On Wed, 26 Oct 2022 18:26:03 -0700, Stephen Boyd wrote:

Quoting Hal Feng (2022-10-22 21:11:41)

quoted

On Wed, 12 Oct 2022 16:05:23 -0700, Stephen Boyd wrote:

quoted

I think we should use auxiliary bus and split the driver logically into
a reset driver in drivers/reset and a clk driver in drivers/clk. That
way the appropriate maintainers can review the code. There is only one
platform device with a single reg property and node in DT, but there are
two drivers. 

Yes, I agree that the reset driver and the clock driver should be split.
However, I think using auxiliary bus is a little bit complicated in this
case, because the reset is not a part of functionality of the clock in 
JH7110. They just share a common register base address.

That is why auxiliary bus exists.

quoted

I think it is 
better to use ioremap for the same address, and the dt will be like

syscrg_clk: clock-controller@13020000 {
        compatible = "starfive,jh7110-clkgen-sys";
        reg = <0x0 0x13020000 0x0 0x10000>;
        ...
};
syscrg_rst: reset-controller@13020000 {
        compatible = "starfive,jh7110-reset-sys";
        reg = <0x0 0x13020000 0x0 0x10000>;
        ...
};

What do you think of this approach? I would appreciate your suggestions.

We shouldn't have two different nodes with the same reg property. Please
ioremap in whatever driver probes and creates the auxiliary device(s)
and then pass the void __iomem * to it.

Okay, I will use auxiliary bus for clock and reset driver on the next version.

Best regards,
Hal