[PATH,RFC,net-next,3/8] net: phy: Add helper to convert MII ADV register to a linkmode

Message ID 1536961136-30453-4-git-send-email-andrew@lunn.ch
State RFC
Delegated to: David Miller
Headers show
Series
  • Continue towards using linkmode in phylib
Related show

Commit Message

Andrew Lunn Sept. 14, 2018, 9:38 p.m.
The phy_mii_ioctl can be used to write a value into the MII_ADVERTISE
register in the PHY. Since this changes the state of the PHY, we need
to make the same change to phydev->advertising. Add a helper which can
convert the register value to a linkmode.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/linux/mii.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

Comments

Florian Fainelli Sept. 14, 2018, 10:23 p.m. | #1
On 09/14/2018 02:38 PM, Andrew Lunn wrote:
> The phy_mii_ioctl can be used to write a value into the MII_ADVERTISE
> register in the PHY. Since this changes the state of the PHY, we need
> to make the same change to phydev->advertising. Add a helper which can
> convert the register value to a linkmode.

It would have been nice if we could eliminate the duplication between
mii_adv_to_ethtool_adv_t() and mii_adv_to_linkmode_adv_t() but I don't
really see how without changing the former function's signature.

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/linux/mii.h | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/include/linux/mii.h b/include/linux/mii.h
> index 567047ef0309..8c7da9473ad9 100644
> --- a/include/linux/mii.h
> +++ b/include/linux/mii.h
> @@ -303,6 +303,37 @@ static inline u32 mii_lpa_to_ethtool_lpa_x(u32 lpa)
>  	return result | mii_adv_to_ethtool_adv_x(lpa);
>  }
>  
> +/**
> + * mii_adv_to_linkmode_adv_t
> + * @advertising:pointer to destination link mode.
> + * @adv: value of the MII_ADVERTISE register
> + *
> + * A small helper function that translates MII_ADVERTISE bits
> + * to linkmode advertisement settings.
> + */
> +static inline void mii_adv_to_linkmode_adv_t(unsigned long *advertising,
> +					     u32 adv)
> +{
> +	linkmode_zero(advertising);
> +
> +	if (adv & ADVERTISE_10HALF)
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
> +				 advertising);
> +	if (adv & ADVERTISE_10FULL)
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
> +				 advertising);
> +	if (adv & ADVERTISE_100HALF)
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
> +				 advertising);
> +	if (adv & ADVERTISE_100FULL)
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
> +				 advertising);
> +	if (adv & ADVERTISE_PAUSE_CAP)
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising);
> +	if (adv & ADVERTISE_PAUSE_ASYM)
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertising);
> +}
> +
>  /**
>   * mii_advertise_flowctrl - get flow control advertisement flags
>   * @cap: Flow control capabilities (FLOW_CTRL_RX, FLOW_CTRL_TX or both)
>
Andrew Lunn Sept. 14, 2018, 11:23 p.m. | #2
On Fri, Sep 14, 2018 at 03:23:14PM -0700, Florian Fainelli wrote:
> On 09/14/2018 02:38 PM, Andrew Lunn wrote:
> > The phy_mii_ioctl can be used to write a value into the MII_ADVERTISE
> > register in the PHY. Since this changes the state of the PHY, we need
> > to make the same change to phydev->advertising. Add a helper which can
> > convert the register value to a linkmode.
> 
> It would have been nice if we could eliminate the duplication between
> mii_adv_to_ethtool_adv_t() and mii_adv_to_linkmode_adv_t() but I don't
> really see how without changing the former function's signature.

Some of these functions are also used by non-phylib MAC drivers. So
the ethtool version cannot be eliminated.

And the UAPI for EEE still uses a u32 for which modes EEE is
advertised :-(

	   Andrew
Maxime Chevallier Sept. 17, 2018, 9:50 a.m. | #3
On Fri, 14 Sep 2018 23:38:51 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

>The phy_mii_ioctl can be used to write a value into the MII_ADVERTISE
>register in the PHY. Since this changes the state of the PHY, we need
>to make the same change to phydev->advertising. Add a helper which can
>convert the register value to a linkmode.
>
>Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Patch

diff --git a/include/linux/mii.h b/include/linux/mii.h
index 567047ef0309..8c7da9473ad9 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -303,6 +303,37 @@  static inline u32 mii_lpa_to_ethtool_lpa_x(u32 lpa)
 	return result | mii_adv_to_ethtool_adv_x(lpa);
 }
 
+/**
+ * mii_adv_to_linkmode_adv_t
+ * @advertising:pointer to destination link mode.
+ * @adv: value of the MII_ADVERTISE register
+ *
+ * A small helper function that translates MII_ADVERTISE bits
+ * to linkmode advertisement settings.
+ */
+static inline void mii_adv_to_linkmode_adv_t(unsigned long *advertising,
+					     u32 adv)
+{
+	linkmode_zero(advertising);
+
+	if (adv & ADVERTISE_10HALF)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+				 advertising);
+	if (adv & ADVERTISE_10FULL)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+				 advertising);
+	if (adv & ADVERTISE_100HALF)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+				 advertising);
+	if (adv & ADVERTISE_100FULL)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+				 advertising);
+	if (adv & ADVERTISE_PAUSE_CAP)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising);
+	if (adv & ADVERTISE_PAUSE_ASYM)
+		linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertising);
+}
+
 /**
  * mii_advertise_flowctrl - get flow control advertisement flags
  * @cap: Flow control capabilities (FLOW_CTRL_RX, FLOW_CTRL_TX or both)