Re: [PATCH] clk: samsung: exynos-clkout: Remove misleading of_match_table/MODULE_DEVICE_TABLE

Krzysztof Kozlowski
2024-04-26T09:26:20Z

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