[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0e3816724876c0040a3462322f8db35aabe109fc.camel@fi.rohmeurope.com>
Date: Mon, 24 May 2021 06:20:25 +0000
From: "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To: "axel.lin@...ics.com" <axel.lin@...ics.com>
CC: "lgirdwood@...il.com" <lgirdwood@...il.com>,
linux-power <linux-power@...rohmeurope.com>,
"broonie@...nel.org" <broonie@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"lee.jones@...aro.org" <lee.jones@...aro.org>
Subject: Re: [PATCH 1/2] regulator: bd70528: Fix off-by-one for buck123
.n_voltages setting
Morning Axel,
On Mon, 2021-05-24 at 14:06 +0800, Axel Lin wrote:
> Matti Vaittinen <matti.vaittinen@...rohmeurope.com> 於 2021年5月24日 週一
> 下午1:41寫道:
> > Hi Axel,
> >
> > On Sun, 2021-05-23 at 15:10 +0800, Axel Lin wrote:
> > > The valid selectors for bd70528 bucks are 0 ~ 0xf, so the
> > > .n_voltages
> > > should be 16 (0x10). Use 0x10 to make it consistent with
> > > BD70528_LDO_VOLTS.
> > > Also remove redundant defines for BD70528_BUCK_VOLTS.
> > >
> > > Signed-off-by: Axel Lin <axel.lin@...ics.com>
> > > ---
> > > I think this fix does not need "Fixes" tag because in original
> > > code
> > > the
> > > .n_voltage is greater than correct one. The latest selector is
> > > not
> > > valid
> > > in the linear range setting anyway.
> > > include/linux/mfd/rohm-bd70528.h | 4 +---
> > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/include/linux/mfd/rohm-bd70528.h
> > > b/include/linux/mfd/rohm-bd70528.h
> > > index a57af878fd0c..4a5966475a35 100644
> > > --- a/include/linux/mfd/rohm-bd70528.h
> > > +++ b/include/linux/mfd/rohm-bd70528.h
> > > @@ -26,9 +26,7 @@ struct bd70528_data {
> > > struct mutex rtc_timer_lock;
> > > };
> > >
> > > -#define BD70528_BUCK_VOLTS 17
> > > -#define BD70528_BUCK_VOLTS 17
> > > -#define BD70528_BUCK_VOLTS 17
> > > +#define BD70528_BUCK_VOLTS 0x10
> >
> > Thank you for fixing this. There really is only 16 valid voltage
> > settings as you pointed out. Regarding changing the define to hex -
> > I
> > would prefer seeing the amount in decimal as it is easier to
> > understand. (I do understand bit-patterns better when in HEX - but
> > "real world" values like voltages, currents or amounts are easier
> > (for
> > me) to understand when in decimals)
>
> Current code already uses hex-decimal (which is the reason I use
> hex-decimal for BD70528_BUCK_VOLTS):
> #define BD70528_LDO_VOLTS 0x20
>
> So do you suggest to change BD70528_LDO_VOLTS to decimal as well?
>
In my opinion that would be better. So if you feel like - then yes
please. After that being said - I am not expecting you to do it - I
appreciate the fix as it is.
Acked-by: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
> Regards,
> Axel
Powered by blists - more mailing lists