[PATCH v14 06/15] clk: Add Lynx 10G SerDes PLL driver

[Sean Anderson]

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