| Message ID | 20240403080702.3509288-17-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | address all -Wunused-const warnings | expand |
On Wed, Apr 03, 2024 at 10:06:34AM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > 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 <andriy.shevchenko@linux.intel.com>
On 03/04/2024 10:06, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > 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 <arnd@arndb.de> > --- > 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
On Wed, Apr 3, 2024, at 11:55, Krzysztof Kozlowski wrote: > On 03/04/2024 10:06, Arnd Bergmann wrote: >> From: Arnd Bergmann <arnd@arndb.de> >> >> 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 <arnd@arndb.de> >> --- >> 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
On Wed, 03 Apr 2024 12:01:13 +0200 "Arnd Bergmann" <arnd@arndb.de> wrote: > On Wed, Apr 3, 2024, at 11:55, Krzysztof Kozlowski wrote: > > On 03/04/2024 10:06, Arnd Bergmann wrote: > >> From: Arnd Bergmann <arnd@arndb.de> > >> > >> 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 <arnd@arndb.de> > >> --- > >> 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
On Wed, 3 Apr 2024 11:55:06 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 03/04/2024 10:06, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > 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 <arnd@arndb.de> > > --- > > 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 >
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,