On 5/16/23 09:22, Vinod Koul wrote:

On 09-05-23, 11:26, Sean Anderson wrote:

quoted

On 5/9/23 09:00, Vinod Koul wrote:

quoted

On 08-05-23, 11:31, Sean Anderson wrote:

quoted

On 5/8/23 05:15, Vinod Koul wrote:

quoted

quoted

quoted

+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/ (local)

--Sean