| Message ID | 20210516172520.1398835-9-jic23@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: adc: Maxim and TI ADC driver cleanups | expand |
On Sun, May 16, 2021 at 10:26 AM Jonathan Cameron <jic23@kernel.org> wrote: > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Simplifies error handling and allows us to drop remove entirely. > > The regulator handling in this driver was unusual as it would try to > acquire the regulator, but if that failed with an error would continue. > > We should get a stub regulator if one isn't provided in DT and an error > could indicate an actual problem preventing the device being powered > (perhaps a need to defer). So this handling is cleaned up (arguably > that might be a fix but given no one has run into it, I haven't broken > it out separately. > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Cc: Matt Ranostay <matt.ranostay@konsulko.com> Acked-by: Matt Ranostay <matt.ranostay@konsulko.com> > --- > drivers/iio/adc/ti-adc161s626.c | 50 ++++++++++++--------------------- > 1 file changed, 18 insertions(+), 32 deletions(-) > > diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c > index 607791ffe7f0..9f2f25cf9a49 100644 > --- a/drivers/iio/adc/ti-adc161s626.c > +++ b/drivers/iio/adc/ti-adc161s626.c > @@ -169,6 +169,11 @@ static const struct iio_info ti_adc_info = { > .read_raw = ti_adc_read_raw, > }; > > +static void ti_adc_reg_disable(void *reg) > +{ > + regulator_disable(reg); > +} > + > static int ti_adc_probe(struct spi_device *spi) > { > struct iio_dev *indio_dev; > @@ -203,42 +208,24 @@ static int ti_adc_probe(struct spi_device *spi) > } > > data->ref = devm_regulator_get(&spi->dev, "vdda"); > - if (!IS_ERR(data->ref)) { > - ret = regulator_enable(data->ref); > - if (ret < 0) > - return ret; > - } > + if (IS_ERR(data->ref)) > + return PTR_ERR(data->ref); > > - ret = iio_triggered_buffer_setup(indio_dev, NULL, > - ti_adc_trigger_handler, NULL); > - if (ret) > - goto error_regulator_disable; > + ret = regulator_enable(data->ref); > + if (ret < 0) > + return ret; > > - ret = iio_device_register(indio_dev); > + ret = devm_add_action_or_reset(&spi->dev, ti_adc_reg_disable, > + data->ref); > if (ret) > - goto error_unreg_buffer; > - > - return 0; > + return ret; > > -error_unreg_buffer: > - iio_triggered_buffer_cleanup(indio_dev); > - > -error_regulator_disable: > - regulator_disable(data->ref); > - > - return ret; > -} > - > -static int ti_adc_remove(struct spi_device *spi) > -{ > - struct iio_dev *indio_dev = spi_get_drvdata(spi); > - struct ti_adc_data *data = iio_priv(indio_dev); > - > - iio_device_unregister(indio_dev); > - iio_triggered_buffer_cleanup(indio_dev); > - regulator_disable(data->ref); > + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL, > + ti_adc_trigger_handler, NULL); > + if (ret) > + return ret; > > - return 0; > + return devm_iio_device_register(&spi->dev, indio_dev); > } > > static const struct of_device_id ti_adc_dt_ids[] = { > @@ -261,7 +248,6 @@ static struct spi_driver ti_adc_driver = { > .of_match_table = ti_adc_dt_ids, > }, > .probe = ti_adc_probe, > - .remove = ti_adc_remove, > .id_table = ti_adc_id, > }; > module_spi_driver(ti_adc_driver); > -- > 2.31.1 >
On Sun, 16 May 2021 at 20:26, Jonathan Cameron <jic23@kernel.org> wrote: > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Simplifies error handling and allows us to drop remove entirely. > > The regulator handling in this driver was unusual as it would try to > acquire the regulator, but if that failed with an error would continue. > > We should get a stub regulator if one isn't provided in DT and an error > could indicate an actual problem preventing the device being powered > (perhaps a need to defer). So this handling is cleaned up (arguably > that might be a fix but given no one has run into it, I haven't broken > it out separately. > This patch forgets to remove "spi_set_drvdata(spi, indio_dev);" With that fixed Reviewed-by: Alexandru Ardelean <aardelean@deviqon.com> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Cc: Matt Ranostay <matt.ranostay@konsulko.com> > --- > drivers/iio/adc/ti-adc161s626.c | 50 ++++++++++++--------------------- > 1 file changed, 18 insertions(+), 32 deletions(-) > > diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c > index 607791ffe7f0..9f2f25cf9a49 100644 > --- a/drivers/iio/adc/ti-adc161s626.c > +++ b/drivers/iio/adc/ti-adc161s626.c > @@ -169,6 +169,11 @@ static const struct iio_info ti_adc_info = { > .read_raw = ti_adc_read_raw, > }; > > +static void ti_adc_reg_disable(void *reg) > +{ > + regulator_disable(reg); > +} > + > static int ti_adc_probe(struct spi_device *spi) > { > struct iio_dev *indio_dev; > @@ -203,42 +208,24 @@ static int ti_adc_probe(struct spi_device *spi) > } > > data->ref = devm_regulator_get(&spi->dev, "vdda"); > - if (!IS_ERR(data->ref)) { > - ret = regulator_enable(data->ref); > - if (ret < 0) > - return ret; > - } > + if (IS_ERR(data->ref)) > + return PTR_ERR(data->ref); > > - ret = iio_triggered_buffer_setup(indio_dev, NULL, > - ti_adc_trigger_handler, NULL); > - if (ret) > - goto error_regulator_disable; > + ret = regulator_enable(data->ref); > + if (ret < 0) > + return ret; > > - ret = iio_device_register(indio_dev); > + ret = devm_add_action_or_reset(&spi->dev, ti_adc_reg_disable, > + data->ref); > if (ret) > - goto error_unreg_buffer; > - > - return 0; > + return ret; > > -error_unreg_buffer: > - iio_triggered_buffer_cleanup(indio_dev); > - > -error_regulator_disable: > - regulator_disable(data->ref); > - > - return ret; > -} > - > -static int ti_adc_remove(struct spi_device *spi) > -{ > - struct iio_dev *indio_dev = spi_get_drvdata(spi); > - struct ti_adc_data *data = iio_priv(indio_dev); > - > - iio_device_unregister(indio_dev); > - iio_triggered_buffer_cleanup(indio_dev); > - regulator_disable(data->ref); > + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL, > + ti_adc_trigger_handler, NULL); > + if (ret) > + return ret; > > - return 0; > + return devm_iio_device_register(&spi->dev, indio_dev); > } > > static const struct of_device_id ti_adc_dt_ids[] = { > @@ -261,7 +248,6 @@ static struct spi_driver ti_adc_driver = { > .of_match_table = ti_adc_dt_ids, > }, > .probe = ti_adc_probe, > - .remove = ti_adc_remove, > .id_table = ti_adc_id, > }; > module_spi_driver(ti_adc_driver); > -- > 2.31.1 >
diff --git a/drivers/iio/adc/ti-adc161s626.c b/drivers/iio/adc/ti-adc161s626.c index 607791ffe7f0..9f2f25cf9a49 100644 --- a/drivers/iio/adc/ti-adc161s626.c +++ b/drivers/iio/adc/ti-adc161s626.c @@ -169,6 +169,11 @@ static const struct iio_info ti_adc_info = { .read_raw = ti_adc_read_raw, }; +static void ti_adc_reg_disable(void *reg) +{ + regulator_disable(reg); +} + static int ti_adc_probe(struct spi_device *spi) { struct iio_dev *indio_dev; @@ -203,42 +208,24 @@ static int ti_adc_probe(struct spi_device *spi) } data->ref = devm_regulator_get(&spi->dev, "vdda"); - if (!IS_ERR(data->ref)) { - ret = regulator_enable(data->ref); - if (ret < 0) - return ret; - } + if (IS_ERR(data->ref)) + return PTR_ERR(data->ref); - ret = iio_triggered_buffer_setup(indio_dev, NULL, - ti_adc_trigger_handler, NULL); - if (ret) - goto error_regulator_disable; + ret = regulator_enable(data->ref); + if (ret < 0) + return ret; - ret = iio_device_register(indio_dev); + ret = devm_add_action_or_reset(&spi->dev, ti_adc_reg_disable, + data->ref); if (ret) - goto error_unreg_buffer; - - return 0; + return ret; -error_unreg_buffer: - iio_triggered_buffer_cleanup(indio_dev); - -error_regulator_disable: - regulator_disable(data->ref); - - return ret; -} - -static int ti_adc_remove(struct spi_device *spi) -{ - struct iio_dev *indio_dev = spi_get_drvdata(spi); - struct ti_adc_data *data = iio_priv(indio_dev); - - iio_device_unregister(indio_dev); - iio_triggered_buffer_cleanup(indio_dev); - regulator_disable(data->ref); + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL, + ti_adc_trigger_handler, NULL); + if (ret) + return ret; - return 0; + return devm_iio_device_register(&spi->dev, indio_dev); } static const struct of_device_id ti_adc_dt_ids[] = { @@ -261,7 +248,6 @@ static struct spi_driver ti_adc_driver = { .of_match_table = ti_adc_dt_ids, }, .probe = ti_adc_probe, - .remove = ti_adc_remove, .id_table = ti_adc_id, }; module_spi_driver(ti_adc_driver);