Re: [PATCH V3 1/3] dt-bindings: clk: sprd: Add bindings for ums512 clock controller

From: Krzysztof Kozlowski
Date: Wed Apr 27 2022 - 02:13:40 EST


On 26/04/2022 07:40, Cixi Geng wrote:
>> You need to help me here with the naming. What is "global registers"
>> range? Let's focus on sharkl3.dtsi and syscon@4035c000 with "rpll".
>>
>> You have a clock controller @4035c000, which provides several clocks,
>> right? Then you have a rpll also @4035c000, so the register range is the
>> same. The register range is the same, isn't it?
>
> the anlg_phy_g5_regs is not a clock controller.
> In fact, this is just to provide an address for other modules to call regmap.
> not provide a clk interface or device.
> The clk configuration of rpll is based on the anlg_phy_g5_regs register.
> The analog_g5 asic document is not only used to configure rpll, but also other
> functions can be configured, but currently our driver is only used to provide
> configuration rpll, so the range of the device node of rpll can be less than or
> equal to the range of anlg_phy_g5_regs.
> Hope this could explains your question

I see, thanks for explanation. Indeed making entire @4035c000
(anlg_phy_g5_regs) a clock controller would not match actual hardware,
since rpll clock is a small part of it. I am afraid though, that you
will duplicate such pattern even for the cases where that
design/register range would be suitable to be a clock controller and a
syscon. In one device.

Please fix the other comments in my review - except this discussed here,
the last one from email:
https://lore.kernel.org/all/714caf6e-5f81-6d73-7629-b2c675f1f1d4@xxxxxxxxxx/


Best regards,
Krzysztof