[PATCH net-next v8 0/9] net: add EEE support for KSZ9477 switch family

[PATCH net-next v8 0/9] net: add EEE support for KSZ9477 switch family

[Oleksij Rempel]

changes v8:
- fix comment for linkmode_to_mii_eee_cap1_t() function
- add Acked-by: Arun Ramadoss <arun.ramadoss@microchip.com>
- add Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>

changes v7:
- update documentation for genphy_c45_eee_is_active()
- address review comments on "net: dsa: microchip: enable EEE support"
  patch

changes v6:
- split patch set and send only first 9 patches
- Add Reviewed-by: Andrew Lunn <andrew@lunn.ch>
- use 0xffff instead of GENMASK
- Document @supported_eee
- use "()" with function name in comments

changes v5:
- spell fixes
- move part of genphy_c45_read_eee_abilities() to
  genphy_c45_read_eee_cap1()
- validate MDIO_PCS_EEE_ABLE register against 0xffff val.
- rename *eee_100_10000* to *eee_cap1*
- use linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)
  instead of !linkmode_empty()
- add documentation to linkmode/register helpers

changes v4:
- remove following helpers:
  mmd_eee_cap_to_ethtool_sup_t
  mmd_eee_adv_to_ethtool_adv_t
  ethtool_adv_to_mmd_eee_adv_t
  and port drivers from this helpers to linkmode helpers.
- rebase against latest net-next
- port phy_init_eee() to genphy_c45_eee_is_active()

changes v3:
- rework some parts of EEE infrastructure and move it to c45 code.
- add supported_eee storage and start using it in EEE code and by the
  micrel driver.
- add EEE support for ar8035 PHY
- add SmartEEE support to FEC i.MX series.

changes v2:
- use phydev->supported instead of reading MII_BMSR regiaster
- fix @get_eee > @set_eee

With this patch series we provide EEE control for KSZ9477 family of
switches and
AR8035 with i.MX6 configuration.
According to my tests, on a system with KSZ8563 switch and 100Mbit idle
link,
we consume 0,192W less power per port if EEE is enabled.

Oleksij Rempel (9):
  net: dsa: microchip: enable EEE support
  net: phy: add genphy_c45_read_eee_abilities() function
  net: phy: micrel: add ksz9477_get_features()
  net: phy: export phy_check_valid() function
  net: phy: add genphy_c45_ethtool_get/set_eee() support
  net: phy: c22: migrate to genphy_c45_write_eee_adv()
  net: phy: c45: migrate to genphy_c45_write_eee_adv()
  net: phy: migrate phy_init_eee() to genphy_c45_eee_is_active()
  net: phy: start using genphy_c45_ethtool_get/set_eee()

 drivers/net/dsa/microchip/ksz_common.c |  66 +++++
 drivers/net/phy/micrel.c               |  21 ++
 drivers/net/phy/phy-c45.c              | 319 ++++++++++++++++++++++++-
 drivers/net/phy/phy.c                  | 153 ++----------
 drivers/net/phy/phy_device.c           |  26 +-
 include/linux/mdio.h                   |  84 +++++++
 include/linux/phy.h                    |  14 ++
 include/uapi/linux/mdio.h              |   8 +
 8 files changed, 554 insertions(+), 137 deletions(-)

-- 
2.30.2

[PATCH net-next v8 1/9] net: dsa: microchip: enable EEE support

[Oleksij Rempel]

Some of KSZ9477 family switches provides EEE support. To enable it, we
just need to register set_mac_eee/set_mac_eee handlers and validate
supported chip version and port.

Currently supported chip variants are: KSZ8563, KSZ9477, KSZ9563,
KSZ9567, KSZ9893, KSZ9896, KSZ9897. KSZ8563 supports EEE only with
100BaseTX/Full.  Other chips support 100BaseTX/Full and 1000BaseTX/Full.
Low Power Idle configuration is not supported and currently not
documented in the datasheets.

EEE PHY specific tunings are not documented in the switch datasheets, but can
overlap with KSZ9131 specification.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 66 ++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 46becc0382d6..729b36eeb2c4 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2673,6 +2673,70 @@ static int ksz_max_mtu(struct dsa_switch *ds, int port)
 	return -EOPNOTSUPP;
 }
 
+static int ksz_validate_eee(struct dsa_switch *ds, int port)
+{
+	struct ksz_device *dev = ds->priv;
+
+	if (!dev->info->internal_phy[port])
+		return -EOPNOTSUPP;
+
+	switch (dev->chip_id) {
+	case KSZ8563_CHIP_ID:
+	case KSZ9477_CHIP_ID:
+	case KSZ9563_CHIP_ID:
+	case KSZ9567_CHIP_ID:
+	case KSZ9893_CHIP_ID:
+	case KSZ9896_CHIP_ID:
+	case KSZ9897_CHIP_ID:
+		return 0;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static int ksz_get_mac_eee(struct dsa_switch *ds, int port,
+			   struct ethtool_eee *e)
+{
+	int ret;
+
+	ret = ksz_validate_eee(ds, port);
+	if (ret)
+		return ret;
+
+	/* There is no documented control of Tx LPI configuration. */
+	e->tx_lpi_enabled = true;
+
+	/* There is no documented control of Tx LPI timer. According to tests
+	 * Tx LPI timer seems to be set by default to minimal value.
+	 */
+	e->tx_lpi_timer = 0;
+
+	return 0;
+}
+
+static int ksz_set_mac_eee(struct dsa_switch *ds, int port,
+			   struct ethtool_eee *e)
+{
+	struct ksz_device *dev = ds->priv;
+	int ret;
+
+	ret = ksz_validate_eee(ds, port);
+	if (ret)
+		return ret;
+
+	if (!e->tx_lpi_enabled) {
+		dev_err(dev->dev, "Disabling EEE Tx LPI is not supported\n");
+		return -EINVAL;
+	}
+
+	if (e->tx_lpi_timer) {
+		dev_err(dev->dev, "Setting EEE Tx LPI timer is not supported\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static void ksz_set_xmii(struct ksz_device *dev, int port,
 			 phy_interface_t interface)
 {
@@ -3130,6 +3194,8 @@ static const struct dsa_switch_ops ksz_switch_ops = {
 	.port_txtstamp		= ksz_port_txtstamp,
 	.port_rxtstamp		= ksz_port_rxtstamp,
 	.port_setup_tc		= ksz_setup_tc,
+	.get_mac_eee		= ksz_get_mac_eee,
+	.set_mac_eee		= ksz_set_mac_eee,
 };
 
 struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
-- 
2.30.2

[PATCH net-next v8 2/9] net: phy: add genphy_c45_read_eee_abilities() function

[Oleksij Rempel]

Add generic function for EEE abilities defined by IEEE 802.3
specification. For now following registers are supported:
- IEEE 802.3-2018 45.2.3.10 EEE control and capability 1 (Register 3.20)
- IEEE 802.3cg-2019 45.2.1.186b 10BASE-T1L PMA status register
  (Register 1.2295)

Since I was not able to find any flag signaling support of these
registers, we should detect link mode abilities first and then based on
these abilities doing EEE link modes detection.

Results of EEE ability detection will be stored into new variable
phydev->supported_eee.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy-c45.c    | 70 ++++++++++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c | 16 +++++++++
 include/linux/mdio.h         | 26 ++++++++++++++
 include/linux/phy.h          |  6 ++++
 4 files changed, 118 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 9f9565a4819d..2d10d22e7684 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -661,6 +661,76 @@ int genphy_c45_read_mdix(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
 
+/**
+ * genphy_c45_read_eee_cap1 - read supported EEE link modes from register 3.20
+ * @phydev: target phy_device struct
+ */
+static int genphy_c45_read_eee_cap1(struct phy_device *phydev)
+{
+	int val;
+
+	/* IEEE 802.3-2018 45.2.3.10 EEE control and capability 1
+	 * (Register 3.20)
+	 */
+	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
+	if (val < 0)
+		return val;
+
+	/* The 802.3 2018 standard says the top 2 bits are reserved and should
+	 * read as 0. Also, it seems unlikely anybody will build a PHY which
+	 * supports 100GBASE-R deep sleep all the way down to 100BASE-TX EEE.
+	 * If MDIO_PCS_EEE_ABLE is 0xffff assume EEE is not supported.
+	 */
+	if (val == 0xffff)
+		return 0;
+
+	mii_eee_cap1_mod_linkmode_t(phydev->supported_eee, val);
+
+	/* Some buggy devices indicate EEE link modes in MDIO_PCS_EEE_ABLE
+	 * which they don't support as indicated by BMSR, ESTATUS etc.
+	 */
+	linkmode_and(phydev->supported_eee, phydev->supported_eee,
+		     phydev->supported);
+
+	return 0;
+}
+
+/**
+ * genphy_c45_read_eee_abilities - read supported EEE link modes
+ * @phydev: target phy_device struct
+ */
+int genphy_c45_read_eee_abilities(struct phy_device *phydev)
+{
+	int val;
+
+	/* There is not indicator whether optional register
+	 * "EEE control and capability 1" (3.20) is supported. Read it only
+	 * on devices with appropriate linkmodes.
+	 */
+	if (linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)) {
+		val = genphy_c45_read_eee_cap1(phydev);
+		if (val)
+			return val;
+	}
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+			      phydev->supported)) {
+		/* IEEE 802.3cg-2019 45.2.1.186b 10BASE-T1L PMA status register
+		 * (Register 1.2295)
+		 */
+		val = phy_read_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_10T1L_STAT);
+		if (val < 0)
+			return val;
+
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+				 phydev->supported_eee,
+				 val & MDIO_PMA_10T1L_STAT_EEE);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(genphy_c45_read_eee_abilities);
+
 /**
  * genphy_c45_pma_read_abilities - read supported link modes from PMA
  * @phydev: target phy_device struct
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a3917c7acbd3..66a4e62009bb 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -132,6 +132,18 @@ static const int phy_10gbit_full_features_array[] = {
 	ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
 };
 
+static const int phy_eee_cap1_features_array[] = {
+	ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+	ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+	ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+	ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+	ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+	ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+};
+
+__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_eee_cap1_features) __ro_after_init;
+EXPORT_SYMBOL_GPL(phy_eee_cap1_features);
+
 static void features_init(void)
 {
 	/* 10/100 half/full*/
@@ -213,6 +225,10 @@ static void features_init(void)
 	linkmode_set_bit_array(phy_10gbit_fec_features_array,
 			       ARRAY_SIZE(phy_10gbit_fec_features_array),
 			       phy_10gbit_fec_features);
+	linkmode_set_bit_array(phy_eee_cap1_features_array,
+			       ARRAY_SIZE(phy_eee_cap1_features_array),
+			       phy_eee_cap1_features);
+
 }
 
 void phy_device_free(struct phy_device *phydev)
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index c0da30d63b1d..e75583f5d967 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -402,6 +402,32 @@ static inline u32 linkmode_adv_to_mii_t1_adv_m_t(unsigned long *advertising)
 	return result;
 }
 
+/**
+ * mii_eee_cap1_mod_linkmode_t()
+ * @adv: target the linkmode advertisement settings
+ * @val: register value
+ *
+ * A function that translates value of following registers to the linkmode:
+ * IEEE 802.3-2018 45.2.3.10 "EEE control and capability 1" register (3.20)
+ * IEEE 802.3-2018 45.2.7.13 "EEE advertisement 1" register (7.60)
+ * IEEE 802.3-2018 45.2.7.14 "EEE "link partner ability 1 register (7.61)
+ */
+static inline void mii_eee_cap1_mod_linkmode_t(unsigned long *adv, u32 val)
+{
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+			 adv, val & MDIO_EEE_100TX);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+			 adv, val & MDIO_EEE_1000T);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+			 adv, val & MDIO_EEE_10GT);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+			 adv, val & MDIO_EEE_1000KX);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+			 adv, val & MDIO_EEE_10GKX4);
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+			 adv, val & MDIO_EEE_10GKR);
+}
+
 int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
 int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
 int __mdiobus_modify_changed(struct mii_bus *bus, int addr, u32 regnum,
diff --git a/include/linux/phy.h b/include/linux/phy.h
index fbeba4fee8d4..c183a8a27986 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -52,6 +52,7 @@ extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_gbit_all_ports_features) __ro_after_
 extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_features) __ro_after_init;
 extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_fec_features) __ro_after_init;
 extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_full_features) __ro_after_init;
+extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_eee_cap1_features) __ro_after_init;
 
 #define PHY_BASIC_FEATURES ((unsigned long *)&phy_basic_features)
 #define PHY_BASIC_T1_FEATURES ((unsigned long *)&phy_basic_t1_features)
@@ -62,6 +63,7 @@ extern __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_10gbit_full_features) __ro_after_ini
 #define PHY_10GBIT_FEATURES ((unsigned long *)&phy_10gbit_features)
 #define PHY_10GBIT_FEC_FEATURES ((unsigned long *)&phy_10gbit_fec_features)
 #define PHY_10GBIT_FULL_FEATURES ((unsigned long *)&phy_10gbit_full_features)
+#define PHY_EEE_CAP1_FEATURES ((unsigned long *)&phy_eee_cap1_features)
 
 extern const int phy_basic_ports_array[3];
 extern const int phy_fibre_port_array[1];
@@ -572,6 +574,7 @@ struct macsec_ops;
  * @supported: Combined MAC/PHY supported linkmodes
  * @advertising: Currently advertised linkmodes
  * @adv_old: Saved advertised while power saving for WoL
+ * @supported_eee: supported PHY EEE linkmodes
  * @lp_advertising: Current link partner advertised linkmodes
  * @host_interfaces: PHY interface modes supported by host
  * @eee_broken_modes: Energy efficient ethernet modes which should be prohibited
@@ -676,6 +679,8 @@ struct phy_device {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
 	/* used with phy_speed_down */
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(adv_old);
+	/* used for eee validation */
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported_eee);
 
 	/* Host supported PHY interface types. Should be ignored if empty. */
 	DECLARE_PHY_INTERFACE_MASK(host_interfaces);
@@ -1737,6 +1742,7 @@ int genphy_c45_an_config_aneg(struct phy_device *phydev);
 int genphy_c45_an_disable_aneg(struct phy_device *phydev);
 int genphy_c45_read_mdix(struct phy_device *phydev);
 int genphy_c45_pma_read_abilities(struct phy_device *phydev);
+int genphy_c45_read_eee_abilities(struct phy_device *phydev);
 int genphy_c45_pma_baset1_read_master_slave(struct phy_device *phydev);
 int genphy_c45_read_status(struct phy_device *phydev);
 int genphy_c45_baset1_read_status(struct phy_device *phydev);
-- 
2.30.2

[PATCH net-next v8 3/9] net: phy: micrel: add ksz9477_get_features()

[Oleksij Rempel]

KSZ8563R, which has same PHYID as KSZ9477 family, will change "EEE control
and capability 1" (Register 3.20) content depending on configuration of
"EEE advertisement 1" (Register 7.60). Changes on the 7.60 will affect
3.20 register.

So, instead of depending on register 3.20, driver should set supported_eee.

Proper supported_eee configuration is needed to make use of generic
PHY c45 set/get_eee functions provided by next patches.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/micrel.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 727de4f4a14d..20738314635e 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1387,6 +1387,26 @@ static int ksz9131_config_aneg(struct phy_device *phydev)
 	return genphy_config_aneg(phydev);
 }
 
+static int ksz9477_get_features(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_read_abilities(phydev);
+	if (ret)
+		return ret;
+
+	/* The "EEE control and capability 1" (Register 3.20) seems to be
+	 * influenced by the "EEE advertisement 1" (Register 7.60). Changes
+	 * on the 7.60 will affect 3.20. So, we need to construct our own list
+	 * of caps.
+	 * KSZ8563R should have 100BaseTX/Full only.
+	 */
+	linkmode_and(phydev->supported_eee, phydev->supported,
+		     PHY_EEE_CAP1_FEATURES);
+
+	return 0;
+}
+
 #define KSZ8873MLL_GLOBAL_CONTROL_4	0x06
 #define KSZ8873MLL_GLOBAL_CONTROL_4_DUPLEX	BIT(6)
 #define KSZ8873MLL_GLOBAL_CONTROL_4_SPEED	BIT(4)
@@ -3597,6 +3617,7 @@ static struct phy_driver ksphy_driver[] = {
 	.handle_interrupt = kszphy_handle_interrupt,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
+	.get_features	= ksz9477_get_features,
 } };
 
 module_phy_driver(ksphy_driver);
-- 
2.30.2

[PATCH net-next v8 4/9] net: phy: export phy_check_valid() function

[Oleksij Rempel]

