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

[Parshuram Raju Thombare <pthombar@cadence.com>]

>> +	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