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]

Message-ID: <20180802140323.GB7462@lunn.ch>
Date:   Thu, 2 Aug 2018 16:03:23 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     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

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

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

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