>> + 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
>supported?
Yes
>In which case, gem_phylink_validate() must clear the support mask when
>SGMII mode is requested to indicate that the interface mode is not
>supported.
>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.
Regards,
Parshuram Thombare
