diff mbox series

[net-next,v2,04/10] net: phy: Automatically fill the generic TP, FIBRE and Backplane modes

Message ID 20190207094939.27369-5-maxime.chevallier@bootlin.com
State Changes Requested
Delegated to: David Miller
Headers show
Series net: phy: Add support for 2.5GBASET PHYs | expand

Commit Message

Maxime Chevallier Feb. 7, 2019, 9:49 a.m. UTC 
PHY advertised and supported linkmodes contain both specific modes such
as 1000BASET Half/Full and generic ones such as TP that represent a
class of modes.

Since some modes such as Fibre, TP or Backplane match a wide range of
specific modes, we can automatically set these bits if one of the
specific modes it corresponds to is present in the list.

The 'TP' bit is set whenever there's a BaseT linkmode in
phydev->supported.

The 'FIBRE' bit is set for BaseL, BaseS and BaseE linkmodes.

Finally, the 'Backplane' is set whenever a BaseK mode is supported.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/phy_device.c | 67 +++++++++++++++++++++++++++++++++++-
 include/linux/linkmode.h     |  6 ++++
 2 files changed, 72 insertions(+), 1 deletion(-)

Comments

Andrew Lunn Feb. 7, 2019, 2:09 p.m. UTC | #1 
On Thu, Feb 07, 2019 at 10:49:33AM +0100, Maxime Chevallier wrote:
> PHY advertised and supported linkmodes contain both specific modes such
> as 1000BASET Half/Full and generic ones such as TP that represent a
> class of modes.
> 
> Since some modes such as Fibre, TP or Backplane match a wide range of
> specific modes, we can automatically set these bits if one of the
> specific modes it corresponds to is present in the list.
> 
> The 'TP' bit is set whenever there's a BaseT linkmode in
> phydev->supported.
> 
> The 'FIBRE' bit is set for BaseL, BaseS and BaseE linkmodes.
> 
> Finally, the 'Backplane' is set whenever a BaseK mode is supported.

Hi Maxime

Interesting idea.

But what exactly are we supposed to be representing here?  That PHY
can do these modes, or that the port exists on the device? The
marvell10g can do fibre, but do all boards have an SFP/SFF, or do some
only have an RJ-45 for TP? Are there boards without TP and just
SFP/SFF?

Is there documentation in ethtool which gives a clue as to what is
expected?

Thanks
	Andrew
Maxime Chevallier Feb. 7, 2019, 2:49 p.m. UTC | #2 
Hello Andrew,

On Thu, 7 Feb 2019 15:09:39 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

>On Thu, Feb 07, 2019 at 10:49:33AM +0100, Maxime Chevallier wrote:
>> PHY advertised and supported linkmodes contain both specific modes such
>> as 1000BASET Half/Full and generic ones such as TP that represent a
>> class of modes.
>> 
>> Since some modes such as Fibre, TP or Backplane match a wide range of
>> specific modes, we can automatically set these bits if one of the
>> specific modes it corresponds to is present in the list.
>> 
>> The 'TP' bit is set whenever there's a BaseT linkmode in
>> phydev->supported.
>> 
>> The 'FIBRE' bit is set for BaseL, BaseS and BaseE linkmodes.
>> 
>> Finally, the 'Backplane' is set whenever a BaseK mode is supported.  
>
>Hi Maxime
>
>Interesting idea.
>
>But what exactly are we supposed to be representing here?  That PHY
>can do these modes, or that the port exists on the device? The
>marvell10g can do fibre, but do all boards have an SFP/SFF, or do some
>only have an RJ-45 for TP? Are there boards without TP and just
>SFP/SFF?

My understanding is that this would represent what the PHY is capable
of, at least when set in the supported field, but I agree that this
doesn't reflect what's on the device.

I extrapolated this logic from the ability detection in marvell10g,
that would set these bits according to the abilities reported by the
PHY. This was done based on the PHY register values, instead of the
linkmodes in the 'supported' field.

