RE: [PATCH v5 2/5] net: macb: add support for sgmii MAC-PHY interface

From: Parshuram Raju Thombare
Date: Tue Jun 25 2019 - 05:26:46 EST

>> + if (change_interface) {
>> + bp->phy_interface = state->interface;
>> + gem_writel(bp, NCR, ~GEM_BIT(TWO_PT_FIVE_GIG) &
>> + gem_readl(bp, NCR));
>This could do with a comment, such as the one I gave in my example.
Sure. I will add a comment here.

>> @@ -493,6 +516,7 @@ static void gem_mac_config(struct phylink_config
>*pl_config, unsigned int mode,
>> reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
>> if (macb_is_gem(bp))
>> reg &= ~GEM_BIT(GBE);
>> +
>Useless change.
Ok, I will remove this empty line.

>> + if (phy_mode == PHY_INTERFACE_MODE_SGMII) {
>> + if (!(bp->caps & MACB_CAPS_PCS))
>> + interface_supported = 0;
>So if bp->caps does not have MACB_CAPS_PCS set, then SGMII mode is not

>In which case, gem_phylink_validate() must clear the support mask when
>SGMII mode is requested to indicate that the interface mode is not
>The same goes for _all_ other PHY link modes that the hardware does not
>actually support, such as PHY_INTERFACE_MODE_10GKR...
If interface is not supported by hardware probe returns with error, so we don't
net interface is not registered at all.
I think what is missing is setting appropriate err value (-ENOTSUPP ?), right now it is returning
default err.

Parshuram Thombare