This function will be needed for genphy_c45_ethtool_get_eee() provided
by next patch.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
 drivers/net/phy/phy.c | 4 ++--
 include/linux/phy.h   | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 3378ca4f49b6..41cfb24c48c1 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -242,11 +242,11 @@ unsigned int phy_supported_speeds(struct phy_device *phy,
  *
  * Description: Returns true if there is a valid setting, false otherwise.
  */
-static inline bool phy_check_valid(int speed, int duplex,
-				   unsigned long *features)
+bool phy_check_valid(int speed, int duplex, unsigned long *features)
 {
 	return !!phy_lookup_setting(speed, duplex, features, true);
 }
+EXPORT_SYMBOL(phy_check_valid);
 
 /**
  * phy_sanitize_settings - make sure the PHY is set to supported speed and duplex
diff --git a/include/linux/phy.h b/include/linux/phy.h
index c183a8a27986..7a8e541de3f3 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1619,6 +1619,7 @@ int phy_start_aneg(struct phy_device *phydev);
 int phy_aneg_done(struct phy_device *phydev);
 int phy_speed_down(struct phy_device *phydev, bool sync);
 int phy_speed_up(struct phy_device *phydev);
+bool phy_check_valid(int speed, int duplex, unsigned long *features);
 
 int phy_restart_aneg(struct phy_device *phydev);
 int phy_reset_after_clk_enable(struct phy_device *phydev);
-- 
2.30.2

[PATCH net-next v8 5/9] net: phy: add genphy_c45_ethtool_get/set_eee() support

[Oleksij Rempel]

Add replacement for phy_ethtool_get/set_eee() functions.

Current phy_ethtool_get/set_eee() implementation is great and it is
possible to make it even better:
- this functionality is for devices implementing parts of IEEE 802.3
  specification beyond Clause 22. The better place for this code is
  phy-c45.c
- currently it is able to do read/write operations on PHYs with
  different abilities to not existing registers. It is better to
  use stored supported_eee abilities to avoid false read/write
  operations.
- the eee_active detection will provide wrong results on not supported
  link modes. It is better to validate speed/duplex properties against
  supported EEE link modes.
- it is able to support only limited amount of link modes. We have more
  EEE link modes...

By refactoring this code I address most of this point except of the last
one. Adding additional EEE link modes will need more work.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy-c45.c | 238 ++++++++++++++++++++++++++++++++++++++
 include/linux/mdio.h      |  58 ++++++++++
 include/linux/phy.h       |   7 ++
 include/uapi/linux/mdio.h |   8 ++
 4 files changed, 311 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 2d10d22e7684..b4910c4c21d7 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -661,6 +661,129 @@ int genphy_c45_read_mdix(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
 
+/**
+ * genphy_c45_write_eee_adv - write advertised EEE link modes
+ * @phydev: target phy_device struct
+ * @adv: the linkmode advertisement settings
+ */
+int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv)
+{
+	int val, changed;
+
+	if (linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)) {
+		val = linkmode_to_mii_eee_cap1_t(adv);
+
+		/* In eee_broken_modes are stored MDIO_AN_EEE_ADV specific raw
+		 * register values.
+		 */
+		val &= ~phydev->eee_broken_modes;
+
+		/* IEEE 802.3-2018 45.2.7.13 EEE advertisement 1
+		 * (Register 7.60)
+		 */
+		val = phy_modify_mmd_changed(phydev, MDIO_MMD_AN,
+					     MDIO_AN_EEE_ADV,
+					     MDIO_EEE_100TX | MDIO_EEE_1000T |
+					     MDIO_EEE_10GT | MDIO_EEE_1000KX |
+					     MDIO_EEE_10GKX4 | MDIO_EEE_10GKR,
+					     val);
+		if (val < 0)
+			return val;
+		if (val > 0)
+			changed = 1;
+	}
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+			      phydev->supported_eee)) {
+		val = linkmode_adv_to_mii_10base_t1_t(adv);
+		/* IEEE 802.3cg-2019 45.2.7.25 10BASE-T1 AN control register
+		 * (Register 7.526)
+		 */
+		val = phy_modify_mmd_changed(phydev, MDIO_MMD_AN,
+					     MDIO_AN_10BT1_AN_CTRL,
+					     MDIO_AN_10BT1_AN_CTRL_ADV_EEE_T1L,
+					     val);
+		if (val < 0)
+			return val;
+		if (val > 0)
+			changed = 1;
+	}
+
+	return changed;
+}
+
+/**
+ * genphy_c45_read_eee_adv - read advertised EEE link modes
+ * @phydev: target phy_device struct
+ * @adv: the linkmode advertisement status
+ */
+static int genphy_c45_read_eee_adv(struct phy_device *phydev,
+				   unsigned long *adv)
+{
+	int val;
+
+	if (linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)) {
+		/* IEEE 802.3-2018 45.2.7.13 EEE advertisement 1
+		 * (Register 7.60)
+		 */
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV);
+		if (val < 0)
+			return val;
+
+		mii_eee_cap1_mod_linkmode_t(adv, val);
+	}
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+			      phydev->supported_eee)) {
+		/* IEEE 802.3cg-2019 45.2.7.25 10BASE-T1 AN control register
+		 * (Register 7.526)
+		 */
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10BT1_AN_CTRL);
+		if (val < 0)
+			return val;
+
+		mii_10base_t1_adv_mod_linkmode_t(adv, val);
+	}
+
+	return 0;
+}
+
+/**
+ * genphy_c45_read_eee_lpa - read advertised LP EEE link modes
+ * @phydev: target phy_device struct
+ * @lpa: the linkmode LP advertisement status
+ */
+static int genphy_c45_read_eee_lpa(struct phy_device *phydev,
+				   unsigned long *lpa)
+{
+	int val;
+
+	if (linkmode_intersects(phydev->supported, PHY_EEE_CAP1_FEATURES)) {
+		/* IEEE 802.3-2018 45.2.7.14 EEE link partner ability 1
+		 * (Register 7.61)
+		 */
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_LPABLE);
+		if (val < 0)
+			return val;
+
+		mii_eee_cap1_mod_linkmode_t(lpa, val);
+	}
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+			      phydev->supported_eee)) {
+		/* IEEE 802.3cg-2019 45.2.7.26 10BASE-T1 AN status register
+		 * (Register 7.527)
+		 */
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_10BT1_AN_STAT);
+		if (val < 0)
+			return val;
+
+		mii_10base_t1_adv_mod_linkmode_t(lpa, val);
+	}
+
+	return 0;
+}
+
 /**
  * genphy_c45_read_eee_cap1 - read supported EEE link modes from register 3.20
  * @phydev: target phy_device struct
@@ -1194,6 +1317,121 @@ int genphy_c45_plca_get_status(struct phy_device *phydev,
 }
 EXPORT_SYMBOL_GPL(genphy_c45_plca_get_status);
 
+/**
+ * genphy_c45_eee_is_active - get EEE status
+ * @phydev: target phy_device struct
+ * @adv: variable to store advertised linkmodes
+ * @lp: variable to store LP advertised linkmodes
+ * @is_enabled: variable to store EEE enabled/disabled configuration value
+ *
+ * Description: this function will read local and link partner PHY
+ * advertisements. Compare them return current EEE state.
+ */
+int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv,
+			     unsigned long *lp, bool *is_enabled)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_adv) = {};
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_lp) = {};
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	bool eee_enabled, eee_active;
+	int ret;
+
+	ret = genphy_c45_read_eee_adv(phydev, tmp_adv);
+	if (ret)
+		return ret;
+
+	ret = genphy_c45_read_eee_lpa(phydev, tmp_lp);
+	if (ret)
+		return ret;
+
+	eee_enabled = !linkmode_empty(tmp_adv);
+	linkmode_and(common, tmp_adv, tmp_lp);
+	if (eee_enabled && !linkmode_empty(common))
+		eee_active = phy_check_valid(phydev->speed, phydev->duplex,
+					     common);
+	else
+		eee_active = false;
+
+	if (adv)
+		linkmode_copy(adv, tmp_adv);
+	if (lp)
+		linkmode_copy(lp, tmp_lp);
+	if (is_enabled)
+		*is_enabled = eee_enabled;
+
+	return eee_active;
+}
+EXPORT_SYMBOL(genphy_c45_eee_is_active);
+
+/**
+ * genphy_c45_ethtool_get_eee - get EEE supported and status
+ * @phydev: target phy_device struct
+ * @data: ethtool_eee data
+ *
+ * Description: it reports the Supported/Advertisement/LP Advertisement
+ * capabilities.
+ */
+int genphy_c45_ethtool_get_eee(struct phy_device *phydev,
+			       struct ethtool_eee *data)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(adv) = {};
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(lp) = {};
+	bool overflow = false, is_enabled;
+	int ret;
+
+	ret = genphy_c45_eee_is_active(phydev, adv, lp, &is_enabled);
+	if (ret < 0)
+		return ret;
+
+	data->eee_enabled = is_enabled;
+	data->eee_active = ret;
+
+	if (!ethtool_convert_link_mode_to_legacy_u32(&data->supported,
+						     phydev->supported_eee))
+		overflow = true;
+	if (!ethtool_convert_link_mode_to_legacy_u32(&data->advertised, adv))
+		overflow = true;
+	if (!ethtool_convert_link_mode_to_legacy_u32(&data->lp_advertised, lp))
+		overflow = true;
+
+	if (overflow)
+		phydev_warn(phydev, "Not all supported or advertised EEE link modes were passed to the user space\n");
+
+	return 0;
+}
+EXPORT_SYMBOL(genphy_c45_ethtool_get_eee);
+
+/**
+ * genphy_c45_ethtool_set_eee - get EEE supported and status
+ * @phydev: target phy_device struct
+ * @data: ethtool_eee data
+ *
+ * Description: it reportes the Supported/Advertisement/LP Advertisement
+ * capabilities.
+ */
+int genphy_c45_ethtool_set_eee(struct phy_device *phydev,
+			       struct ethtool_eee *data)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(adv) = {};
+	int ret;
+
+	if (data->eee_enabled) {
+		if (data->advertised)
+			adv[0] = data->advertised;
+		else
+			linkmode_copy(adv, phydev->supported_eee);
+	}
+
+	ret = genphy_c45_write_eee_adv(phydev, adv);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		return phy_restart_aneg(phydev);
+
+	return 0;
+}
+EXPORT_SYMBOL(genphy_c45_ethtool_set_eee);
+
 struct phy_driver genphy_c45_driver = {
 	.phy_id         = 0xffffffff,
 	.phy_id_mask    = 0xffffffff,
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index e75583f5d967..27013d6bf24a 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -428,6 +428,64 @@ static inline void mii_eee_cap1_mod_linkmode_t(unsigned long *adv, u32 val)
 			 adv, val & MDIO_EEE_10GKR);
 }
 
+/**
+ * linkmode_to_mii_eee_cap1_t()
+ * @adv: the linkmode advertisement settings
+ *
+ * A function that translates linkmode to value for IEEE 802.3-2018 45.2.7.13
+ * "EEE advertisement 1" register (7.60)
+ */
+static inline u32 linkmode_to_mii_eee_cap1_t(unsigned long *adv)
+{
+	u32 result = 0;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, adv))
+		result |= MDIO_EEE_100TX;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, adv))
+		result |= MDIO_EEE_1000T;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, adv))
+		result |= MDIO_EEE_10GT;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT, adv))
+		result |= MDIO_EEE_1000KX;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT, adv))
+		result |= MDIO_EEE_10GKX4;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, adv))
+		result |= MDIO_EEE_10GKR;
+
+	return result;
+}
+
+/**
+ * mii_10base_t1_adv_mod_linkmode_t()
+ * @adv: linkmode advertisement settings
+ * @val: register value
+ *
+ * A function that translates IEEE 802.3cg-2019 45.2.7.26 "10BASE-T1 AN status"
+ * register (7.527) value to the linkmode.
+ */
+static inline void mii_10base_t1_adv_mod_linkmode_t(unsigned long *adv, u16 val)
+{
+	linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT,
+			 adv, val & MDIO_AN_10BT1_AN_CTRL_ADV_EEE_T1L);
+}
+
+/**
+ * linkmode_adv_to_mii_10base_t1_t()
+ * @adv: linkmode advertisement settings
+ *
+ * A function that translates the linkmode to IEEE 802.3cg-2019 45.2.7.25
+ * "10BASE-T1 AN control" register (7.526) value.
+ */
+static inline u32 linkmode_adv_to_mii_10base_t1_t(unsigned long *adv)
+{
+	u32 result = 0;
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT1L_Full_BIT, adv))
+		result |= MDIO_AN_10BT1_AN_CTRL_ADV_EEE_T1L;
+
+	return result;
+}
+
 int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
 int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
 int __mdiobus_modify_changed(struct mii_bus *bus, int addr, u32 regnum,
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7a8e541de3f3..727bff531a14 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1758,6 +1758,13 @@ int genphy_c45_plca_set_cfg(struct phy_device *phydev,
 			    const struct phy_plca_cfg *plca_cfg);
 int genphy_c45_plca_get_status(struct phy_device *phydev,
 			       struct phy_plca_status *plca_st);
+int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv,
+			     unsigned long *lp, bool *is_enabled);
+int genphy_c45_ethtool_get_eee(struct phy_device *phydev,
+			       struct ethtool_eee *data);
+int genphy_c45_ethtool_set_eee(struct phy_device *phydev,
+			       struct ethtool_eee *data);
+int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv);
 
 /* Generic C45 PHY driver */
 extern struct phy_driver genphy_c45_driver;
diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index 75b7257a51e1..256b463e47a6 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -79,6 +79,8 @@
 #define MDIO_AN_T1_LP_L		517	/* BASE-T1 AN LP Base Page ability register [15:0] */
 #define MDIO_AN_T1_LP_M		518	/* BASE-T1 AN LP Base Page ability register [31:16] */
 #define MDIO_AN_T1_LP_H		519	/* BASE-T1 AN LP Base Page ability register [47:32] */
+#define MDIO_AN_10BT1_AN_CTRL	526	/* 10BASE-T1 AN control register */
+#define MDIO_AN_10BT1_AN_STAT	527	/* 10BASE-T1 AN status register */
 #define MDIO_PMA_PMD_BT1_CTRL	2100	/* BASE-T1 PMA/PMD control register */
 
 /* LASI (Link Alarm Status Interrupt) registers, defined by XENPAK MSA. */
@@ -340,6 +342,12 @@
 #define MDIO_AN_T1_LP_H_10L_TX_HI_REQ	0x1000	/* 10BASE-T1L High Level LP Transmit Request */
 #define MDIO_AN_T1_LP_H_10L_TX_HI	0x2000	/* 10BASE-T1L High Level LP Transmit Ability */
 
+/* 10BASE-T1 AN control register */
+#define MDIO_AN_10BT1_AN_CTRL_ADV_EEE_T1L	0x4000 /* 10BASE-T1L EEE ability advertisement */
+
+/* 10BASE-T1 AN status register */
+#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_CFG_MST	0x4000 /* MASTER-SLAVE config value */
 
-- 
2.30.2

Re: [PATCH net-next v8 5/9] net: phy: add genphy_c45_ethtool_get/set_eee() support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 6965 bytes --]

On Sat, Feb 11, 2023 at 08:41:09AM +0100, Oleksij Rempel wrote:
> Add replacement for phy_ethtool_get/set_eee() functions.
> 
> Current phy_ethtool_get/set_eee() implementation is great and it is
> possible to make it even better:
> - this functionality is for devices implementing parts of IEEE 802.3
>   specification beyond Clause 22. The better place for this code is
>   phy-c45.c

Currently mainline is failing to bring up networking on the Libre
Computer AML-S905X-CC, with a bisect pointing at this commit,
022c3f87f88 upstream (although I'm not 100% sure I trust the bisect it
seems to be in roughly the right place).  I've not dug into what's going
on more than running the bisect yet.

The boot dies with:

