Message ID | 20170612133237.E20945085BC@solo.franken.de |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On 06/12/2017 05:54 AM, Thomas Bogendoerfer wrote: > From: Thomas Bogendoerfer <tbogendoerfer@suse.de> > > Give back all modes advertised by the link partner. This change brings > the marvell phy driver in line with all other phy drivers. > > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> I thought Russell had a similar patch but I can't find it applied in net-next, so: Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> drivers/net/phy/lxt.c has a similar pattern that would be worth fixing too. > --- > drivers/net/phy/marvell.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > index 4c5246fed69b..8400403b3f62 100644 > --- a/drivers/net/phy/marvell.c > +++ b/drivers/net/phy/marvell.c > @@ -1139,7 +1139,6 @@ static int marvell_read_status_page_an(struct phy_device *phydev, > int status; > int lpa; > int lpagb; > - int adv; > > status = phy_read(phydev, MII_M1011_PHY_STATUS); > if (status < 0) > @@ -1153,12 +1152,6 @@ static int marvell_read_status_page_an(struct phy_device *phydev, > if (lpagb < 0) > return lpagb; > > - adv = phy_read(phydev, MII_ADVERTISE); > - if (adv < 0) > - return adv; > - > - lpa &= adv; > - > if (status & MII_M1011_PHY_STATUS_FULLDUPLEX) > phydev->duplex = DUPLEX_FULL; > else >
From: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Date: Mon, 12 Jun 2017 14:54:57 +0200 > From: Thomas Bogendoerfer <tbogendoerfer@suse.de> > > Give back all modes advertised by the link partner. This change brings > the marvell phy driver in line with all other phy drivers. > > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> Applied, thanks.
On Mon, Jun 12, 2017 at 09:05:04AM -0700, Florian Fainelli wrote: > On 06/12/2017 05:54 AM, Thomas Bogendoerfer wrote: > > From: Thomas Bogendoerfer <tbogendoerfer@suse.de> > > > > Give back all modes advertised by the link partner. This change brings > > the marvell phy driver in line with all other phy drivers. > > > > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> > > I thought Russell had a similar patch but I can't find it applied in > net-next, so: I do, it has a subject line of "net: phy: fix marvell phy status reading" and it's already been sent (first link). DaveM said he applied it (second link): https://www.mail-archive.com/netdev@vger.kernel.org/msg170743.html https://www.mail-archive.com/netdev@vger.kernel.org/msg171036.html However, Thomas' patch removes slightly more code (I didn't spot that "adv" is no longer used and we don't need to read the MII_ADVERTISE register anymore.)
On Mon, Jun 12, 2017 at 09:05:04AM -0700, Florian Fainelli wrote: > On 06/12/2017 05:54 AM, Thomas Bogendoerfer wrote: > > From: Thomas Bogendoerfer <tbogendoerfer@suse.de> > > > > Give back all modes advertised by the link partner. This change brings > > the marvell phy driver in line with all other phy drivers. > > > > Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de> > > I thought Russell had a similar patch but I can't find it applied in > net-next, so: > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > > drivers/net/phy/lxt.c has a similar pattern that would be worth fixing too. that's different and correct. The lpa value is not exported as lp_advertising, but only used internal. Well the bug here is IMHO, that it doesn't export lpa to lp_advertising at all as it's done in genphy_read_status(). And from a quick grep there are more phy drivers doing that... I'll have a look later. Thomas.
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 4c5246fed69b..8400403b3f62 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -1139,7 +1139,6 @@ static int marvell_read_status_page_an(struct phy_device *phydev, int status; int lpa; int lpagb; - int adv; status = phy_read(phydev, MII_M1011_PHY_STATUS); if (status < 0) @@ -1153,12 +1152,6 @@ static int marvell_read_status_page_an(struct phy_device *phydev, if (lpagb < 0) return lpagb; - adv = phy_read(phydev, MII_ADVERTISE); - if (adv < 0) - return adv; - - lpa &= adv; - if (status & MII_M1011_PHY_STATUS_FULLDUPLEX) phydev->duplex = DUPLEX_FULL; else