All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode
@ 2019-02-14 21:12 Heiner Kallweit
  2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Heiner Kallweit @ 2019-02-14 21:12 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev

Improve phy_resolve_aneg_linkmode and use it in genphy_read_status.

Heiner Kallweit (2):
  net: phy: improve phy_resolve_aneg_linkmode
  net: phy: use phy_resolve_aneg_linkmode in genphy_read_status

 drivers/net/phy/phy-core.c   | 43 ++++++------------------------------
 drivers/net/phy/phy_device.c | 24 +-------------------
 2 files changed, 8 insertions(+), 59 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode
  2019-02-14 21:12 [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode Heiner Kallweit
@ 2019-02-14 21:15 ` Heiner Kallweit
  2019-02-15 13:57   ` Andrew Lunn
  2019-02-14 21:16 ` [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status Heiner Kallweit
  2019-02-17 23:22 ` [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Heiner Kallweit @ 2019-02-14 21:15 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev

We have the settings array of modes which is sorted based on aneg
priority. Instead of checking each mode manually let's simply iterate
over the sorted settings.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy-core.c | 43 +++++++-------------------------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index cdea028d1..5d43106fe 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
 void phy_resolve_aneg_linkmode(struct phy_device *phydev)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	int i;
 
 	linkmode_and(common, phydev->lp_advertising, phydev->advertising);
 
-	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) {
-		phydev->speed = SPEED_10000;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_5000;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_2500;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_1000;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
-				     common)) {
-		phydev->speed = SPEED_1000;
-		phydev->duplex = DUPLEX_HALF;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_100;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
-				     common)) {
-		phydev->speed = SPEED_100;
-		phydev->duplex = DUPLEX_HALF;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_10;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
-				     common)) {
-		phydev->speed = SPEED_10;
-		phydev->duplex = DUPLEX_HALF;
-	}
+	for (i = 0; i < ARRAY_SIZE(settings); i++)
+		if (test_bit(settings[i].bit, common)) {
+			phydev->speed = settings[i].speed;
+			phydev->duplex = settings[i].duplex;
+			break;
+		}
 
 	if (phydev->duplex == DUPLEX_FULL) {
 		phydev->pause = linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status
  2019-02-14 21:12 [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode Heiner Kallweit
  2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit
@ 2019-02-14 21:16 ` Heiner Kallweit
  2019-02-17 23:22 ` [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2019-02-14 21:16 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev

Now that we have phy_resolve_aneg_linkmode() we can make
genphy_read_status() much simpler.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2c61282a2..d7ccbd94f 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1747,8 +1747,6 @@ int genphy_read_status(struct phy_device *phydev)
 	int err;
 	int lpa;
 	int lpagb = 0;
-	int common_adv;
-	int common_adv_gb = 0;
 
 	/* Update the link, but return if there was an error */
 	err = genphy_update_link(phydev);
@@ -1780,7 +1778,6 @@ int genphy_read_status(struct phy_device *phydev)
 
 			mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising,
 							lpagb);
-			common_adv_gb = lpagb & adv << 2;
 		}
 
 		lpa = phy_read(phydev, MII_LPA);
@@ -1793,31 +1790,12 @@ int genphy_read_status(struct phy_device *phydev)
 		if (adv < 0)
 			return adv;
 
-		common_adv = lpa & adv;
-
 		phydev->speed = SPEED_10;
 		phydev->duplex = DUPLEX_HALF;
 		phydev->pause = 0;
 		phydev->asym_pause = 0;
 
-		if (common_adv_gb & (LPA_1000FULL | LPA_1000HALF)) {
-			phydev->speed = SPEED_1000;
-
-			if (common_adv_gb & LPA_1000FULL)
-				phydev->duplex = DUPLEX_FULL;
-		} else if (common_adv & (LPA_100FULL | LPA_100HALF)) {
-			phydev->speed = SPEED_100;
-
-			if (common_adv & LPA_100FULL)
-				phydev->duplex = DUPLEX_FULL;
-		} else
-			if (common_adv & LPA_10FULL)
-				phydev->duplex = DUPLEX_FULL;
-
-		if (phydev->duplex == DUPLEX_FULL) {
-			phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
-			phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
-		}
+		phy_resolve_aneg_linkmode(phydev);
 	} else {
 		int bmcr = phy_read(phydev, MII_BMCR);
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode
  2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit
@ 2019-02-15 13:57   ` Andrew Lunn
  2019-02-15 18:57     ` Heiner Kallweit
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2019-02-15 13:57 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev

On Thu, Feb 14, 2019 at 10:15:31PM +0100, Heiner Kallweit wrote:
> We have the settings array of modes which is sorted based on aneg
> priority. Instead of checking each mode manually let's simply iterate
> over the sorted settings.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/phy/phy-core.c | 43 +++++++-------------------------------
>  1 file changed, 7 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
> index cdea028d1..5d43106fe 100644
> --- a/drivers/net/phy/phy-core.c
> +++ b/drivers/net/phy/phy-core.c
> @@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
>  void phy_resolve_aneg_linkmode(struct phy_device *phydev)
>  {
>  	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
> +	int i;
>  
>  	linkmode_and(common, phydev->lp_advertising, phydev->advertising);
>  
> -	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) {
> -		phydev->speed = SPEED_10000;
> -		phydev->duplex = DUPLEX_FULL;
> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
> -				     common)) {
> -		phydev->speed = SPEED_5000;
> -		phydev->duplex = DUPLEX_FULL;
> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
> -				     common)) {
> -		phydev->speed = SPEED_2500;
> -		phydev->duplex = DUPLEX_FULL;
> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
> -				     common)) {
> -		phydev->speed = SPEED_1000;
> -		phydev->duplex = DUPLEX_FULL;
> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
> -				     common)) {
> -		phydev->speed = SPEED_1000;
> -		phydev->duplex = DUPLEX_HALF;
> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
> -				     common)) {
> -		phydev->speed = SPEED_100;
> -		phydev->duplex = DUPLEX_FULL;
> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
> -				     common)) {
> -		phydev->speed = SPEED_100;
> -		phydev->duplex = DUPLEX_HALF;
> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
> -				     common)) {
> -		phydev->speed = SPEED_10;
> -		phydev->duplex = DUPLEX_FULL;
> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
> -				     common)) {
> -		phydev->speed = SPEED_10;
> -		phydev->duplex = DUPLEX_HALF;
> -	}
> +	for (i = 0; i < ARRAY_SIZE(settings); i++)
> +		if (test_bit(settings[i].bit, common)) {
> +			phydev->speed = settings[i].speed;
> +			phydev->duplex = settings[i].duplex;
> +			break;
> +		}

