Messages in this thread | | | Date | Tue, 30 Apr 2024 09:31:44 -0700 | Subject | Re: [PATCH v2 2/7] hwmon: (adc128d818) Use devm_regulator_get_enable_read_voltage() | From | Guenter Roeck <> |
| |
On 4/29/24 16:40, David Lechner wrote: > We can reduce boilerplate code and eliminate the driver remove() > function by using devm_regulator_get_enable_read_voltage(). > > A new external_vref flag is added since we no longer have the handle > to the regulator to check if it is present. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: David Lechner <dlechner@baylibre.com>
Nit below in case you resend, otherwise
Acked-by: Guenter Roeck <linux@roeck-us.net>
> --- > > v2 changes: > * rename to devm_regulator_get_enable_read_voltage() > * use vref instead of err for return value > * simplify last error check to return PTR_ERR directly > --- > drivers/hwmon/adc128d818.c | 57 ++++++++++++++-------------------------------- > 1 file changed, 17 insertions(+), 40 deletions(-) > > diff --git a/drivers/hwmon/adc128d818.c b/drivers/hwmon/adc128d818.c > index 46e3c8c50765..2a35acb011eb 100644 > --- a/drivers/hwmon/adc128d818.c > +++ b/drivers/hwmon/adc128d818.c > @@ -58,7 +58,6 @@ static const u8 num_inputs[] = { 7, 8, 4, 6 }; > > struct adc128_data { > struct i2c_client *client; > - struct regulator *regulator; > int vref; /* Reference voltage in mV */ > struct mutex update_lock; > u8 mode; /* Operation mode */ > @@ -389,7 +388,7 @@ static int adc128_detect(struct i2c_client *client, struct i2c_board_info *info) > return 0; > } > > -static int adc128_init_client(struct adc128_data *data) > +static int adc128_init_client(struct adc128_data *data, bool external_vref) > { > struct i2c_client *client = data->client; > int err; > @@ -408,7 +407,7 @@ static int adc128_init_client(struct adc128_data *data) > regval |= data->mode << 1; > > /* If external vref is selected, configure the chip to use it */ > - if (data->regulator) > + if (external_vref) > regval |= 0x01; > > /* Write advanced configuration register */ > @@ -430,9 +429,9 @@ static int adc128_init_client(struct adc128_data *data) > static int adc128_probe(struct i2c_client *client) > { > struct device *dev = &client->dev; > - struct regulator *regulator; > struct device *hwmon_dev; > struct adc128_data *data; > + bool external_vref; > int err, vref; > > data = devm_kzalloc(dev, sizeof(struct adc128_data), GFP_KERNEL); > @@ -440,20 +439,15 @@ static int adc128_probe(struct i2c_client *client) > return -ENOMEM; > > /* vref is optional. If specified, is used as chip reference voltage */ > - regulator = devm_regulator_get_optional(dev, "vref"); > - if (!IS_ERR(regulator)) { > - data->regulator = regulator; > - err = regulator_enable(regulator); > - if (err < 0) > - return err; > - vref = regulator_get_voltage(regulator); > - if (vref < 0) { > - err = vref; > - goto error; > - } > - data->vref = DIV_ROUND_CLOSEST(vref, 1000); > - } else { > + vref = devm_regulator_get_enable_read_voltage(dev, "vref"); > + if (vref == -ENODEV) { > + external_vref = false; > data->vref = 2560; /* 2.56V, in mV */ > + } else if (vref < 0) { > + return vref; > + } else { > + external_vref = true; > + data->vref = DIV_ROUND_CLOSEST(vref, 1000); > } > > /* Operation mode is optional. If unspecified, keep current mode */ > @@ -461,13 +455,12 @@ static int adc128_probe(struct i2c_client *client) > if (data->mode > 3) { > dev_err(dev, "invalid operation mode %d\n", > data->mode); > - err = -EINVAL; > - goto error; > + return -EINVAL; > } > } else { > err = i2c_smbus_read_byte_data(client, ADC128_REG_CONFIG_ADV); > if (err < 0) > - goto error; > + return err; > data->mode = (err >> 1) & ADC128_REG_MASK; > } > > @@ -476,31 +469,16 @@ static int adc128_probe(struct i2c_client *client) > mutex_init(&data->update_lock); > > /* Initialize the chip */ > - err = adc128_init_client(data); > + err = adc128_init_client(data, external_vref); > if (err < 0) > - goto error; > + return err; > > hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name, > data, adc128_groups); > - if (IS_ERR(hwmon_dev)) { > - err = PTR_ERR(hwmon_dev); > - goto error; > - } > + if (IS_ERR(hwmon_dev)) > + return PTR_ERR(hwmon_dev); > > return 0;
return PTR_ERR_OR_ZERO(hwmon_dev);
> - > -error: > - if (data->regulator) > - regulator_disable(data->regulator); > - return err; > -} > - > -static void adc128_remove(struct i2c_client *client) > -{ > - struct adc128_data *data = i2c_get_clientdata(client); > - > - if (data->regulator) > - regulator_disable(data->regulator); > } > > static const struct i2c_device_id adc128_id[] = { > @@ -522,7 +500,6 @@ static struct i2c_driver adc128_driver = { > .of_match_table = of_match_ptr(adc128_of_match), > }, > .probe = adc128_probe, > - .remove = adc128_remove, > .id_table = adc128_id, > .detect = adc128_detect, > .address_list = normal_i2c, >
| |