Re: [PATCH v1 5/5] power: regulator: tps65941: Add TPS65224 PMIC regulator support

On Fri, 23 Feb 2024 14:37:48 +0300, Dan Carpenter wrote:

On Fri, Feb 23, 2024 at 02:42:12PM +0530, Bhargav Raviprakash wrote:

...

• int mask = TPS65224_LDO_VOLT_MASK >> 1;
• if (idx > 0) {

• base = TPS65224_LDO23_VOLT_MIN;
  

• max = TPS65224_LDO23_VOLT_MAX;
  

• reg_base = TPS65224_LDO23_VOLT_MIN_HEX;
  

• reg_max = TPS65224_LDO23_VOLT_MAX_HEX;
  

• }
• val = val >> 1;
• if (val > mask || val < 0)

• return -EINVAL;
  

• else if (val >= reg_max)

• return max;
  

• else if (val <= reg_base)

• return base;
  

• else if (val >= 0)

• return base + (step * (val - reg_base));
  

• else

• return -EINVAL;
  

Instead of "if (val >= 0)" it would be more clear to write "if (value > reg_base)". Or something like this:

val = val >> 1; if (val < 0 || val > mask) return -EINVAL;

if (val <= reg_base) return base; if (val >= reg_max) return max;

return base + (step * (val - reg_base));

regards, dan carpenter

Thanks! Will change it accordingly in next version.

Regards, Bhargav