[PATCH v1 0/2] TPS65219 U-Boot Driver Clean-Up
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(-)
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; }
-----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");@@ -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");-- 2.34.1
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
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;} @@ -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;} @@ -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;} @@ -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);} 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
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;@@ -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;@@ -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;@@ -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);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
-----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;@@ -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;@@ -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;@@ -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);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
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;@@ -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;@@ -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;@@ -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);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
-----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;@@ -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;@@ -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;@@ -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);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
participants (2)
-
Jaehoon Chung -
Shree Ramamoorthy