Re: [PATCH] regulator: axp20x: Get rid of AXP20X_xxx_START/END/STEPS defines

From: Olliver Schinagl
Date: Wed Feb 20 2019 - 18:06:45 EST

Hey Axel,

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