On 16-05-23, 11:11, Sean Anderson wrote: > 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
I agree that was not the right recommendation back then and your view was right -- ~Vinod