* [PATCH v1 0/2] TPS65219 U-Boot Driver Clean-Up @ 2024-12-18 17:12 Shree Ramamoorthy 2024-12-18 17:12 ` [PATCH v1 1/2] power: regulator: replace printf() with pr_err() Shree Ramamoorthy 2024-12-18 17:12 ` [PATCH v1 2/2] power: replace magic numbers with macros Shree Ramamoorthy 0 siblings, 2 replies; 9+ messages in thread From: Shree Ramamoorthy @ 2024-12-18 17:12 UTC (permalink / raw) To: u-boot, jh80.chung, trini; +Cc: m-leonard, praneeth This clean-up series is sent in preparation to add 2 PMIC devices to the TPS65219 Driver. The changes involve replacing magic numbers with macros and replacing printf() instances with pr_err(). The intention is to remove unnecessary noise from the new PMIC device patches added to this driver. Test Log (AM62B-P1 + TPS65219 EVM): https://gist.github.com/ramamoorthyhs/8c81e33cf6d46c8b5fc135424a5f224b Kernel baseline used in test log: Reg: https://lore.kernel.org/all/20241217204526.1010989-1-s-ramamoorthy@ti.com/ GPIO: https://lore.kernel.org/all/20241217204755.1011731-1-s-ramamoorthy@ti.com/ MFD: https://lore.kernel.org/all/20241217204935.1012106-1-s-ramamoorthy@ti.com/ Shree Ramamoorthy (2): power: regulator: replace printf() with pr_err() power: replace magic numbers with macros drivers/power/regulator/tps65219_regulator.c | 30 ++++++++++---------- include/power/tps65219.h | 14 +++++++-- 2 files changed, 27 insertions(+), 17 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 1/2] power: regulator: replace printf() with pr_err() 2024-12-18 17:12 [PATCH v1 0/2] TPS65219 U-Boot Driver Clean-Up Shree Ramamoorthy @ 2024-12-18 17:12 ` Shree Ramamoorthy 2024-12-18 22:44 ` Jaehoon Chung 2024-12-18 17:12 ` [PATCH v1 2/2] power: replace magic numbers with macros Shree Ramamoorthy 1 sibling, 1 reply; 9+ messages in thread From: Shree Ramamoorthy @ 2024-12-18 17:12 UTC (permalink / raw) To: u-boot, jh80.chung, trini; +Cc: m-leonard, praneeth Replace printf() with pr_err() because pr_err() has a uniform print format and takes into consideration the log levels supported. Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com> --- drivers/power/regulator/tps65219_regulator.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/power/regulator/tps65219_regulator.c b/drivers/power/regulator/tps65219_regulator.c index b7124fed0245..4b0fb205909a 100644 --- a/drivers/power/regulator/tps65219_regulator.c +++ b/drivers/power/regulator/tps65219_regulator.c @@ -250,7 +250,7 @@ static int tps65219_ldo_probe(struct udevice *dev) /* idx must be in 1..TPS65219_LDO_NUM */ idx = dev->driver_data; if (idx < 1 || idx > TPS65219_LDO_NUM) { - printf("Wrong ID for regulator\n"); + pr_err("Wrong ID for regulator\n"); return -EINVAL; } @@ -271,7 +271,7 @@ static int tps65219_buck_probe(struct udevice *dev) /* idx must be in 1..TPS65219_BUCK_NUM */ idx = dev->driver_data; if (idx < 1 || idx > TPS65219_BUCK_NUM) { - printf("Wrong ID for regulator\n"); + pr_err("Wrong ID for regulator\n"); return -EINVAL; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: [PATCH v1 1/2] power: regulator: replace printf() with pr_err() 2024-12-18 17:12 ` [PATCH v1 1/2] power: regulator: replace printf() with pr_err() Shree Ramamoorthy @ 2024-12-18 22:44 ` Jaehoon Chung 0 siblings, 0 replies; 9+ messages in thread From: Jaehoon Chung @ 2024-12-18 22:44 UTC (permalink / raw) To: 'Shree Ramamoorthy', u-boot, trini; +Cc: m-leonard, praneeth > -----Original Message----- > From: Shree Ramamoorthy <s-ramamoorthy@ti.com> > Sent: Thursday, December 19, 2024 2:13 AM > > Replace printf() with pr_err() because pr_err() has a uniform print format > and takes into consideration the log levels supported. > > Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> Best Regards, Jaehoon Chung > --- > drivers/power/regulator/tps65219_regulator.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/power/regulator/tps65219_regulator.c > b/drivers/power/regulator/tps65219_regulator.c > index b7124fed0245..4b0fb205909a 100644 > --- a/drivers/power/regulator/tps65219_regulator.c > +++ b/drivers/power/regulator/tps65219_regulator.c > @@ -250,7 +250,7 @@ static int tps65219_ldo_probe(struct udevice *dev) > /* idx must be in 1..TPS65219_LDO_NUM */ > idx = dev->driver_data; > if (idx < 1 || idx > TPS65219_LDO_NUM) { > - printf("Wrong ID for regulator\n"); > + pr_err("Wrong ID for regulator\n"); > return -EINVAL; > } > > @@ -271,7 +271,7 @@ static int tps65219_buck_probe(struct udevice *dev) > /* idx must be in 1..TPS65219_BUCK_NUM */ > idx = dev->driver_data; > if (idx < 1 || idx > TPS65219_BUCK_NUM) { > - printf("Wrong ID for regulator\n"); > + pr_err("Wrong ID for regulator\n"); > return -EINVAL; > } > > -- > 2.34.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 2/2] power: replace magic numbers with macros 2024-12-18 17:12 [PATCH v1 0/2] TPS65219 U-Boot Driver Clean-Up Shree Ramamoorthy 2024-12-18 17:12 ` [PATCH v1 1/2] power: regulator: replace printf() with pr_err() Shree Ramamoorthy @ 2024-12-18 17:12 ` Shree Ramamoorthy 2024-12-18 22:44 ` Jaehoon Chung 1 sibling, 1 reply; 9+ messages in thread From: Shree Ramamoorthy @ 2024-12-18 17:12 UTC (permalink / raw) To: u-boot, jh80.chung, trini; +Cc: m-leonard, praneeth Replace magic numbers in buckval2votl() & buckvolt2val() with macros to help with clarity and correlate what the numbers correspond to in the TPS65219 datasheet. Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com> --- drivers/power/regulator/tps65219_regulator.c | 26 ++++++++++---------- include/power/tps65219.h | 14 +++++++++-- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/drivers/power/regulator/tps65219_regulator.c b/drivers/power/regulator/tps65219_regulator.c index 4b0fb205909a..88abc896b3a2 100644 --- a/drivers/power/regulator/tps65219_regulator.c +++ b/drivers/power/regulator/tps65219_regulator.c @@ -72,12 +72,12 @@ static int tps65219_buck_enable(struct udevice *dev, int op, bool *enable) static int tps65219_buck_volt2val(int uV) { - if (uV > TPS65219_BUCK_VOLT_MAX) + if (uV > TPS65219_BUCK_3V4) return -EINVAL; - else if (uV >= 1400000) - return (uV - 1400000) / 100000 + 0x20; - else if (uV >= 600000) - return (uV - 600000) / 25000 + 0x00; + else if (uV >= TPS65219_BUCK_1V4) + return (uV - TPS65219_BUCK_1V4) / TPS65219_VOLT_STEP_100MV + TPS65219_BUCK_REG_1V4; + else if (uV >= TPS65219_BUCK_0V6) + return (uV - TPS65219_BUCK_0V6) / TPS65219_VOLT_STEP_25MV + TPS65219_BUCK_REG_0V6; else return -EINVAL; } @@ -86,12 +86,12 @@ static int tps65219_buck_val2volt(int val) { if (val > TPS65219_VOLT_MASK) return -EINVAL; - else if (val > 0x34) - return TPS65219_BUCK_VOLT_MAX; - else if (val > 0x20) - return 1400000 + (val - 0x20) * 100000; - else if (val >= 0) - return 600000 + val * 25000; + else if (val > TPS65219_BUCK_REG_3V4) + return TPS65219_BUCK_3V4; + else if (val > TPS65219_BUCK_REG_1V4) + return TPS65219_BUCK_1V4 + (val - TPS65219_BUCK_REG_1V4) * TPS65219_VOLT_STEP_100MV; + else if (val >= TPS65219_BUCK_REG_0V6) + return TPS65219_BUCK_0V6 + val * TPS65219_VOLT_STEP_25MV; else return -EINVAL; } @@ -161,7 +161,7 @@ static int tps65219_ldo_volt2val(int idx, int uV) if (uV > max) return -EINVAL; else if (uV >= base) - return (uV - TPS65219_LDO12_VOLT_MIN) / 50000; + return (uV - TPS65219_LDO12_VOLT_MIN) / TPS65219_VOLT_STEP_50MV; else return -EINVAL; } @@ -187,7 +187,7 @@ static int tps65219_ldo_val2volt(int idx, int val) else if (val <= reg_base) return base; else if (val >= 0) - return TPS65219_LDO12_VOLT_MIN + (50000 * val); + return TPS65219_LDO12_VOLT_MIN + (TPS65219_VOLT_STEP_50MV * val); else return -EINVAL; } diff --git a/include/power/tps65219.h b/include/power/tps65219.h index aa81b92266fd..e8780af2d811 100644 --- a/include/power/tps65219.h +++ b/include/power/tps65219.h @@ -17,10 +17,20 @@ #define TPS65219_BUCK_DRIVER "tps65219_buck" #define TPS65219_VOLT_MASK 0x3F -#define TPS65219_BUCK_VOLT_MAX 3400000 - #define TPS65219_ENABLE_CTRL_REG 0x2 +#define TPS65219_VOLT_STEP_25MV 25000 +#define TPS65219_VOLT_STEP_50MV 50000 +#define TPS65219_VOLT_STEP_100MV 100000 + +#define TPS65219_BUCK_0V6 600000 +#define TPS65219_BUCK_1V4 1400000 +#define TPS65219_BUCK_3V4 3400000 + +#define TPS65219_BUCK_REG_0V6 0x00 +#define TPS65219_BUCK_REG_1V4 0x20 +#define TPS65219_BUCK_REG_3V4 0x34 + #define TPS65219_BUCK1_VOUT_REG 0xa #define TPS65219_BUCK2_VOUT_REG 0x9 #define TPS65219_BUCK3_VOUT_REG 0x8 -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: [PATCH v1 2/2] power: replace magic numbers with macros 2024-12-18 17:12 ` [PATCH v1 2/2] power: replace magic numbers with macros Shree Ramamoorthy @ 2024-12-18 22:44 ` Jaehoon Chung 2024-12-18 22:55 ` Shree Ramamoorthy 0 siblings, 1 reply; 9+ messages in thread From: Jaehoon Chung @ 2024-12-18 22:44 UTC (permalink / raw) To: 'Shree Ramamoorthy', u-boot, trini; +Cc: m-leonard, praneeth Hi, > -----Original Message----- > From: Shree Ramamoorthy <s-ramamoorthy@ti.com> > Sent: Thursday, December 19, 2024 2:13 AM > > Replace magic numbers in buckval2votl() & buckvolt2val() with macros to > help with clarity and correlate what the numbers correspond to in the > TPS65219 datasheet. > > Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com> > --- > drivers/power/regulator/tps65219_regulator.c | 26 ++++++++++---------- > include/power/tps65219.h | 14 +++++++++-- > 2 files changed, 25 insertions(+), 15 deletions(-) > > diff --git a/drivers/power/regulator/tps65219_regulator.c > b/drivers/power/regulator/tps65219_regulator.c > index 4b0fb205909a..88abc896b3a2 100644 > --- a/drivers/power/regulator/tps65219_regulator.c > +++ b/drivers/power/regulator/tps65219_regulator.c > @@ -72,12 +72,12 @@ static int tps65219_buck_enable(struct udevice *dev, int op, bool *enable) > > static int tps65219_buck_volt2val(int uV) > { > - if (uV > TPS65219_BUCK_VOLT_MAX) > + if (uV > TPS65219_BUCK_3V4) > return -EINVAL; > - else if (uV >= 1400000) > - return (uV - 1400000) / 100000 + 0x20; > - else if (uV >= 600000) > - return (uV - 600000) / 25000 + 0x00; > + else if (uV >= TPS65219_BUCK_1V4) > + return (uV - TPS65219_BUCK_1V4) / TPS65219_VOLT_STEP_100MV + TPS65219_BUCK_REG_1V4; Even though Not relevant to this subject. If uV is 340000, the return value is correct? Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> Best Regards, Jaehoon Chung > + else if (uV >= TPS65219_BUCK_0V6) > + return (uV - TPS65219_BUCK_0V6) / TPS65219_VOLT_STEP_25MV + TPS65219_BUCK_REG_0V6; > else > return -EINVAL; > } > @@ -86,12 +86,12 @@ static int tps65219_buck_val2volt(int val) > { > if (val > TPS65219_VOLT_MASK) > return -EINVAL; > - else if (val > 0x34) > - return TPS65219_BUCK_VOLT_MAX; > - else if (val > 0x20) > - return 1400000 + (val - 0x20) * 100000; > - else if (val >= 0) > - return 600000 + val * 25000; > + else if (val > TPS65219_BUCK_REG_3V4) > + return TPS65219_BUCK_3V4; > + else if (val > TPS65219_BUCK_REG_1V4) > + return TPS65219_BUCK_1V4 + (val - TPS65219_BUCK_REG_1V4) * TPS65219_VOLT_STEP_100MV; > + else if (val >= TPS65219_BUCK_REG_0V6) > + return TPS65219_BUCK_0V6 + val * TPS65219_VOLT_STEP_25MV; > else > return -EINVAL; > } > @@ -161,7 +161,7 @@ static int tps65219_ldo_volt2val(int idx, int uV) > if (uV > max) > return -EINVAL; > else if (uV >= base) > - return (uV - TPS65219_LDO12_VOLT_MIN) / 50000; > + return (uV - TPS65219_LDO12_VOLT_MIN) / TPS65219_VOLT_STEP_50MV; > else > return -EINVAL; > } > @@ -187,7 +187,7 @@ static int tps65219_ldo_val2volt(int idx, int val) > else if (val <= reg_base) > return base; > else if (val >= 0) > - return TPS65219_LDO12_VOLT_MIN + (50000 * val); > + return TPS65219_LDO12_VOLT_MIN + (TPS65219_VOLT_STEP_50MV * val); > else > return -EINVAL; > } > diff --git a/include/power/tps65219.h b/include/power/tps65219.h > index aa81b92266fd..e8780af2d811 100644 > --- a/include/power/tps65219.h > +++ b/include/power/tps65219.h > @@ -17,10 +17,20 @@ > #define TPS65219_BUCK_DRIVER "tps65219_buck" > > #define TPS65219_VOLT_MASK 0x3F > -#define TPS65219_BUCK_VOLT_MAX 3400000 > - > #define TPS65219_ENABLE_CTRL_REG 0x2 > > +#define TPS65219_VOLT_STEP_25MV 25000 > +#define TPS65219_VOLT_STEP_50MV 50000 > +#define TPS65219_VOLT_STEP_100MV 100000 > + > +#define TPS65219_BUCK_0V6 600000 > +#define TPS65219_BUCK_1V4 1400000 > +#define TPS65219_BUCK_3V4 3400000 > + > +#define TPS65219_BUCK_REG_0V6 0x00 > +#define TPS65219_BUCK_REG_1V4 0x20 > +#define TPS65219_BUCK_REG_3V4 0x34 > + > #define TPS65219_BUCK1_VOUT_REG 0xa > #define TPS65219_BUCK2_VOUT_REG 0x9 > #define TPS65219_BUCK3_VOUT_REG 0x8 > -- > 2.34.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] power: replace magic numbers with macros 2024-12-18 22:44 ` Jaehoon Chung @ 2024-12-18 22:55 ` Shree Ramamoorthy 2024-12-18 23:26 ` Jaehoon Chung 0 siblings, 1 reply; 9+ messages in thread From: Shree Ramamoorthy @ 2024-12-18 22:55 UTC (permalink / raw) To: Jaehoon Chung, u-boot, trini; +Cc: m-leonard, praneeth Hi. On 12/18/24 4:44 PM, Jaehoon Chung wrote: > Hi, > >> -----Original Message----- >> From: Shree Ramamoorthy <s-ramamoorthy@ti.com> >> Sent: Thursday, December 19, 2024 2:13 AM >> >> Replace magic numbers in buckval2votl() & buckvolt2val() with macros to >> help with clarity and correlate what the numbers correspond to in the >> TPS65219 datasheet. >> >> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com> >> --- >> drivers/power/regulator/tps65219_regulator.c | 26 ++++++++++---------- >> include/power/tps65219.h | 14 +++++++++-- >> 2 files changed, 25 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/power/regulator/tps65219_regulator.c >> b/drivers/power/regulator/tps65219_regulator.c >> index 4b0fb205909a..88abc896b3a2 100644 >> --- a/drivers/power/regulator/tps65219_regulator.c >> +++ b/drivers/power/regulator/tps65219_regulator.c >> @@ -72,12 +72,12 @@ static int tps65219_buck_enable(struct udevice *dev, int op, bool *enable) >> >> static int tps65219_buck_volt2val(int uV) >> { >> - if (uV > TPS65219_BUCK_VOLT_MAX) >> + if (uV > TPS65219_BUCK_3V4) >> return -EINVAL; >> - else if (uV >= 1400000) >> - return (uV - 1400000) / 100000 + 0x20; >> - else if (uV >= 600000) >> - return (uV - 600000) / 25000 + 0x00; >> + else if (uV >= TPS65219_BUCK_1V4) >> + return (uV - TPS65219_BUCK_1V4) / TPS65219_VOLT_STEP_100MV + TPS65219_BUCK_REG_1V4; > Even though Not relevant to this subject. If uV is 340000, the return value is correct? > > Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> > > Best Regards, > Jaehoon Chung Thank you for reviewing! The allowed max uV is 3.4V inclusive. If 340000 uV is detected, the first 'else if' statement should be taken. Best, Shree >> + else if (uV >= TPS65219_BUCK_0V6) >> + return (uV - TPS65219_BUCK_0V6) / TPS65219_VOLT_STEP_25MV + TPS65219_BUCK_REG_0V6; >> else >> return -EINVAL; >> } >> @@ -86,12 +86,12 @@ static int tps65219_buck_val2volt(int val) >> { >> if (val > TPS65219_VOLT_MASK) >> return -EINVAL; >> - else if (val > 0x34) >> - return TPS65219_BUCK_VOLT_MAX; >> - else if (val > 0x20) >> - return 1400000 + (val - 0x20) * 100000; >> - else if (val >= 0) >> - return 600000 + val * 25000; >> + else if (val > TPS65219_BUCK_REG_3V4) >> + return TPS65219_BUCK_3V4; >> + else if (val > TPS65219_BUCK_REG_1V4) >> + return TPS65219_BUCK_1V4 + (val - TPS65219_BUCK_REG_1V4) * TPS65219_VOLT_STEP_100MV; >> + else if (val >= TPS65219_BUCK_REG_0V6) >> + return TPS65219_BUCK_0V6 + val * TPS65219_VOLT_STEP_25MV; >> else >> return -EINVAL; >> } >> @@ -161,7 +161,7 @@ static int tps65219_ldo_volt2val(int idx, int uV) >> if (uV > max) >> return -EINVAL; >> else if (uV >= base) >> - return (uV - TPS65219_LDO12_VOLT_MIN) / 50000; >> + return (uV - TPS65219_LDO12_VOLT_MIN) / TPS65219_VOLT_STEP_50MV; >> else >> return -EINVAL; >> } >> @@ -187,7 +187,7 @@ static int tps65219_ldo_val2volt(int idx, int val) >> else if (val <= reg_base) >> return base; >> else if (val >= 0) >> - return TPS65219_LDO12_VOLT_MIN + (50000 * val); >> + return TPS65219_LDO12_VOLT_MIN + (TPS65219_VOLT_STEP_50MV * val); >> else >> return -EINVAL; >> } >> diff --git a/include/power/tps65219.h b/include/power/tps65219.h >> index aa81b92266fd..e8780af2d811 100644 >> --- a/include/power/tps65219.h >> +++ b/include/power/tps65219.h >> @@ -17,10 +17,20 @@ >> #define TPS65219_BUCK_DRIVER "tps65219_buck" >> >> #define TPS65219_VOLT_MASK 0x3F >> -#define TPS65219_BUCK_VOLT_MAX 3400000 >> - >> #define TPS65219_ENABLE_CTRL_REG 0x2 >> >> +#define TPS65219_VOLT_STEP_25MV 25000 >> +#define TPS65219_VOLT_STEP_50MV 50000 >> +#define TPS65219_VOLT_STEP_100MV 100000 >> + >> +#define TPS65219_BUCK_0V6 600000 >> +#define TPS65219_BUCK_1V4 1400000 >> +#define TPS65219_BUCK_3V4 3400000 >> + >> +#define TPS65219_BUCK_REG_0V6 0x00 >> +#define TPS65219_BUCK_REG_1V4 0x20 >> +#define TPS65219_BUCK_REG_3V4 0x34 >> + >> #define TPS65219_BUCK1_VOUT_REG 0xa >> #define TPS65219_BUCK2_VOUT_REG 0x9 >> #define TPS65219_BUCK3_VOUT_REG 0x8 >> -- >> 2.34.1 > > -- Best, Shree Ramamoorthy PMIC Software Engineer ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v1 2/2] power: replace magic numbers with macros 2024-12-18 22:55 ` Shree Ramamoorthy @ 2024-12-18 23:26 ` Jaehoon Chung 2024-12-19 17:47 ` Shree Ramamoorthy 0 siblings, 1 reply; 9+ messages in thread From: Jaehoon Chung @ 2024-12-18 23:26 UTC (permalink / raw) To: 'Shree Ramamoorthy', u-boot, trini; +Cc: m-leonard, praneeth > -----Original Message----- > From: Shree Ramamoorthy <s-ramamoorthy@ti.com> > Sent: Thursday, December 19, 2024 7:55 AM > > Hi. > > > On 12/18/24 4:44 PM, Jaehoon Chung wrote: > > Hi, > > > >> -----Original Message----- > >> From: Shree Ramamoorthy <s-ramamoorthy@ti.com> > >> Sent: Thursday, December 19, 2024 2:13 AM > >> > >> Replace magic numbers in buckval2votl() & buckvolt2val() with macros to > >> help with clarity and correlate what the numbers correspond to in the > >> TPS65219 datasheet. > >> > >> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com> > >> --- > >> drivers/power/regulator/tps65219_regulator.c | 26 ++++++++++---------- > >> include/power/tps65219.h | 14 +++++++++-- > >> 2 files changed, 25 insertions(+), 15 deletions(-) > >> > >> diff --git a/drivers/power/regulator/tps65219_regulator.c > >> b/drivers/power/regulator/tps65219_regulator.c > >> index 4b0fb205909a..88abc896b3a2 100644 > >> --- a/drivers/power/regulator/tps65219_regulator.c > >> +++ b/drivers/power/regulator/tps65219_regulator.c > >> @@ -72,12 +72,12 @@ static int tps65219_buck_enable(struct udevice *dev, int op, bool *enable) > >> > >> static int tps65219_buck_volt2val(int uV) > >> { > >> - if (uV > TPS65219_BUCK_VOLT_MAX) > >> + if (uV > TPS65219_BUCK_3V4) > >> return -EINVAL; > >> - else if (uV >= 1400000) > >> - return (uV - 1400000) / 100000 + 0x20; > >> - else if (uV >= 600000) > >> - return (uV - 600000) / 25000 + 0x00; > >> + else if (uV >= TPS65219_BUCK_1V4) > >> + return (uV - TPS65219_BUCK_1V4) / TPS65219_VOLT_STEP_100MV + TPS65219_BUCK_REG_1V4; > > Even though Not relevant to this subject. If uV is 340000, the return value is correct? > > > > Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> > > > > Best Regards, > > Jaehoon Chung > > Thank you for reviewing! > The allowed max uV is 3.4V inclusive. > If 340000 uV is detected, the first 'else if' statement should be taken. Ah, I missed wrong read. Thanks for checking. If you can do, other tpsXXX can also be changed to readable macro. Best Regards, Jaehoon Chung > > Best, > Shree > > >> + else if (uV >= TPS65219_BUCK_0V6) > >> + return (uV - TPS65219_BUCK_0V6) / TPS65219_VOLT_STEP_25MV + TPS65219_BUCK_REG_0V6; > >> else > >> return -EINVAL; > >> } > >> @@ -86,12 +86,12 @@ static int tps65219_buck_val2volt(int val) > >> { > >> if (val > TPS65219_VOLT_MASK) > >> return -EINVAL; > >> - else if (val > 0x34) > >> - return TPS65219_BUCK_VOLT_MAX; > >> - else if (val > 0x20) > >> - return 1400000 + (val - 0x20) * 100000; > >> - else if (val >= 0) > >> - return 600000 + val * 25000; > >> + else if (val > TPS65219_BUCK_REG_3V4) > >> + return TPS65219_BUCK_3V4; > >> + else if (val > TPS65219_BUCK_REG_1V4) > >> + return TPS65219_BUCK_1V4 + (val - TPS65219_BUCK_REG_1V4) * TPS65219_VOLT_STEP_100MV; > >> + else if (val >= TPS65219_BUCK_REG_0V6) > >> + return TPS65219_BUCK_0V6 + val * TPS65219_VOLT_STEP_25MV; > >> else > >> return -EINVAL; > >> } > >> @@ -161,7 +161,7 @@ static int tps65219_ldo_volt2val(int idx, int uV) > >> if (uV > max) > >> return -EINVAL; > >> else if (uV >= base) > >> - return (uV - TPS65219_LDO12_VOLT_MIN) / 50000; > >> + return (uV - TPS65219_LDO12_VOLT_MIN) / TPS65219_VOLT_STEP_50MV; > >> else > >> return -EINVAL; > >> } > >> @@ -187,7 +187,7 @@ static int tps65219_ldo_val2volt(int idx, int val) > >> else if (val <= reg_base) > >> return base; > >> else if (val >= 0) > >> - return TPS65219_LDO12_VOLT_MIN + (50000 * val); > >> + return TPS65219_LDO12_VOLT_MIN + (TPS65219_VOLT_STEP_50MV * val); > >> else > >> return -EINVAL; > >> } > >> diff --git a/include/power/tps65219.h b/include/power/tps65219.h > >> index aa81b92266fd..e8780af2d811 100644 > >> --- a/include/power/tps65219.h > >> +++ b/include/power/tps65219.h > >> @@ -17,10 +17,20 @@ > >> #define TPS65219_BUCK_DRIVER "tps65219_buck" > >> > >> #define TPS65219_VOLT_MASK 0x3F > >> -#define TPS65219_BUCK_VOLT_MAX 3400000 > >> - > >> #define TPS65219_ENABLE_CTRL_REG 0x2 > >> > >> +#define TPS65219_VOLT_STEP_25MV 25000 > >> +#define TPS65219_VOLT_STEP_50MV 50000 > >> +#define TPS65219_VOLT_STEP_100MV 100000 > >> + > >> +#define TPS65219_BUCK_0V6 600000 > >> +#define TPS65219_BUCK_1V4 1400000 > >> +#define TPS65219_BUCK_3V4 3400000 > >> + > >> +#define TPS65219_BUCK_REG_0V6 0x00 > >> +#define TPS65219_BUCK_REG_1V4 0x20 > >> +#define TPS65219_BUCK_REG_3V4 0x34 > >> + > >> #define TPS65219_BUCK1_VOUT_REG 0xa > >> #define TPS65219_BUCK2_VOUT_REG 0x9 > >> #define TPS65219_BUCK3_VOUT_REG 0x8 > >> -- > >> 2.34.1 > > > > > > -- > Best, > Shree Ramamoorthy > PMIC Software Engineer ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] power: replace magic numbers with macros 2024-12-18 23:26 ` Jaehoon Chung @ 2024-12-19 17:47 ` Shree Ramamoorthy 2024-12-20 2:04 ` Jaehoon Chung 0 siblings, 1 reply; 9+ messages in thread From: Shree Ramamoorthy @ 2024-12-19 17:47 UTC (permalink / raw) To: Jaehoon Chung, u-boot, trini; +Cc: m-leonard, praneeth Hi, On 12/18/24 5:26 PM, Jaehoon Chung wrote: > >> -----Original Message----- >> From: Shree Ramamoorthy <s-ramamoorthy@ti.com> >> Sent: Thursday, December 19, 2024 7:55 AM >> >> Hi. >> >> >> On 12/18/24 4:44 PM, Jaehoon Chung wrote: >>> Hi, >>> >>>> -----Original Message----- >>>> From: Shree Ramamoorthy <s-ramamoorthy@ti.com> >>>> Sent: Thursday, December 19, 2024 2:13 AM >>>> >>>> Replace magic numbers in buckval2votl() & buckvolt2val() with macros to >>>> help with clarity and correlate what the numbers correspond to in the >>>> TPS65219 datasheet. >>>> >>>> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com> >>>> --- >>>> drivers/power/regulator/tps65219_regulator.c | 26 ++++++++++---------- >>>> include/power/tps65219.h | 14 +++++++++-- >>>> 2 files changed, 25 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/drivers/power/regulator/tps65219_regulator.c >>>> b/drivers/power/regulator/tps65219_regulator.c >>>> index 4b0fb205909a..88abc896b3a2 100644 >>>> --- a/drivers/power/regulator/tps65219_regulator.c >>>> +++ b/drivers/power/regulator/tps65219_regulator.c >>>> @@ -72,12 +72,12 @@ static int tps65219_buck_enable(struct udevice *dev, int op, bool *enable) >>>> >>>> static int tps65219_buck_volt2val(int uV) >>>> { >>>> - if (uV > TPS65219_BUCK_VOLT_MAX) >>>> + if (uV > TPS65219_BUCK_3V4) >>>> return -EINVAL; >>>> - else if (uV >= 1400000) >>>> - return (uV - 1400000) / 100000 + 0x20; >>>> - else if (uV >= 600000) >>>> - return (uV - 600000) / 25000 + 0x00; >>>> + else if (uV >= TPS65219_BUCK_1V4) >>>> + return (uV - TPS65219_BUCK_1V4) / TPS65219_VOLT_STEP_100MV + TPS65219_BUCK_REG_1V4; >>> Even though Not relevant to this subject. If uV is 340000, the return value is correct? >>> >>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> >>> >>> Best Regards, >>> Jaehoon Chung >> Thank you for reviewing! >> The allowed max uV is 3.4V inclusive. >> If 340000 uV is detected, the first 'else if' statement should be taken. > Ah, I missed wrong read. Thanks for checking. If you can do, other tpsXXX can also be changed to readable macro. > > Best Regards, > Jaehoon Chung Absolutely, I will make a note to work on this. Besides tps6594, are there any specific tpsXXX drivers you had in mind? Or work on patches for any TI PMIC tpsXXX U-Boot drivers that include magic numbers? >> Best, >> Shree >> >>>> + else if (uV >= TPS65219_BUCK_0V6) >>>> + return (uV - TPS65219_BUCK_0V6) / TPS65219_VOLT_STEP_25MV + TPS65219_BUCK_REG_0V6; >>>> else >>>> return -EINVAL; >>>> } >>>> @@ -86,12 +86,12 @@ static int tps65219_buck_val2volt(int val) >>>> { >>>> if (val > TPS65219_VOLT_MASK) >>>> return -EINVAL; >>>> - else if (val > 0x34) >>>> - return TPS65219_BUCK_VOLT_MAX; >>>> - else if (val > 0x20) >>>> - return 1400000 + (val - 0x20) * 100000; >>>> - else if (val >= 0) >>>> - return 600000 + val * 25000; >>>> + else if (val > TPS65219_BUCK_REG_3V4) >>>> + return TPS65219_BUCK_3V4; >>>> + else if (val > TPS65219_BUCK_REG_1V4) >>>> + return TPS65219_BUCK_1V4 + (val - TPS65219_BUCK_REG_1V4) * TPS65219_VOLT_STEP_100MV; >>>> + else if (val >= TPS65219_BUCK_REG_0V6) >>>> + return TPS65219_BUCK_0V6 + val * TPS65219_VOLT_STEP_25MV; >>>> else >>>> return -EINVAL; >>>> } >>>> @@ -161,7 +161,7 @@ static int tps65219_ldo_volt2val(int idx, int uV) >>>> if (uV > max) >>>> return -EINVAL; >>>> else if (uV >= base) >>>> - return (uV - TPS65219_LDO12_VOLT_MIN) / 50000; >>>> + return (uV - TPS65219_LDO12_VOLT_MIN) / TPS65219_VOLT_STEP_50MV; >>>> else >>>> return -EINVAL; >>>> } >>>> @@ -187,7 +187,7 @@ static int tps65219_ldo_val2volt(int idx, int val) >>>> else if (val <= reg_base) >>>> return base; >>>> else if (val >= 0) >>>> - return TPS65219_LDO12_VOLT_MIN + (50000 * val); >>>> + return TPS65219_LDO12_VOLT_MIN + (TPS65219_VOLT_STEP_50MV * val); >>>> else >>>> return -EINVAL; >>>> } >>>> diff --git a/include/power/tps65219.h b/include/power/tps65219.h >>>> index aa81b92266fd..e8780af2d811 100644 >>>> --- a/include/power/tps65219.h >>>> +++ b/include/power/tps65219.h >>>> @@ -17,10 +17,20 @@ >>>> #define TPS65219_BUCK_DRIVER "tps65219_buck" >>>> >>>> #define TPS65219_VOLT_MASK 0x3F >>>> -#define TPS65219_BUCK_VOLT_MAX 3400000 >>>> - >>>> #define TPS65219_ENABLE_CTRL_REG 0x2 >>>> >>>> +#define TPS65219_VOLT_STEP_25MV 25000 >>>> +#define TPS65219_VOLT_STEP_50MV 50000 >>>> +#define TPS65219_VOLT_STEP_100MV 100000 >>>> + >>>> +#define TPS65219_BUCK_0V6 600000 >>>> +#define TPS65219_BUCK_1V4 1400000 >>>> +#define TPS65219_BUCK_3V4 3400000 >>>> + >>>> +#define TPS65219_BUCK_REG_0V6 0x00 >>>> +#define TPS65219_BUCK_REG_1V4 0x20 >>>> +#define TPS65219_BUCK_REG_3V4 0x34 >>>> + >>>> #define TPS65219_BUCK1_VOUT_REG 0xa >>>> #define TPS65219_BUCK2_VOUT_REG 0x9 >>>> #define TPS65219_BUCK3_VOUT_REG 0x8 >>>> -- >>>> 2.34.1 >>> >> -- >> Best, >> Shree Ramamoorthy >> PMIC Software Engineer > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v1 2/2] power: replace magic numbers with macros 2024-12-19 17:47 ` Shree Ramamoorthy @ 2024-12-20 2:04 ` Jaehoon Chung 0 siblings, 0 replies; 9+ messages in thread From: Jaehoon Chung @ 2024-12-20 2:04 UTC (permalink / raw) To: 'Shree Ramamoorthy', u-boot, trini; +Cc: m-leonard, praneeth > -----Original Message----- > From: Shree Ramamoorthy <s-ramamoorthy@ti.com> > Sent: Friday, December 20, 2024 2:47 AM > > Hi, > > On 12/18/24 5:26 PM, Jaehoon Chung wrote: > > > >> -----Original Message----- > >> From: Shree Ramamoorthy <s-ramamoorthy@ti.com> > >> Sent: Thursday, December 19, 2024 7:55 AM > >> > >> Hi. > >> > >> > >> On 12/18/24 4:44 PM, Jaehoon Chung wrote: > >>> Hi, > >>> > >>>> -----Original Message----- > >>>> From: Shree Ramamoorthy <s-ramamoorthy@ti.com> > >>>> Sent: Thursday, December 19, 2024 2:13 AM > >>>> > >>>> Replace magic numbers in buckval2votl() & buckvolt2val() with macros to > >>>> help with clarity and correlate what the numbers correspond to in the > >>>> TPS65219 datasheet. > >>>> > >>>> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@ti.com> > >>>> --- > >>>> drivers/power/regulator/tps65219_regulator.c | 26 ++++++++++---------- > >>>> include/power/tps65219.h | 14 +++++++++-- > >>>> 2 files changed, 25 insertions(+), 15 deletions(-) > >>>> > >>>> diff --git a/drivers/power/regulator/tps65219_regulator.c > >>>> b/drivers/power/regulator/tps65219_regulator.c > >>>> index 4b0fb205909a..88abc896b3a2 100644 > >>>> --- a/drivers/power/regulator/tps65219_regulator.c > >>>> +++ b/drivers/power/regulator/tps65219_regulator.c > >>>> @@ -72,12 +72,12 @@ static int tps65219_buck_enable(struct udevice *dev, int op, bool *enable) > >>>> > >>>> static int tps65219_buck_volt2val(int uV) > >>>> { > >>>> - if (uV > TPS65219_BUCK_VOLT_MAX) > >>>> + if (uV > TPS65219_BUCK_3V4) > >>>> return -EINVAL; > >>>> - else if (uV >= 1400000) > >>>> - return (uV - 1400000) / 100000 + 0x20; > >>>> - else if (uV >= 600000) > >>>> - return (uV - 600000) / 25000 + 0x00; > >>>> + else if (uV >= TPS65219_BUCK_1V4) > >>>> + return (uV - TPS65219_BUCK_1V4) / TPS65219_VOLT_STEP_100MV + TPS65219_BUCK_REG_1V4; > >>> Even though Not relevant to this subject. If uV is 340000, the return value is correct? > >>> > >>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> > >>> > >>> Best Regards, > >>> Jaehoon Chung > >> Thank you for reviewing! > >> The allowed max uV is 3.4V inclusive. > >> If 340000 uV is detected, the first 'else if' statement should be taken. > > Ah, I missed wrong read. Thanks for checking. If you can do, other tpsXXX can also be changed to > readable macro. > > > > Best Regards, > > Jaehoon Chung > > Absolutely, I will make a note to work on this. Besides tps6594, are there any specific tpsXXX drivers > you had in mind? > Or work on patches for any TI PMIC tpsXXX U-Boot drivers that include magic numbers? I didn't check the entire tpsXXX drivers in more detail. But I don't like using magic numbers. :) It's a difficult to know what purpose its value is. Best Regards, Jaehoon Chung > > >> Best, > >> Shree > >> > >>>> + else if (uV >= TPS65219_BUCK_0V6) > >>>> + return (uV - TPS65219_BUCK_0V6) / TPS65219_VOLT_STEP_25MV + TPS65219_BUCK_REG_0V6; > >>>> else > >>>> return -EINVAL; > >>>> } > >>>> @@ -86,12 +86,12 @@ static int tps65219_buck_val2volt(int val) > >>>> { > >>>> if (val > TPS65219_VOLT_MASK) > >>>> return -EINVAL; > >>>> - else if (val > 0x34) > >>>> - return TPS65219_BUCK_VOLT_MAX; > >>>> - else if (val > 0x20) > >>>> - return 1400000 + (val - 0x20) * 100000; > >>>> - else if (val >= 0) > >>>> - return 600000 + val * 25000; > >>>> + else if (val > TPS65219_BUCK_REG_3V4) > >>>> + return TPS65219_BUCK_3V4; > >>>> + else if (val > TPS65219_BUCK_REG_1V4) > >>>> + return TPS65219_BUCK_1V4 + (val - TPS65219_BUCK_REG_1V4) * TPS65219_VOLT_STEP_100MV; > >>>> + else if (val >= TPS65219_BUCK_REG_0V6) > >>>> + return TPS65219_BUCK_0V6 + val * TPS65219_VOLT_STEP_25MV; > >>>> else > >>>> return -EINVAL; > >>>> } > >>>> @@ -161,7 +161,7 @@ static int tps65219_ldo_volt2val(int idx, int uV) > >>>> if (uV > max) > >>>> return -EINVAL; > >>>> else if (uV >= base) > >>>> - return (uV - TPS65219_LDO12_VOLT_MIN) / 50000; > >>>> + return (uV - TPS65219_LDO12_VOLT_MIN) / TPS65219_VOLT_STEP_50MV; > >>>> else > >>>> return -EINVAL; > >>>> } > >>>> @@ -187,7 +187,7 @@ static int tps65219_ldo_val2volt(int idx, int val) > >>>> else if (val <= reg_base) > >>>> return base; > >>>> else if (val >= 0) > >>>> - return TPS65219_LDO12_VOLT_MIN + (50000 * val); > >>>> + return TPS65219_LDO12_VOLT_MIN + (TPS65219_VOLT_STEP_50MV * val); > >>>> else > >>>> return -EINVAL; > >>>> } > >>>> diff --git a/include/power/tps65219.h b/include/power/tps65219.h > >>>> index aa81b92266fd..e8780af2d811 100644 > >>>> --- a/include/power/tps65219.h > >>>> +++ b/include/power/tps65219.h > >>>> @@ -17,10 +17,20 @@ > >>>> #define TPS65219_BUCK_DRIVER "tps65219_buck" > >>>> > >>>> #define TPS65219_VOLT_MASK 0x3F > >>>> -#define TPS65219_BUCK_VOLT_MAX 3400000 > >>>> - > >>>> #define TPS65219_ENABLE_CTRL_REG 0x2 > >>>> > >>>> +#define TPS65219_VOLT_STEP_25MV 25000 > >>>> +#define TPS65219_VOLT_STEP_50MV 50000 > >>>> +#define TPS65219_VOLT_STEP_100MV 100000 > >>>> + > >>>> +#define TPS65219_BUCK_0V6 600000 > >>>> +#define TPS65219_BUCK_1V4 1400000 > >>>> +#define TPS65219_BUCK_3V4 3400000 > >>>> + > >>>> +#define TPS65219_BUCK_REG_0V6 0x00 > >>>> +#define TPS65219_BUCK_REG_1V4 0x20 > >>>> +#define TPS65219_BUCK_REG_3V4 0x34 > >>>> + > >>>> #define TPS65219_BUCK1_VOUT_REG 0xa > >>>> #define TPS65219_BUCK2_VOUT_REG 0x9 > >>>> #define TPS65219_BUCK3_VOUT_REG 0x8 > >>>> -- > >>>> 2.34.1 > >>> > >> -- > >> Best, > >> Shree Ramamoorthy > >> PMIC Software Engineer > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-12-20 2:05 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-18 17:12 [PATCH v1 0/2] TPS65219 U-Boot Driver Clean-Up Shree Ramamoorthy 2024-12-18 17:12 ` [PATCH v1 1/2] power: regulator: replace printf() with pr_err() Shree Ramamoorthy 2024-12-18 22:44 ` Jaehoon Chung 2024-12-18 17:12 ` [PATCH v1 2/2] power: replace magic numbers with macros Shree Ramamoorthy 2024-12-18 22:44 ` Jaehoon Chung 2024-12-18 22:55 ` Shree Ramamoorthy 2024-12-18 23:26 ` Jaehoon Chung 2024-12-19 17:47 ` Shree Ramamoorthy 2024-12-20 2:04 ` Jaehoon Chung
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.