| Message ID | 8b1193fdefb231a6d721e2bded52c48e56039c20.1660934107.git.mazziesaccount@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Use devm helpers for regulator get and enable | expand |
> From: Matti Vaittinen <mazziesaccount@gmail.com> > Sent: Friday, August 19, 2022 9:21 PM > To: Matti Vaittinen <mazziesaccount@gmail.com>; Matti Vaittinen > <matti.vaittinen@fi.rohmeurope.com> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael > <Michael.Hennerich@analog.com>; Jonathan Cameron > <jic23@kernel.org>; linux-iio@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: [PATCH v3 14/14] iio: hmc425a: simplify using > devm_regulator_get_enable() > > [External] > > Drop open-coded pattern: 'devm_regulator_get(), regulator_enable(), > add_action_or_reset(regulator_disable)' and use the > devm_regulator_get_enable() and drop the pointer to the regulator. > This simplifies code and makes it less tempting to add manual control > for the regulator which is also controlled by devm. > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > > --- Acked-by: Nuno Sá <nuno.sa@analog.com> (I see that in this patch you are not using dev_err_probe(). Maybe some consistency in the series and where applicable would be appropriate :))
On 8/30/22 14:49, Sa, Nuno wrote: > >> From: Matti Vaittinen <mazziesaccount@gmail.com> >> Sent: Friday, August 19, 2022 9:21 PM >> To: Matti Vaittinen <mazziesaccount@gmail.com>; Matti Vaittinen >> <matti.vaittinen@fi.rohmeurope.com> >> Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael >> <Michael.Hennerich@analog.com>; Jonathan Cameron >> <jic23@kernel.org>; linux-iio@vger.kernel.org; linux- >> kernel@vger.kernel.org >> Subject: [PATCH v3 14/14] iio: hmc425a: simplify using >> devm_regulator_get_enable() >> >> [External] >> >> Drop open-coded pattern: 'devm_regulator_get(), regulator_enable(), >> add_action_or_reset(regulator_disable)' and use the >> devm_regulator_get_enable() and drop the pointer to the regulator. >> This simplifies code and makes it less tempting to add manual control >> for the regulator which is also controlled by devm. >> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> >> >> --- > > Acked-by: Nuno Sá <nuno.sa@analog.com> > > (I see that in this patch you are not using dev_err_probe(). Maybe some > consistency in the series and where applicable would be appropriate :)) I don't think the driver did originally print an error if regulator get or enable failed. I didn't want to add any new prints - just converted the existing ones to use dev_err_probe(). I believe adding new prints would've been somewhat unrelated change :) Yours, -- Matti
> -----Original Message----- > From: Matti Vaittinen <mazziesaccount@gmail.com> > Sent: Tuesday, August 30, 2022 3:00 PM > To: Sa, Nuno <Nuno.Sa@analog.com>; Matti Vaittinen > <matti.vaittinen@fi.rohmeurope.com> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael > <Michael.Hennerich@analog.com>; Jonathan Cameron > <jic23@kernel.org>; linux-iio@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v3 14/14] iio: hmc425a: simplify using > devm_regulator_get_enable() > > [External] > > On 8/30/22 14:49, Sa, Nuno wrote: > > > >> From: Matti Vaittinen <mazziesaccount@gmail.com> > >> Sent: Friday, August 19, 2022 9:21 PM > >> To: Matti Vaittinen <mazziesaccount@gmail.com>; Matti Vaittinen > >> <matti.vaittinen@fi.rohmeurope.com> > >> Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael > >> <Michael.Hennerich@analog.com>; Jonathan Cameron > >> <jic23@kernel.org>; linux-iio@vger.kernel.org; linux- > >> kernel@vger.kernel.org > >> Subject: [PATCH v3 14/14] iio: hmc425a: simplify using > >> devm_regulator_get_enable() > >> > >> [External] > >> > >> Drop open-coded pattern: 'devm_regulator_get(), > regulator_enable(), > >> add_action_or_reset(regulator_disable)' and use the > >> devm_regulator_get_enable() and drop the pointer to the > regulator. > >> This simplifies code and makes it less tempting to add manual > control > >> for the regulator which is also controlled by devm. > >> > >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > >> > >> --- > > > > Acked-by: Nuno Sá <nuno.sa@analog.com> > > > > (I see that in this patch you are not using dev_err_probe(). Maybe > some > > consistency in the series and where applicable would be appropriate > :)) > > I don't think the driver did originally print an error if regulator get > or enable failed. I didn't want to add any new prints - just converted > the existing ones to use dev_err_probe(). I believe adding new prints > would've been somewhat unrelated change :) > Ahh that makes sense. I failed to see the print that you were replacing in the ad7606 patch... - Nuno Sá
On Tue, 30 Aug 2022 13:55:38 +0000 "Sa, Nuno" <Nuno.Sa@analog.com> wrote: > > -----Original Message----- > > From: Matti Vaittinen <mazziesaccount@gmail.com> > > Sent: Tuesday, August 30, 2022 3:00 PM > > To: Sa, Nuno <Nuno.Sa@analog.com>; Matti Vaittinen > > <matti.vaittinen@fi.rohmeurope.com> > > Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael > > <Michael.Hennerich@analog.com>; Jonathan Cameron > > <jic23@kernel.org>; linux-iio@vger.kernel.org; linux- > > kernel@vger.kernel.org > > Subject: Re: [PATCH v3 14/14] iio: hmc425a: simplify using > > devm_regulator_get_enable() > > > > [External] > > > > On 8/30/22 14:49, Sa, Nuno wrote: > > > > > >> From: Matti Vaittinen <mazziesaccount@gmail.com> > > >> Sent: Friday, August 19, 2022 9:21 PM > > >> To: Matti Vaittinen <mazziesaccount@gmail.com>; Matti Vaittinen > > >> <matti.vaittinen@fi.rohmeurope.com> > > >> Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael > > >> <Michael.Hennerich@analog.com>; Jonathan Cameron > > >> <jic23@kernel.org>; linux-iio@vger.kernel.org; linux- > > >> kernel@vger.kernel.org > > >> Subject: [PATCH v3 14/14] iio: hmc425a: simplify using > > >> devm_regulator_get_enable() > > >> > > >> [External] > > >> > > >> Drop open-coded pattern: 'devm_regulator_get(), > > regulator_enable(), > > >> add_action_or_reset(regulator_disable)' and use the > > >> devm_regulator_get_enable() and drop the pointer to the > > regulator. > > >> This simplifies code and makes it less tempting to add manual > > control > > >> for the regulator which is also controlled by devm. > > >> > > >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > > >> > > >> --- > > > > > > Acked-by: Nuno Sá <nuno.sa@analog.com> > > > > > > (I see that in this patch you are not using dev_err_probe(). Maybe > > some > > > consistency in the series and where applicable would be appropriate > > :)) > > > > I don't think the driver did originally print an error if regulator get > > or enable failed. I didn't want to add any new prints - just converted > > the existing ones to use dev_err_probe(). I believe adding new prints > > would've been somewhat unrelated change :) > > > > Ahh that makes sense. I failed to see the print that you were replacing > in the ad7606 patch... > Applied to the togreg branch of iio.git though for now that's just pushed out as testing. As I mentioned on an earlier patch I forgot these existed and was about to duplicate some of the work.. Anyhow, we only overlapped on a few patches so I'll send the rest of my set out shortly. There are probably other cases still in IIO but they are fiddly to grep for! Jonathan > - Nuno Sá
diff --git a/drivers/iio/amplifiers/hmc425a.c b/drivers/iio/amplifiers/hmc425a.c index ce80e0c916f4..108f0f1685ef 100644 --- a/drivers/iio/amplifiers/hmc425a.c +++ b/drivers/iio/amplifiers/hmc425a.c @@ -34,7 +34,6 @@ struct hmc425a_chip_info { }; struct hmc425a_state { - struct regulator *reg; struct mutex lock; /* protect sensor state */ struct hmc425a_chip_info *chip_info; struct gpio_descs *gpios; @@ -162,13 +161,6 @@ static const struct of_device_id hmc425a_of_match[] = { }; MODULE_DEVICE_TABLE(of, hmc425a_of_match); -static void hmc425a_reg_disable(void *data) -{ - struct hmc425a_state *st = data; - - regulator_disable(st->reg); -} - static struct hmc425a_chip_info hmc425a_chip_info_tbl[] = { [ID_HMC425A] = { .name = "hmc425a", @@ -211,14 +203,7 @@ static int hmc425a_probe(struct platform_device *pdev) return -ENODEV; } - st->reg = devm_regulator_get(&pdev->dev, "vcc-supply"); - if (IS_ERR(st->reg)) - return PTR_ERR(st->reg); - - ret = regulator_enable(st->reg); - if (ret) - return ret; - ret = devm_add_action_or_reset(&pdev->dev, hmc425a_reg_disable, st); + ret = devm_regulator_get_enable(&pdev->dev, "vcc-supply"); if (ret) return ret;
Drop open-coded pattern: 'devm_regulator_get(), regulator_enable(), add_action_or_reset(regulator_disable)' and use the devm_regulator_get_enable() and drop the pointer to the regulator. This simplifies code and makes it less tempting to add manual control for the regulator which is also controlled by devm. Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- v2 => v3: New patch --- drivers/iio/amplifiers/hmc425a.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-)