Re: [PATCH net-next v3 7/7] net: pcs: rzn1-miic: Init RX clock early if MAC requires it

[Romain Gantois <romain.gantois@bootlin.com>]

Hi Maxime,

On Mon, 12 Feb 2024, Maxime Chevallier wrote:

> > +static int miic_pre_init(struct phylink_pcs *pcs)
> > +{
> > +	struct miic_port *miic_port = phylink_pcs_to_miic_port(pcs);
> > +	struct miic *miic = miic_port->miic;
> > +	u32 val;
> > +
> > +	/* Start RX clock if required */
> > +	if (pcs->rxc_always_on) {
> > +		/* In MII through mode, the clock signals will be driven by the
> > +		 * external PHY, which might not be initialized yet. Set RMII
> > +		 * as default mode to ensure that a reference clock signal is
> > +		 * generated.
> > +		 */
> > +		miic_port->interface = PHY_INTERFACE_MODE_RMII;
> 
> There's this check in miic_config :
> 
> 	if (interface != miic_port->interface) {
> 		val |= FIELD_PREP(MIIC_CONVCTRL_CONV_SPEED, speed);
> 		mask |= MIIC_CONVCTRL_CONV_SPEED;
> 		miic_port->interface = interface;
> 	}
> 
> As you set the interface to RMII and set the CONV_MODE below without
> really looking at the speed, is there any risk of a mismatch between
> the configured mode and the speed ?
Good point, it is probably necessary to set the default RMII speed in 
miic_pre_init(), since miic_config will not do it if the link mode hasn't 
changed in the meantime. However, this is only an issue if the link isn't 
already up when miic_config() is called. If the link is up, then that means that 
miic_link_up() has already been called and has set the appropriate speed anyway.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com