[   15.532108] meson8b-dwmac c9410000.ethernet end0: Register
M the information provided will be safe.
[   15.569305] meson8b-dwmac c9410000.ethernet end0: PHY [mdio_mux-0.e40908ff:08] driver [Meson GXL Internal PHY] (irq=45)
[   15.585168] meson8b-dwmac c9410000.ethernet end0: No Safety Features support found
[   15.587169] meson8b-dwmac c9410000.ethernet end0: PTP not supported by HW
[   15.594673] meson8b-dwmac c9410000.ethernet end0: configuring for phy/rmii link mode
[   15.601802] ------------[ cut here ]---------that are being provided
--- [   15.606093] WARNING: CPU: 1 PID: 57 at drivers/net/phy/phy.c:1168
phy_error+0x14/0x60 [   15.613854] Modules linked in: snd_soc_hdmi_codec
dw_hdmi_i2s_audio meson_gxl dwmac_generic meson_drm lima
drm_shmem_helper gpu_sched dwmac_meson8b stmmac_platform crct10dif_ce
stmmac amlogic_gxl_crypto pcs_xpcs drm_dma_helper crypto_engine
meson_canvas meson_gxbb_wdt meson_rng meson_dw_hdmi rng_core dw_hdmi cec
drm_display_helper meson_ir rc_core snd_soc_meson_aiu
snd_soc_meson_codec_glue snd_soc_meson_t9015 snd_soc_meson_gx_sound_card
snd_soc_meson_card_utils snd_soc_simple_amplifier display_conne
drm_kms_helper drm nvmem_meson_efuse [   15.661291] CPU: 1 PID: 57 Comm:
kworker/u8:2 Not tainted 6.2.0-rc7-01626-g8b68710a3121 #10
[   15.669568] Hardware name: Libre Computer AML-S905X-CC (DT)
[   15.675090] Workqueue: events_power_efficient phy_state_machine
[   15.680954] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   15.687853] pc : phy_error+0x14/0x60
[   15.691389] lr : phy_state_machine+0xa0/0x280
[   15.695701] sp : ffff80000a8a3d40
[   15.698979] x29: ffff80000a8a3d40 x28: 0000000000000000 x27: 0000000000000000
[   15.706051] x26: ffff80000a170000 x25: ffff00007ba884b0 x24: ffff000001009105
[   15.713124] x23: 00000000ffffffa1 x22: ffff00007ba884a8 x21: ffff00007ba88500
[   15.720196] x20: 0000000000000003 x19: ffff00007ba88000 x18: 0000000000000000
[   15.727269] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[   15.734341] x14: 00000000000001d6 x13: 00000000000001d6 x12: 0000000000000001
[   15.741414] x11: 0000000000000001 x10: ffff00007ba88420 x9 : ffff00007ba88418
[   15.748487] x8 : 0000000000000000 x7 : 0000000000000020 x6 : 0000000000000040
[   15.755559] x5 : 0000000000000000 x4 : 0000000000000000 x3 : ffff00007ba88500
[   15.762631] x2 : 0000000000000000 x1 : ffff000001105700 x0 : ffff00007ba88000
[   15.769705] Call trace:
[   15.772120]  phy_error+0x14/.  It didn't really cross my mind that
something would be hard coded like this.
[   15.786868]  kthread+0x108/0x10c
[   15.790059]  ret_from_fork+0x10/0x20
[   15.793596] ---[ end trace 0000000000000000 ]---

followed by there being no network so no NFS root.  Bisect log:

git bisect start
# good: [c9c3395d5e3dcc6daee66c6908354d47bf98cb0c] Linux 6.2
git bisect good c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
# bad: [2fcd07b7ccd5fd10b2120d298363e4e6c53ccf9c] mm/mprotect: Fix successful vma_merge() of next in do_mprotect_pkey()
git bisect bad 2fcd07b7ccd5fd10b2120d298363e4e6c53ccf9c
# bad: [d5176cdbf64ce7d4eebfbec23118e9f72] Merge tag 'pinctrl-v6.3-1' of
# git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
git bisect bad d5176cdbf64ce7d4eebf339205f17c23118e9f72
# skip: [69308402ca6f5b80a5a090ade0b13bd146891420] Merge tag
# 'platform-drivers-x86-v6.3-1' of
# git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86
git bisect skip 69308402ca6f5b80a5a090ade0b13bd146891420
# good: [bc61761394ce0f0cc35c6fc60426f08d83d0d488] ipv6: ICMPV6: Use
# swap() instead of open coding it
git bisect good bc61761394ce0f0cc35c6fc60426f08d83d0d488
# good: [1b72607d7321e66829e11148712b3a2ba1dc83e7] Merge tag 'thermal-6.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
git bisect good 1b72607d7321e66829e11148712b3a2ba1dc83e7
# bad: [d1fabc68f8e0541d41657096dc713cb01775652d] Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
git bisect bad d1fabc68f8e0541d41657096dc713cb01775652d
# good: [31de4105f00d64570139bc5494a20 after figuring out what system
# it's on1b0bdf] bpf: Add BPF_FIB_LOOKUP_SKIP_NEIGH for bpf_fib_lookup
git bisect good 31de4105f00d64570139bc5494a201b0bd57349f
# good: [1a30a6b25f263686dbf2028d56041ac012b10dcb] wifi: brcmfmac: p2p:.
git bisect good 1a30a6b25f263686dbf2028d56041ac012b10dcb
# bad: [14743ddd2495c96caa18e382625c034e49a812e2] sfc: add devlink info support for ef100
git bisect bad 14743ddd2495c96caa18e382625c034e49a812e2
# bad: [1daa8e25ed971eca8cd8c8dfd4d6d6541b1d62a2] Merge branch 'net-make-kobj_type-structures-constant'
git bisect bad 1daa8e25ed971eca8cd8c8dfd4d6d6541b1d62a2
# bad: [1daa8e25ed971eca8cd8c8dfd4d6d6541b1d62a2] Merge branch 'net-make-kobj_type-structures-constant'
git bisect bad 1daa8e25ed971eca8cd8c8dfd4d6d6541b1d62a2
# good: [75da437a2f172759b227309 (or whatever)1a938772e687242d0] Merge
# branch '40GbE' of
# git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue
git bisect good 75da437a2f172759b2273091a938772e687242d0
# bad: [8b68710a3121e0475b123a20c4220f66a728770e] net: phy: start using genphy_c45_ethtool_get/set_eee()
git bisect bad 8b68710a3121e0475b123a20c4220f66a728770e
# good: [d1ce6395d4648b41cf762714934e34ae57f0d1a4] net: ipa: define IPA v3.1 GSI event ring register offsets
git bisect good d1ce6395d4648b41cf762714934e34ae57f0d1a4
# good: [79cdf17e5131ccdee0792f6f25d3db0e34861998] Merge branch 'ionic-on-chip-desc'
git bisect good 79cdf17e5131ccdee0792f6f25d3db0e34861998
# bad: [ when the device is
# registered022c3f87f88e2d68e90be7687d981c9cb893a3b1] net: phy: add
# genphy_c45_ethtool_get/set_eee() support
git bisect bad 022c3f87f88e2d68e90be7687d981c9cb893a3b1
# good: [14e47d1fb8f9596acc90a06a66808657a9c512b5] net: phy: add
# genphy_c45_read_eee_abilities() function
git bisect good 14e47d1fb8f9596acc90a06a66808657a9c512b5
# good: [48fb19940f2ba6b50dfea70f671be9340fb63d60] net: phy: micrel: add ksz9477_get_features()
git bisect good cf9f6079696840093aa6ea3c0ee405a553afe2fb
# first bad commit: [022c3f87f88e2d68e90be7687d981c9cb893a3b1] net: phy: add genphy_c45_ethtool_get/set_eee() support
 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH net-next v8 5/9] net: phy: add genphy_c45_ethtool_get/set_eee() support

[Oleksij Rempel]

Hi Mark,

On Sun, Feb 26, 2023 at 06:06:48PM +0000, Mark Brown wrote:
> On Sat, Feb 11, 2023 at 08:41:09AM +0100, Oleksij Rempel wrote:
> > Add replacement for phy_ethtool_get/set_eee() functions.
> > 
> > Current phy_ethtool_get/set_eee() implementation is great and it is
> > possible to make it even better:
> > - this functionality is for devices implementing parts of IEEE 802.3
> >   specification beyond Clause 22. The better place for this code is
> >   phy-c45.c
> 
> Currently mainline is failing to bring up networking on the Libre
> Computer AML-S905X-CC, with a bisect pointing at this commit,
> 022c3f87f88 upstream (although I'm not 100% sure I trust the bisect it
> seems to be in roughly the right place).  I've not dug into what's going
> on more than running the bisect yet.

Can you please test following fixes:
https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/
https://lore.kernel.org/all/20230225071644.2754893-1-o.rempel@pengutronix.de/

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v8 5/9] net: phy: add genphy_c45_ethtool_get/set_eee() support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 931 bytes --]

On Mon, Feb 27, 2023 at 06:52:41AM +0100, Oleksij Rempel wrote:
> On Sun, Feb 26, 2023 at 06:06:48PM +0000, Mark Brown wrote:

> > Currently mainline is failing to bring up networking on the Libre
> > Computer AML-S905X-CC, with a bisect pointing at this commit,
> > 022c3f87f88 upstream (although I'm not 100% sure I trust the bisect it
> > seems to be in roughly the right place).  I've not dug into what's going
> > on more than running the bisect yet.

> Can you please test following fixes:
> https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/
> https://lore.kernel.org/all/20230225071644.2754893-1-o.rempel@pengutronix.de/

They seem to work, thanks!  I had found and tried the second patch but
it doesn't apply without the first series.  Will those patches be going
to Linus for -rc1?  It's pretty disruptive to a bunch of the test
infrastructure to not be able to NFS boot.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH net-next v8 5/9] net: phy: add genphy_c45_ethtool_get/set_eee() support

[Jakub Kicinski]

On Mon, 27 Feb 2023 13:06:10 +0000 Mark Brown wrote:
> They seem to work, thanks!  I had found and tried the second patch but
> it doesn't apply without the first series.  Will those patches be going
> to Linus for -rc1?  It's pretty disruptive to a bunch of the test
> infrastructure to not be able to NFS boot.

Should be in Linus's tree later today 🤞️

[PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Oleksij Rempel]

Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().

It should work as before except write operation to the EEE adv registers
will be done only if some EEE abilities was detected.

If some driver will have a regression, related driver should provide own
.get_features callback. See micrel.c:ksz9477_get_features() as example.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 66a4e62009bb..8d927c5e3bf8 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2231,7 +2231,10 @@ int __genphy_config_aneg(struct phy_device *phydev, bool changed)
 {
 	int err;
 
-	if (genphy_config_eee_advert(phydev))
+	err = genphy_c45_write_eee_adv(phydev, phydev->supported_eee);
+	if (err < 0)
+		return err;
+	else if (err)
 		changed = true;
 
 	err = genphy_setup_master_slave(phydev);
@@ -2653,6 +2656,11 @@ int genphy_read_abilities(struct phy_device *phydev)
 				 phydev->supported, val & ESTATUS_1000_XFULL);
 	}
 
