[PATCH RFT] regulator: atc260x: Fix n_voltages and min_sel for pickable linear ranges

[PATCH RFT] regulator: atc260x: Fix n_voltages and min_sel for pickable linear ranges

[Axel Lin]

The .n_voltages was missed for pickable linear ranges, fix it.
The min_sel for each pickable range should be starting from 0.

Fixes: 3b15ccac161a ("regulator: Add regulator driver for ATC260x PMICs")
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
Hi Cristian,
I don't have this h/w to test, please review and test this patch.
Thanks,
Axel
 drivers/regulator/atc260x-regulator.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/atc260x-regulator.c b/drivers/regulator/atc260x-regulator.c
index d8b429955d33..5dee02394cd9 100644
--- a/drivers/regulator/atc260x-regulator.c
+++ b/drivers/regulator/atc260x-regulator.c
@@ -28,12 +28,12 @@ static const struct linear_range atc2609a_dcdc_voltage_ranges[] = {
 
 static const struct linear_range atc2609a_ldo_voltage_ranges0[] = {
 	REGULATOR_LINEAR_RANGE(700000, 0, 15, 100000),
-	REGULATOR_LINEAR_RANGE(2100000, 16, 28, 100000),
+	REGULATOR_LINEAR_RANGE(2100000, 0, 12, 100000),
 };
 
 static const struct linear_range atc2609a_ldo_voltage_ranges1[] = {
 	REGULATOR_LINEAR_RANGE(850000, 0, 15, 100000),
-	REGULATOR_LINEAR_RANGE(2100000, 16, 27, 100000),
+	REGULATOR_LINEAR_RANGE(2100000, 0, 11, 100000),
 };
 
 static const unsigned int atc260x_ldo_voltage_range_sel[] = {
@@ -411,7 +411,7 @@ enum atc2609a_reg_ids {
 	.owner = THIS_MODULE, \
 }
 
-#define atc2609a_reg_desc_ldo_range_pick(num, n_range) { \
+#define atc2609a_reg_desc_ldo_range_pick(num, n_range, n_volt) { \
 	.name = "LDO"#num, \
 	.supply_name = "ldo"#num, \
 	.of_match = of_match_ptr("ldo"#num), \
@@ -421,6 +421,7 @@ enum atc2609a_reg_ids {
 	.type = REGULATOR_VOLTAGE, \
 	.linear_ranges = atc2609a_ldo_voltage_ranges##n_range, \
 	.n_linear_ranges = ARRAY_SIZE(atc2609a_ldo_voltage_ranges##n_range), \
+	.n_voltages = n_volt, \
 	.vsel_reg = ATC2609A_PMU_LDO##num##_CTL0, \
 	.vsel_mask = GENMASK(4, 1), \
 	.vsel_range_reg = ATC2609A_PMU_LDO##num##_CTL0, \
@@ -458,12 +459,12 @@ static const struct regulator_desc atc2609a_reg[] = {
 	atc2609a_reg_desc_ldo_bypass(0),
 	atc2609a_reg_desc_ldo_bypass(1),
 	atc2609a_reg_desc_ldo_bypass(2),
-	atc2609a_reg_desc_ldo_range_pick(3, 0),
-	atc2609a_reg_desc_ldo_range_pick(4, 0),
+	atc2609a_reg_desc_ldo_range_pick(3, 0, 29),
+	atc2609a_reg_desc_ldo_range_pick(4, 0, 29),
 	atc2609a_reg_desc_ldo(5),
-	atc2609a_reg_desc_ldo_range_pick(6, 1),
-	atc2609a_reg_desc_ldo_range_pick(7, 0),
-	atc2609a_reg_desc_ldo_range_pick(8, 0),
+	atc2609a_reg_desc_ldo_range_pick(6, 1, 28),
+	atc2609a_reg_desc_ldo_range_pick(7, 0, 29),
+	atc2609a_reg_desc_ldo_range_pick(8, 0, 29),
 	atc2609a_reg_desc_ldo_fixed(9),
 };
 
-- 
2.25.1

Re: [PATCH RFT] regulator: atc260x: Fix n_voltages and min_sel for pickable linear ranges

[Cristian Ciocaltea]

Hi Axel,

On Fri, May 28, 2021 at 08:38:26PM +0800, Axel Lin wrote:
> The .n_voltages was missed for pickable linear ranges, fix it.

Nice spot! I'm not quite sure if this is really mandatory to be manually
specified or gets computed internally based on the provided linear
ranges, but let's be on the safe side.

> The min_sel for each pickable range should be starting from 0.

Right, although in this particular case the driver behaves correctly
because the overflow bit of every item in the 2nd range is exactly the
range selector (bit 5 - LDO<N>_VOL_SEL in datasheet). Additionally, this
bit is not cleared internally as the selector mask (vsel_mask) is not
applied directly, but in conjunction with the range mask, according to
the implementation of "regulator_set_voltage_sel_pickable_regmap()":

	[...]
		ret = regmap_update_bits(rdev->regmap,
					 rdev->desc->vsel_reg,
					 rdev->desc->vsel_range_mask |
					 rdev->desc->vsel_mask, sel | range);
	[...]

As a matter of fact, using 0-based ranges will break the driver because
the 2nd item in the range sel array "atc260x_ldo_voltage_range_sel" is
not correct, it should be 0x20 instead of 0x1 (0x1<<5, since it is not
shifted automatically as in the case of the voltage selectors).

> Fixes: 3b15ccac161a ("regulator: Add regulator driver for ATC260x PMICs")
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
> Hi Cristian,
> I don't have this h/w to test, please review and test this patch.

Unfortunately I can only test the ATC2603C variant, I haven't been able
to get a 2609A based board yet.

Thanks,
Cristi

> Thanks,
> Axel
>  drivers/regulator/atc260x-regulator.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/regulator/atc260x-regulator.c b/drivers/regulator/atc260x-regulator.c
> index d8b429955d33..5dee02394cd9 100644
> --- a/drivers/regulator/atc260x-regulator.c
> +++ b/drivers/regulator/atc260x-regulator.c
> @@ -28,12 +28,12 @@ static const struct linear_range atc2609a_dcdc_voltage_ranges[] = {
>  
>  static const struct linear_range atc2609a_ldo_voltage_ranges0[] = {
>  	REGULATOR_LINEAR_RANGE(700000, 0, 15, 100000),
> -	REGULATOR_LINEAR_RANGE(2100000, 16, 28, 100000),
> +	REGULATOR_LINEAR_RANGE(2100000, 0, 12, 100000),
>  };
>  
>  static const struct linear_range atc2609a_ldo_voltage_ranges1[] = {
>  	REGULATOR_LINEAR_RANGE(850000, 0, 15, 100000),
> -	REGULATOR_LINEAR_RANGE(2100000, 16, 27, 100000),
> +	REGULATOR_LINEAR_RANGE(2100000, 0, 11, 100000),
>  };
>  
>  static const unsigned int atc260x_ldo_voltage_range_sel[] = {
> @@ -411,7 +411,7 @@ enum atc2609a_reg_ids {
>  	.owner = THIS_MODULE, \
>  }
>  
> -#define atc2609a_reg_desc_ldo_range_pick(num, n_range) { \
> +#define atc2609a_reg_desc_ldo_range_pick(num, n_range, n_volt) { \
>  	.name = "LDO"#num, \
>  	.supply_name = "ldo"#num, \
>  	.of_match = of_match_ptr("ldo"#num), \
> @@ -421,6 +421,7 @@ enum atc2609a_reg_ids {
>  	.type = REGULATOR_VOLTAGE, \
>  	.linear_ranges = atc2609a_ldo_voltage_ranges##n_range, \
>  	.n_linear_ranges = ARRAY_SIZE(atc2609a_ldo_voltage_ranges##n_range), \
> +	.n_voltages = n_volt, \
>  	.vsel_reg = ATC2609A_PMU_LDO##num##_CTL0, \
>  	.vsel_mask = GENMASK(4, 1), \
>  	.vsel_range_reg = ATC2609A_PMU_LDO##num##_CTL0, \
> @@ -458,12 +459,12 @@ static const struct regulator_desc atc2609a_reg[] = {
>  	atc2609a_reg_desc_ldo_bypass(0),
>  	atc2609a_reg_desc_ldo_bypass(1),
>  	atc2609a_reg_desc_ldo_bypass(2),
> -	atc2609a_reg_desc_ldo_range_pick(3, 0),
> -	atc2609a_reg_desc_ldo_range_pick(4, 0),
> +	atc2609a_reg_desc_ldo_range_pick(3, 0, 29),
> +	atc2609a_reg_desc_ldo_range_pick(4, 0, 29),
>  	atc2609a_reg_desc_ldo(5),
> -	atc2609a_reg_desc_ldo_range_pick(6, 1),
> -	atc2609a_reg_desc_ldo_range_pick(7, 0),
> -	atc2609a_reg_desc_ldo_range_pick(8, 0),
> +	atc2609a_reg_desc_ldo_range_pick(6, 1, 28),
> +	atc2609a_reg_desc_ldo_range_pick(7, 0, 29),
> +	atc2609a_reg_desc_ldo_range_pick(8, 0, 29),
>  	atc2609a_reg_desc_ldo_fixed(9),
>  };
>  
> -- 
> 2.25.1
> 

Re: [PATCH RFT] regulator: atc260x: Fix n_voltages and min_sel for pickable linear ranges

[Mark Brown]

On Fri, 28 May 2021 20:38:26 +0800, Axel Lin wrote:
> The .n_voltages was missed for pickable linear ranges, fix it.
> The min_sel for each pickable range should be starting from 0.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: atc260x: Fix n_voltages and min_sel for pickable linear ranges
      commit: 1963fa67d78674a110bc9b2a8b1e226967692f05

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