On Wed, Mar 12, 2025 at 04:43:51PM -0700, Stephen Boyd wrote: > Quoting Inochi Amaoto (2025-03-12 16:29:43) > > On Wed, Mar 12, 2025 at 04:14:37PM -0700, Stephen Boyd wrote: > > > Quoting Inochi Amaoto (2025-03-11 16:31:29) > > > > > > > > > or if that syscon node should just have the #clock-cells property as > > > > > part of the node instead. > > > > > > > > This is not match the hardware I think. The pll area is on the middle > > > > of the syscon and is hard to be separated as a subdevice of the syscon > > > > or just add "#clock-cells" to the syscon device. It is better to handle > > > > them in one device/driver. So let the clock device reference it. > > > > > > This happens all the time. We don't need a syscon for that unless the > > > registers for the pll are both inside the syscon and in the register > > > space 0x50002000. Is that the case? > > > > Yes, the clock has two areas, one in the clk controller and one in > > the syscon, the vendor said this design is a heritage from other SoC. > > My question is more if the PLL clk_ops need to access both the syscon > register range and the clk controller register range. What part of the > PLL clk_ops needs to access the clk controller at 0x50002000? > The PLL clk_ops does nothing, but there is an implicit dependency: When the PLL change rate, the mux attached to it must switch to another source to keep the output clock stable. This is the only thing it needed. > > > > > This looks like you want there to be one node for clks on the system > > > because logically that is clean, when the reality is that there is a > > > PLL block exposed in the syscon (someone forgot to put it in the clk > > > controller?) and a non-PLL block for the other clks. > > > > That is true, I prefer to keep clean and make less mistakes. Although > > the PLL is exposed in the syscon, the pll need to be tight with other > > clocks in the space 0x50002000 (especially between the PLL and mux). > > In this view, it is more like a mistake made by the hardware design. > > And I prefer not to add a subnode for the syscon. > > Ok. You wouldn't add a subnode for the syscon. You would just have > #clock-cells in that syscon node and register an auxiliary device to > provide the PLL(s) from there. Then in drivers/clk we would have an > auxiliary driver that uses a regmap or gets an iomem pointer from the > parent device somehow so that we can logically put the PLL code in > drivers/clk while having one node in DT for the "miscellaneous register > area" where the hardware engineer had to expose the PLL control to > software. Cool, I understand what you mean. It is a good idea, I will have a try. Regards, Inochi
