Hi Andrew,
On 02-08-2018 15:03, Andrew Lunn wrote:
> On Thu, Aug 02, 2018 at 09:26:28AM +0100, Jose Abreu wrote:
>> 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 ...
> Please add that to the commit message. It is useful for people to know
> this is untested above 1G, and so probably broken....
Ok, will do.
>
>> 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.
> MDIO is the control plane used to manage the PHY. But here we are
> talking about the data plane. As i said, the link between the MAC and
> PHY will need to change depending on what the PHY is doing. SGMII will
> work for 10/100/1000, but nothing above that.
Sorry, I made a mistake. Where it reads SGMII in my reply I was
referring to XGMII.
> It could be this speed
> register also changes the SERDES configuration, but you really should
> confirm this and find out exactly what it is doing. There can be
> multiple ways of doing one speed, e.g. SGMII at 1G. So if the PHY
> wants you to do 1000Base-X and the MAC can only do SGMII, you need to
> be raising an error. phylink makes this simpler. It ask the MAC driver
> for all the modes it supports. It will then not ask the MAC to swap to
> something it does not support.
Ok. XGMII support is optional in the MAC so I will need to add a
check for that.
Thanks and Best Regards,
Jose Miguel Abreu
>
> I suggest you get the datasheet for the PHY you are expecting to get,
> once shipping is fixed. See what it says about its MAC side interface.
> You can also look at the Marvell 10G driver, e.g.
> mv3310_update_interface().
>
> Andrew
