Re: [PATCH v1 1/2] iio: adc: ti-adc128s052: Switch to use spi_get_device_match_data()

From: Jonathan Cameron
Date: Fri Dec 23 2022 - 10:31:47 EST


On Fri, 23 Dec 2022 15:22:42 +0000
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

> On Wed, 14 Dec 2022 13:49:43 +0200
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> > The spi_get_device_match_data() helps to get driver data from the
> > firmware node or SPI ID table. Use it instead of open coding.
> >
> > While at it, switch ID tables to provide an acrual pointers to
> > the configuration data.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> >
> > Requires aea672d054a2 ("spi: Introduce spi_get_device_match_data()
> > helper") which is part of upstream as of today.
>
> I rebased to get that (will rebase again on rc1).
>
> Applied to the togreg branch of iio.git and pushed out as testing
> to keep 0-day busy over my holidays.

I take it back... Build test failed...

>
> Jonathan
>
> >
> > drivers/iio/adc/ti-adc128s052.c | 39 +++++++++++++++------------------
> > 1 file changed, 18 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> > index b3d5b9b7255b..9dfc625100b6 100644
> > --- a/drivers/iio/adc/ti-adc128s052.c
> > +++ b/drivers/iio/adc/ti-adc128s052.c
> > @@ -139,16 +139,11 @@ static void adc128_disable_regulator(void *reg)
> >
> > static int adc128_probe(struct spi_device *spi)
> > {
> > + const struct adc128_configuration *config;
> > struct iio_dev *indio_dev;
> > - unsigned int config;
> > struct adc128 *adc;
> > int ret;
> >
> > - if (dev_fwnode(&spi->dev))
> > - config = (unsigned long) device_get_match_data(&spi->dev);
> > - else
> > - config = spi_get_device_id(spi)->driver_data;
> > -
> > indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> > if (!indio_dev)
> > return -ENOMEM;
> > @@ -160,6 +155,8 @@ static int adc128_probe(struct spi_device *spi)
> > indio_dev->modes = INDIO_DIRECT_MODE;
> > indio_dev->info = &adc128_info;
> >
> > + config = spi_get_device_match_data(&spi->dev);
Takes a struct spi_device*

Also, having opened code up to fix this, we have a spi_get_device_id() left
just above that needs dealing with.

And a lot of uses of config below that need fixing.

I'm dropping this series until that's all fixed up.

Jonathan



> > +
> > indio_dev->channels = adc128_config[config].channels;
> > indio_dev->num_channels = adc128_config[config].num_channels;
> >
> > @@ -181,32 +178,32 @@ static int adc128_probe(struct spi_device *spi)
> > }
> >
> > static const struct of_device_id adc128_of_match[] = {
> > - { .compatible = "ti,adc128s052", .data = (void*)0L, },
> > - { .compatible = "ti,adc122s021", .data = (void*)1L, },
> > - { .compatible = "ti,adc122s051", .data = (void*)1L, },
> > - { .compatible = "ti,adc122s101", .data = (void*)1L, },
> > - { .compatible = "ti,adc124s021", .data = (void*)2L, },
> > - { .compatible = "ti,adc124s051", .data = (void*)2L, },
> > - { .compatible = "ti,adc124s101", .data = (void*)2L, },
> > + { .compatible = "ti,adc128s052", .data = &adc128_config[0] },
> > + { .compatible = "ti,adc122s021", .data = &adc128_config[1] },
> > + { .compatible = "ti,adc122s051", .data = &adc128_config[1] },
> > + { .compatible = "ti,adc122s101", .data = &adc128_config[1] },
> > + { .compatible = "ti,adc124s021", .data = &adc128_config[2] },
> > + { .compatible = "ti,adc124s051", .data = &adc128_config[2] },
> > + { .compatible = "ti,adc124s101", .data = &adc128_config[2] },
> > { /* sentinel */ },
> > };
> > MODULE_DEVICE_TABLE(of, adc128_of_match);
> >
> > static const struct spi_device_id adc128_id[] = {
> > - { "adc128s052", 0 }, /* index into adc128_config */
> > - { "adc122s021", 1 },
> > - { "adc122s051", 1 },
> > - { "adc122s101", 1 },
> > - { "adc124s021", 2 },
> > - { "adc124s051", 2 },
> > - { "adc124s101", 2 },
> > + { "adc128s052", (kernel_ulong_t)&adc128_config[0] },
> > + { "adc122s021", (kernel_ulong_t)&adc128_config[1] },
> > + { "adc122s051", (kernel_ulong_t)&adc128_config[1] },
> > + { "adc122s101", (kernel_ulong_t)&adc128_config[1] },
> > + { "adc124s021", (kernel_ulong_t)&adc128_config[2] },
> > + { "adc124s051", (kernel_ulong_t)&adc128_config[2] },
> > + { "adc124s101", (kernel_ulong_t)&adc128_config[2] },
> > { }
> > };
> > MODULE_DEVICE_TABLE(spi, adc128_id);
> >
> > #ifdef CONFIG_ACPI
> > static const struct acpi_device_id adc128_acpi_match[] = {
> > - { "AANT1280", 2 }, /* ADC124S021 compatible ACPI ID */
> > + { "AANT1280", (kernel_ulong_t)&adc128_config[2] },
> > { }
> > };
> > MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
>