Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1663514 > unrolled thread

[net-next] net: phy: marvell: Show complete link partner advertising

Started byThomas Bogendoerfer <tsbogend@alpha.franken.de>
First post2017-06-12 15:40 +0200
Last post2017-06-13 10:40 +0200
Articles 5 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [net-next] net: phy: marvell: Show complete link partner advertising Thomas Bogendoerfer <tsbogend@alpha.franken.de> - 2017-06-12 15:40 +0200
    Re: [net-next] net: phy: marvell: Show complete link partner  advertising David Miller <davem@davemloft.net> - 2017-06-12 18:10 +0200
    Re: [net-next] net: phy: marvell: Show complete link partner  advertising Florian Fainelli <f.fainelli@gmail.com> - 2017-06-12 18:10 +0200
      Re: [net-next] net: phy: marvell: Show complete link partner  advertising Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-06-12 18:20 +0200
      Re: [net-next] net: phy: marvell: Show complete link partner  advertising Tom Bogendoerfer <tsbogend@alpha.franken.de> - 2017-06-13 10:40 +0200

#1663514 — [net-next] net: phy: marvell: Show complete link partner advertising

FromThomas Bogendoerfer <tsbogend@alpha.franken.de>
Date2017-06-12 15:40 +0200
Subject[net-next] net: phy: marvell: Show complete link partner advertising
Message-ID<tRxJw-2GO-17@gated-at.bofh.it>
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>
---
 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
-- 
1.8.5.6

[toc] | [next] | [standalone]


#1663813 — Re: [net-next] net: phy: marvell: Show complete link partner advertising

FromDavid Miller <davem@davemloft.net>
Date2017-06-12 18:10 +0200
SubjectRe: [net-next] net: phy: marvell: Show complete link partner advertising
Message-ID<tRA4G-4ju-13@gated-at.bofh.it>
In reply to#1663514
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.

[toc] | [prev] | [next] | [standalone]


#1663838 — Re: [net-next] net: phy: marvell: Show complete link partner advertising

FromFlorian Fainelli <f.fainelli@gmail.com>
Date2017-06-12 18:10 +0200
SubjectRe: [net-next] net: phy: marvell: Show complete link partner advertising
Message-ID<tRA4J-4ju-75@gated-at.bofh.it>
In reply to#1663514
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
> 


-- 
Florian

[toc] | [prev] | [next] | [standalone]


#1663860 — Re: [net-next] net: phy: marvell: Show complete link partner advertising

FromRussell King - ARM Linux <linux@armlinux.org.uk>
Date2017-06-12 18:20 +0200
SubjectRe: [net-next] net: phy: marvell: Show complete link partner advertising
Message-ID<tRAen-4ne-43@gated-at.bofh.it>
In reply to#1663838
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.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]


#1664596 — Re: [net-next] net: phy: marvell: Show complete link partner advertising

FromTom Bogendoerfer <tsbogend@alpha.franken.de>
Date2017-06-13 10:40 +0200
SubjectRe: [net-next] net: phy: marvell: Show complete link partner advertising
Message-ID<tRPwK-5EB-9@gated-at.bofh.it>
In reply to#1663838
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.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web