LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Olliver Schinagl <oliver@schinagl.nl>
To: Axel Lin <axel.lin@ingics.com>, Mark Brown <broonie@kernel.org>
Cc: Chen-Yu Tsai <wens@csie.org>, Priit Laes <plaes@plaes.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: axp20x: Get rid of AXP20X_xxx_START/END/STEPS defines
Date: Wed, 20 Feb 2019 22:38:23 +0100	[thread overview]
Message-ID: <24E35288-677D-4223-B94A-52A4F37792A8@schinagl.nl> (raw)
In-Reply-To: <20190220165013.12774-1-axel.lin@ingics.com>

Hey Axel,

On February 20, 2019 5:50:13 PM GMT+01:00, Axel Lin <axel.lin@ingics.com> wrote:
>The AXP20X_xxx_START/END/STEPS defines make the code hard to read and
>very hard to check the linear range settings because it needs to check
>the defines one-by-one.
>The original code without the defines is very good in readability
>as the meaning of each field of REGULATOR_LINEAR_RANGE is clear.
>So I suggest to get rid of AXP20X_xxx_START/END/STEPS defines.
Are you suggesting that magic values and hex numbers are more readable?

Maybe you can explain what your problem is, as it appears you are trying to debug something?

>
>Signed-off-by: Axel Lin <axel.lin@ingics.com>
>---
> drivers/regulator/axp20x-regulator.c | 176 +++------------------------
> 1 file changed, 19 insertions(+), 157 deletions(-)
>
>diff --git a/drivers/regulator/axp20x-regulator.c
>b/drivers/regulator/axp20x-regulator.c
>index fba8f58ab769..46b5de53d1a3 100644
>--- a/drivers/regulator/axp20x-regulator.c
>+++ b/drivers/regulator/axp20x-regulator.c
>@@ -64,26 +64,6 @@
> #define AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK		BIT_MASK(3)
> #define AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN		BIT(3)
> 
>-#define AXP20X_LDO4_V_OUT_1250mV_START	0x0
>-#define AXP20X_LDO4_V_OUT_1250mV_STEPS	0
>-#define AXP20X_LDO4_V_OUT_1250mV_END	\
>-	(AXP20X_LDO4_V_OUT_1250mV_START + AXP20X_LDO4_V_OUT_1250mV_STEPS)
>-#define AXP20X_LDO4_V_OUT_1300mV_START	0x1
>-#define AXP20X_LDO4_V_OUT_1300mV_STEPS	7
>-#define AXP20X_LDO4_V_OUT_1300mV_END	\
>-	(AXP20X_LDO4_V_OUT_1300mV_START + AXP20X_LDO4_V_OUT_1300mV_STEPS)
>-#define AXP20X_LDO4_V_OUT_2500mV_START	0x9
>-#define AXP20X_LDO4_V_OUT_2500mV_STEPS	0
>-#define AXP20X_LDO4_V_OUT_2500mV_END	\
>-	(AXP20X_LDO4_V_OUT_2500mV_START + AXP20X_LDO4_V_OUT_2500mV_STEPS)
>-#define AXP20X_LDO4_V_OUT_2700mV_START	0xa
>-#define AXP20X_LDO4_V_OUT_2700mV_STEPS	1
>-#define AXP20X_LDO4_V_OUT_2700mV_END	\
>-	(AXP20X_LDO4_V_OUT_2700mV_START + AXP20X_LDO4_V_OUT_2700mV_STEPS)
>-#define AXP20X_LDO4_V_OUT_3000mV_START	0xc
>-#define AXP20X_LDO4_V_OUT_3000mV_STEPS	3
>-#define AXP20X_LDO4_V_OUT_3000mV_END	\
>-	(AXP20X_LDO4_V_OUT_3000mV_START + AXP20X_LDO4_V_OUT_3000mV_STEPS)
> #define AXP20X_LDO4_V_OUT_NUM_VOLTAGES	16
> 
> #define AXP22X_IO_ENABLED		0x03
>@@ -156,44 +136,9 @@
> #define AXP803_DCDC23_POLYPHASE_DUAL	BIT(6)
> #define AXP803_DCDC56_POLYPHASE_DUAL	BIT(5)
> 
>-#define AXP803_DCDC234_500mV_START	0x00
>-#define AXP803_DCDC234_500mV_STEPS	70
>-#define AXP803_DCDC234_500mV_END	\
>-	(AXP803_DCDC234_500mV_START + AXP803_DCDC234_500mV_STEPS)
>-#define AXP803_DCDC234_1220mV_START	0x47
>-#define AXP803_DCDC234_1220mV_STEPS	4
>-#define AXP803_DCDC234_1220mV_END	\
>-	(AXP803_DCDC234_1220mV_START + AXP803_DCDC234_1220mV_STEPS)
> #define AXP803_DCDC234_NUM_VOLTAGES	76
>-
>-#define AXP803_DCDC5_800mV_START	0x00
>-#define AXP803_DCDC5_800mV_STEPS	32
>-#define AXP803_DCDC5_800mV_END		\
>-	(AXP803_DCDC5_800mV_START + AXP803_DCDC5_800mV_STEPS)
>-#define AXP803_DCDC5_1140mV_START	0x21
>-#define AXP803_DCDC5_1140mV_STEPS	35
>-#define AXP803_DCDC5_1140mV_END		\
>-	(AXP803_DCDC5_1140mV_START + AXP803_DCDC5_1140mV_STEPS)
> #define AXP803_DCDC5_NUM_VOLTAGES	68
>-
>-#define AXP803_DCDC6_600mV_START	0x00
>-#define AXP803_DCDC6_600mV_STEPS	50
>-#define AXP803_DCDC6_600mV_END		\
>-	(AXP803_DCDC6_600mV_START + AXP803_DCDC6_600mV_STEPS)
>-#define AXP803_DCDC6_1120mV_START	0x33
>-#define AXP803_DCDC6_1120mV_STEPS	14
>-#define AXP803_DCDC6_1120mV_END		\
>-	(AXP803_DCDC6_1120mV_START + AXP803_DCDC6_1120mV_STEPS)
> #define AXP803_DCDC6_NUM_VOLTAGES	72
>-
>-#define AXP803_DLDO2_700mV_START	0x00
>-#define AXP803_DLDO2_700mV_STEPS	26
>-#define AXP803_DLDO2_700mV_END		\
>-	(AXP803_DLDO2_700mV_START + AXP803_DLDO2_700mV_STEPS)
>-#define AXP803_DLDO2_3400mV_START	0x1b
>-#define AXP803_DLDO2_3400mV_STEPS	4
>-#define AXP803_DLDO2_3400mV_END		\
>-	(AXP803_DLDO2_3400mV_START + AXP803_DLDO2_3400mV_STEPS)
> #define AXP803_DLDO2_NUM_VOLTAGES	32
> 
> #define AXP806_DCDCA_V_CTRL_MASK	GENMASK(6, 0)
>@@ -235,34 +180,8 @@
> 
> #define AXP806_DCDCDE_POLYPHASE_DUAL	BIT(5)
> 
>-#define AXP806_DCDCA_600mV_START	0x00
>-#define AXP806_DCDCA_600mV_STEPS	50
>-#define AXP806_DCDCA_600mV_END		\
>-	(AXP806_DCDCA_600mV_START + AXP806_DCDCA_600mV_STEPS)
>-#define AXP806_DCDCA_1120mV_START	0x33
>-#define AXP806_DCDCA_1120mV_STEPS	14
>-#define AXP806_DCDCA_1120mV_END		\
>-	(AXP806_DCDCA_1120mV_START + AXP806_DCDCA_1120mV_STEPS)
> #define AXP806_DCDCA_NUM_VOLTAGES	72
>-
>-#define AXP806_DCDCD_600mV_START	0x00
>-#define AXP806_DCDCD_600mV_STEPS	45
>-#define AXP806_DCDCD_600mV_END		\
>-	(AXP806_DCDCD_600mV_START + AXP806_DCDCD_600mV_STEPS)
>-#define AXP806_DCDCD_1600mV_START	0x2e
>-#define AXP806_DCDCD_1600mV_STEPS	17
>-#define AXP806_DCDCD_1600mV_END		\
>-	(AXP806_DCDCD_1600mV_START + AXP806_DCDCD_1600mV_STEPS)
> #define AXP806_DCDCD_NUM_VOLTAGES	64
>-
>-#define AXP809_DCDC4_600mV_START	0x00
>-#define AXP809_DCDC4_600mV_STEPS	47
>-#define AXP809_DCDC4_600mV_END		\
>-	(AXP809_DCDC4_600mV_START + AXP809_DCDC4_600mV_STEPS)
>-#define AXP809_DCDC4_1800mV_START	0x30
>-#define AXP809_DCDC4_1800mV_STEPS	8
>-#define AXP809_DCDC4_1800mV_END		\
>-	(AXP809_DCDC4_1800mV_START + AXP809_DCDC4_1800mV_STEPS)
> #define AXP809_DCDC4_NUM_VOLTAGES	57
> 
> #define AXP813_DCDC7_V_OUT_MASK		GENMASK(6, 0)
>@@ -517,26 +436,11 @@ static const struct regulator_ops axp20x_ops_sw =
>{
> };
> 
> static const struct regulator_linear_range axp20x_ldo4_ranges[] = {
>-	REGULATOR_LINEAR_RANGE(1250000,
>-			       AXP20X_LDO4_V_OUT_1250mV_START,
>-			       AXP20X_LDO4_V_OUT_1250mV_END,
>-			       0),
>-	REGULATOR_LINEAR_RANGE(1300000,
>-			       AXP20X_LDO4_V_OUT_1300mV_START,
>-			       AXP20X_LDO4_V_OUT_1300mV_END,
>-			       100000),
>-	REGULATOR_LINEAR_RANGE(2500000,
>-			       AXP20X_LDO4_V_OUT_2500mV_START,
>-			       AXP20X_LDO4_V_OUT_2500mV_END,
>-			       0),
>-	REGULATOR_LINEAR_RANGE(2700000,
>-			       AXP20X_LDO4_V_OUT_2700mV_START,
>-			       AXP20X_LDO4_V_OUT_2700mV_END,
>-			       100000),
>-	REGULATOR_LINEAR_RANGE(3000000,
>-			       AXP20X_LDO4_V_OUT_3000mV_START,
>-			       AXP20X_LDO4_V_OUT_3000mV_END,
>-			       100000),
>+	REGULATOR_LINEAR_RANGE(1250000, 0x0, 0x0, 0),
>+	REGULATOR_LINEAR_RANGE(1300000, 0x1, 0x8, 100000),
>+	REGULATOR_LINEAR_RANGE(2500000, 0x9, 0x9, 0),
>+	REGULATOR_LINEAR_RANGE(2700000, 0xa, 0xb, 100000),
>+	REGULATOR_LINEAR_RANGE(3000000, 0xc, 0xf, 100000),
> };
> 
> static const struct regulator_desc axp20x_regulators[] = {
>@@ -645,48 +549,24 @@ static const struct regulator_desc
>axp22x_drivevbus_regulator = {
> 
> /* DCDC ranges shared with AXP813 */
> static const struct regulator_linear_range axp803_dcdc234_ranges[] = {
>-	REGULATOR_LINEAR_RANGE(500000,
>-			       AXP803_DCDC234_500mV_START,
>-			       AXP803_DCDC234_500mV_END,
>-			       10000),
>-	REGULATOR_LINEAR_RANGE(1220000,
>-			       AXP803_DCDC234_1220mV_START,
>-			       AXP803_DCDC234_1220mV_END,
>-			       20000),
>+	REGULATOR_LINEAR_RANGE(500000, 0x0, 0x46, 10000),
>+	REGULATOR_LINEAR_RANGE(1220000, 0x47, 0x4b, 20000),
> };
> 
> static const struct regulator_linear_range axp803_dcdc5_ranges[] = {
>-	REGULATOR_LINEAR_RANGE(800000,
>-			       AXP803_DCDC5_800mV_START,
>-			       AXP803_DCDC5_800mV_END,
>-			       10000),
>-	REGULATOR_LINEAR_RANGE(1140000,
>-			       AXP803_DCDC5_1140mV_START,
>-			       AXP803_DCDC5_1140mV_END,
>-			       20000),
>+	REGULATOR_LINEAR_RANGE(800000, 0x0, 0x20, 10000),
>+	REGULATOR_LINEAR_RANGE(1140000, 0x21, 0x44, 20000),
> };
> 
> static const struct regulator_linear_range axp803_dcdc6_ranges[] = {
>-	REGULATOR_LINEAR_RANGE(600000,
>-			       AXP803_DCDC6_600mV_START,
>-			       AXP803_DCDC6_600mV_END,
>-			       10000),
>-	REGULATOR_LINEAR_RANGE(1120000,
>-			       AXP803_DCDC6_1120mV_START,
>-			       AXP803_DCDC6_1120mV_END,
>-			       20000),
>+	REGULATOR_LINEAR_RANGE(600000, 0x0, 0x32, 10000),
>+	REGULATOR_LINEAR_RANGE(1120000, 0x33, 0x47, 20000),
> };
> 
> /* AXP806's CLDO2 and AXP809's DLDO1 share the same range */
> static const struct regulator_linear_range axp803_dldo2_ranges[] = {
>-	REGULATOR_LINEAR_RANGE(700000,
>-			       AXP803_DLDO2_700mV_START,
>-			       AXP803_DLDO2_700mV_END,
>-			       100000),
>-	REGULATOR_LINEAR_RANGE(3400000,
>-			       AXP803_DLDO2_3400mV_START,
>-			       AXP803_DLDO2_3400mV_END,
>-			       200000),
>+	REGULATOR_LINEAR_RANGE(700000, 0x0, 0x1a, 100000),
>+	REGULATOR_LINEAR_RANGE(3400000, 0x1b, 0x1f, 200000),
> };
> 
> static const struct regulator_desc axp803_regulators[] = {
>@@ -765,25 +645,13 @@ static const struct regulator_desc
>axp803_regulators[] = {
> };
> 
> static const struct regulator_linear_range axp806_dcdca_ranges[] = {
>-	REGULATOR_LINEAR_RANGE(600000,
>-			       AXP806_DCDCA_600mV_START,
>-			       AXP806_DCDCA_600mV_END,
>-			       10000),
>-	REGULATOR_LINEAR_RANGE(1120000,
>-			       AXP806_DCDCA_1120mV_START,
>-			       AXP806_DCDCA_1120mV_END,
>-			       20000),
>+	REGULATOR_LINEAR_RANGE(600000, 0x0, 0x32, 10000),
>+	REGULATOR_LINEAR_RANGE(1120000, 0x33, 0x47, 20000),
> };
> 
> static const struct regulator_linear_range axp806_dcdcd_ranges[] = {
>-	REGULATOR_LINEAR_RANGE(600000,
>-			       AXP806_DCDCD_600mV_START,
>-			       AXP806_DCDCD_600mV_END,
>-			       20000),
>-	REGULATOR_LINEAR_RANGE(1600000,
>-			       AXP806_DCDCD_600mV_START,
>-			       AXP806_DCDCD_600mV_END,
>-			       100000),
>+	REGULATOR_LINEAR_RANGE(600000, 0x0, 0x2d, 20000),
>+	REGULATOR_LINEAR_RANGE(1600000, 0x2e, 0x3f, 100000),
> };
> 
> static const struct regulator_desc axp806_regulators[] = {
>@@ -841,14 +709,8 @@ static const struct regulator_desc
>axp806_regulators[] = {
> };
> 
> static const struct regulator_linear_range axp809_dcdc4_ranges[] = {
>-	REGULATOR_LINEAR_RANGE(600000,
>-			       AXP809_DCDC4_600mV_START,
>-			       AXP809_DCDC4_600mV_END,
>-			       20000),
>-	REGULATOR_LINEAR_RANGE(1800000,
>-			       AXP809_DCDC4_1800mV_START,
>-			       AXP809_DCDC4_1800mV_END,
>-			       100000),
>+	REGULATOR_LINEAR_RANGE(600000, 0x0, 0x2f, 20000),
>+	REGULATOR_LINEAR_RANGE(1800000, 0x30, 0x38, 100000),
> };
> 
> static const struct regulator_desc axp809_regulators[] = {

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

  reply	other threads:[~2019-02-20 23:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-20 16:50 Axel Lin
2019-02-20 21:38 ` Olliver Schinagl [this message]
2019-02-21  0:22   ` Axel Lin
2019-02-21  9:42     ` Mark Brown
2019-02-23  7:55       ` Olliver Schinagl
2019-02-23 12:54         ` Axel Lin
2019-02-23 20:37           ` Olliver Schinagl
2019-02-25 17:25             ` Mark Brown
2019-02-27 19:41               ` Olliver Schinagl
2019-02-27 20:05                 ` Mark Brown
2019-02-27 20:26                   ` Olliver Schinagl
2019-03-23 13:41             ` Axel Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=24E35288-677D-4223-B94A-52A4F37792A8@schinagl.nl \
    --to=oliver@schinagl.nl \
    --cc=axel.lin@ingics.com \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=plaes@plaes.org \
    --cc=wens@csie.org \
    --subject='Re: [PATCH] regulator: axp20x: Get rid of AXP20X_xxx_START/END/STEPS defines' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).