+	/* This is optional functionality. If not supported, we may get an error
+	 * which should be ignored.
+	 */
+	genphy_c45_read_eee_abilities(phydev);
+
 	return 0;
 }
 EXPORT_SYMBOL(genphy_read_abilities);
-- 
2.30.2

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> 
> It should work as before except write operation to the EEE adv registers
> will be done only if some EEE abilities was detected.
> 
> If some driver will have a regression, related driver should provide own
> .get_features callback. See micrel.c:ksz9477_get_features() as example.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

This patch causes network interface failures with all my xtensa qemu
emulations. Reverting it fixes the problem. Bisect log is attached
for reference.

Guenter

---
# bad: [e4bc15889506723d7b93c053ad4a75cd58248d74] Merge tag 'leds-next-6.3' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/leds
# good: [c9c3395d5e3dcc6daee66c6908354d47bf98cb0c] Linux 6.2
git bisect start 'HEAD' 'c9c3395d5e3d'
# bad: [5b7c4cabbb65f5c469464da6c5f614cbd7f730f2] Merge tag 'net-next-6.3' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
git bisect bad 5b7c4cabbb65f5c469464da6c5f614cbd7f730f2
# good: [877934769e5b91798d304d4641647900ee614ce8] Merge tag 'x86_cpu_for_v6.3_rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 877934769e5b91798d304d4641647900ee614ce8
# good: [c5ebba75c7625e5cb62cb5423883cc3764779420] net: ipa: use bitmasks for GSI IRQ values
git bisect good c5ebba75c7625e5cb62cb5423883cc3764779420
# bad: [871489dd01b67483248edc8873c389a66e469f30] Merge tag 'ieee802154-for-net-next-2023-02-20' of git://git.kernel.org/pub/scm/linux/kernel/git/sschmidt/wpan-next
git bisect bad 871489dd01b67483248edc8873c389a66e469f30
# good: [986e43b19ae9176093da35e0a844e65c8bf9ede7] wifi: mac80211: fix receiving A-MSDU frames on mesh interfaces
git bisect good 986e43b19ae9176093da35e0a844e65c8bf9ede7
# bad: [ca0df43d211039dded5a8f8553356414c9a74731] Merge tag 'wireless-next-2023-03-16' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next
git bisect bad ca0df43d211039dded5a8f8553356414c9a74731
# bad: [388a9c907a51489bf566165c72e4e8aa4d62ab49] Merge branch 'devlink-cleanups-and-move-devlink-health-functionality-to-separate-file'
git bisect bad 388a9c907a51489bf566165c72e4e8aa4d62ab49
# bad: [1a940b00013a468c0c9dd79dbb485c3ad273939e] net: stmmac: dwc-qos: Make struct dwc_eth_dwmac_data::remove return void
git bisect bad 1a940b00013a468c0c9dd79dbb485c3ad273939e
# good: [8024edf3590c83f467374857d7c3082d4b3bf079] Merge branch 'net-ipa-GSI-regs'
git bisect good 8024edf3590c83f467374857d7c3082d4b3bf079
# bad: [9b01c885be364526d8c05794f8358b3e563b7ff8] net: phy: c22: migrate to genphy_c45_write_eee_adv()
git bisect bad 9b01c885be364526d8c05794f8358b3e563b7ff8
# good: [79cdf17e5131ccdee0792f6f25d3db0e34861998] Merge branch 'ionic-on-chip-desc'
git bisect good 79cdf17e5131ccdee0792f6f25d3db0e34861998
# good: [48fb19940f2ba6b50dfea70f671be9340fb63d60] net: phy: micrel: add ksz9477_get_features()
git bisect good 48fb19940f2ba6b50dfea70f671be9340fb63d60
# good: [022c3f87f88e2d68e90be7687d981c9cb893a3b1] net: phy: add genphy_c45_ethtool_get/set_eee() support
git bisect good 022c3f87f88e2d68e90be7687d981c9cb893a3b1
# first bad commit: [9b01c885be364526d8c05794f8358b3e563b7ff8] net: phy: c22: migrate to genphy_c45_write_eee_adv()

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
> On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> > 
> > It should work as before except write operation to the EEE adv registers
> > will be done only if some EEE abilities was detected.
> > 
> > If some driver will have a regression, related driver should provide own
> > .get_features callback. See micrel.c:ksz9477_get_features() as example.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
> This patch causes network interface failures with all my xtensa qemu
> emulations. Reverting it fixes the problem. Bisect log is attached
> for reference.
> 

Also affected are arm:cubieboard emulations, with same symptom.
arm:bletchley-bmc emulations crash. In both cases, reverting this patch
fixes the problem.

Guenter

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Oleksij Rempel]

Hallo Guenter,

On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote:
> On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
> > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> > > 
> > > It should work as before except write operation to the EEE adv registers
> > > will be done only if some EEE abilities was detected.
> > > 
> > > If some driver will have a regression, related driver should provide own
> > > .get_features callback. See micrel.c:ksz9477_get_features() as example.
> > > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > 
> > This patch causes network interface failures with all my xtensa qemu
> > emulations. Reverting it fixes the problem. Bisect log is attached
> > for reference.
> > 
> 
> Also affected are arm:cubieboard emulations, with same symptom.
> arm:bletchley-bmc emulations crash. In both cases, reverting this patch
> fixes the problem.

Please test this fixes:
https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

On 2/23/23 20:53, Oleksij Rempel wrote:
> Hallo Guenter,
> 
> On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote:
>> On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
>>> On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
>>>> Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
>>>>
>>>> It should work as before except write operation to the EEE adv registers
>>>> will be done only if some EEE abilities was detected.
>>>>
>>>> If some driver will have a regression, related driver should provide own
>>>> .get_features callback. See micrel.c:ksz9477_get_features() as example.
>>>>
>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>>>
>>> This patch causes network interface failures with all my xtensa qemu
>>> emulations. Reverting it fixes the problem. Bisect log is attached
>>> for reference.
>>>
>>
>> Also affected are arm:cubieboard emulations, with same symptom.
>> arm:bletchley-bmc emulations crash. In both cases, reverting this patch
>> fixes the problem.
> 
> Please test this fixes:
> https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/
> 

Applied and tested

77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes
068a35a8d62c net: phy: do not force EEE support
66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function
ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation

on top of

d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm

No change for xtensa and arm:cubieboard; network interfaces still fail.
On the plus side, the failures with arm:bletchley-bmc (warnings, crash)
are longer seen.

Guenter

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Oleksij Rempel]

On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote:
> On 2/23/23 20:53, Oleksij Rempel wrote:
> > Hallo Guenter,
> > 
> > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote:
> > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
> > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> > > > > 
> > > > > It should work as before except write operation to the EEE adv registers
> > > > > will be done only if some EEE abilities was detected.
> > > > > 
> > > > > If some driver will have a regression, related driver should provide own
> > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example.
> > > > > 
> > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > > > 
> > > > This patch causes network interface failures with all my xtensa qemu
> > > > emulations. Reverting it fixes the problem. Bisect log is attached
> > > > for reference.
> > > > 
> > > 
> > > Also affected are arm:cubieboard emulations, with same symptom.
> > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch
> > > fixes the problem.
> > 
> > Please test this fixes:
> > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/
> > 
> 
> Applied and tested
> 
> 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes
> 068a35a8d62c net: phy: do not force EEE support
> 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function
> ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation
> 
> on top of
> 
> d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> 
> No change for xtensa and arm:cubieboard; network interfaces still fail.

Huh, interesting.

can you please send me the kernel logs.

> On the plus side, the failures with arm:bletchley-bmc (warnings, crash)
> are longer seen.

s/longer/no longer/ ?

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

On Fri, Feb 24, 2023 at 05:52:13PM +0100, Oleksij Rempel wrote:
> On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote:
> > On 2/23/23 20:53, Oleksij Rempel wrote:
> > > Hallo Guenter,
> > > 
> > > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote:
> > > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
> > > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> > > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> > > > > > 
> > > > > > It should work as before except write operation to the EEE adv registers
> > > > > > will be done only if some EEE abilities was detected.
> > > > > > 
> > > > > > If some driver will have a regression, related driver should provide own
> > > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example.
> > > > > > 
> > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > > > > 
> > > > > This patch causes network interface failures with all my xtensa qemu
> > > > > emulations. Reverting it fixes the problem. Bisect log is attached
> > > > > for reference.
> > > > > 
> > > > 
> > > > Also affected are arm:cubieboard emulations, with same symptom.
> > > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch
> > > > fixes the problem.
> > > 
> > > Please test this fixes:
> > > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/
> > > 
> > 
> > Applied and tested
> > 
> > 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes
> > 068a35a8d62c net: phy: do not force EEE support
> > 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function
> > ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation
> > 
> > on top of
> > 
> > d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > 
> > No change for xtensa and arm:cubieboard; network interfaces still fail.
> 
> Huh, interesting.
> 
> can you please send me the kernel logs.
> 
There is nothing useful there, or at least I don't see anything useful.
The Ethernet interfaces (sun4i-emac for cubieboard and ethoc for xtensa)
just don't come up.

Sample logs:

cubieboard:

https://kerneltests.org/builders/qemu-arm-v7-master/builds/531/steps/qemubuildcommand/logs/stdio

xtensa:

https://kerneltests.org/builders/qemu-xtensa-master/builds/2177/steps/qemubuildcommand/logs/stdio

and, for completeness, bletchley-bmc:

https://kerneltests.org/builders/qemu-arm-aspeed-master/builds/531/steps/qemubuildcommand/logs/stdio

Those logs are without the above set of patches, but I don't see a
difference with the patches applied for cubieboard and xtensa. I
started a complete test run (for all emulations) with the patches
applied; that should take about an hour to complete. 
I could also add some debug logging, but you'd have to give me
some hints about what to add and where.

> > On the plus side, the failures with arm:bletchley-bmc (warnings, crash)
> > are longer seen.
> 
> s/longer/no longer/ ?

Yes, sorry.

Thanks,
Guenter

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Oleksij Rempel]

