Hello Russell,
On Tue, 30 Jan 2024, Russell King (Oracle) wrote:
...
> > +int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs)
> > +{
> > + int ret = 0;
> > +
> > + /* Signal to PCS driver that MAC requires RX clock for init */
> > + if (pl->config->mac_requires_rxc)
> > + pcs->rxc_always_on = true;
> > +
> > + if (pcs->ops->pcs_pre_init)
> > + ret = pcs->ops->pcs_pre_init(pcs, pl->link_config.interface);
>
> Given that:
> 1) phylink supports switching between mutliple different interfaces,
> 2) from what I can see you are only calling this from stmmac's
> initialisation path,
> 3) you pass the interface mode to the PCS here
>
> then we don't want the PCS to configure itself for the interface mode
> passed in, because this function won't be called when the interface
> mode changes - and PCS driver authors will have to bear that in mind.
> So...
>
...
> However, do we really need it - if the PCS is supplying the RXC to
> the MAC, then is the interface mode between the PCS and PHY all that
> relevant at this point?
If a PCS can set the needed clock signal without configuring the details
of a particular link mode, then passing the interface mode to pcs_pre_init()
would indeed not be relevant. Generally, I agree that setting the interface mode
shouldn't be the concern of the pre-initialization function. I'll dig a bit
more into the PCS datasheet and run more tests to see if I can get away with
enabling the RX clock selectively for this particular PCS model. If not, then
maybe I can hardcode a "default" interface mode for the pre-initialization that
will not interfere with the rest of the link setup process.
Best Regards,
--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
