Re: [PATCH 2/2] regulator: bd71828: Fix .n_voltages settings

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 2/2] regulator: bd71828: Fix .n_voltages settings
Date: Mon, 24 May 2021 09:17:11 +0300	[thread overview]
Message-ID: <5639c1af646367caf345d0c346dded4e52178030.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <20210523071045.2168904-2-axel.lin@ingics.com>

On Sun, 2021-05-23 at 15:10 +0800, Axel Lin wrote:
> Current .n_voltages settings do not cover the latest 2 valid
> selectors,
> so it fails to set voltage for the hightest voltage support.
> The latest linear range has step_uV = 0, so it does not matter if we
> count the .n_voltages to maximum selector + 1 or the first selector
> of
> latest linear range + 1.
> To simplify calculating the n_voltages, let's just set the
> .n_voltages to maximum selector + 1.
> 
> Fixes: 522498f8cb8c ("regulator: bd71828: Basic support for ROHM
> bd71828 PMIC regulators")
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---

Thank you Axel. I never stop being surprized by your accuracy what
comes spotting errors like this. I had to look-up my calculator and the
data-sheet to verify your fix - and you did find this just by reviewing
the existing code(?) Really impressive. It seems the biggest supported
voltage (2V) was really not reachable as it only belonged to the last
(step 0) range. Big thanks!

Reviewed-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

>  include/linux/mfd/rohm-bd71828.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mfd/rohm-bd71828.h
> b/include/linux/mfd/rohm-bd71828.h
> index c7ab69c87ee8..3b5f3a7db4bd 100644
> --- a/include/linux/mfd/rohm-bd71828.h
> +++ b/include/linux/mfd/rohm-bd71828.h
> @@ -26,11 +26,11 @@ enum {
>  	BD71828_REGULATOR_AMOUNT,
>  };
>  
> -#define BD71828_BUCK1267_VOLTS		0xEF
> -#define BD71828_BUCK3_VOLTS		0x10
> -#define BD71828_BUCK4_VOLTS		0x20
> -#define BD71828_BUCK5_VOLTS		0x10
> -#define BD71828_LDO_VOLTS		0x32
> +#define BD71828_BUCK1267_VOLTS		0x100
> +#define BD71828_BUCK3_VOLTS		0x20
> +#define BD71828_BUCK4_VOLTS		0x40
> +#define BD71828_BUCK5_VOLTS		0x20
> +#define BD71828_LDO_VOLTS		0x40
>  /* LDO6 is fixed 1.8V voltage */
>  #define BD71828_LDO_6_VOLTAGE		1800000
>  