Re: [PATCH v3 07/42] soc: Add SoC driver for Cirrus ep93xx

[Krzysztof Kozlowski]

On 24/07/2023 20:04, Arnd Bergmann wrote:
> 
>>>>> +
>>>>> +       pr_info("EP93xx SoC revision %s\n", attrs->revision);
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +core_initcall(ep93xx_soc_init);
>>>>
>>>> That's not the way to add soc driver. You need a proper driver for it
>>>
>>> What is a proper driver for these ? 
>>
>> Usually platform_driver, e.g. exynos-chipid.
> 
> Using a platform driver sounds like the right thing to do here,
> it's cleaner and should not be hard to do if the driver just matches
> the cirrus,ep9301-syscon node. I would have just merged
> the v3 version, but this is something that makes sense changing
> in v4.
> 
>>> Even the latest additions in drivers/soc/* go with *_initcall.
>>
>> Which one? Ones which call platform_driver_register()? That's quite
>> different story, isn't it? I don't see other recent cases, except
>> regulator couplers. They might be, some people send and accept poor
>> code, so what do you expect from us? Crappy code got in once, so more of
>> it can go?
> 
> That's not what is happening here, this is all part of an incremental
> improvement of the existing code. If the DT bindings make sense, I'm
> happy to take some shortcuts with the implementation that keep it
> closer to the existing implementation and either clean them up over
> time or just throw out the platform in five or ten years when the last
> machines are dead.

I am not saying that this is happening here, but the argument that once
we took some patch is not the reason to keep going. For sure we could
cut here some slack, unlike for big SoC vendors...

Best regards,
Krzysztof