2023-06-09 08:12:32

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH v2] regulator: Use bitfield values for range selectors

Right now the regulator helpers expect raw register values for the range
selectors. This is different from the voltage selectors, which are
normalized as bitfield values. This leads to a bit of confusion. Also,
raw values are harder to copy from datasheets or match up with them,
as datasheets will typically have bitfield values.

Make the helpers expect bitfield values, and convert existing users.
Include bitops.h explicitly for ffs(), and reorder the header include
statements. While at it, also replace module.h with export.h, since the
only use is EXPORT_SYMBOL_GPL.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
Changes since v1:
- Fixed up bd71847_buck4_volt_range_sel in bd718x7-regulator.c

drivers/regulator/atc260x-regulator.c | 2 +-
drivers/regulator/bd718x7-regulator.c | 10 +++++-----
drivers/regulator/helpers.c | 9 ++++++---
drivers/regulator/tps6287x-regulator.c | 2 +-
4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/atc260x-regulator.c b/drivers/regulator/atc260x-regulator.c
index 87e237d740bc..0bba33955a1a 100644
--- a/drivers/regulator/atc260x-regulator.c
+++ b/drivers/regulator/atc260x-regulator.c
@@ -37,7 +37,7 @@ static const struct linear_range atc2609a_ldo_voltage_ranges1[] = {
};

static const unsigned int atc260x_ldo_voltage_range_sel[] = {
- 0x0, 0x20,
+ 0x0, 0x1,
};

static int atc260x_dcdc_set_voltage_time_sel(struct regulator_dev *rdev,
diff --git a/drivers/regulator/bd718x7-regulator.c b/drivers/regulator/bd718x7-regulator.c
index b0b9938c20a1..211ed9861417 100644
--- a/drivers/regulator/bd718x7-regulator.c
+++ b/drivers/regulator/bd718x7-regulator.c
@@ -289,7 +289,7 @@ static const struct linear_range bd71837_buck5_volts[] = {
* and 0x1 for last 3 ranges.
*/
static const unsigned int bd71837_buck5_volt_range_sel[] = {
- 0x0, 0x0, 0x0, 0x80, 0x80, 0x80
+ 0x0, 0x0, 0x0, 0x1, 0x1, 0x1
};

/*
@@ -309,7 +309,7 @@ static const struct linear_range bd71847_buck3_volts[] = {
};

static const unsigned int bd71847_buck3_volt_range_sel[] = {
- 0x0, 0x0, 0x0, 0x40, 0x80, 0x80, 0x80
+ 0x0, 0x0, 0x0, 0x1, 0x2, 0x2, 0x2
};

static const struct linear_range bd71847_buck4_volts[] = {
@@ -317,7 +317,7 @@ static const struct linear_range bd71847_buck4_volts[] = {
REGULATOR_LINEAR_RANGE(2600000, 0x00, 0x03, 100000),
};

-static const unsigned int bd71847_buck4_volt_range_sel[] = { 0x0, 0x40 };
+static const unsigned int bd71847_buck4_volt_range_sel[] = { 0x0, 0x1 };

/*
* BUCK6
@@ -360,7 +360,7 @@ static const struct linear_range bd718xx_ldo1_volts[] = {
REGULATOR_LINEAR_RANGE(1600000, 0x00, 0x03, 100000),
};

-static const unsigned int bd718xx_ldo1_volt_range_sel[] = { 0x0, 0x20 };
+static const unsigned int bd718xx_ldo1_volt_range_sel[] = { 0x0, 0x1 };

/*
* LDO2
@@ -403,7 +403,7 @@ static const struct linear_range bd71847_ldo5_volts[] = {
REGULATOR_LINEAR_RANGE(800000, 0x00, 0x0F, 100000),
};

-static const unsigned int bd71847_ldo5_volt_range_sel[] = { 0x0, 0x20 };
+static const unsigned int bd71847_ldo5_volt_range_sel[] = { 0x0, 0x1 };

/*
* LDO6
diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index 586f42e378ee..8bdafea70d36 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -5,13 +5,14 @@
// Copyright 2007, 2008 Wolfson Microelectronics PLC.
// Copyright 2008 SlimLogic Ltd.

-#include <linux/kernel.h>
-#include <linux/err.h>
+#include <linux/bitops.h>
#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/regulator/driver.h>
-#include <linux/module.h>

#include "internal.h"

@@ -108,6 +109,7 @@ static int regulator_range_selector_to_index(struct regulator_dev *rdev,
return -EINVAL;

rval &= rdev->desc->vsel_range_mask;
+ rval >>= ffs(rdev->desc->vsel_range_mask) - 1;

for (i = 0; i < rdev->desc->n_linear_ranges; i++) {
if (rdev->desc->linear_range_selectors[i] == rval)
@@ -195,6 +197,7 @@ int regulator_set_voltage_sel_pickable_regmap(struct regulator_dev *rdev,
sel += rdev->desc->linear_ranges[i].min_sel;

range = rdev->desc->linear_range_selectors[i];
+ range <<= ffs(rdev->desc->vsel_mask) - 1;

if (rdev->desc->vsel_reg == rdev->desc->vsel_range_reg) {
ret = regmap_update_bits(rdev->regmap,
diff --git a/drivers/regulator/tps6287x-regulator.c b/drivers/regulator/tps6287x-regulator.c
index 870e63ce3ff2..3139a0cbb6f7 100644
--- a/drivers/regulator/tps6287x-regulator.c
+++ b/drivers/regulator/tps6287x-regulator.c
@@ -41,7 +41,7 @@ static const struct linear_range tps6287x_voltage_ranges[] = {
};

static const unsigned int tps6287x_voltage_range_sel[] = {
- 0x0, 0x4, 0x8, 0xC
+ 0x0, 0x1, 0x2, 0x3
};

static const unsigned int tps6287x_ramp_table[] = {
--
2.41.0.162.gfafddb0af9-goog



2023-06-09 11:07:42

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH v2] regulator: Use bitfield values for range selectors

On 6/9/23 10:50, Chen-Yu Tsai wrote:
> Right now the regulator helpers expect raw register values for the range
> selectors. This is different from the voltage selectors, which are
> normalized as bitfield values. This leads to a bit of confusion. Also,
> raw values are harder to copy from datasheets or match up with them,
> as datasheets will typically have bitfield values.
>
> Make the helpers expect bitfield values, and convert existing users.
> Include bitops.h explicitly for ffs(), and reorder the header include
> statements. While at it, also replace module.h with export.h, since the
> only use is EXPORT_SYMBOL_GPL.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
For the helpers.c and bd718x7
Reviewed-by: Matti Vaittinen <[email protected]>

Please ignore my tag in v1. I accidentally replied to wrong mail, the
tag was meant for v2. Thanks for the heads-up Mark.

Yours,
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


2023-07-25 20:37:36

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2] regulator: Use bitfield values for range selectors

On Fri, 09 Jun 2023 15:50:30 +0800, Chen-Yu Tsai wrote:
> Right now the regulator helpers expect raw register values for the range
> selectors. This is different from the voltage selectors, which are
> normalized as bitfield values. This leads to a bit of confusion. Also,
> raw values are harder to copy from datasheets or match up with them,
> as datasheets will typically have bitfield values.
>
> Make the helpers expect bitfield values, and convert existing users.
> Include bitops.h explicitly for ffs(), and reorder the header include
> statements. While at it, also replace module.h with export.h, since the
> only use is EXPORT_SYMBOL_GPL.
>
> [...]

Applied to

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

Thanks!

[1/1] regulator: Use bitfield values for range selectors
commit: 269cb04b601dd8c35bbee180a9800335b93111fb

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