lkml.org 
[lkml]   [2024]   [Apr]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 17/34] greybus: arche-ctrl: move device table to its right location
    From
    On 4/3/24 3:06 AM, Arnd Bergmann wrote:
    > From: Arnd Bergmann <arnd@arndb.de>
    >
    > 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@linaro.org>

    >
    > Fixes: 7b62b61c752a ("greybus: arche-ctrl: Don't expose driver internals to arche-platform driver")
    > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
    > ---
    > 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,


    \
     
     \ /
      Last update: 2024-05-27 18:11    [W:3.921 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site