netdev - Re: [PATCH net] net: phy: Fix qca8081 with speeds lower than 2.5Gb/s

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: <YfasMniiA8wn+isu@earth.li>
Date:   Sun, 30 Jan 2022 15:18:10 +0000
From:   Jonathan McDowell <noodles@...th.li>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Luo Jie <luoj@...eaurora.org>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Robert Marko <robimarko@...il.com>
Subject: Re: [PATCH net] net: phy: Fix qca8081 with speeds lower than 2.5Gb/s

On Sun, Jan 30, 2022 at 12:40:57PM +0000, Russell King (Oracle) wrote:
> On Sun, Jan 30, 2022 at 10:25:28AM +0000, Jonathan McDowell wrote:
> > A typo in qca808x_read_status means we try to set SMII mode on the port
> > rather than SGMII when the link speed is not 2.5Gb/s. This results in no
> > traffic due to the mismatch in configuration between the phy and the
> > mac.
> > 
> > Fixes: 79c7bc0521545 ("net: phy: add qca8081 read_status")
> > Signed-off-by: Jonathan McDowell <noodles@...th.li>
> > ---
> >  drivers/net/phy/at803x.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> > index 5b6c0d120e09..7077e3a92d31 100644
> > --- a/drivers/net/phy/at803x.c
> > +++ b/drivers/net/phy/at803x.c
> > @@ -1691,7 +1691,7 @@ static int qca808x_read_status(struct phy_device *phydev)
> >  	if (phydev->link && phydev->speed == SPEED_2500)
> >  		phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
> >  	else
> > -		phydev->interface = PHY_INTERFACE_MODE_SMII;
> > +		phydev->interface = PHY_INTERFACE_MODE_SGMII;
> 
> Is it intentional to set the interface to SGMII also when there is no
> link?

My reading of the code is that if this was just a GigE capable phy the
interface would be set once and never changed/unset. The only reason
it happens here is because the link changes to support the 2.5G mode, so
there's no problem with it defaulting to SGMII even when the external
link isn't actually up. Perhap Luo can confirm if this is the case?

J.

-- 
Web [  101 things you can't have too much of : 30 - Comfy sofas.   ]
site: https:// [                                          ]      Made by
www.earth.li/~noodles/  [                      ]         HuggieTag 0.0.24