Re: [PATCH 17/34] greybus: arche-ctrl: move device table to its right location

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/3/24 3:06 AM, Arnd Bergmann wrote:
From: Arnd Bergmann <arnd@xxxxxxxx>

The arche-ctrl has two platform drivers and three of_device_id tables,
but one table is only used for the the module loader, while the other
two seem to be associated with their drivers.

This leads to a W=1 warning when the driver is built-in:

drivers/staging/greybus/arche-platform.c:623:34: error: 'arche_combined_id' defined but not used [-Werror=unused-const-variable=]
   623 | static const struct of_device_id arche_combined_id[] = {

Drop the extra table and register both tables that are actually
used as the ones for the module loader instead.

So what I see is that this commit added arche_combined_id[]:
1e5dd1f8279a8 greybus: arche-platform: merge arche-apb-ctrl and arche-platform

That moved the arche_apb_ctrl_device_driver struct and some other
associated bits out of arche-apb-ctrl.c and into arche-platform.c.
It *kept* arche_platform_of_match[] as the of_match_table for
arche_platform_device_driver, but defined arche_combined_id[] and
declared it for modalias to indicate both drivers were implemented
in the single kernel module.

The later commit (the one you references in "Fixes") then moved
the arche_apb_ctrl_device_driver etc. back to arche-apb-ctrl.c.
That commit did *not* use MODULE_DEVICE_TABLE() to declare
arche_apb_ctrl_of_match[] for modalias.  And it simply kept the
(no longer correct) arche_combined_id[] table to be used by the
arche_platform_device_driver.

So your fix:
- Declares for modalias that arche_apb_ctrl_of_match[] is the
  of_match_table for arche_apb_ctrl_device_driver.
- Declares for modalias that arche_platform_of_match[] is the
  of_match_table for arche_platform_device_driver.
- Gets rid of arche_combined_id[], which is no longer used.

In short:  looks good to me.

Reviewed-by: Alex Elder <elder@xxxxxxxxxx>


Fixes: 7b62b61c752a ("greybus: arche-ctrl: Don't expose driver internals to arche-platform driver")
Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
---
  drivers/staging/greybus/arche-apb-ctrl.c | 1 +
  drivers/staging/greybus/arche-platform.c | 9 +--------
  2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/greybus/arche-apb-ctrl.c b/drivers/staging/greybus/arche-apb-ctrl.c
index 8541995008da..aa6f266b62a1 100644
--- a/drivers/staging/greybus/arche-apb-ctrl.c
+++ b/drivers/staging/greybus/arche-apb-ctrl.c
@@ -466,6 +466,7 @@ static const struct of_device_id arche_apb_ctrl_of_match[] = {
  	{ .compatible = "usbffff,2", },
  	{ },
  };
+MODULE_DEVICE_TABLE(of, arche_apb_ctrl_of_match);
static struct platform_driver arche_apb_ctrl_device_driver = {
  	.probe		= arche_apb_ctrl_probe,
diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
index 891b75327d7f..b33977ccd527 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -619,14 +619,7 @@ static const struct of_device_id arche_platform_of_match[] = {
  	{ .compatible = "google,arche-platform", },
  	{ },
  };
-
-static const struct of_device_id arche_combined_id[] = {
-	/* Use PID/VID of SVC device */
-	{ .compatible = "google,arche-platform", },
-	{ .compatible = "usbffff,2", },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, arche_combined_id);
+MODULE_DEVICE_TABLE(of, arche_platform_of_match);
static struct platform_driver arche_platform_device_driver = {
  	.probe		= arche_platform_probe,

_______________________________________________
greybus-dev mailing list -- greybus-dev@xxxxxxxxxxxxxxxx
To unsubscribe send an email to greybus-dev-leave@xxxxxxxxxxxxxxxx



[Index of Archives]     [Asterisk App Development]     [PJ SIP]     [Gnu Gatekeeper]     [IETF Sipping]     [Info Cyrus]     [ALSA User]     [Fedora Linux Users]     [Linux SCTP]     [DCCP]     [Gimp]     [Yosemite News]     [Deep Creek Hot Springs]     [Yosemite Campsites]     [ISDN Cause Codes]     [Asterisk Books]

  Powered by Linux