Hi Udit,
On 29/01/26 20:45, Kumar, Udit wrote:
>
> On 1/29/2026 12:58 PM, Neha Malcom Francis wrote:
>> Depending on the phase selection (single or multi), the FPWM bits
>> configured forces the regulator to operate in PWM mode. In case of
>> multi-phase selection, the FPWM_MP bits enforce the regulator to also
>> operate in multi-phase. This fixes correct multi-phase operation.
>>
>> Fixes: 065a452ae6a1 ("power: regulator: tps65941: add regulator support")
>> Link: https://www.ti.com/lit/ds/symlink/tps6594-q1.pdf
>> Signed-off-by: Keerthy <[email protected]>
>> Signed-off-by: Takuma Fujiwara <[email protected]>
>> Signed-off-by: Neha Malcom Francis <[email protected]>
>> ---
>> Tested on J721S2-EVM
>>
>> https://gist.github.com/nehamalcom/a74fb2a3b59421d0e04693c7d244fdb4
>>
>> Before the patch, the kernel TPS6594 regulator driver would warn about
>> overcurrent on a buck, this was due to incorrect multi-phase
>> configuration.
>> This patch resolves it.
>>
>> drivers/power/regulator/tps65941_regulator.c | 13 ++++++++++---
>> include/power/tps65941.h | 2 ++
>> 2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/power/regulator/tps65941_regulator.c b/drivers/
>> power/regulator/tps65941_regulator.c
>> index 2561d6f4c6c..e0297302914 100644
>> --- a/drivers/power/regulator/tps65941_regulator.c
>> +++ b/drivers/power/regulator/tps65941_regulator.c
>> @@ -63,13 +63,14 @@ static inline int tps65941_get_chip_id(struct
>> udevice *dev)
>> static int tps65941_buck_enable(struct udevice *dev, int op, bool
>> *enable)
>> {
>> - int ret;
>> + int ret, idx;
>> unsigned int adr;
>> struct dm_regulator_uclass_plat *uc_pdata;
>> uc_pdata = dev_get_uclass_plat(dev);
>> adr = uc_pdata->ctrl_reg;
>> + idx = dev->driver_data;
>> ret = pmic_reg_read(dev->parent, adr);
>> if (ret < 0)
>> return ret;
>> @@ -84,10 +85,16 @@ static int tps65941_buck_enable(struct udevice
>> *dev, int op, bool *enable)
>> return 0;
>> } else if (op == PMIC_OP_SET) {
>> - if (*enable)
>> + if (*enable) {
>> ret |= TPS65941_BUCK_MODE_MASK;
>> - else
>> + /* Enable FPWM */
>> + ret |= TPS65941_BUCK_FPWM_MASK;
>
>
> I am wondering, why you need to set this bit , I see default/reset value
> is 1.
Even though the reset value is 1, NVM configurations done by HW can
override this.
>
>
>> + /* If Multiphase enable FPWM_MP */
>> + if (idx == 12 || idx == 123 || idx == 1234)
>> + ret |= TPS65941_BUCK_FPWM_MP_MASK;
>
> you can use TPS65941_BUCK_ID_12, TPS65941_BUCK_ID_123 and and
> TPS65941_BUCK_ID_1234
>
> instead of 12, 123 and 1234
Will make this change
>
>
>> + } else {
>> ret &= ~TPS65941_BUCK_MODE_MASK;
>> + }
>> ret = pmic_reg_write(dev->parent, adr, ret);
>> if (ret)
>> return ret;
>> diff --git a/include/power/tps65941.h b/include/power/tps65941.h
>> index a026ec56958..a8b1044b2f9 100644
>> --- a/include/power/tps65941.h
>> +++ b/include/power/tps65941.h
>> @@ -20,6 +20,8 @@
>> #define TPS65941_BUCK_VOLT_MAX_HEX 0xFF
>> #define TPS65941_BUCK_VOLT_MAX 3340000
>> #define TPS65941_BUCK_MODE_MASK 0x1
>> +#define TPS65941_BUCK_FPWM_MASK 0x2
>> +#define TPS65941_BUCK_FPWM_MP_MASK 0x4
>
>
> Check for alignment please
Will rectify
>
>
>> #define TPS65941_LDO_VOLT_MASK 0x7E
>> #define TPS65941_LDO_VOLT_MAX_HEX 0x3A
Thanks for the review!
--
Thanking You
Neha Malcom Francis
