Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main driver flow

[Jose Abreu <Jose.Abreu@synopsys.com>]

Hi Andrew,

Thanks for the review!

On 01-08-2018 16:23, Andrew Lunn wrote:
>> @@ -842,6 +863,12 @@ static void stmmac_adjust_link(struct net_device *dev)
>>  			new_state = true;
>>  			ctrl &= ~priv->hw->link.speed_mask;
>>  			switch (phydev->speed) {
>> +			case SPEED_10000:
>> +				ctrl |= priv->hw->link.speed10000;
>> +				break;
>> +			case SPEED_2500:
>> +				ctrl |= priv->hw->link.speed2500;
>> +				break;
>>  			case SPEED_1000:
>>  				ctrl |= priv->hw->link.speed1000;
>>  				break;
> Hi Jose
>
> What PHY did you test this with?

We had some shipping issues with the 10G phy so right now I'm
using a 1G phy ... I would expect that as MDIO is used in both
phys then phylib would take care of everything as long as I
specify in the DT the right interface (SGMII) ... Am I making a
wrong assumption?

>
> 10G phys change the interface mode when the speed change. In general,
> 10/100/1000G copper uses SGMII. A 1G SFP optical module generally
> wants 1000Base-X. 2.5G wants 2500Base-X, 10G copper wants 10GKR, etc.
>
> So your adjust link callback needs to look at phydev->interface and
> reconfigure the MAC as requested.

Sorry, I'm not a phy expert but as long as I use MDIO shouldn't
this be transparent to MAC? I mean, there are no registers about
the interface to use in XGMAC2, there is only this speed
selection register that its implemented already in the
stmmac_adjust_link.

>
> You might also want to consider moving from phylib to phylink. It has
> a better interface for things like this, and makes support for SFP
> interfaces much easier. A MAC which supports 10G is likely to be used
> with SFPs...

Ok, I will take a look into it.

Thanks and Best Regards,
Jose Miguel Abreu

>
>      Andrew