Hey Axel,

On February 20, 2019 5:50:13 PM GMT+01:00, Axel Lin <[email protected]> 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 <[email protected]>
>---
> 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 via email to