diff mbox series

[RFC,net-next,4/8] net: phylink: update supported_interfaces with modes from fwnode

Message ID 20211110190709.16505-5-kabel@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Extend `phy-mode` to string array | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 390 this patch: 390
netdev/cc_maintainers warning 4 maintainers not CCed: kuba@kernel.org linux@armlinux.org.uk hkallweit1@gmail.com davem@davemloft.net
netdev/build_clang success Errors and warnings before: 315 this patch: 315
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 380 this patch: 380
netdev/checkpatch warning WARNING: 'compatiblity' may be misspelled - perhaps 'compatibility'?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Marek Behún Nov. 10, 2021, 7:07 p.m. UTC
Now that the 'phy-mode' property can be a string array containing more
PHY modes (all that are supported by the board), update the bitmap of
interfaces supported by the MAC with this property.

Normally this would be a simple intersection (of interfaces supported by
the current implementation of the driver and interfaces supported by the
board), but we need to keep being backwards compatible with older DTs,
which may only define one mode, since, as Russell King says,
  conventionally phy-mode has meant "this is the mode we want to operate
  the PHY interface in" which was fine when PHYs didn't change their
  mode depending on the media speed

An example is DT defining
  phy-mode = "sgmii";
but the board supporting also 1000base-x and 2500base-x.

Add the following logic to keep this backwards compatiblity:
- if more PHY modes are defined, do a simple intersection
- if one PHY mode is defined:
  - if it is sgmii, 1000base-x or 2500base-x, add all three and then do
    the intersection
  - if it is 10gbase-r or usxgmii, add both, and also 5gbase-r,
    2500base-x, 1000base-x and sgmii, and then do the intersection

This is simple enough and should work for all boards.

Nonetheless it is possible (although extremely unlikely, in my opinion)
that a board will be found that (for example) defines
  phy-mode = "sgmii";
and the MAC drivers supports sgmii, 1000base-x and 2500base-x, but the
board DOESN'T support 2500base-x, because of electrical reasons (since
the frequency is 2.5x of sgmii).
Our code will in this case incorrectly infer also support for
2500base-x. To avoid this, the board maintainer should either change DTS
to
  phy-mode = "sgmii", "1000base-x";
and update device tree on all boards, or, if that is impossible, add a
fix into the function we are introducing in this commit.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/net/phy/phylink.c | 63 +++++++++++++++++++++++++++++++++++++++
 include/linux/phy.h       |  6 ++++
 2 files changed, 69 insertions(+)
diff mbox series

Patch

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 3ad7397b8119..4a8a0adf8c6c 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -311,6 +311,67 @@  static int phylink_parse_fixedlink(struct phylink *pl,
 	return 0;
 }
 
+static void phylink_update_phy_modes(struct phylink *pl,
+				     struct fwnode_handle *fwnode)
+{
+	unsigned long *supported = pl->config->supported_interfaces;
+	DECLARE_PHY_INTERFACE_MASK(modes);
+
+	if (fwnode_get_phy_modes(fwnode, modes) < 0)
+		return;
+
+	if (phy_interface_empty(modes))
+		return;
+
+	/* If supported is empty, just copy modes defined in fwnode. */
+	if (phy_interface_empty(supported))
+		return phy_interface_copy(supported, modes);
+
+	/* We want the intersection of given supported modes with those defined
+	 * in DT.
+	 *
+	 * Some older device-trees mention only one of `sgmii`, `1000base-x` or
+	 * `2500base-x`, while supporting all three. Other mention `10gbase-r`
+	 * or `usxgmii`, while supporting both, and also `sgmii`, `1000base-x`,
+	 * `2500base-x` and `5gbase-r`.
+	 * For backwards compatibility with these older DTs, make it so that if
+	 * one of these modes is mentioned in DT and MAC supports more of them,
+	 * keep all that are supported according to the logic above.
+	 *
+	 * Nonetheless it is possible that a device may support only one mode,
+	 * for example 1000base-x, due to strapping pins or some other reasons.
+	 * If a specific device supports only the mode mentioned in DT, the
+	 * exception should be made here with of_machine_is_compatible().
+	 */
+	if (bitmap_weight(modes, PHY_INTERFACE_MODE_MAX) == 1) {
+		bool lower = false;
+
+		if (test_bit(PHY_INTERFACE_MODE_10GBASER, modes) ||
+		    test_bit(PHY_INTERFACE_MODE_USXGMII, modes)) {
+			if (test_bit(PHY_INTERFACE_MODE_5GBASER, supported))
+				__set_bit(PHY_INTERFACE_MODE_5GBASER, modes);
+			if (test_bit(PHY_INTERFACE_MODE_10GBASER, supported))
+				__set_bit(PHY_INTERFACE_MODE_10GBASER, modes);
+			if (test_bit(PHY_INTERFACE_MODE_USXGMII, supported))
+				__set_bit(PHY_INTERFACE_MODE_USXGMII, modes);
+			lower = true;
+		}
+
+		if (lower || (test_bit(PHY_INTERFACE_MODE_SGMII, modes) ||
+			      test_bit(PHY_INTERFACE_MODE_1000BASEX, modes) ||
+			      test_bit(PHY_INTERFACE_MODE_2500BASEX, modes))) {
+			if (test_bit(PHY_INTERFACE_MODE_SGMII, supported))
+				__set_bit(PHY_INTERFACE_MODE_SGMII, modes);
+			if (test_bit(PHY_INTERFACE_MODE_1000BASEX, supported))
+				__set_bit(PHY_INTERFACE_MODE_1000BASEX, modes);
+			if (test_bit(PHY_INTERFACE_MODE_2500BASEX, supported))
+				__set_bit(PHY_INTERFACE_MODE_2500BASEX, modes);
+		}
+	}
+
+	phy_interface_and(supported, supported, modes);
+}
+
 static int phylink_parse_mode(struct phylink *pl, struct fwnode_handle *fwnode)
 {
 	struct fwnode_handle *dn;
@@ -904,6 +965,8 @@  struct phylink *phylink_create(struct phylink_config *config,
 	__set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
 	timer_setup(&pl->link_poll, phylink_fixed_poll, 0);
 
+	phylink_update_phy_modes(pl, fwnode);
+
 	bitmap_fill(pl->supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
 	linkmode_copy(pl->link_config.advertising, pl->supported);
 	phylink_validate(pl, pl->supported, &pl->link_config);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 96e43fbb2dd8..effa4137f173 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -169,6 +169,12 @@  static inline bool phy_interface_empty(const unsigned long *intf)
 	return bitmap_empty(intf, PHY_INTERFACE_MODE_MAX);
 }
 
+static inline void phy_interface_copy(unsigned long *dst,
+				      const unsigned long *src)
+{
+	bitmap_copy(dst, src, PHY_INTERFACE_MODE_MAX);
+}
+
 static inline void phy_interface_and(unsigned long *dst, const unsigned long *a,
 				     const unsigned long *b)
 {