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

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

[Thomas Bogendoerfer]

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

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

[Florian Fainelli]

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

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

[Russell King - ARM Linux]

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.

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

[Tom Bogendoerfer]

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 ]

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

[David Miller]

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.