RE: [PATCH v3] net: macb: add support for high speed interface

From: Parshuram Raju Thombare
Date: Thu Oct 22 2020 - 07:03:43 EST


>If you're going to support pcs_ops for the 10GBASE-R mode, can you
>also convert macb to use pcs_ops for the lower speed modes as well?

Ok, I will add pcs_ops for low speed as well. In fact, having single
pcs_ops and using check for interface type within each method of
pcs_ops to decide appropriate action will also solve the below mentioned
issue when phylink requests 10GBASE-R and subsequently selects a
different interface mode.

Since macb_mac_an_restart is empty, macb_mac_pcs_get_state only
sets "state->link = 0" and there is nothing to be done in pcs_config, there will
be no changes in pcs_ops methods pcs_get_state, pcs_config and pcs_link_up,
except guarding current code in these methods by
interface == PHY_INTERFACE_MODE_10GBASER check.

pcs_config and pcs_link_up passes "interface" as an argument, and in
pcs_get_state call "state->interface" appeared to be populated just before
calling it and hence should be valid.

528 state->interface = pl->link_config.interface;
...
535
536 if (pl->pcs_ops)
537 pl->pcs_ops->pcs_get_state(pl->pcs, state);
538 else if (pl->mac_ops->mac_pcs_get_state)
539 pl->mac_ops->mac_pcs_get_state(pl->config, state);
540 else
541 state->link = 0;

>> + val = gem_readl(bp, NCFGR);
>> + val = GEM_BIT(PCSSEL) | (~GEM_BIT(SGMIIEN) & val);
>> + gem_writel(bp, NCFGR, val);
>
>This looks like it's configuring the MAC rather than the PCS - it
>should probably be in mac_prepare() or mac_config().

Ok

>What happens if phylink requests 10GBASE-R and subsequently selects
>a different interface mode? You end up with the PCS still registered
>and phylink will use it even when not in 10GBASE-R mode - so its
>functions will also be called.