Re: [PATCH 1/2] net: phy: Helper function for reading strapped configuration values

[Esben Haabendal]

Florian Fainelli  writes:

> On 04/05/2018 04:44 AM, esben.haaben...@gmail.com wrote:
>> From: Esben Haabendal 
>> 
>> Add a function for use in PHY driver probe functions, reading current
>> autoneg, speed and duplex configuration from BMCR register.
>> 
>> Useful for PHY that supports hardware strapped configuration, enabling
>> Linux to respect that configuration (i.e. strapped non-autoneg
>> configuration).
>> 
>> Signed-off-by: Esben Haabendal 
>> Cc: Rasmus Villemoes 
>> ---
>>  drivers/net/phy/phy_device.c | 41 +
>>  include/linux/phy.h  |  1 +
>>  2 files changed, 42 insertions(+)
>> 
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 74664a6c0cdc..cc52ff2a2344 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -1673,6 +1673,47 @@ int genphy_config_init(struct phy_device *phydev)
>>  }
>>  EXPORT_SYMBOL(genphy_config_init);
>>  
>> +/**
>> + * genphy_read_config - read configuration from PHY
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: Reads MII_BMCR and sets phydev autoneg, speed and duplex
>> + * accordingly.  For use in driver probe functions, to respect strapped
>> + * configuration settings.
>> + */
>> +int genphy_read_config(struct phy_device *phydev)
>
> This duplicates what already exists, in part at least within
> genphy_read_status() can you find a way to use that?

Make a small static function for updating duplex and speed fields from a
BMCR value.  It will not be big re-use, but it would make sense.  I will
do that in next patch version.

/Esben

Re: [PATCH 1/2] net: phy: Helper function for reading strapped configuration values

[Esben Haabendal]

Florian Fainelli  writes:

> On 04/05/2018 04:44 AM, esben.haaben...@gmail.com wrote:
>> From: Esben Haabendal 
>> 
>> Add a function for use in PHY driver probe functions, reading current
>> autoneg, speed and duplex configuration from BMCR register.
>> 
>> Useful for PHY that supports hardware strapped configuration, enabling
>> Linux to respect that configuration (i.e. strapped non-autoneg
>> configuration).
>> 
>> Signed-off-by: Esben Haabendal 
>> Cc: Rasmus Villemoes 
>> ---
>>  drivers/net/phy/phy_device.c | 41 +
>>  include/linux/phy.h  |  1 +
>>  2 files changed, 42 insertions(+)
>> 
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 74664a6c0cdc..cc52ff2a2344 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -1673,6 +1673,47 @@ int genphy_config_init(struct phy_device *phydev)
>>  }
>>  EXPORT_SYMBOL(genphy_config_init);
>>  
>> +/**
>> + * genphy_read_config - read configuration from PHY
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: Reads MII_BMCR and sets phydev autoneg, speed and duplex
>> + * accordingly.  For use in driver probe functions, to respect strapped
>> + * configuration settings.
>> + */
>> +int genphy_read_config(struct phy_device *phydev)
>
> This duplicates what already exists, in part at least within
> genphy_read_status() can you find a way to use that?

Make a small static function for updating duplex and speed fields from a
BMCR value.  It will not be big re-use, but it would make sense.  I will
do that in next patch version.

/Esben

Re: [PATCH 1/2] net: phy: Helper function for reading strapped configuration values

[Florian Fainelli]

On 04/05/2018 04:44 AM, esben.haaben...@gmail.com wrote:
> From: Esben Haabendal 
> 
> Add a function for use in PHY driver probe functions, reading current
> autoneg, speed and duplex configuration from BMCR register.
> 
> Useful for PHY that supports hardware strapped configuration, enabling
> Linux to respect that configuration (i.e. strapped non-autoneg
> configuration).
> 
> Signed-off-by: Esben Haabendal 
> Cc: Rasmus Villemoes 
> ---
>  drivers/net/phy/phy_device.c | 41 +
>  include/linux/phy.h  |  1 +
>  2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 74664a6c0cdc..cc52ff2a2344 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1673,6 +1673,47 @@ int genphy_config_init(struct phy_device *phydev)
>  }
>  EXPORT_SYMBOL(genphy_config_init);
>  
> +/**
> + * genphy_read_config - read configuration from PHY
> + * @phydev: target phy_device struct
> + *
> + * Description: Reads MII_BMCR and sets phydev autoneg, speed and duplex
> + * accordingly.  For use in driver probe functions, to respect strapped
> + * configuration settings.
> + */
> +int genphy_read_config(struct phy_device *phydev)

This duplicates what already exists, in part at least within
genphy_read_status() can you find a way to use that?

I really wonder how this is going to work though because an user can
decide to force the PHY to have auto-negotiation disabled just like a
MAC could actually attempt to do that while connecting to the PHY...
more comments in patch 2.
-- 
Florian

[PATCH 1/2] net: phy: Helper function for reading strapped configuration values

[esben . haabendal]

From: Esben Haabendal 

Add a function for use in PHY driver probe functions, reading current
autoneg, speed and duplex configuration from BMCR register.

Useful for PHY that supports hardware strapped configuration, enabling
Linux to respect that configuration (i.e. strapped non-autoneg
configuration).

Signed-off-by: Esben Haabendal 
Cc: Rasmus Villemoes 
---
 drivers/net/phy/phy_device.c | 41 +
 include/linux/phy.h  |  1 +
 2 files changed, 42 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 74664a6c0cdc..cc52ff2a2344 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1673,6 +1673,47 @@ int genphy_config_init(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_config_init);
 
+/**
+ * genphy_read_config - read configuration from PHY
+ * @phydev: target phy_device struct
+ *
+ * Description: Reads MII_BMCR and sets phydev autoneg, speed and duplex
+ * accordingly.  For use in driver probe functions, to respect strapped
+ * configuration settings.
+ */
+int genphy_read_config(struct phy_device *phydev)
+{
+   int bmcr;
+
+   bmcr = phy_read(phydev, MII_BMCR);
+   if (bmcr < 0)
+   return bmcr;
+
+   if (BMCR_ANENABLE & bmcr) {
+   phydev->autoneg = AUTONEG_ENABLE;
+
+   phydev->speed = 0;
+   phydev->duplex = -1;
+   } else {
+   phydev->autoneg = AUTONEG_DISABLE;
+
+   if (bmcr & BMCR_FULLDPLX)
+   phydev->duplex = DUPLEX_FULL;
+   else
+   phydev->duplex = DUPLEX_HALF;
+
+   if (bmcr & BMCR_SPEED1000)
+   phydev->speed = SPEED_1000;
+   else if (bmcr & BMCR_SPEED100)
+   phydev->speed = SPEED_100;
+   else
+   phydev->speed = SPEED_10;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(genphy_read_config);
+
 /* This is used for the phy device which doesn't support the MMD extended
  * register access, but it does have side effect when we are trying to access
  * the MMD register via indirect method.
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7c4c2379e010..5a8821c8105d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -970,6 +970,7 @@ void phy_attached_info(struct phy_device *phydev);
 
 /* Clause 22 PHY */
 int genphy_config_init(struct phy_device *phydev);
+int genphy_read_config(struct phy_device *phydev);
 int genphy_setup_forced(struct phy_device *phydev);
 int genphy_restart_aneg(struct phy_device *phydev);
 int genphy_config_aneg(struct phy_device *phydev);
-- 
2.16.3