Re: [PATCH 1/2] regulator: bd70528: Fix off-by-one for buck123 .n_voltages setting

From: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
To: Axel Lin <axel.lin@ingics.com>, Mark Brown <broonie@kernel.org>
Cc: Lee Jones <lee.jones@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	linux-kernel@vger.kernel.org, linux-power@fi.rohmeurope.com
Subject: Re: [PATCH 1/2] regulator: bd70528: Fix off-by-one for buck123 .n_voltages setting
Date: Mon, 24 May 2021 08:41:06 +0300	[thread overview]
Message-ID: <4189cb2094b4fe43e351eb1d80ca6c4cb1ac08ed.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <20210523071045.2168904-1-axel.lin@ingics.com>

[-- Attachment #1: Type: text/plain, Size: 1815 bytes --]

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@ingics.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)

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)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]