drivers/iio/magnetometer/si7210.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)
Simplify probe by using devm_regulator_get_enable_read_voltage() to
get, enable and read the regulator voltage in a single call.
Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
Changes in v2:
- use vdd_uV naming convention
- remove unnecessary div_s64() for 32-bit division
drivers/iio/magnetometer/si7210.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/iio/magnetometer/si7210.c b/drivers/iio/magnetometer/si7210.c
index 27e3feba7a0f..8b98dd4dafa4 100644
--- a/drivers/iio/magnetometer/si7210.c
+++ b/drivers/iio/magnetometer/si7210.c
@@ -128,13 +128,13 @@ static const struct regmap_config si7210_regmap_conf = {
struct si7210_data {
struct regmap *regmap;
struct i2c_client *client;
- struct regulator *vdd;
struct mutex fetch_lock; /* lock for a single measurement fetch */
s8 temp_offset;
s8 temp_gain;
s8 scale_20_a[A_REGS_COUNT];
s8 scale_200_a[A_REGS_COUNT];
u8 curr_scale;
+ unsigned int vdd_uV;
};
static const struct iio_chan_spec si7210_channels[] = {
@@ -221,12 +221,8 @@ static int si7210_read_raw(struct iio_dev *indio_dev,
temp *= (1 + (data->temp_gain / 2048));
temp += (int)(MICRO / 16) * data->temp_offset;
- ret = regulator_get_voltage(data->vdd);
- if (ret < 0)
- return ret;
-
/* temp -= 0.222 * VDD */
- temp -= 222 * div_s64(ret, MILLI);
+ temp -= 222 * (data->vdd_uV / MILLI);
*val = div_s64(temp, MILLI);
@@ -396,14 +392,11 @@ static int si7210_probe(struct i2c_client *client)
return dev_err_probe(&client->dev, PTR_ERR(data->regmap),
"failed to register regmap\n");
- data->vdd = devm_regulator_get(&client->dev, "vdd");
- if (IS_ERR(data->vdd))
- return dev_err_probe(&client->dev, PTR_ERR(data->vdd),
- "failed to get VDD regulator\n");
-
- ret = regulator_enable(data->vdd);
- if (ret)
- return ret;
+ ret = devm_regulator_get_enable_read_voltage(&client->dev, "vdd");
+ if (ret < 0)
+ return dev_err_probe(&client->dev, ret,
+ "Failed to get vdd regulator\n");
+ data->vdd_uV = ret;
indio_dev->name = dev_name(&client->dev);
indio_dev->modes = INDIO_DIRECT_MODE;
--
2.43.0
On Wed, 11 Feb 2026 10:18:28 +0200
Antoniu Miclaus <antoniu.miclaus@analog.com> wrote:
> Simplify probe by using devm_regulator_get_enable_read_voltage() to
> get, enable and read the regulator voltage in a single call.
This needs to state that this is a functional change, but in real
deployments it is very rare for VDD to change after initial probe.
Note I once had a board where it did depending on driver
probe ordering as devices had slightly different requirements and
it was cut and paste from the driver I wrote for that which probably
led to so many drivers assuming it was dynamic.
Jonathan
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> Changes in v2:
> - use vdd_uV naming convention
> - remove unnecessary div_s64() for 32-bit division
> drivers/iio/magnetometer/si7210.c | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/si7210.c b/drivers/iio/magnetometer/si7210.c
> index 27e3feba7a0f..8b98dd4dafa4 100644
> --- a/drivers/iio/magnetometer/si7210.c
> +++ b/drivers/iio/magnetometer/si7210.c
> @@ -128,13 +128,13 @@ static const struct regmap_config si7210_regmap_conf = {
> struct si7210_data {
> struct regmap *regmap;
> struct i2c_client *client;
> - struct regulator *vdd;
> struct mutex fetch_lock; /* lock for a single measurement fetch */
> s8 temp_offset;
> s8 temp_gain;
> s8 scale_20_a[A_REGS_COUNT];
> s8 scale_200_a[A_REGS_COUNT];
> u8 curr_scale;
> + unsigned int vdd_uV;
> };
>
> static const struct iio_chan_spec si7210_channels[] = {
> @@ -221,12 +221,8 @@ static int si7210_read_raw(struct iio_dev *indio_dev,
> temp *= (1 + (data->temp_gain / 2048));
> temp += (int)(MICRO / 16) * data->temp_offset;
>
> - ret = regulator_get_voltage(data->vdd);
> - if (ret < 0)
> - return ret;
> -
> /* temp -= 0.222 * VDD */
> - temp -= 222 * div_s64(ret, MILLI);
> + temp -= 222 * (data->vdd_uV / MILLI);
>
> *val = div_s64(temp, MILLI);
>
> @@ -396,14 +392,11 @@ static int si7210_probe(struct i2c_client *client)
> return dev_err_probe(&client->dev, PTR_ERR(data->regmap),
> "failed to register regmap\n");
>
> - data->vdd = devm_regulator_get(&client->dev, "vdd");
> - if (IS_ERR(data->vdd))
> - return dev_err_probe(&client->dev, PTR_ERR(data->vdd),
> - "failed to get VDD regulator\n");
> -
> - ret = regulator_enable(data->vdd);
> - if (ret)
> - return ret;
> + ret = devm_regulator_get_enable_read_voltage(&client->dev, "vdd");
> + if (ret < 0)
> + return dev_err_probe(&client->dev, ret,
> + "Failed to get vdd regulator\n");
> + data->vdd_uV = ret;
>
> indio_dev->name = dev_name(&client->dev);
> indio_dev->modes = INDIO_DIRECT_MODE;
On Wed, Feb 11, 2026 at 10:18:28AM +0200, Antoniu Miclaus wrote:
> Simplify probe by using devm_regulator_get_enable_read_voltage() to
> get, enable and read the regulator voltage in a single call.
One nit-pick below, otherwise LGTM,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
...
> struct si7210_data {
> struct regmap *regmap;
> struct i2c_client *client;
> - struct regulator *vdd;
> struct mutex fetch_lock; /* lock for a single measurement fetch */
> s8 temp_offset;
> s8 temp_gain;
> s8 scale_20_a[A_REGS_COUNT];
> s8 scale_200_a[A_REGS_COUNT];
> u8 curr_scale;
> + unsigned int vdd_uV;
> };
Haven't noticed, but I believe this is not the best place to put a new member.
Does `pahole` agree with me or with you?
...
> /* temp -= 0.222 * VDD */
> - temp -= 222 * div_s64(ret, MILLI);
> + temp -= 222 * (data->vdd_uV / MILLI);
Just a side note: Here is a possible loss in precision, wouldn't be better to
have it as
temp -= 222 * data->vdd_uV / MILLI;
?
Note, if you agree this needs to be checked with the datasheet, etc. and sent
in a separate patch. For now the original way seems okay to me.
--
With Best Regards,
Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.