>Is there documentation in ethtool which gives a clue as to what is
>expected?

Not really, at least to my knowledge. I think this would be used by
"ethtool -s ethX port tp|fibre|aui|etc". Maybe this could be useful if
we decide to implement port selection with ethtool on the MCBin, but I'm
getting a bit ahead of myself.

Thanks,

Maxime
diff mbox series

Patch

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a4cbc5a6f09d..942cfa7548c4 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1066,15 +1066,80 @@  static int phy_poll_reset(struct phy_device *phydev)
  * straightforward to maintain, since PHYs and MACs are subject to quirks and
  * erratas. This function re-builds the list of the supported pause parameters
  * by taking into account the parameters expressed in the driver's features
- * list.
+ * list. It also sets the generic bits indicating Twisted Pair, Fibre and
+ * Backaplane link modes support based on the detailed list that can be built
+ * from the PHY's ability list.
  */
 static void phy_update_linkmodes(struct phy_device *phydev)
 {
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_baset_features) = { 0, };
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_fibre_features) = { 0, };
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_backplane_features) = { 0, };
 	struct device_driver *drv = phydev->mdio.dev.driver;
 	struct phy_driver *phydrv = to_phy_driver(drv);
 
+	const int phy_baset_features_array[] = {
+		ETHTOOL_LINK_MODE_10baseT_Half_BIT,
+		ETHTOOL_LINK_MODE_10baseT_Full_BIT,
+		ETHTOOL_LINK_MODE_100baseT_Half_BIT,
+		ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+		ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
+		ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+	};
+
+	const int phy_fibre_features_array[] = {
+		ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT,
+		ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT,
+		ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT,
+		ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT,
+		ETHTOOL_LINK_MODE_25000baseSR_Full_BIT,
+		ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT,
+		ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT,
+		ETHTOOL_LINK_MODE_50000baseSR2_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseSR_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseLR_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
+	};
+
+	const int phy_backplane_features_array[] = {
+		ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+		ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+		ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT,
+		ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+		ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT,
+		ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
+		ETHTOOL_LINK_MODE_50000baseKR2_Full_BIT,
+		ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT,
+	};
+
+	linkmode_set_bit_array(phy_baset_features_array,
+			       ARRAY_SIZE(phy_baset_features_array),
+			       phy_baset_features);
+
+	linkmode_set_bit_array(phy_fibre_features_array,
+			       ARRAY_SIZE(phy_fibre_features_array),
+			       phy_fibre_features);
+
+	linkmode_set_bit_array(phy_backplane_features_array,
+			       ARRAY_SIZE(phy_backplane_features_array),
+			       phy_backplane_features);
+
 	mutex_lock(&phydev->lock);
 
+	if (linkmode_intersects(phydev->supported, phy_baset_features))
+		linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT, phydev->supported);
+
+	if (linkmode_intersects(phydev->supported, phy_fibre_features))
+		linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
+				 phydev->supported);
+
+	if (linkmode_intersects(phydev->supported, phy_backplane_features))
+		linkmode_set_bit(ETHTOOL_LINK_MODE_Backplane_BIT,
+				 phydev->supported);
+
 	/* The Pause Frame bits indicate that the PHY can support passing
 	 * pause frames. During autonegotiation, the PHYs will determine if
 	 * they should allow pause frames to pass.  The MAC driver should then
diff --git a/include/linux/linkmode.h b/include/linux/linkmode.h
index a99c58866860..49ab6415e1e0 100644
--- a/include/linux/linkmode.h
+++ b/include/linux/linkmode.h
@@ -82,4 +82,10 @@  static inline int linkmode_equal(const unsigned long *src1,
 	return bitmap_equal(src1, src2, __ETHTOOL_LINK_MODE_MASK_NBITS);
 }
 
+static inline bool linkmode_intersects(const unsigned long *src1,
+				       const unsigned long *src2)
+{
+	return bitmap_intersects(src1, src2, __ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+
 #endif /* __LINKMODE_H */