| Message ID | 20221214114944.83790-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1,1/2] iio: adc: ti-adc128s052: Switch to use spi_get_device_match_data() | expand |
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. 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); > + > 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);
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); >
On Fri, Dec 23, 2022 at 03:44:50PM +0000, Jonathan Cameron wrote: > 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... As comment on the first patch stays this requires an SPI core patch, which is now the part of the v6.2-rc1. Can you reapply it taking the above into consideration?
On Wed, 28 Dec 2022 11:59:53 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Fri, Dec 23, 2022 at 03:44:50PM +0000, Jonathan Cameron wrote: > > 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... > > As comment on the first patch stays this requires an SPI core patch, which is > now the part of the v6.2-rc1. > > Can you reapply it taking the above into consideration? > I should have been more specific though I do mention rebasing to get the patch above.. Doesn't build with it. Signature of spi_get_device_match_data is: extern const void * spi_get_device_match_data(const struct spi_device *sdev); and you are passing it a struct device * which rather implies you didn't successfully build test this. Jonathan
On Sat, Dec 31, 2022 at 02:45:58PM +0000, Jonathan Cameron wrote: > On Wed, 28 Dec 2022 11:59:53 +0200 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: ... > I should have been more specific though I do mention rebasing to get the > patch above.. Doesn't build with it. > > Signature of spi_get_device_match_data is: > extern const void * > spi_get_device_match_data(const struct spi_device *sdev); > > and you are passing it a struct device * which rather implies you didn't > successfully build test this. Definitely. Thanks for spotting this, I'll investigate what happened on my side that it wasn't built.
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); + 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);
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. drivers/iio/adc/ti-adc128s052.c | 39 +++++++++++++++------------------ 1 file changed, 18 insertions(+), 21 deletions(-)