Hi Heiner

Nice simplification.

I just have one thought about this. The original code was limited to
baseT. The new code could in theory return a non BaseT speed. For that
to happen, it would require that phydev->lp_advertising and
phydev->advertising contain a non BaseT link mode? Is that possible?
I don't think it is.

  Andrew

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode
  2019-02-15 13:57   ` Andrew Lunn
@ 2019-02-15 18:57     ` Heiner Kallweit
  0 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2019-02-15 18:57 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev

On 15.02.2019 14:57, Andrew Lunn wrote:
> On Thu, Feb 14, 2019 at 10:15:31PM +0100, Heiner Kallweit wrote:
>> We have the settings array of modes which is sorted based on aneg
>> priority. Instead of checking each mode manually let's simply iterate
>> over the sorted settings.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/net/phy/phy-core.c | 43 +++++++-------------------------------
>>  1 file changed, 7 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
>> index cdea028d1..5d43106fe 100644
>> --- a/drivers/net/phy/phy-core.c
>> +++ b/drivers/net/phy/phy-core.c
>> @@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
>>  void phy_resolve_aneg_linkmode(struct phy_device *phydev)
>>  {
>>  	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
>> +	int i;
>>  
>>  	linkmode_and(common, phydev->lp_advertising, phydev->advertising);
>>  
>> -	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) {
>> -		phydev->speed = SPEED_10000;
>> -		phydev->duplex = DUPLEX_FULL;
>> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
>> -				     common)) {
>> -		phydev->speed = SPEED_5000;
>> -		phydev->duplex = DUPLEX_FULL;
>> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> -				     common)) {
>> -		phydev->speed = SPEED_2500;
>> -		phydev->duplex = DUPLEX_FULL;
>> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
>> -				     common)) {
>> -		phydev->speed = SPEED_1000;
>> -		phydev->duplex = DUPLEX_FULL;
>> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
>> -				     common)) {
>> -		phydev->speed = SPEED_1000;
>> -		phydev->duplex = DUPLEX_HALF;
>> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
>> -				     common)) {
>> -		phydev->speed = SPEED_100;
>> -		phydev->duplex = DUPLEX_FULL;
>> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
>> -				     common)) {
>> -		phydev->speed = SPEED_100;
>> -		phydev->duplex = DUPLEX_HALF;
>> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
>> -				     common)) {
>> -		phydev->speed = SPEED_10;
>> -		phydev->duplex = DUPLEX_FULL;
>> -	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
>> -				     common)) {
>> -		phydev->speed = SPEED_10;
>> -		phydev->duplex = DUPLEX_HALF;
>> -	}
>> +	for (i = 0; i < ARRAY_SIZE(settings); i++)
>> +		if (test_bit(settings[i].bit, common)) {
>> +			phydev->speed = settings[i].speed;
>> +			phydev->duplex = settings[i].duplex;
>> +			break;
>> +		}
> 
> Hi Heiner
> 
> Nice simplification.
> 
> I just have one thought about this. The original code was limited to
> baseT. The new code could in theory return a non BaseT speed. For that
> to happen, it would require that phydev->lp_advertising and
> phydev->advertising contain a non BaseT link mode? Is that possible?
> I don't think it is.
> 
Currently we set only BaseT modes because that's what the clause 45
standard registers offer. However drivers may come that use vendor
registers for e.g. backplane auto-negotiation (IIRC clause 73).

Now it's even better because this function shouldn't be (and doesn't
have to be) limited to a specific physical link type.

>   Andrew
> 
Heiner


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode
  2019-02-14 21:12 [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode Heiner Kallweit
  2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit
  2019-02-14 21:16 ` [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status Heiner Kallweit
@ 2019-02-17 23:22 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2019-02-17 23:22 UTC (permalink / raw)
  To: hkallweit1; +Cc: andrew, f.fainelli, netdev

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Thu, 14 Feb 2019 22:12:59 +0100

> Improve phy_resolve_aneg_linkmode and use it in genphy_read_status.

Series applied, thanks Heiner.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-02-17 23:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 21:12 [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode Heiner Kallweit
2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit
2019-02-15 13:57   ` Andrew Lunn
2019-02-15 18:57     ` Heiner Kallweit
2019-02-14 21:16 ` [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status Heiner Kallweit
2019-02-17 23:22 ` [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.