lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 20 Feb 2019 22:38:23 +0100
From:   Olliver Schinagl <oliver@...inagl.nl>
To:     Axel Lin <axel.lin@...ics.com>, Mark Brown <broonie@...nel.org>
CC:     Chen-Yu Tsai <wens@...e.org>, Priit Laes <plaes@...es.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] regulator: axp20x: Get rid of AXP20X_xxx_START/END/STEPS defines

Hey Axel,

On February 20, 2019 5:50:13 PM GMT+01:00, Axel Lin <axel.lin@...ics.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@...ics.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.

Powered by blists - more mailing lists