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 <[email protected]>
---
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
#define BD70528_LDO_VOLTS 0x20
#define BD70528_REG_BUCK1_EN 0x0F
--
2.25.1
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 <[email protected]>
> ---
> 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)
Best Regards
Matti Vaittinen
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland
SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
"non cogito me" dixit Rene Descarte, deinde evanescavit
(Thanks for the translation Simon)
Matti Vaittinen <[email protected]> 於 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 <[email protected]>
> > ---
> > 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?
Regards,
Axel
Morning Axel,
On Mon, 2021-05-24 at 14:06 +0800, Axel Lin wrote:
> Matti Vaittinen <[email protected]> 於 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 <[email protected]>
> > > ---
> > > 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 <[email protected]>
> Regards,
> Axel
On Sun, 23 May 2021 15:10:44 +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.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/2] regulator: bd70528: Fix off-by-one for buck123 .n_voltages setting
commit: 0514582a1a5b4ac1a3fd64792826d392d7ae9ddc
[2/2] regulator: bd71828: Fix .n_voltages settings
commit: 4c668630bf8ea90a041fc69c9984486e0f56682d
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark