Messages in this thread Patch in this message | | | Date | Mon, 31 Jan 2022 08:41:26 +0000 | From | Jonathan McDowell <> | Subject | Re: [PATCH net] net: phy: Fix qca8081 with speeds lower than 2.5Gb/s |
| |
On Sun, Jan 30, 2022 at 06:09:37PM +0000, Russell King (Oracle) wrote: > 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.
I'm not aware of any reason to do so. I've rolled a v2 that only changes the interface mode when the link is up and it behaves as expected in my testing, so I'll get that sent out later.
J.
-- /-\ | 101 things you can't have too much |@/ Debian GNU/Linux Developer | of : 52 - IRC. \- |
| |