| Message ID | 20240425091429.948467-1-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: samsung: exynos-clkout: Remove misleading of_match_table/MODULE_DEVICE_TABLE | expand |
On 04/25/2024, Marek Szyprowski wrote: > Since commit 9484f2cb8332 ("clk: samsung: exynos-clkout: convert to > module driver") this driver is instantiated as MFD-cell (matched by > platform device name) not as a real platform device created by OF code. > Remove then of_match_table and related MODULE_DEVICE_TABLE to avoid > further confusion. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/clk/samsung/clk-exynos-clkout.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/clk/samsung/clk-exynos-clkout.c b/drivers/clk/samsung/clk-exynos-clkout.c > index 503c6f5b20d5..0c7f4e2aa366 100644 > --- a/drivers/clk/samsung/clk-exynos-clkout.c > +++ b/drivers/clk/samsung/clk-exynos-clkout.c > @@ -75,7 +75,6 @@ static const struct of_device_id exynos_clkout_ids[] = { > .data = &exynos_clkout_exynos5, > }, { } > }; > -MODULE_DEVICE_TABLE(of, exynos_clkout_ids); I understand these are duplicates of the exynos-pmu driver, but was wondering if this will impact the exynos-clkout module from getting auto-loaded? Without the MODULE_DEVICE_TABLE() defined, the aliases won't be created that trigger udev to load this module and the mfd driver is obviously not going to load it. Thanks, Will
On 26.04.2024 02:24, William McVicker wrote: > On 04/25/2024, Marek Szyprowski wrote: >> Since commit 9484f2cb8332 ("clk: samsung: exynos-clkout: convert to >> module driver") this driver is instantiated as MFD-cell (matched by >> platform device name) not as a real platform device created by OF code. >> Remove then of_match_table and related MODULE_DEVICE_TABLE to avoid >> further confusion. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> drivers/clk/samsung/clk-exynos-clkout.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/clk/samsung/clk-exynos-clkout.c b/drivers/clk/samsung/clk-exynos-clkout.c >> index 503c6f5b20d5..0c7f4e2aa366 100644 >> --- a/drivers/clk/samsung/clk-exynos-clkout.c >> +++ b/drivers/clk/samsung/clk-exynos-clkout.c >> @@ -75,7 +75,6 @@ static const struct of_device_id exynos_clkout_ids[] = { >> .data = &exynos_clkout_exynos5, >> }, { } >> }; >> -MODULE_DEVICE_TABLE(of, exynos_clkout_ids); > I understand these are duplicates of the exynos-pmu driver, but was wondering > if this will impact the exynos-clkout module from getting auto-loaded? Without > the MODULE_DEVICE_TABLE() defined, the aliases won't be created that trigger > udev to load this module and the mfd driver is obviously not going to load it. This driver loaded and matched only against the platform device name ("exynos-clkout") since the mentioned commit 9484f2cb8332. These OF IDs defined as MODULE_DEVICE_TABLE and of_match_table are leftovers from the old (pre-9484f2cb8332) instantiating method.and should be removed by that commit too. Best regards
On 26/04/2024 07:24, Marek Szyprowski wrote: > On 26.04.2024 02:24, William McVicker wrote: >> On 04/25/2024, Marek Szyprowski wrote: >>> Since commit 9484f2cb8332 ("clk: samsung: exynos-clkout: convert to >>> module driver") this driver is instantiated as MFD-cell (matched by >>> platform device name) not as a real platform device created by OF code. >>> Remove then of_match_table and related MODULE_DEVICE_TABLE to avoid >>> further confusion. >>> >>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>> --- >>> drivers/clk/samsung/clk-exynos-clkout.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/drivers/clk/samsung/clk-exynos-clkout.c b/drivers/clk/samsung/clk-exynos-clkout.c >>> index 503c6f5b20d5..0c7f4e2aa366 100644 >>> --- a/drivers/clk/samsung/clk-exynos-clkout.c >>> +++ b/drivers/clk/samsung/clk-exynos-clkout.c >>> @@ -75,7 +75,6 @@ static const struct of_device_id exynos_clkout_ids[] = { >>> .data = &exynos_clkout_exynos5, >>> }, { } >>> }; >>> -MODULE_DEVICE_TABLE(of, exynos_clkout_ids); >> I understand these are duplicates of the exynos-pmu driver, but was wondering >> if this will impact the exynos-clkout module from getting auto-loaded? Without >> the MODULE_DEVICE_TABLE() defined, the aliases won't be created that trigger >> udev to load this module and the mfd driver is obviously not going to load it. > > This driver loaded and matched only against the platform device name Matched yes, but "loaded"? As in module loaded? Are you sure? There is no MODULE_ALIAS, no platform_id_table with MODULE_DEVICE_TABLE, so with this patch all aliases are gone. The old aliases indeed could have been used to load the clkout, even if not used for matching. > ("exynos-clkout") since the mentioned commit 9484f2cb8332. These OF IDs > defined as MODULE_DEVICE_TABLE and of_match_table are leftovers from the > old (pre-9484f2cb8332) instantiating method.and should be removed by > that commit too. Best regards, Krzysztof
On 26.04.2024 11:02, Krzysztof Kozlowski wrote: > On 26/04/2024 07:24, Marek Szyprowski wrote: >> On 26.04.2024 02:24, William McVicker wrote: >>> On 04/25/2024, Marek Szyprowski wrote: >>>> Since commit 9484f2cb8332 ("clk: samsung: exynos-clkout: convert to >>>> module driver") this driver is instantiated as MFD-cell (matched by >>>> platform device name) not as a real platform device created by OF code. >>>> Remove then of_match_table and related MODULE_DEVICE_TABLE to avoid >>>> further confusion. >>>> >>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>> --- >>>> drivers/clk/samsung/clk-exynos-clkout.c | 2 -- >>>> 1 file changed, 2 deletions(-) >>>> >>>> diff --git a/drivers/clk/samsung/clk-exynos-clkout.c b/drivers/clk/samsung/clk-exynos-clkout.c >>>> index 503c6f5b20d5..0c7f4e2aa366 100644 >>>> --- a/drivers/clk/samsung/clk-exynos-clkout.c >>>> +++ b/drivers/clk/samsung/clk-exynos-clkout.c >>>> @@ -75,7 +75,6 @@ static const struct of_device_id exynos_clkout_ids[] = { >>>> .data = &exynos_clkout_exynos5, >>>> }, { } >>>> }; >>>> -MODULE_DEVICE_TABLE(of, exynos_clkout_ids); >>> I understand these are duplicates of the exynos-pmu driver, but was wondering >>> if this will impact the exynos-clkout module from getting auto-loaded? Without >>> the MODULE_DEVICE_TABLE() defined, the aliases won't be created that trigger >>> udev to load this module and the mfd driver is obviously not going to load it. >> This driver loaded and matched only against the platform device name > Matched yes, but "loaded"? As in module loaded? Are you sure? There is > no MODULE_ALIAS, no platform_id_table with MODULE_DEVICE_TABLE, so with > this patch all aliases are gone. > > The old aliases indeed could have been used to load the clkout, even if > not used for matching. Right, the proper solution is to add: MODULE_ALIAS("platform:exynos-clkout"); I was convinced that the module_platform_driver() macro adds simple module alias based on the driver name, but it looks that's not true. Thanks for pointing this out. I will send v2 with the above alias then. Best regards
On 26/04/2024 11:21, Marek Szyprowski wrote: > On 26.04.2024 11:02, Krzysztof Kozlowski wrote: >> On 26/04/2024 07:24, Marek Szyprowski wrote: >>> On 26.04.2024 02:24, William McVicker wrote: >>>> On 04/25/2024, Marek Szyprowski wrote: >>>>> Since commit 9484f2cb8332 ("clk: samsung: exynos-clkout: convert to >>>>> module driver") this driver is instantiated as MFD-cell (matched by >>>>> platform device name) not as a real platform device created by OF code. >>>>> Remove then of_match_table and related MODULE_DEVICE_TABLE to avoid >>>>> further confusion. >>>>> >>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>>> --- >>>>> drivers/clk/samsung/clk-exynos-clkout.c | 2 -- >>>>> 1 file changed, 2 deletions(-) >>>>> >>>>> diff --git a/drivers/clk/samsung/clk-exynos-clkout.c b/drivers/clk/samsung/clk-exynos-clkout.c >>>>> index 503c6f5b20d5..0c7f4e2aa366 100644 >>>>> --- a/drivers/clk/samsung/clk-exynos-clkout.c >>>>> +++ b/drivers/clk/samsung/clk-exynos-clkout.c >>>>> @@ -75,7 +75,6 @@ static const struct of_device_id exynos_clkout_ids[] = { >>>>> .data = &exynos_clkout_exynos5, >>>>> }, { } >>>>> }; >>>>> -MODULE_DEVICE_TABLE(of, exynos_clkout_ids); >>>> I understand these are duplicates of the exynos-pmu driver, but was wondering >>>> if this will impact the exynos-clkout module from getting auto-loaded? Without >>>> the MODULE_DEVICE_TABLE() defined, the aliases won't be created that trigger >>>> udev to load this module and the mfd driver is obviously not going to load it. >>> This driver loaded and matched only against the platform device name >> Matched yes, but "loaded"? As in module loaded? Are you sure? There is >> no MODULE_ALIAS, no platform_id_table with MODULE_DEVICE_TABLE, so with >> this patch all aliases are gone. >> >> The old aliases indeed could have been used to load the clkout, even if >> not used for matching. > > Right, the proper solution is to add: > > MODULE_ALIAS("platform:exynos-clkout"); > > I was convinced that the module_platform_driver() macro adds simple > module alias based on the driver name, but it looks that's not true. > Thanks for pointing this out. I will send v2 with the above alias then. So just add simple platform ID table and MODULE_DEVICE_TABLE. It still allows growing and in general is preferred than spreading aliases manually. Best regards, Krzysztof
diff --git a/drivers/clk/samsung/clk-exynos-clkout.c b/drivers/clk/samsung/clk-exynos-clkout.c index 503c6f5b20d5..0c7f4e2aa366 100644 --- a/drivers/clk/samsung/clk-exynos-clkout.c +++ b/drivers/clk/samsung/clk-exynos-clkout.c @@ -75,7 +75,6 @@ static const struct of_device_id exynos_clkout_ids[] = { .data = &exynos_clkout_exynos5, }, { } }; -MODULE_DEVICE_TABLE(of, exynos_clkout_ids); /* * Device will be instantiated as child of PMU device without its own @@ -237,7 +236,6 @@ static SIMPLE_DEV_PM_OPS(exynos_clkout_pm_ops, exynos_clkout_suspend, static struct platform_driver exynos_clkout_driver = { .driver = { .name = "exynos-clkout", - .of_match_table = exynos_clkout_ids, .pm = &exynos_clkout_pm_ops, }, .probe = exynos_clkout_probe,
Since commit 9484f2cb8332 ("clk: samsung: exynos-clkout: convert to module driver") this driver is instantiated as MFD-cell (matched by platform device name) not as a real platform device created by OF code. Remove then of_match_table and related MODULE_DEVICE_TABLE to avoid further confusion. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/clk/samsung/clk-exynos-clkout.c | 2 -- 1 file changed, 2 deletions(-)