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

[Jonathan Cameron <jic23@kernel.org>]

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

> On Wed, 14 Dec 2022 13:49:43 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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@linux.intel.com>
> > ---
> > 
> > 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);  
>