On Fri, Feb 24, 2023 at 09:41:32AM -0800, Guenter Roeck wrote:
> On Fri, Feb 24, 2023 at 05:52:13PM +0100, Oleksij Rempel wrote:
> > On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote:
> > > On 2/23/23 20:53, Oleksij Rempel wrote:
> > > > Hallo Guenter,
> > > > 
> > > > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote:
> > > > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
> > > > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> > > > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> > > > > > > 
> > > > > > > It should work as before except write operation to the EEE adv registers
> > > > > > > will be done only if some EEE abilities was detected.
> > > > > > > 
> > > > > > > If some driver will have a regression, related driver should provide own
> > > > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example.
> > > > > > > 
> > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > > > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > > > > > 
> > > > > > This patch causes network interface failures with all my xtensa qemu
> > > > > > emulations. Reverting it fixes the problem. Bisect log is attached
> > > > > > for reference.
> > > > > > 
> > > > > 
> > > > > Also affected are arm:cubieboard emulations, with same symptom.
> > > > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch
> > > > > fixes the problem.
> > > > 
> > > > Please test this fixes:
> > > > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/
> > > > 
> > > 
> > > Applied and tested
> > > 
> > > 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes
> > > 068a35a8d62c net: phy: do not force EEE support
> > > 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function
> > > ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation
> > > 
> > > on top of
> > > 
> > > d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > 
> > > No change for xtensa and arm:cubieboard; network interfaces still fail.
> > 
> > Huh, interesting.
> > 
> > can you please send me the kernel logs.
> > 
> There is nothing useful there, or at least I don't see anything useful.
> The Ethernet interfaces (sun4i-emac for cubieboard and ethoc for xtensa)
> just don't come up.
> 
> Sample logs:
> 
> cubieboard:
> 
> https://kerneltests.org/builders/qemu-arm-v7-master/builds/531/steps/qemubuildcommand/logs/stdio
> 
> xtensa:
> 
> https://kerneltests.org/builders/qemu-xtensa-master/builds/2177/steps/qemubuildcommand/logs/stdio
> 
> and, for completeness, bletchley-bmc:
> 
> https://kerneltests.org/builders/qemu-arm-aspeed-master/builds/531/steps/qemubuildcommand/logs/stdio
> 
> Those logs are without the above set of patches, but I don't see a
> difference with the patches applied for cubieboard and xtensa. I
> started a complete test run (for all emulations) with the patches
> applied; that should take about an hour to complete. 
> I could also add some debug logging, but you'd have to give me
> some hints about what to add and where.

OK, interesting. These are emulated PHYs. QEMU seems to return 0 or
0xFFFF on unsupported registers. May be I'm wrong.
All EEE read/write accesses depend on initial capability read
genphy_c45_read_eee_cap1()

Can you please add this trace:

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index f595acd0a895..67dac9f0e71d 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev)
         * (Register 3.20)
         */
        val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
+       printk("MDIO_PCS_EEE_ABLE = 0x%04x", val);
        if (val < 0)
                return val;

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

On 2/24/23 10:36, Oleksij Rempel wrote:
> On Fri, Feb 24, 2023 at 09:41:32AM -0800, Guenter Roeck wrote:
>> On Fri, Feb 24, 2023 at 05:52:13PM +0100, Oleksij Rempel wrote:
>>> On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote:
>>>> On 2/23/23 20:53, Oleksij Rempel wrote:
>>>>> Hallo Guenter,
>>>>>
>>>>> On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote:
>>>>>> On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
>>>>>>> On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
>>>>>>>> Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
>>>>>>>>
>>>>>>>> It should work as before except write operation to the EEE adv registers
>>>>>>>> will be done only if some EEE abilities was detected.
>>>>>>>>
>>>>>>>> If some driver will have a regression, related driver should provide own
>>>>>>>> .get_features callback. See micrel.c:ksz9477_get_features() as example.
>>>>>>>>
>>>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>>>>>>>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>>>>>>>
>>>>>>> This patch causes network interface failures with all my xtensa qemu
>>>>>>> emulations. Reverting it fixes the problem. Bisect log is attached
>>>>>>> for reference.
>>>>>>>
>>>>>>
>>>>>> Also affected are arm:cubieboard emulations, with same symptom.
>>>>>> arm:bletchley-bmc emulations crash. In both cases, reverting this patch
>>>>>> fixes the problem.
>>>>>
>>>>> Please test this fixes:
>>>>> https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/
>>>>>
>>>>
>>>> Applied and tested
>>>>
>>>> 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes
>>>> 068a35a8d62c net: phy: do not force EEE support
>>>> 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function
>>>> ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation
>>>>
>>>> on top of
>>>>
>>>> d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>
>>>> No change for xtensa and arm:cubieboard; network interfaces still fail.
>>>
>>> Huh, interesting.
>>>
>>> can you please send me the kernel logs.
>>>
>> There is nothing useful there, or at least I don't see anything useful.
>> The Ethernet interfaces (sun4i-emac for cubieboard and ethoc for xtensa)
>> just don't come up.
>>
>> Sample logs:
>>
>> cubieboard:
>>
>> https://kerneltests.org/builders/qemu-arm-v7-master/builds/531/steps/qemubuildcommand/logs/stdio
>>
>> xtensa:
>>
>> https://kerneltests.org/builders/qemu-xtensa-master/builds/2177/steps/qemubuildcommand/logs/stdio
>>
>> and, for completeness, bletchley-bmc:
>>
>> https://kerneltests.org/builders/qemu-arm-aspeed-master/builds/531/steps/qemubuildcommand/logs/stdio
>>
>> Those logs are without the above set of patches, but I don't see a
>> difference with the patches applied for cubieboard and xtensa. I
>> started a complete test run (for all emulations) with the patches
>> applied; that should take about an hour to complete.
>> I could also add some debug logging, but you'd have to give me
>> some hints about what to add and where.
> 
> OK, interesting. These are emulated PHYs. QEMU seems to return 0 or
> 0xFFFF on unsupported registers. May be I'm wrong.
> All EEE read/write accesses depend on initial capability read
> genphy_c45_read_eee_cap1()
> 
> Can you please add this trace:
> 
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index f595acd0a895..67dac9f0e71d 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev)
>           * (Register 3.20)
>           */
>          val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
> +       printk("MDIO_PCS_EEE_ABLE = 0x%04x", val);
>          if (val < 0)
>                  return val;
> 

For cubieboard:

MDIO_PCS_EEE_ABLE = 0x0000

qemu reports attempts to access unsupported registers.

I had a look at the Allwinner mdio driver. There is no indication suggesting
what the real hardware would return when trying to access unsupported registers,
and the Ethernet controller datasheet is not public.

For xtensa:

MDIO_PCS_EEE_ABLE = 0x0014

I didn't try to find out what that means.

qemu did not report attempts to access unsupported registers.

Guenter

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Oleksij Rempel]

On Fri, Feb 24, 2023 at 11:17:24AM -0800, Guenter Roeck wrote:
> On 2/24/23 10:36, Oleksij Rempel wrote:
> > On Fri, Feb 24, 2023 at 09:41:32AM -0800, Guenter Roeck wrote:
> > > On Fri, Feb 24, 2023 at 05:52:13PM +0100, Oleksij Rempel wrote:
> > > > On Fri, Feb 24, 2023 at 08:00:57AM -0800, Guenter Roeck wrote:
> > > > > On 2/23/23 20:53, Oleksij Rempel wrote:
> > > > > > Hallo Guenter,
> > > > > > 
> > > > > > On Thu, Feb 23, 2023 at 08:16:04PM -0800, Guenter Roeck wrote:
> > > > > > > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
> > > > > > > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> > > > > > > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> > > > > > > > > 
> > > > > > > > > It should work as before except write operation to the EEE adv registers
> > > > > > > > > will be done only if some EEE abilities was detected.
> > > > > > > > > 
> > > > > > > > > If some driver will have a regression, related driver should provide own
> > > > > > > > > .get_features callback. See micrel.c:ksz9477_get_features() as example.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > > > > > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > > > > > > > 
> > > > > > > > This patch causes network interface failures with all my xtensa qemu
> > > > > > > > emulations. Reverting it fixes the problem. Bisect log is attached
> > > > > > > > for reference.
> > > > > > > > 
> > > > > > > 
> > > > > > > Also affected are arm:cubieboard emulations, with same symptom.
> > > > > > > arm:bletchley-bmc emulations crash. In both cases, reverting this patch
> > > > > > > fixes the problem.
> > > > > > 
> > > > > > Please test this fixes:
> > > > > > https://lore.kernel.org/all/167715661799.11159.2057121677394149658.git-patchwork-notify@kernel.org/
> > > > > > 
> > > > > 
> > > > > Applied and tested
> > > > > 
> > > > > 77c39beb5efa (HEAD -> master) net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes
> > > > > 068a35a8d62c net: phy: do not force EEE support
> > > > > 66d358a5fac6 net: phy: c45: add genphy_c45_an_config_eee_aneg() function
> > > > > ecea1bf8b04c net: phy: c45: use "supported_eee" instead of supported for access validation
> > > > > 
> > > > > on top of
> > > > > 
> > > > > d2980d8d8265 (upstream/master, origin/master, origin/HEAD, local/master) Merge tag 'mm-nonmm-stable-2023-02-20-15-29' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > 
> > > > > No change for xtensa and arm:cubieboard; network interfaces still fail.
> > > > 
> > > > Huh, interesting.
> > > > 
> > > > can you please send me the kernel logs.
> > > > 
> > > There is nothing useful there, or at least I don't see anything useful.
> > > The Ethernet interfaces (sun4i-emac for cubieboard and ethoc for xtensa)
> > > just don't come up.
> > > 
> > > Sample logs:
> > > 
> > > cubieboard:
> > > 
> > > https://kerneltests.org/builders/qemu-arm-v7-master/builds/531/steps/qemubuildcommand/logs/stdio
> > > 
> > > xtensa:
> > > 
> > > https://kerneltests.org/builders/qemu-xtensa-master/builds/2177/steps/qemubuildcommand/logs/stdio
> > > 
> > > and, for completeness, bletchley-bmc:
> > > 
> > > https://kerneltests.org/builders/qemu-arm-aspeed-master/builds/531/steps/qemubuildcommand/logs/stdio
> > > 
> > > Those logs are without the above set of patches, but I don't see a
> > > difference with the patches applied for cubieboard and xtensa. I
> > > started a complete test run (for all emulations) with the patches
> > > applied; that should take about an hour to complete.
> > > I could also add some debug logging, but you'd have to give me
> > > some hints about what to add and where.
> > 
> > OK, interesting. These are emulated PHYs. QEMU seems to return 0 or
> > 0xFFFF on unsupported registers. May be I'm wrong.
> > All EEE read/write accesses depend on initial capability read
> > genphy_c45_read_eee_cap1()
> > 
> > Can you please add this trace:
> > 
> > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> > index f595acd0a895..67dac9f0e71d 100644
> > --- a/drivers/net/phy/phy-c45.c
> > +++ b/drivers/net/phy/phy-c45.c
> > @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev)
> >           * (Register 3.20)
> >           */
> >          val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
> > +       printk("MDIO_PCS_EEE_ABLE = 0x%04x", val);
> >          if (val < 0)
> >                  return val;
> > 
> 
> For cubieboard:
> 
> MDIO_PCS_EEE_ABLE = 0x0000
> 
> qemu reports attempts to access unsupported registers.
> 
> I had a look at the Allwinner mdio driver. There is no indication suggesting
> what the real hardware would return when trying to access unsupported registers,
> and the Ethernet controller datasheet is not public.

These are PHY accesses over MDIO bus. Ethernet controller should not
care about content of this operations. But on qemu side, it is implemented as
part of Ethernet controller emulation...

Since MDIO_PCS_EEE_ABLE == 0x0000, phydev->supported_eee should prevent
other EEE related operations. But may be actual phy_read_mmd() went
wrong. It is a combination of simple phy_read/write to different
registers.

