2024-04-03 08:24:23

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 16/34] iio: ad5755: hook up of_device_id lookup to platform driver

From: Arnd Bergmann <[email protected]>

When the driver is built-in, 'make W=1' warns about an unused
ID table:

drivers/iio/dac/ad5755.c:866:34: error: 'ad5755_of_match' defined but not used [-Werror=unused-const-variable=]
866 | static const struct of_device_id ad5755_of_match[] = {

While the data is duplicated in the spi_device_id, it's common
to use the actual OF compatible strings in the driver.

Since there are no in-tree users of plain platform devices, the
spi_device_id table could actually be dropped entirely with this.

Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/iio/dac/ad5755.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
index 404865e35460..5c1e7f428c25 100644
--- a/drivers/iio/dac/ad5755.c
+++ b/drivers/iio/dac/ad5755.c
@@ -876,6 +876,7 @@ MODULE_DEVICE_TABLE(of, ad5755_of_match);
static struct spi_driver ad5755_driver = {
.driver = {
.name = "ad5755",
+ .of_match_table = ad5755_of_match,
},
.probe = ad5755_probe,
.id_table = ad5755_id,
--
2.39.2



2024-04-03 09:36:26

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH 16/34] iio: ad5755: hook up of_device_id lookup to platform driver

On Wed, Apr 03, 2024 at 10:06:34AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When the driver is built-in, 'make W=1' warns about an unused
> ID table:
>
> drivers/iio/dac/ad5755.c:866:34: error: 'ad5755_of_match' defined but not used [-Werror=unused-const-variable=]
> 866 | static const struct of_device_id ad5755_of_match[] = {
>
> While the data is duplicated in the spi_device_id, it's common
> to use the actual OF compatible strings in the driver.
>
> Since there are no in-tree users of plain platform devices, the
> spi_device_id table could actually be dropped entirely with this.

Reviewed-by: Andy Shevchenko <[email protected]>

--
With Best Regards,
Andy Shevchenko



2024-04-03 09:56:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 16/34] iio: ad5755: hook up of_device_id lookup to platform driver

On 03/04/2024 10:06, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When the driver is built-in, 'make W=1' warns about an unused
> ID table:
>
> drivers/iio/dac/ad5755.c:866:34: error: 'ad5755_of_match' defined but not used [-Werror=unused-const-variable=]
> 866 | static const struct of_device_id ad5755_of_match[] = {
>
> While the data is duplicated in the spi_device_id, it's common
> to use the actual OF compatible strings in the driver.
>
> Since there are no in-tree users of plain platform devices, the
> spi_device_id table could actually be dropped entirely with this.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/iio/dac/ad5755.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
> index 404865e35460..5c1e7f428c25 100644
> --- a/drivers/iio/dac/ad5755.c
> +++ b/drivers/iio/dac/ad5755.c
> @@ -876,6 +876,7 @@ MODULE_DEVICE_TABLE(of, ad5755_of_match);
> static struct spi_driver ad5755_driver = {
> .driver = {
> .name = "ad5755",
> + .of_match_table = ad5755_of_match,

I was working on this as well and have a bit bigger solution, following
Jonathan's preference (I think):

https://lore.kernel.org/all/20240226192555.14aa178e@jic23-huawei/

I need to send v3, somehow I missed his comments.

Jonathan,
Do you want me to still work on this according to your comments (which I
missed, I am sorry).

Best regards,
Krzysztof


2024-04-03 10:01:47

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 16/34] iio: ad5755: hook up of_device_id lookup to platform driver

On Wed, Apr 3, 2024, at 11:55, Krzysztof Kozlowski wrote:
> On 03/04/2024 10:06, Arnd Bergmann wrote:
>> From: Arnd Bergmann <[email protected]>
>>
>> When the driver is built-in, 'make W=1' warns about an unused
>> ID table:
>>
>> drivers/iio/dac/ad5755.c:866:34: error: 'ad5755_of_match' defined but not used [-Werror=unused-const-variable=]
>> 866 | static const struct of_device_id ad5755_of_match[] = {
>>
>> While the data is duplicated in the spi_device_id, it's common
>> to use the actual OF compatible strings in the driver.
>>
>> Since there are no in-tree users of plain platform devices, the
>> spi_device_id table could actually be dropped entirely with this.
>>
>> Signed-off-by: Arnd Bergmann <[email protected]>
>> ---
>> drivers/iio/dac/ad5755.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
>> index 404865e35460..5c1e7f428c25 100644
>> --- a/drivers/iio/dac/ad5755.c
>> +++ b/drivers/iio/dac/ad5755.c
>> @@ -876,6 +876,7 @@ MODULE_DEVICE_TABLE(of, ad5755_of_match);
>> static struct spi_driver ad5755_driver = {
>> .driver = {
>> .name = "ad5755",
>> + .of_match_table = ad5755_of_match,
>
> I was working on this as well and have a bit bigger solution, following
> Jonathan's preference (I think):
>
> https://lore.kernel.org/all/20240226192555.14aa178e@jic23-huawei/
>
> I need to send v3, somehow I missed his comments.

Yes, that looks good as well, though you might need to drop
spi_device_id table if you convert it to using pointers.

Arnd

2024-04-06 15:30:36

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 16/34] iio: ad5755: hook up of_device_id lookup to platform driver

On Wed, 03 Apr 2024 12:01:13 +0200
"Arnd Bergmann" <[email protected]> wrote:

> On Wed, Apr 3, 2024, at 11:55, Krzysztof Kozlowski wrote:
> > On 03/04/2024 10:06, Arnd Bergmann wrote:
> >> From: Arnd Bergmann <[email protected]>
> >>
> >> When the driver is built-in, 'make W=1' warns about an unused
> >> ID table:
> >>
> >> drivers/iio/dac/ad5755.c:866:34: error: 'ad5755_of_match' defined but not used [-Werror=unused-const-variable=]
> >> 866 | static const struct of_device_id ad5755_of_match[] = {
> >>
> >> While the data is duplicated in the spi_device_id, it's common
> >> to use the actual OF compatible strings in the driver.
> >>
> >> Since there are no in-tree users of plain platform devices, the
> >> spi_device_id table could actually be dropped entirely with this.
> >>
> >> Signed-off-by: Arnd Bergmann <[email protected]>
> >> ---
> >> drivers/iio/dac/ad5755.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
> >> index 404865e35460..5c1e7f428c25 100644
> >> --- a/drivers/iio/dac/ad5755.c
> >> +++ b/drivers/iio/dac/ad5755.c
> >> @@ -876,6 +876,7 @@ MODULE_DEVICE_TABLE(of, ad5755_of_match);
> >> static struct spi_driver ad5755_driver = {
> >> .driver = {
> >> .name = "ad5755",
> >> + .of_match_table = ad5755_of_match,
> >
> > I was working on this as well and have a bit bigger solution, following
> > Jonathan's preference (I think):
> >
> > https://lore.kernel.org/all/20240226192555.14aa178e@jic23-huawei/
> >
> > I need to send v3, somehow I missed his comments.
>
> Yes, that looks good as well, though you might need to drop
> spi_device_id table if you convert it to using pointers.

Put them in there as well (with appropriate cast). We don't want
to stop supporting probe paths that might be using that.

My preference is for both tables, same pointers in each then
the use of the relevant bus specific wrapper - here
spi_get_device_match_data() which will happily deal with matches
in either table (thus avoiding the potential mess of them getting out
of sync)

Jonathan


>
> Arnd


2024-04-06 15:32:17

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 16/34] iio: ad5755: hook up of_device_id lookup to platform driver

On Wed, 3 Apr 2024 11:55:06 +0200
Krzysztof Kozlowski <[email protected]> wrote:

> On 03/04/2024 10:06, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > When the driver is built-in, 'make W=1' warns about an unused
> > ID table:
> >
> > drivers/iio/dac/ad5755.c:866:34: error: 'ad5755_of_match' defined but not used [-Werror=unused-const-variable=]
> > 866 | static const struct of_device_id ad5755_of_match[] = {
> >
> > While the data is duplicated in the spi_device_id, it's common
> > to use the actual OF compatible strings in the driver.
> >
> > Since there are no in-tree users of plain platform devices, the
> > spi_device_id table could actually be dropped entirely with this.
> >
> > Signed-off-by: Arnd Bergmann <[email protected]>
> > ---
> > drivers/iio/dac/ad5755.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
> > index 404865e35460..5c1e7f428c25 100644
> > --- a/drivers/iio/dac/ad5755.c
> > +++ b/drivers/iio/dac/ad5755.c
> > @@ -876,6 +876,7 @@ MODULE_DEVICE_TABLE(of, ad5755_of_match);
> > static struct spi_driver ad5755_driver = {
> > .driver = {
> > .name = "ad5755",
> > + .of_match_table = ad5755_of_match,
>
> I was working on this as well and have a bit bigger solution, following
> Jonathan's preference (I think):
>
> https://lore.kernel.org/all/20240226192555.14aa178e@jic23-huawei/
>
> I need to send v3, somehow I missed his comments.
>
> Jonathan,
> Do you want me to still work on this according to your comments (which I
> missed, I am sorry).

No problem on missing the reply! (I'd forgotten all about this!)
I would prefer that solution to this one that relies on the two
tables having equivalent entries. So I'll not pick up this patch.

Thanks,

Jonathan

>
> Best regards,
> Krzysztof
>