linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Jonathan McDowell <noodles@earth.li>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	David Miller <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Luo Jie <luoj@codeaurora.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Robert Marko <robimarko@gmail.com>
Subject: Re: [PATCH net] net: phy: Fix qca8081 with speeds lower than 2.5Gb/s
Date: Sun, 30 Jan 2022 18:09:37 +0000	[thread overview]
Message-ID: <YfbUYcOLiikv9Pyv@shell.armlinux.org.uk> (raw)
In-Reply-To: <YfasMniiA8wn+isu@earth.li>

On Sun, Jan 30, 2022 at 03:18:10PM +0000, Jonathan McDowell wrote:
> 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@earth.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?

My point is that other PHY drivers only change the interface mode when
the link comes up, and we ought to have consistency between PHY drivers
rather than each PHY driver deciding on different behaviours - unless
there is a good reason to be different.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2022-01-30 18:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-30 10:25 [PATCH net] net: phy: Fix qca8081 with speeds lower than 2.5Gb/s Jonathan McDowell
2022-01-30 12:40 ` Russell King (Oracle)
2022-01-30 15:18   ` Jonathan McDowell
2022-01-30 18:09     ` Russell King (Oracle) [this message]
2022-01-31  8:41       ` Jonathan McDowell
2022-01-31 13:56 ` [PATCH net v2] " Jonathan McDowell
2022-01-31 14:19   ` Russell King (Oracle)
2022-02-01 14:30   ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YfbUYcOLiikv9Pyv@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luoj@codeaurora.org \
    --cc=netdev@vger.kernel.org \
    --cc=noodles@earth.li \
    --cc=robimarko@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).