On 5/16/23 09:22, Vinod Koul wrote: > On 09-05-23, 11:26, Sean Anderson wrote: >> On 5/9/23 09:00, Vinod Koul wrote: >> > On 08-05-23, 11:31, Sean Anderson wrote: >> >> On 5/8/23 05:15, Vinod Koul wrote: >> > >> >> >> +int lynx_clks_init(struct device *dev, struct regmap *regmap, >> >> >> + struct clk *plls[2], struct clk *ex_dlys[2], bool compat); >> >> > >> >> > so you have an exported symbol for clk driver init in phy driver header? >> >> > can you please explain why..? >> >> >> >> So that it can be called at the appropriate time during the phy's probe function. >> >> >> >> This is really an integral part of the phy driver, but I was directed to split it >> >> off and put it in another subsystem's directory. >> > >> > That is right clock should be belong to clk driver. IIUC the hardware is >> > phy along with clocks and you are doing the clk init. I think that may >> > not be correct model, you should really have a device tree node to >> > represent the clock and the phy node >> > >> > >> > What stops this from being modelled as it is in the hardware? >> >> It *is* modeled as it is in hardware. With just the serdes compatible, >> we have all the information we need to create the clocks. So we do so. >> There's no need for a separate device just to create four clocks. >> >> These clocks cannot be used by any other device (except possibly by >> putting a lane into test mode). So there is no benefit from making them >> a separate device, except an increase in complexity due to ordering and >> dynamic lookup. By doing things this way we know that either there was >> an error or the clocks all exist, and the lifetime of the clocks matches >> the serdes. > > Okay that does makes sense. > > In that case why should this not be in the phy driver itself. There are > already few examples og phy drivers having inbuild clk where is makes > sense. You register the clk_ops as part of phy driver You told me to do it this way https://lore.kernel.org/linux-phy/YsW+4fm%2F613ByK09@matsya/ --Sean