diff mbox series

[31/34] spi: remove incorrect of_match_ptr annotations

Message ID 20240403080702.3509288-32-arnd@kernel.org
State New
Headers show
Series None | expand

Commit Message

Arnd Bergmann April 3, 2024, 8:06 a.m. UTC 
From: Arnd Bergmann <arnd@arndb.de>

When building with CONFIG_OF  disabled but W=1 extra warnings enabled,
a couple of driver cause a warning about an unused ID table:

drivers/spi/spi-armada-3700.c:806:34: error: unused variable 'a3700_spi_dt_ids' [-Werror,-Wunused-const-variable]
drivers/spi/spi-orion.c:614:34: error: unused variable 'orion_spi_of_match_table' [-Werror,-Wunused-const-variable]
drivers/spi/spi-pic32-sqi.c:673:34: error: unused variable 'pic32_sqi_of_ids' [-Werror,-Wunused-const-variable]
drivers/spi/spi-pic32.c:850:34: error: unused variable 'pic32_spi_of_match' [-Werror,-Wunused-const-variable]
drivers/spi/spi-rockchip.c:1020:34: error: unused variable 'rockchip_spi_dt_match' [-Werror,-Wunused-const-variable]
drivers/spi/spi-s3c64xx.c:1642:34: error: unused variable 's3c64xx_spi_dt_match' [-Werror,-Wunused-const-variable]
drivers/spi/spi-st-ssc4.c:439:34: error: unused variable 'stm_spi_match' [-Werror,-Wunused-const-variable]

These appear to all be copied from the same original driver, so fix them at the
same time by removing the unnecessary of_match_ptr() annotation. As far as I
can tell, all these drivers are only actually used on configurations that
have CONFIG_OF enabled.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/spi/spi-armada-3700.c | 2 +-
 drivers/spi/spi-img-spfi.c    | 2 +-
 drivers/spi/spi-meson-spicc.c | 2 +-
 drivers/spi/spi-meson-spifc.c | 2 +-
 drivers/spi/spi-orion.c       | 2 +-
 drivers/spi/spi-pic32-sqi.c   | 2 +-
 drivers/spi/spi-pic32.c       | 2 +-
 drivers/spi/spi-rockchip.c    | 2 +-
 drivers/spi/spi-s3c64xx.c     | 2 +-
 drivers/spi/spi-st-ssc4.c     | 2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

Comments

Uwe Kleine-König April 3, 2024, 9:05 p.m. UTC | #1 
On Wed, Apr 03, 2024 at 10:56:58AM +0100, Mark Brown wrote:
> On Wed, Apr 03, 2024 at 10:06:49AM +0200, Arnd Bergmann wrote:
> 
> > These appear to all be copied from the same original driver, so fix them at the
> > same time by removing the unnecessary of_match_ptr() annotation. As far as I
> > can tell, all these drivers are only actually used on configurations that
> > have CONFIG_OF enabled.
> 
> Why are we not fixing of_match_ptr() here, or at least adding the ifdefs
> in case someone does end up wanting to run without OF?

Fixing of_match_ptr =

diff --git a/include/linux/of.h b/include/linux/of.h
index a0bedd038a05..d980bccffda0 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -890,7 +890,7 @@ static inline const void *of_device_get_match_data(const struct device *dev)
 	return NULL;
 }
 
-#define of_match_ptr(_ptr)	NULL
+#define of_match_ptr(_ptr)	(0 ? (_ptr) : NULL)
 #define of_match_node(_matches, _node)	NULL
 #endif /* CONFIG_OF */
 
?

Assuming this helps, I agree this would be the better fix.

