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