> For xtensa:
> 
> MDIO_PCS_EEE_ABLE = 0x0014
> 
> I didn't try to find out what that means.

These will be interpreted as the PHY supports 1000KX and 1000T EEE modes.
Starting from this point all EEE read write operations will be allowed.

> qemu did not report attempts to access unsupported registers.

Hm. What is the best way to proceed? Remove genphy_c45_read_eee_abilities()
out of genphy_read_abilities() and let add it to PHYs known to support
it? Or go deeper and fix QEMU if needed?

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

On 2/24/23 12:02, Oleksij Rempel wrote:
[ ... ]
>>
>> For cubieboard:
>>
>> MDIO_PCS_EEE_ABLE = 0x0000
>>
>> qemu reports attempts to access unsupported registers.
>>
>> I had a look at the Allwinner mdio driver. There is no indication suggesting
>> what the real hardware would return when trying to access unsupported registers,
>> and the Ethernet controller datasheet is not public.
> 
> These are PHY accesses over MDIO bus. Ethernet controller should not
> care about content of this operations. But on qemu side, it is implemented as
> part of Ethernet controller emulation...
> 
> Since MDIO_PCS_EEE_ABLE == 0x0000, phydev->supported_eee should prevent
> other EEE related operations. But may be actual phy_read_mmd() went
> wrong. It is a combination of simple phy_read/write to different
> registers.
> 

Adding MDD read/write support in qemu doesn't help. Something else in your patch
prevents the PHY from coming up. After reverting your patch, I see

sun4i-emac 1c0b000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready

in the log. This is missing with your patch in place.

Anyway, the key difference is not really the qemu emulation, but the added
unconditional call to genphy_c45_write_eee_adv() in your patch. If you look
closely into that function, you may notice that the 'changed' variable is
never set to 0.

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 3813b86689d0..fee514b96ab1 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -672,7 +672,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
   */
  int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv)
  {
-       int val, changed;
+       int val, changed = 0;

         if (linkmode_intersects(phydev->supported_eee, PHY_EEE_CAP1_FEATURES)) {
                 val = linkmode_to_mii_eee_cap1_t(adv);

fixes the problem, both for cubieboard and xtensa.

Guenter

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Oleksij Rempel]

On Fri, Feb 24, 2023 at 04:09:40PM -0800, Guenter Roeck wrote:
> On 2/24/23 12:02, Oleksij Rempel wrote:
> [ ... ]
> > > 
> > > For cubieboard:
> > > 
> > > MDIO_PCS_EEE_ABLE = 0x0000
> > > 
> > > qemu reports attempts to access unsupported registers.
> > > 
> > > I had a look at the Allwinner mdio driver. There is no indication suggesting
> > > what the real hardware would return when trying to access unsupported registers,
> > > and the Ethernet controller datasheet is not public.
> > 
> > These are PHY accesses over MDIO bus. Ethernet controller should not
> > care about content of this operations. But on qemu side, it is implemented as
> > part of Ethernet controller emulation...
> > 
> > Since MDIO_PCS_EEE_ABLE == 0x0000, phydev->supported_eee should prevent
> > other EEE related operations. But may be actual phy_read_mmd() went
> > wrong. It is a combination of simple phy_read/write to different
> > registers.
> > 
> 
> Adding MDD read/write support in qemu doesn't help. Something else in your patch
> prevents the PHY from coming up. After reverting your patch, I see
> 
> sun4i-emac 1c0b000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
> IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> 
> in the log. This is missing with your patch in place.
> 
> Anyway, the key difference is not really the qemu emulation, but the added
> unconditional call to genphy_c45_write_eee_adv() in your patch. If you look
> closely into that function, you may notice that the 'changed' variable is
> never set to 0.
> 
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index 3813b86689d0..fee514b96ab1 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -672,7 +672,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
>   */
>  int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv)
>  {
> -       int val, changed;
> +       int val, changed = 0;
> 
>         if (linkmode_intersects(phydev->supported_eee, PHY_EEE_CAP1_FEATURES)) {
>                 val = linkmode_to_mii_eee_cap1_t(adv);
> 
> fixes the problem, both for cubieboard and xtensa.

Good point! Thx for finding it!

Do you wont to send the fix against net?
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

On 2/24/23 22:08, Oleksij Rempel wrote:
> On Fri, Feb 24, 2023 at 04:09:40PM -0800, Guenter Roeck wrote:
>> On 2/24/23 12:02, Oleksij Rempel wrote:
>> [ ... ]
>>>>
>>>> For cubieboard:
>>>>
>>>> MDIO_PCS_EEE_ABLE = 0x0000
>>>>
>>>> qemu reports attempts to access unsupported registers.
>>>>
>>>> I had a look at the Allwinner mdio driver. There is no indication suggesting
>>>> what the real hardware would return when trying to access unsupported registers,
>>>> and the Ethernet controller datasheet is not public.
>>>
>>> These are PHY accesses over MDIO bus. Ethernet controller should not
>>> care about content of this operations. But on qemu side, it is implemented as
>>> part of Ethernet controller emulation...
>>>
>>> Since MDIO_PCS_EEE_ABLE == 0x0000, phydev->supported_eee should prevent
>>> other EEE related operations. But may be actual phy_read_mmd() went
>>> wrong. It is a combination of simple phy_read/write to different
>>> registers.
>>>
>>
>> Adding MDD read/write support in qemu doesn't help. Something else in your patch
>> prevents the PHY from coming up. After reverting your patch, I see
>>
>> sun4i-emac 1c0b000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
>> IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>>
>> in the log. This is missing with your patch in place.
>>
>> Anyway, the key difference is not really the qemu emulation, but the added
>> unconditional call to genphy_c45_write_eee_adv() in your patch. If you look
>> closely into that function, you may notice that the 'changed' variable is
>> never set to 0.
>>
>> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
>> index 3813b86689d0..fee514b96ab1 100644
>> --- a/drivers/net/phy/phy-c45.c
>> +++ b/drivers/net/phy/phy-c45.c
>> @@ -672,7 +672,7 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
>>    */
>>   int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv)
>>   {
>> -       int val, changed;
>> +       int val, changed = 0;
>>
>>          if (linkmode_intersects(phydev->supported_eee, PHY_EEE_CAP1_FEATURES)) {
>>                  val = linkmode_to_mii_eee_cap1_t(adv);
>>
>> fixes the problem, both for cubieboard and xtensa.
> 
> Good point! Thx for finding it!
> 
> Do you wont to send the fix against net?

No, please go ahead and do it.

Guenter

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Andrew Lunn]

> > diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> > index f595acd0a895..67dac9f0e71d 100644
> > --- a/drivers/net/phy/phy-c45.c
> > +++ b/drivers/net/phy/phy-c45.c
> > @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev)
> >           * (Register 3.20)
> >           */
> >          val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
> > +       printk("MDIO_PCS_EEE_ABLE = 0x%04x", val);
> >          if (val < 0)
> >                  return val;
> > 
> 
> For cubieboard:
> 
> MDIO_PCS_EEE_ABLE = 0x0000
> 
> qemu reports attempts to access unsupported registers.

MDIO is a serial bus with two lines, clock driven by the bus master
and data. There is a pull up on the data line, so if the device does
not respond to a read request, you get 0xffff. That value is all i've
ever seen a real PHY do when asked to read a register which does not
exist. So i would say QEMU could be better emulate this.

The code actually looks for the value 0xffff and then decides that EEE
is not supporting in the PHY.

The value of 0x0 is probably being interpreted as meaning EEE is
supported, but none of the link modes, 10Mbps, 100Mbps etc support
EEE. I would say it is then legitimate to read/write other EEE
registers, so long as those writes take into account that no link
modes are actually supported.

Reading the other messages in this thread, a bug has been found in the
patches. But i would also say QEMU could do better.

      Andrew

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

On 2/27/23 05:08, Andrew Lunn wrote:
>>> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
>>> index f595acd0a895..67dac9f0e71d 100644
>>> --- a/drivers/net/phy/phy-c45.c
>>> +++ b/drivers/net/phy/phy-c45.c
>>> @@ -799,6 +799,7 @@ static int genphy_c45_read_eee_cap1(struct phy_device *phydev)
>>>            * (Register 3.20)
>>>            */
>>>           val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
>>> +       printk("MDIO_PCS_EEE_ABLE = 0x%04x", val);
>>>           if (val < 0)
>>>                   return val;
>>>
>>
>> For cubieboard:
>>
>> MDIO_PCS_EEE_ABLE = 0x0000
>>
>> qemu reports attempts to access unsupported registers.
> 
> MDIO is a serial bus with two lines, clock driven by the bus master
> and data. There is a pull up on the data line, so if the device does
> not respond to a read request, you get 0xffff. That value is all i've
> ever seen a real PHY do when asked to read a register which does not
> exist. So i would say QEMU could be better emulate this.
> 
> The code actually looks for the value 0xffff and then decides that EEE
> is not supporting in the PHY.
> 
> The value of 0x0 is probably being interpreted as meaning EEE is
> supported, but none of the link modes, 10Mbps, 100Mbps etc support
> EEE. I would say it is then legitimate to read/write other EEE
> registers, so long as those writes take into account that no link
> modes are actually supported.
> 
> Reading the other messages in this thread, a bug has been found in the
> patches. But i would also say QEMU could do better.
> 

Sure, it could. Always. That is why I checked the qemu code and
actually tried to implement some of the EEE handling, only to
realize that it didn't help. The emulated PHY does support EEE
and would return either 0x0001 or 0x0003 depending on the
underlying hardware. However, returning that and returning/
accepting reasonable values for other EEE registers didn't make
a difference due to the kernel bug.

Thanks,
Guenter

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

Copying regzbot.
  
#regzbot ^introduced 9b01c885be36
#regzbot title Network interface initialization failures on xtensa, arm:cubieboard
#regzbot ignore-activity

On Thu, Feb 23, 2023 at 08:16:06PM -0800, Guenter Roeck wrote:
> On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
> > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> > > 
> > > It should work as before except write operation to the EEE adv registers
> > > will be done only if some EEE abilities was detected.
> > > 
> > > If some driver will have a regression, related driver should provide own
> > > .get_features callback. See micrel.c:ksz9477_get_features() as example.
> > > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > 
> > This patch causes network interface failures with all my xtensa qemu
> > emulations. Reverting it fixes the problem. Bisect log is attached
> > for reference.
> > 
> 
> Also affected are arm:cubieboard emulations, with same symptom.
> arm:bletchley-bmc emulations crash. In both cases, reverting this patch
> fixes the problem.
> 
> Guenter

Re: [PATCH net-next v8 6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()

[Guenter Roeck]

Letting regzbot know about the fix:

#regzbot fixed-by: 972074ea8840

On Fri, Feb 24, 2023 at 09:20:04AM -0800, Guenter Roeck wrote:
> Copying regzbot.
>   
> #regzbot ^introduced 9b01c885be36
> #regzbot title Network interface initialization failures on xtensa, arm:cubieboard
> #regzbot ignore-activity
> 
> On Thu, Feb 23, 2023 at 08:16:06PM -0800, Guenter Roeck wrote:
> > On Thu, Feb 23, 2023 at 07:55:55PM -0800, Guenter Roeck wrote:
> > > On Sat, Feb 11, 2023 at 08:41:10AM +0100, Oleksij Rempel wrote:
> > > > Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().
> > > > 
> > > > It should work as before except write operation to the EEE adv registers
> > > > will be done only if some EEE abilities was detected.
> > > > 
> > > > If some driver will have a regression, related driver should provide own
> > > > .get_features callback. See micrel.c:ksz9477_get_features() as example.
> > > > 
> > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > > 
> > > This patch causes network interface failures with all my xtensa qemu
> > > emulations. Reverting it fixes the problem. Bisect log is attached
> > > for reference.
> > > 
> > 
> > Also affected are arm:cubieboard emulations, with same symptom.
> > arm:bletchley-bmc emulations crash. In both cases, reverting this patch
> > fixes the problem.
> > 
> > Guenter

[PATCH net-next v8 7/9] net: phy: c45: migrate to genphy_c45_write_eee_adv()

[Oleksij Rempel]

Migrate from genphy_config_eee_advert() to genphy_c45_write_eee_adv().

It should work as before except write operation to the EEE adv registers
will be done only if some EEE abilities was detected.

If some driver will have a regression, related driver should provide own
.get_features callback. See micrel.c:ksz9477_get_features() as example.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy-c45.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index b4910c4c21d7..ef36582adbeb 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -262,7 +262,11 @@ int genphy_c45_an_config_aneg(struct phy_device *phydev)
 	linkmode_and(phydev->advertising, phydev->advertising,
 		     phydev->supported);
 
-	changed = genphy_config_eee_advert(phydev);
+	ret = genphy_c45_write_eee_adv(phydev, phydev->supported_eee);
+	if (ret < 0)
+		return ret;
+	else if (ret)
+		changed = true;
 
 	if (genphy_c45_baset1_able(phydev))
 		return genphy_c45_baset1_an_config_aneg(phydev);
@@ -968,6 +972,11 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev)
 		}
 	}
 