Best regards
Uwe
Arnd Bergmann April 3, 2024, 10:19 p.m. UTC | #2 
On Wed, Apr 3, 2024, at 23:13, Andy Shevchenko wrote:
> Wed, Apr 03, 2024 at 11:05:51PM +0200, Uwe Kleine-König kirjoitti:
>> On Wed, Apr 03, 2024 at 10:56:58AM +0100, Mark Brown wrote:
>> > On Wed, Apr 03, 2024 at 10:06:49AM +0200, Arnd Bergmann wrote:
>> > 
>> > > These appear to all be copied from the same original driver, so fix them at the
>> > > same time by removing the unnecessary of_match_ptr() annotation. As far as I
>> > > can tell, all these drivers are only actually used on configurations that
>> > > have CONFIG_OF enabled.
>> > 
>> > Why are we not fixing of_match_ptr() here, or at least adding the ifdefs
>> > in case someone does end up wanting to run without OF?
>> 
>> Fixing of_match_ptr =
>> 
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index a0bedd038a05..d980bccffda0 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -890,7 +890,7 @@ static inline const void *of_device_get_match_data(const struct device *dev)
>>  	return NULL;
>>  }
>>  
>> -#define of_match_ptr(_ptr)	NULL
>> +#define of_match_ptr(_ptr)	(0 ? (_ptr) : NULL)

This would require removing several hundred "#ifdef CONFIG_OF"
checks around the of_device_id tables at the same time
unfortunately. Most of those are completely unnecessary, so
if we wanted to remove those, we should remove the of_match_ptr()
instances at the same time.

>>  #define of_match_node(_matches, _node)	NULL
>>  #endif /* CONFIG_OF */
>>  
>> ?
>> 
>> Assuming this helps, I agree this would be the better fix.
>
> Why? I mean why do we need to even have this API? It's always
> good to know which devices are supported by the module even
> if you have no need in such support or it's not compiled in.
> One of the reasons why is to be able to google for compatible hardware,
> for example.

As far as I can tell, the of_match_ptr() helper was a historic
mistake, it makes pretty much no sense in its current form.

The version that Uwe proposes would have made sense but we
can't change it now.

      Arnd
diff mbox series

Patch

diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
index 3c9ed412932f..2fc2e7c0daf2 100644
--- a/drivers/spi/spi-armada-3700.c
+++ b/drivers/spi/spi-armada-3700.c
@@ -902,7 +902,7 @@  static int a3700_spi_probe(struct platform_device *pdev)
 static struct platform_driver a3700_spi_driver = {
 	.driver = {
 		.name	= DRIVER_NAME,
-		.of_match_table = of_match_ptr(a3700_spi_dt_ids),
+		.of_match_table = a3700_spi_dt_ids,
 	},
 	.probe		= a3700_spi_probe,
 };
diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index d8360f94d3b7..0d7f89301f8d 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -753,7 +753,7 @@  static struct platform_driver img_spfi_driver = {
 	.driver = {
 		.name = "img-spfi",
 		.pm = &img_spfi_pm_ops,
-		.of_match_table = of_match_ptr(img_spfi_of_match),
+		.of_match_table = img_spfi_of_match,
 	},
 	.probe = img_spfi_probe,
 	.remove_new = img_spfi_remove,
diff --git a/drivers/spi/spi-meson-spicc.c b/drivers/spi/spi-meson-spicc.c
index fc75492e50ff..4189d4434e37 100644
--- a/drivers/spi/spi-meson-spicc.c
+++ b/drivers/spi/spi-meson-spicc.c
@@ -946,7 +946,7 @@  static struct platform_driver meson_spicc_driver = {
 	.remove_new = meson_spicc_remove,
 	.driver  = {
 		.name = "meson-spicc",
-		.of_match_table = of_match_ptr(meson_spicc_of_match),
+		.of_match_table = meson_spicc_of_match,
 	},
 };
 
diff --git a/drivers/spi/spi-meson-spifc.c b/drivers/spi/spi-meson-spifc.c
index fd8b26dd4a79..0c39d59d0af8 100644
--- a/drivers/spi/spi-meson-spifc.c
+++ b/drivers/spi/spi-meson-spifc.c
@@ -432,7 +432,7 @@  static struct platform_driver meson_spifc_driver = {
 	.remove_new = meson_spifc_remove,
 	.driver	= {
 		.name		= "meson-spifc",
-		.of_match_table	= of_match_ptr(meson_spifc_dt_match),
+		.of_match_table	= meson_spifc_dt_match,
 		.pm		= &meson_spifc_pm_ops,
 	},
 };
diff --git a/drivers/spi/spi-orion.c b/drivers/spi/spi-orion.c
index eee9ff4bfa5b..a0029e8dc0be 100644
--- a/drivers/spi/spi-orion.c
+++ b/drivers/spi/spi-orion.c
@@ -843,7 +843,7 @@  static struct platform_driver orion_spi_driver = {
 	.driver = {
 		.name	= DRIVER_NAME,
 		.pm	= &orion_spi_pm_ops,
-		.of_match_table = of_match_ptr(orion_spi_of_match_table),
+		.of_match_table = orion_spi_of_match_table,
 	},
 	.probe		= orion_spi_probe,
 	.remove_new	= orion_spi_remove,
diff --git a/drivers/spi/spi-pic32-sqi.c b/drivers/spi/spi-pic32-sqi.c
index 3f1e5b27776b..458c1a8d5719 100644
--- a/drivers/spi/spi-pic32-sqi.c
+++ b/drivers/spi/spi-pic32-sqi.c
@@ -679,7 +679,7 @@  MODULE_DEVICE_TABLE(of, pic32_sqi_of_ids);
 static struct platform_driver pic32_sqi_driver = {
 	.driver = {
 		.name = "sqi-pic32",
-		.of_match_table = of_match_ptr(pic32_sqi_of_ids),
+		.of_match_table = pic32_sqi_of_ids,
 	},
 	.probe = pic32_sqi_probe,
 	.remove_new = pic32_sqi_remove,
diff --git a/drivers/spi/spi-pic32.c b/drivers/spi/spi-pic32.c
index 709edb70ad7d..e8bd32c1fc7c 100644
--- a/drivers/spi/spi-pic32.c
+++ b/drivers/spi/spi-pic32.c
@@ -856,7 +856,7 @@  MODULE_DEVICE_TABLE(of, pic32_spi_of_match);
 static struct platform_driver pic32_spi_driver = {
 	.driver = {
 		.name = "spi-pic32",
-		.of_match_table = of_match_ptr(pic32_spi_of_match),
+		.of_match_table = pic32_spi_of_match,
 	},
 	.probe = pic32_spi_probe,
 	.remove_new = pic32_spi_remove,
diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index e1ecd96c7858..4dbb5127a9e5 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -1042,7 +1042,7 @@  static struct platform_driver rockchip_spi_driver = {
 	.driver = {
 		.name	= DRIVER_NAME,
 		.pm = &rockchip_spi_pm,
-		.of_match_table = of_match_ptr(rockchip_spi_dt_match),
+		.of_match_table = rockchip_spi_dt_match,
 	},
 	.probe = rockchip_spi_probe,
 	.remove_new = rockchip_spi_remove,
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index f726d8670428..dbb89f6cb3dd 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1677,7 +1677,7 @@  static struct platform_driver s3c64xx_spi_driver = {
 	.driver = {
 		.name	= "s3c64xx-spi",
 		.pm = &s3c64xx_spi_pm,
-		.of_match_table = of_match_ptr(s3c64xx_spi_dt_match),
+		.of_match_table = s3c64xx_spi_dt_match,
 	},
 	.probe = s3c64xx_spi_probe,
 	.remove_new = s3c64xx_spi_remove,
diff --git a/drivers/spi/spi-st-ssc4.c b/drivers/spi/spi-st-ssc4.c
index e064025e2fd6..6d288497c500 100644
--- a/drivers/spi/spi-st-ssc4.c
+++ b/drivers/spi/spi-st-ssc4.c
@@ -446,7 +446,7 @@  static struct platform_driver spi_st_driver = {
 	.driver = {
 		.name = "spi-st",
 		.pm = &spi_st_pm,
-		.of_match_table = of_match_ptr(stm_spi_match),
+		.of_match_table = stm_spi_match,
 	},
 	.probe = spi_st_probe,
 	.remove_new = spi_st_remove,