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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 Aug 2018 15:15:19 +0100
From:   Jose Abreu <Jose.Abreu@...opsys.com>
To:     Andrew Lunn <andrew@...n.ch>, Jose Abreu <Jose.Abreu@...opsys.com>
CC:     <netdev@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>,
        "Joao Pinto" <Joao.Pinto@...opsys.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>
Subject: Re: [PATCH net-next 7/9] net: stmmac: Integrate XGMAC into main
 driver flow

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