+	/* This is optional functionality. If not supported, we may get an error
+	 * which should be ignored.
+	 */
+	genphy_c45_read_eee_abilities(phydev);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(genphy_c45_pma_read_abilities);
-- 
2.30.2

[PATCH net-next v8 8/9] net: phy: migrate phy_init_eee() to genphy_c45_eee_is_active()

[Oleksij Rempel]

Reduce code duplicated by migrating phy_init_eee() to
genphy_c45_eee_is_active().

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy.c | 89 +++++++------------------------------------
 1 file changed, 14 insertions(+), 75 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 41cfb24c48c1..36533746630e 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1457,30 +1457,6 @@ void phy_mac_interrupt(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_mac_interrupt);
 
-static void mmd_eee_adv_to_linkmode(unsigned long *advertising, u16 eee_adv)
-{
-	linkmode_zero(advertising);
-
-	if (eee_adv & MDIO_EEE_100TX)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
-				 advertising);
-	if (eee_adv & MDIO_EEE_1000T)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
-				 advertising);
-	if (eee_adv & MDIO_EEE_10GT)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
-				 advertising);
-	if (eee_adv & MDIO_EEE_1000KX)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
-				 advertising);
-	if (eee_adv & MDIO_EEE_10GKX4)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
-				 advertising);
-	if (eee_adv & MDIO_EEE_10GKR)
-		linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
-				 advertising);
-}
-
 /**
  * phy_init_eee - init and check the EEE feature
  * @phydev: target phy_device struct
@@ -1493,62 +1469,25 @@ static void mmd_eee_adv_to_linkmode(unsigned long *advertising, u16 eee_adv)
  */
 int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
 {
+	int ret;
+
 	if (!phydev->drv)
 		return -EIO;
 
-	/* According to 802.3az,the EEE is supported only in full duplex-mode.
-	 */
-	if (phydev->duplex == DUPLEX_FULL) {
-		__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
-		__ETHTOOL_DECLARE_LINK_MODE_MASK(lp);
-		__ETHTOOL_DECLARE_LINK_MODE_MASK(adv);
-		int eee_lp, eee_cap, eee_adv;
-		int status;
-		u32 cap;
-
-		/* Read phy status to properly get the right settings */
-		status = phy_read_status(phydev);
-		if (status)
-			return status;
-
-		/* First check if the EEE ability is supported */
-		eee_cap = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
-		if (eee_cap <= 0)
-			goto eee_exit_err;
-
-		cap = mmd_eee_cap_to_ethtool_sup_t(eee_cap);
-		if (!cap)
-			goto eee_exit_err;
-
-		/* Check which link settings negotiated and verify it in
-		 * the EEE advertising registers.
-		 */
-		eee_lp = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_LPABLE);
-		if (eee_lp <= 0)
-			goto eee_exit_err;
-
-		eee_adv = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV);
-		if (eee_adv <= 0)
-			goto eee_exit_err;
-
-		mmd_eee_adv_to_linkmode(adv, eee_adv);
-		mmd_eee_adv_to_linkmode(lp, eee_lp);
-		linkmode_and(common, adv, lp);
-
-		if (!phy_check_valid(phydev->speed, phydev->duplex, common))
-			goto eee_exit_err;
+	ret = genphy_c45_eee_is_active(phydev, NULL, NULL, NULL);
+	if (ret < 0)
+		return ret;
+	if (!ret)
+		return -EPROTONOSUPPORT;
 
-		if (clk_stop_enable)
-			/* Configure the PHY to stop receiving xMII
-			 * clock while it is signaling LPI.
-			 */
-			phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MDIO_CTRL1,
-					 MDIO_PCS_CTRL1_CLKSTOP_EN);
+	if (clk_stop_enable)
+		/* Configure the PHY to stop receiving xMII
+		 * clock while it is signaling LPI.
+		 */
+		ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MDIO_CTRL1,
+				       MDIO_PCS_CTRL1_CLKSTOP_EN);
 
-		return 0; /* EEE supported */
-	}
-eee_exit_err:
-	return -EPROTONOSUPPORT;
+	return ret < 0 ? ret : 0;
 }
 EXPORT_SYMBOL(phy_init_eee);
 
-- 
2.30.2

[PATCH net-next v8 9/9] net: phy: start using genphy_c45_ethtool_get/set_eee()

[Oleksij Rempel]

All preparations are done. Now we can start using new functions and remove
the old code.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy.c | 60 ++-----------------------------------------
 1 file changed, 2 insertions(+), 58 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 36533746630e..2f1041a7211e 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1517,33 +1517,10 @@ EXPORT_SYMBOL(phy_get_eee_err);
  */
 int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data)
 {
-	int val;
-
 	if (!phydev->drv)
 		return -EIO;
 
-	/* Get Supported EEE */
-	val = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
-	if (val < 0)
-		return val;
-	data->supported = mmd_eee_cap_to_ethtool_sup_t(val);
-
-	/* Get advertisement EEE */
-	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV);
-	if (val < 0)
-		return val;
-	data->advertised = mmd_eee_adv_to_ethtool_adv_t(val);
-	data->eee_enabled = !!data->advertised;
-
-	/* Get LP advertisement EEE */
-	val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_LPABLE);
-	if (val < 0)
-		return val;
-	data->lp_advertised = mmd_eee_adv_to_ethtool_adv_t(val);
-
-	data->eee_active = !!(data->advertised & data->lp_advertised);
-
-	return 0;
+	return genphy_c45_ethtool_get_eee(phydev, data);
 }
 EXPORT_SYMBOL(phy_ethtool_get_eee);
 
@@ -1556,43 +1533,10 @@ EXPORT_SYMBOL(phy_ethtool_get_eee);
  */
 int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
 {
-	int cap, old_adv, adv = 0, ret;
-
 	if (!phydev->drv)
 		return -EIO;
 
-	/* Get Supported EEE */
-	cap = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE);
-	if (cap < 0)
-		return cap;
-
-	old_adv = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV);
-	if (old_adv < 0)
-		return old_adv;
-
-	if (data->eee_enabled) {
-		adv = !data->advertised ? cap :
-		      ethtool_adv_to_mmd_eee_adv_t(data->advertised) & cap;
-		/* Mask prohibited EEE modes */
-		adv &= ~phydev->eee_broken_modes;
-	}
-
-	if (old_adv != adv) {
-		ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, adv);
-		if (ret < 0)
-			return ret;
-
-		/* Restart autonegotiation so the new modes get sent to the
-		 * link partner.
-		 */
-		if (phydev->autoneg == AUTONEG_ENABLE) {
-			ret = phy_restart_aneg(phydev);
-			if (ret < 0)
-				return ret;
-		}
-	}
-
-	return 0;
+	return genphy_c45_ethtool_set_eee(phydev, data);
 }
 EXPORT_SYMBOL(phy_ethtool_set_eee);
 
-- 
2.30.2

Re: [PATCH net-next v8 0/9] net: add EEE support for KSZ9477 switch family

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Sat, 11 Feb 2023 08:41:04 +0100 you wrote:
> changes v8:
> - fix comment for linkmode_to_mii_eee_cap1_t() function
> - add Acked-by: Arun Ramadoss <arun.ramadoss@microchip.com>
> - add Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
> 
> changes v7:
> - update documentation for genphy_c45_eee_is_active()
> - address review comments on "net: dsa: microchip: enable EEE support"
>   patch
> 
> [...]

Here is the summary with links:
  - [net-next,v8,1/9] net: dsa: microchip: enable EEE support
    https://git.kernel.org/netdev/net-next/c/69d3b36ca045
  - [net-next,v8,2/9] net: phy: add genphy_c45_read_eee_abilities() function
    https://git.kernel.org/netdev/net-next/c/14e47d1fb8f9
  - [net-next,v8,3/9] net: phy: micrel: add ksz9477_get_features()
    https://git.kernel.org/netdev/net-next/c/48fb19940f2b
  - [net-next,v8,4/9] net: phy: export phy_check_valid() function
    https://git.kernel.org/netdev/net-next/c/cf9f60796968
  - [net-next,v8,5/9] net: phy: add genphy_c45_ethtool_get/set_eee() support
    https://git.kernel.org/netdev/net-next/c/022c3f87f88e
  - [net-next,v8,6/9] net: phy: c22: migrate to genphy_c45_write_eee_adv()
    https://git.kernel.org/netdev/net-next/c/9b01c885be36
  - [net-next,v8,7/9] net: phy: c45: migrate to genphy_c45_write_eee_adv()
    https://git.kernel.org/netdev/net-next/c/5827b168125d
  - [net-next,v8,8/9] net: phy: migrate phy_init_eee() to genphy_c45_eee_is_active()
    https://git.kernel.org/netdev/net-next/c/6340f9fd43d5
  - [net-next,v8,9/9] net: phy: start using genphy_c45_ethtool_get/set_eee()
    https://git.kernel.org/netdev/net-next/c/8b68710a3121

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html