net: phy: c45: select BASE-T1 type for the forced speed
Subsystems (3)
PATCH v3 1/1 net: phy: c45: select BASE-T1 type for the forced speed
Commit 25108a834e14 ("net: phy: c45: add support for 1000BASE-T1 forced setup") made genphy_c45_pma_setup_forced() write the type selection field (bits 3:0) of the BASE-T1 PMA/PMD control register (1.2100) on every forced setup: 1000BASE-T1 for SPEED_1000, and 0 for everything else. 0 selects 100BASE-T1, so forcing a 10BASE-T1L PHY to 10 Mbit/s now also switches its PMA to 100BASE-T1, a mode it does not implement.
Seen on an ADIN1110 (internal ADIN1100 PHY): after
ethtool -s ethX autoneg off speed 10 master-slave forced-master
1.2100 reads 0xc000 instead of the 0xc002. Since the field is not restored by re-enabling autoneg, the PHY keeps the wrong type until the next forced setup or reset.
Add a helper that returns the type selection value for the forced speed and the supported link modes: 100BASE-T1, 1000BASE-T1, and 10BASE-T1L or 10BASE-T1S for SPEED_10 depending on the supported 10 Mbit/s modes. Fail with -EINVAL when no type matches instead of writing 100BASE-T1. This also applies to forced 2.5G, 5G and 10G on BASE-T1 capable PHYs, for which there are no link modes yet.
The field is called "type selection" in IEEE 802.3-2022 (45.2.1.214, Table 45-178). Keep the old STRAP names as aliases.
Tested on EVAL-ADIN1110EBZ: with this change, forcing 10 Mbit/s writes 1.2100 = 0xc002 (10BASE-T1L) instead of the invalid 0xc000.
---
Changes in v3 (review by Andrew Lunn):
- Sort the cases of genphy_c45_baset1_type_sel() by speed.
- Add Reviewed-by from Andrew Lunn.
- No functional change.
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2 (review by Andrew Lunn):
- Name the type selection values after 802.3-2022 (Table 45-178) and
define all of them, including 2.5GBASE-T1, 5GBASE-T1 and 10GBASE-T1.
The old STRAP name comes from 6f1c646d88c5 and is kept as an alias
for userspace.
- Add a helper that returns the type selection for the forced speed and
the supported link modes, also covering 10BASE-T1S. It returns
-EINVAL when no type matches instead of writing 100BASE-T1, which
also applies to forced 2.5G, 5G and 10G on BASE-T1 capable PHYs.
- Resolve the type before the first register write, so a failure leaves
the PHY untouched (reported by the Sashiko review bot).
- Retitle the patch, as it is no longer specific to 10BASE-T1L.
- Link to v1: https://lore.kernel.org/r/[email protected]
drivers/net/phy/phy-c45.c | 46 ++++++++++++++++++++++++++++++++++----- include/uapi/linux/mdio.h | 13 +++++++++-- 2 files changed, 52 insertions(+), 7 deletions(-)
drivers/net/phy/phy-c45.c +41 -5
@@ -104,6 +104,36 @@ int genphy_c45_pma_baset1_setup_master_slave(struct phy_device *phydev) }EXPORT_SYMBOL_GPL (genphy_c45_pma_baset1_setup_master_slave); +/* Type selection value for the BASE-T1 PMA/PMD control register (1.2100, +* 802.3-2022 Table 45-178) matching the forced speed. 10 Mbit/s is ambiguous +* between 10BASE-T1L and 10BASE-T1S, so the supported link modes decide. +* 2.5G, 5G and 10GBASE-T1 are not handled as there are no link modes for them +* yet. +*/ +static int genphy_c45_baset1_type_sel (struct phy_device *phydev) +{ +switch (phydev->speed) { +case SPEED_10 : +if (linkmode_test_bit (ETHTOOL_LINK_MODE_10baseT1L_Full_BIT, + phydev->supported)) +return MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_10BT1L ; +if (linkmode_test_bit (ETHTOOL_LINK_MODE_10baseT1S_Full_BIT, + phydev->supported) || +linkmode_test_bit (ETHTOOL_LINK_MODE_10baseT1S_Half_BIT, + phydev->supported) || +linkmode_test_bit (ETHTOOL_LINK_MODE_10baseT1S_P2MP_Half_BIT, + phydev->supported)) +return MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_10BT1S ; +return -EINVAL ; +case SPEED_100 : +return MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_100BT1 ; +case SPEED_1000 : +return MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_1000BT1 ; +default : +return -EINVAL ; + } +} +/** * genphy_c45_pma_setup_forced - configures a forced speed * @phydev: target phy_device struct
@@ -166,6 +196,16 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev)return -EINVAL ; } +/* Resolve the type before the first write, so a speed without a +* matching type leaves the PHY untouched. +*/ + bt1_ctrl =0 ; +if (genphy_c45_baset1_able (phydev)) { + bt1_ctrl =genphy_c45_baset1_type_sel (phydev); +if (bt1_ctrl <0 ) +return bt1_ctrl; + } + ret =phy_write_mmd (phydev,MDIO_MMD_PMAPMD ,MDIO_CTRL1 , ctrl1);if (ret <0 )return ret;
@@ -179,12 +219,8 @@ int genphy_c45_pma_setup_forced(struct phy_device *phydev)if (ret <0 )return ret; - bt1_ctrl =0 ; -if (phydev->speed ==SPEED_1000 ) - bt1_ctrl =MDIO_PMA_PMD_BT1_CTRL_STRAP_B1000 ; - ret =phy_modify_mmd (phydev,MDIO_MMD_PMAPMD ,MDIO_PMA_PMD_BT1_CTRL , -MDIO_PMA_PMD_BT1_CTRL_STRAP , bt1_ctrl); +MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL , bt1_ctrl);if (ret <0 )return ret; }
include/uapi/linux/mdio.h +11 -2
@@ -417,8 +417,17 @@#define MDIO_AN_10BT1_AN_STAT_LPA_EEE_T1L 0x4000/* 10BASE-T1L LP EEE ability advertisement */ /* BASE-T1 PMA/PMD control register */ -#define MDIO_PMA_PMD_BT1_CTRL_STRAP 0x000F/* Type selection (Strap) */ -#define MDIO_PMA_PMD_BT1_CTRL_STRAP_B1000 0x0001/* Select 1000BASE-T1 */ +#define MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL 0x000F/* Type selection */ +#define MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_100BT1 0x0000/* Select 100BASE-T1 */ +#define MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_1000BT1 0x0001/* Select 1000BASE-T1 */ +#define MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_10BT1L 0x0002/* Select 10BASE-T1L */ +#define MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_10BT1S 0x0003/* Select 10BASE-T1S */ +#define MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_2P5GBT1 0x0004/* Select 2.5GBASE-T1 */ +#define MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_5GBT1 0x0005/* Select 5GBASE-T1 */ +#define MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_10GBT1 0x0006/* Select 10GBASE-T1 */ +/* Legacy names, kept for userspace */ +#define MDIO_PMA_PMD_BT1_CTRL_STRAP MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL +#define MDIO_PMA_PMD_BT1_CTRL_STRAP_B1000 MDIO_PMA_PMD_BT1_CTRL_TYPE_SEL_1000BT1#define MDIO_PMA_PMD_BT1_CTRL_CFG_MST 0x4000/* MASTER-SLAVE config value */ /* 1000BASE-T1 PCS control register */
base-commit: 241dec6f9ab74d8c3ef246764f3611666b96b9e1
Signature · 1 line
2.43.0