All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6
@ 2023-01-30  8:06 Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 01/15] net: dsa: microchip: enable EEE support Oleksij Rempel
                   ` (14 more replies)
  0 siblings, 15 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:06 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

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 (15):
  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: start using genphy_c45_ethtool_get/set_eee()
  net: phy: add driver specific get/set_eee support
  net: phy: at803x: implement ethtool access to SmartEEE functionality
  net: phy: at803x: ar8035: fix EEE support for half duplex links
  net: phy: add PHY specifica flag to signal SmartEEE support
  net: phy: at803x: add PHY_SMART_EEE flag to AR8035
  net: phy: add phy_has_smarteee() helper
  net: fec: add support for PHYs with SmartEEE support

 drivers/net/dsa/microchip/ksz_common.c    |  66 ++++++
 drivers/net/ethernet/freescale/fec_main.c |  22 +-
 drivers/net/phy/at803x.c                  | 142 +++++++++++-
 drivers/net/phy/micrel.c                  |  21 ++
 drivers/net/phy/phy-c45.c                 | 255 +++++++++++++++++++++-
 drivers/net/phy/phy.c                     |  66 +-----
 drivers/net/phy/phy_device.c              |  26 ++-
 include/linux/mdio.h                      |  53 +++++
 include/linux/phy.h                       |  26 +++
 include/uapi/linux/mdio.h                 |   8 +
 10 files changed, 615 insertions(+), 70 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 01/15] net: dsa: microchip: enable EEE support
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30 17:59   ` Vladimir Oltean
  2023-01-30  8:07 ` [PATCH net-next v3 02/15] net: phy: add genphy_c45_read_eee_abilities() function Oleksij Rempel
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

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.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 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 5e1e5bd555d2..bef36e2fa257 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2645,6 +2645,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 testes
+	 * 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)
 {
@@ -3006,6 +3070,8 @@ static const struct dsa_switch_ops ksz_switch_ops = {
 	.port_hwtstamp_set	= ksz_hwtstamp_set,
 	.port_txtstamp		= ksz_port_txtstamp,
 	.port_rxtstamp		= ksz_port_rxtstamp,
+	.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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 02/15] net: phy: add genphy_c45_read_eee_abilities() function
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 01/15] net: dsa: microchip: enable EEE support Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30 21:55   ` Andrew Lunn
  2023-01-30  8:07 ` [PATCH net-next v3 03/15] net: phy: micrel: add ksz9477_get_features() Oleksij Rempel
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

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 this
registers, we should detect link mode abilities first and then based on
this abilities doing EEE link modes detection.

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

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy-c45.c    | 49 ++++++++++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c | 16 ++++++++++++
 include/linux/mdio.h         | 17 +++++++++++++
 include/linux/phy.h          |  5 ++++
 4 files changed, 87 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index cff83220595c..03af19796848 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -661,6 +661,55 @@ int genphy_c45_read_mdix(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
 
+/**
+ * genphy_c45_read_eee_abilities - read supported EEE link modes
+ * @phydev: target phy_device struct
+ *
+ * Read supported EEE link modes.
+ */
+int genphy_c45_read_eee_abilities(struct phy_device *phydev)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	int val;
+
+	linkmode_and(common, phydev->supported, PHY_EEE_100_10000_FEATURES);
+	/* There is not indicator if optional register
+	 * "EEE control and capability 1" (3.20) is supported. Read it only
+	 * on devices with appropriate linkmodes.
+	 */
+	if (!linkmode_empty(common)) {
+		/* 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;
+
+		mii_eee_100_10000_adv_mod_linkmode_t(phydev->supported_eee, val);
+
+		/* Some buggy devices claim not supported EEE link modes */
+		linkmode_and(phydev->supported_eee, phydev->supported_eee,
+			     phydev->supported);
+	}
+
+	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 0d371a0a49f2..698549ef2981 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_100_10000_features_array[6] = {
+	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_100_10000_features) __ro_after_init;
+EXPORT_SYMBOL_GPL(phy_eee_100_10000_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_100_10000_features_array,
+			       ARRAY_SIZE(phy_eee_100_10000_features_array),
+			       phy_eee_100_10000_features);
+
 }
 
 void phy_device_free(struct phy_device *phydev)
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 220f3ca8702d..ea4f7d08d1a6 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -410,6 +410,23 @@ static inline u32 linkmode_adv_to_mii_t1_adv_m_t(unsigned long *advertising)
 	return result;
 }
 
+static inline void mii_eee_100_10000_adv_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 b3cf1e08e880..42a4a5d66472 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_100_10000_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_100_10000_FEATURES ((unsigned long *)&phy_eee_100_10000_features)
 
 extern const int phy_basic_ports_array[3];
 extern const int phy_fibre_port_array[1];
@@ -684,6 +686,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);
@@ -1745,6 +1749,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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 03/15] net: phy: micrel: add ksz9477_get_features()
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 01/15] net: dsa: microchip: enable EEE support Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 02/15] net: phy: add genphy_c45_read_eee_abilities() function Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 04/15] net: phy: export phy_check_valid() function Oleksij Rempel
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

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>
---
 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 d5b80c31ab91..767c4111cb18 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1370,6 +1370,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_100_10000_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)
@@ -3422,6 +3442,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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 04/15] net: phy: export phy_check_valid() function
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (2 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 03/15] net: phy: micrel: add ksz9477_get_features() Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 05/15] net: phy: add genphy_c45_ethtool_get/set_eee() support Oleksij Rempel
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

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

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 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 42a4a5d66472..950e5ee0d8f2 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1626,6 +1626,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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 05/15] net: phy: add genphy_c45_ethtool_get/set_eee() support
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (3 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 04/15] net: phy: export phy_check_valid() function Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30 22:06   ` Andrew Lunn
  2023-01-30  8:07 ` [PATCH net-next v3 06/15] net: phy: c22: migrate to genphy_c45_write_eee_adv() Oleksij Rempel
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

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 | 195 ++++++++++++++++++++++++++++++++++++++
 include/linux/mdio.h      |  36 +++++++
 include/linux/phy.h       |   5 +
 include/uapi/linux/mdio.h |   8 ++
 4 files changed, 244 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 03af19796848..d89627ee0ec8 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -661,6 +661,127 @@ int genphy_c45_read_mdix(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_read_mdix);
 
+/**
+ * genphy_c45_write_eee_adv - read advertised EEE link modes
+ * @phydev: target phy_device struct
+ */
+int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	int val, changed;
+
+	linkmode_and(common, phydev->supported_eee, PHY_EEE_100_10000_FEATURES);
+	if (!linkmode_empty(common)) {
+		val = linkmode_adv_to_mii_eee_100_10000_adv_t(adv);
+
+		/* In eee_broken_modes are stored MDIO_AN_EEE_ADV specific raw
+		 * register values.
+		 */
+		val &= ~phydev->eee_broken_modes;
+
+		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);
+
+		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
+ */
+static int genphy_c45_read_eee_adv(struct phy_device *phydev,
+				   unsigned long *adv)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	int val;
+
+	linkmode_and(common, phydev->supported_eee, PHY_EEE_100_10000_FEATURES);
+	if (!linkmode_empty(common)) {
+		/* 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_100_10000_adv_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
+ */
+static int genphy_c45_read_eee_lpa(struct phy_device *phydev,
+				   unsigned long *lpa)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	int val;
+
+	linkmode_and(common, phydev->supported_eee, PHY_EEE_100_10000_FEATURES);
+	if (!linkmode_empty(common)) {
+		/* 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_100_10000_adv_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_abilities - read supported EEE link modes
  * @phydev: target phy_device struct
@@ -1173,6 +1294,80 @@ int genphy_c45_plca_get_status(struct phy_device *phydev,
 }
 EXPORT_SYMBOL_GPL(genphy_c45_plca_get_status);
 
+/**
+ * genphy_c45_ethtool_get_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_get_eee(struct phy_device *phydev,
+			       struct ethtool_eee *data)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(adv) = {};
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(lp) = {};
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	int ret;
+
+	ret = genphy_c45_read_eee_adv(phydev, adv);
+	if (ret)
+		return ret;
+
+	ret = genphy_c45_read_eee_lpa(phydev, lp);
+	if (ret)
+		return ret;
+
+	data->eee_enabled = !linkmode_empty(adv);
+	linkmode_and(common, adv, lp);
+	if (data->eee_enabled && !linkmode_empty(common))
+		data->eee_active = phy_check_valid(phydev->speed,
+						   phydev->duplex, common);
+	else
+		data->eee_active = false;
+
+	/* FIXME: EEE ethtool interface currently do not support full set of
+	 * possible EEE link modes.
+	 */
+	data->supported = phydev->supported_eee[0];
+	data->advertised = adv[0];
+	data->lp_advertised = lp[0];
+
+	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 ea4f7d08d1a6..231cf02671a7 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -427,6 +427,42 @@ static inline void mii_eee_100_10000_adv_mod_linkmode_t(unsigned long *adv,
 			 adv, val & MDIO_EEE_10GKR);
 }
 
+static inline u32 linkmode_adv_to_mii_eee_100_10000_adv_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;
+}
+
+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);
+}
+
+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 950e5ee0d8f2..d7eacb150eac 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1765,6 +1765,11 @@ 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_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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 06/15] net: phy: c22: migrate to genphy_c45_write_eee_adv()
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (4 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 05/15] net: phy: add genphy_c45_ethtool_get/set_eee() support Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 07/15] net: phy: c45: " Oleksij Rempel
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

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>
---
 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 698549ef2981..abe6124eb9dc 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 07/15] net: phy: c45: migrate to genphy_c45_write_eee_adv()
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (5 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 06/15] net: phy: c22: migrate to genphy_c45_write_eee_adv() Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 08/15] net: phy: start using genphy_c45_ethtool_get/set_eee() Oleksij Rempel
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

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>
---
 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 d89627ee0ec8..3a990e0a8d51 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);
@@ -945,6 +949,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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 08/15] net: phy: start using genphy_c45_ethtool_get/set_eee()
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (6 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 07/15] net: phy: c45: " Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 09/15] net: phy: add driver specific get/set_eee support Oleksij Rempel
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

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>
---
 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 41cfb24c48c1..54b62a604238 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1578,33 +1578,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);
 
@@ -1617,43 +1594,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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 09/15] net: phy: add driver specific get/set_eee support
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (7 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 08/15] net: phy: start using genphy_c45_ethtool_get/set_eee() Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 10/15] net: phy: at803x: implement ethtool access to SmartEEE functionality Oleksij Rempel
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

Not all PHYs can be handled by generic phy_ethtool_get/set_eee()
functions. So, add driver specific get/set_eee support.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy.c | 6 ++++++
 include/linux/phy.h   | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 54b62a604238..1e0e81510f10 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1581,6 +1581,9 @@ int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data)
 	if (!phydev->drv)
 		return -EIO;
 
+	if (phydev->drv->get_eee)
+		return phydev->drv->get_eee(phydev, data);
+
 	return genphy_c45_ethtool_get_eee(phydev, data);
 }
 EXPORT_SYMBOL(phy_ethtool_get_eee);
@@ -1597,6 +1600,9 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
 	if (!phydev->drv)
 		return -EIO;
 
+	if (phydev->drv->set_eee)
+		return phydev->drv->set_eee(phydev, data);
+
 	return genphy_c45_ethtool_set_eee(phydev, data);
 }
 EXPORT_SYMBOL(phy_ethtool_set_eee);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index d7eacb150eac..f6619f4aaa4b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1059,6 +1059,11 @@ struct phy_driver {
 	/** @get_plca_status: Return the current PLCA status info */
 	int (*get_plca_status)(struct phy_device *dev,
 			       struct phy_plca_status *plca_st);
+
+	/** @get_eee: Return the current EEE configuration */
+	int (*get_eee)(struct phy_device *phydev, struct ethtool_eee *e);
+	/** @set_eee: Set the EEE configuration */
+	int (*set_eee)(struct phy_device *phydev, struct ethtool_eee *e);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),		\
 				      struct phy_driver, mdiodrv)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 10/15] net: phy: at803x: implement ethtool access to SmartEEE functionality
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (8 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 09/15] net: phy: add driver specific get/set_eee support Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 11/15] net: phy: at803x: ar8035: fix EEE support for half duplex links Oleksij Rempel
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

If AR8035 PHY is used with a MAC without EEE support
(iMX6, etc), then we need to process ethtool_eee::tx_lpi_timer and
tx_lpi_enabled by the PHY driver. So, add get/set_eee support for this
functionality.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/at803x.c | 109 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 22f4458274aa..9eb4439b0afc 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -166,8 +166,18 @@
 
 #define AT803X_MMD3_SMARTEEE_CTL1		0x805b
 #define AT803X_MMD3_SMARTEEE_CTL2		0x805c
+#define AT803X_MMD3_SMARTEEE_LPI_TIME_LOW	GENMASK(15, 0)
+#define AT803X_MMD3_SMARTEEE_LPI_TIME_15_0	GENMASK(15, 0)
 #define AT803X_MMD3_SMARTEEE_CTL3		0x805d
 #define AT803X_MMD3_SMARTEEE_CTL3_LPI_EN	BIT(8)
+#define AT803X_MMD3_SMARTEEE_LPI_TIME_HIGH	GENMASK(7, 0)
+#define AT803X_MMD3_SMARTEEE_LPI_TIME_23_16	GENMASK(23, 16)
+/* Tx LPI timer resolution */
+#define AT803X_MMD3_SMARTEEE_LPI_TIME_RESOL_NS	163840
+#define AT803X_MMD3_SMARTEEE_LPI_TIME_MAX_US	\
+	((GENMASK(23, 0) * AT803X_MMD3_SMARTEEE_LPI_TIME_RESOL_NS) / \
+	       NSEC_PER_USEC)
+#define AT803X_MMD3_SMARTEEE_LPI_TIME_DEF_US	335544
 
 #define ATH9331_PHY_ID				0x004dd041
 #define ATH8030_PHY_ID				0x004dd076
@@ -951,17 +961,26 @@ static int at803x_get_features(struct phy_device *phydev)
 	return 0;
 }
 
-static int at803x_smarteee_config(struct phy_device *phydev)
+static int at803x_smarteee_config(struct phy_device *phydev, bool enable,
+				  u32 tx_lpi_timer_us)
 {
 	struct at803x_priv *priv = phydev->priv;
+	u64 tx_lpi_timer_raw;
+	u64 tx_lpi_timer_ns;
 	u16 mask = 0, val = 0;
 	int ret;
 
-	if (priv->flags & AT803X_DISABLE_SMARTEEE)
+	if (priv->flags & AT803X_DISABLE_SMARTEEE || !enable)
 		return phy_modify_mmd(phydev, MDIO_MMD_PCS,
 				      AT803X_MMD3_SMARTEEE_CTL3,
 				      AT803X_MMD3_SMARTEEE_CTL3_LPI_EN, 0);
 
+	if (tx_lpi_timer_us > AT803X_MMD3_SMARTEEE_LPI_TIME_MAX_US) {
+		phydev_err(phydev, "Max LPI timer is %lu microsecs\n",
+			   AT803X_MMD3_SMARTEEE_LPI_TIME_MAX_US);
+		return -EINVAL;
+	}
+
 	if (priv->smarteee_lpi_tw_1g) {
 		mask |= 0xff00;
 		val |= priv->smarteee_lpi_tw_1g << 8;
@@ -978,9 +997,27 @@ static int at803x_smarteee_config(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
+	tx_lpi_timer_ns = tx_lpi_timer_us * NSEC_PER_USEC;
+	tx_lpi_timer_raw =
+		DIV_ROUND_CLOSEST_ULL(tx_lpi_timer_ns,
+				      AT803X_MMD3_SMARTEEE_LPI_TIME_RESOL_NS);
+	val = FIELD_PREP(AT803X_MMD3_SMARTEEE_LPI_TIME_LOW,
+			 FIELD_GET(AT803X_MMD3_SMARTEEE_LPI_TIME_15_0,
+				   tx_lpi_timer_raw));
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_PCS, AT803X_MMD3_SMARTEEE_CTL2,
+			    val);
+	if (ret)
+		return ret;
+
+	val = AT803X_MMD3_SMARTEEE_CTL3_LPI_EN |
+		FIELD_PREP(AT803X_MMD3_SMARTEEE_LPI_TIME_HIGH,
+			   FIELD_GET(AT803X_MMD3_SMARTEEE_LPI_TIME_23_16,
+				     tx_lpi_timer_raw));
+
 	return phy_modify_mmd(phydev, MDIO_MMD_PCS, AT803X_MMD3_SMARTEEE_CTL3,
-			      AT803X_MMD3_SMARTEEE_CTL3_LPI_EN,
-			      AT803X_MMD3_SMARTEEE_CTL3_LPI_EN);
+			      AT803X_MMD3_SMARTEEE_CTL3_LPI_EN |
+			      AT803X_MMD3_SMARTEEE_LPI_TIME_HIGH, val);
 }
 
 static int at803x_clk_out_config(struct phy_device *phydev)
@@ -1067,7 +1104,8 @@ static int at803x_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	ret = at803x_smarteee_config(phydev);
+	ret = at803x_smarteee_config(phydev, true,
+				     AT803X_MMD3_SMARTEEE_LPI_TIME_DEF_US);
 	if (ret < 0)
 		return ret;
 
@@ -1612,6 +1650,65 @@ static int at803x_cable_test_start(struct phy_device *phydev)
 	return 0;
 }
 
+static int at803x_get_eee(struct phy_device *phydev, struct ethtool_eee *data)
+{
+	struct at803x_priv *priv = phydev->priv;
+	u32 tx_timer_raw;
+	u64 tx_timer_ns;
+	int ret;
+
+	/* If SmartEEE is not enabled, it is expected that tx_lpi_* fields
+	 * are processed by the MAC driver.
+	 */
+	if (priv->flags & AT803X_DISABLE_SMARTEEE)
+		return genphy_c45_ethtool_get_eee(phydev, data);
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+			   AT803X_MMD3_SMARTEEE_CTL2);
+	tx_timer_raw = FIELD_PREP(AT803X_MMD3_SMARTEEE_LPI_TIME_15_0,
+				  FIELD_GET(AT803X_MMD3_SMARTEEE_LPI_TIME_LOW,
+					    ret));
+	if (ret < 0)
+		return ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_PCS,
+			   AT803X_MMD3_SMARTEEE_CTL3);
+	if (ret < 0)
+		return ret;
+
+	tx_timer_raw |= FIELD_PREP(AT803X_MMD3_SMARTEEE_LPI_TIME_23_16,
+				   FIELD_GET(AT803X_MMD3_SMARTEEE_LPI_TIME_HIGH,
+					     ret));
+	tx_timer_ns = tx_timer_raw * AT803X_MMD3_SMARTEEE_LPI_TIME_RESOL_NS;
+	data->tx_lpi_timer = DIV_ROUND_CLOSEST_ULL(tx_timer_ns, NSEC_PER_USEC);
+
+	data->tx_lpi_enabled = !!(ret & AT803X_MMD3_SMARTEEE_CTL3_LPI_EN);
+
+	return genphy_c45_ethtool_get_eee(phydev, data);
+}
+
+static int at803x_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
+{
+	struct at803x_priv *priv = phydev->priv;
+	int ret;
+
+	/* If SmartEEE is not enabled, it is expected that tx_lpi_* fields
+	 * are processed by the MAC driver.
+	 */
+	if (priv->flags & AT803X_DISABLE_SMARTEEE)
+		return genphy_c45_ethtool_set_eee(phydev, data);
+
+	/* Changing Tx LPI on/off or Tx LPI timer settings
+	 * do not require link reset.
+	 */
+	ret = at803x_smarteee_config(phydev, data->tx_lpi_enabled,
+				     data->tx_lpi_timer);
+	if (ret)
+		return ret;
+
+	return genphy_c45_ethtool_set_eee(phydev, data);
+}
+
 static int qca83xx_config_init(struct phy_device *phydev)
 {
 	u8 switch_revision;
@@ -2038,6 +2135,8 @@ static struct phy_driver at803x_driver[] = {
 	.set_tunable		= at803x_set_tunable,
 	.cable_test_start	= at803x_cable_test_start,
 	.cable_test_get_status	= at803x_cable_test_get_status,
+	.get_eee		= at803x_get_eee,
+	.set_eee		= at803x_set_eee,
 }, {
 	/* Qualcomm Atheros AR8030 */
 	.phy_id			= ATH8030_PHY_ID,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 11/15] net: phy: at803x: ar8035: fix EEE support for half duplex links
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (9 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 10/15] net: phy: at803x: implement ethtool access to SmartEEE functionality Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 12/15] net: phy: add PHY specifica flag to signal SmartEEE support Oleksij Rempel
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

If AR8035 is running with enabled EEE and LPI, it will not be able to
establish an 100BaseTX/Half or 1000BaseT/Half link. Similar issue we
will have with 100BaseTX/Full and LPI TX timer configured to less then
80msec.

To avoid this issue, we need to keep LPI disabled before link is
establish and enable it only we detected supported link configuration.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/at803x.c | 41 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 9eb4439b0afc..5ab43eb63581 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -313,6 +313,7 @@ struct at803x_priv {
 	u8 smarteee_lpi_tw_100m;
 	bool is_fiber;
 	bool is_1000basex;
+	bool tx_lpi_on;
 	struct regulator_dev *vddio_rdev;
 	struct regulator_dev *vddh_rdev;
 	struct regulator *vddio;
@@ -970,6 +971,8 @@ static int at803x_smarteee_config(struct phy_device *phydev, bool enable,
 	u16 mask = 0, val = 0;
 	int ret;
 
+	priv->tx_lpi_on = enable;
+
 	if (priv->flags & AT803X_DISABLE_SMARTEEE || !enable)
 		return phy_modify_mmd(phydev, MDIO_MMD_PCS,
 				      AT803X_MMD3_SMARTEEE_CTL3,
@@ -1010,10 +1013,15 @@ static int at803x_smarteee_config(struct phy_device *phydev, bool enable,
 	if (ret)
 		return ret;
 
-	val = AT803X_MMD3_SMARTEEE_CTL3_LPI_EN |
-		FIELD_PREP(AT803X_MMD3_SMARTEEE_LPI_TIME_HIGH,
-			   FIELD_GET(AT803X_MMD3_SMARTEEE_LPI_TIME_23_16,
-				     tx_lpi_timer_raw));
+	val = FIELD_PREP(AT803X_MMD3_SMARTEEE_LPI_TIME_HIGH,
+			 FIELD_GET(AT803X_MMD3_SMARTEEE_LPI_TIME_23_16,
+				   tx_lpi_timer_raw));
+
+	if (phydev->state == PHY_RUNNING &&
+	    phy_check_valid(phydev->speed, phydev->duplex,
+			    phydev->supported_eee)) {
+		val |= AT803X_MMD3_SMARTEEE_CTL3_LPI_EN;
+	}
 
 	return phy_modify_mmd(phydev, MDIO_MMD_PCS, AT803X_MMD3_SMARTEEE_CTL3,
 			      AT803X_MMD3_SMARTEEE_CTL3_LPI_EN |
@@ -1682,7 +1690,7 @@ static int at803x_get_eee(struct phy_device *phydev, struct ethtool_eee *data)
 	tx_timer_ns = tx_timer_raw * AT803X_MMD3_SMARTEEE_LPI_TIME_RESOL_NS;
 	data->tx_lpi_timer = DIV_ROUND_CLOSEST_ULL(tx_timer_ns, NSEC_PER_USEC);
 
-	data->tx_lpi_enabled = !!(ret & AT803X_MMD3_SMARTEEE_CTL3_LPI_EN);
+	data->tx_lpi_enabled = priv->tx_lpi_on;
 
 	return genphy_c45_ethtool_get_eee(phydev, data);
 }
@@ -1709,6 +1717,28 @@ static int at803x_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
 	return genphy_c45_ethtool_set_eee(phydev, data);
 }
 
+static void at8035_link_change_notify(struct phy_device *phydev)
+{
+	struct at803x_priv *priv = phydev->priv;
+
+	if (priv->flags & AT803X_DISABLE_SMARTEEE)
+		return;
+
+	if (phydev->state == PHY_RUNNING) {
+		if (priv->tx_lpi_on && phy_check_valid(phydev->speed,
+						       phydev->duplex,
+						       phydev->supported_eee))
+			phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
+					 AT803X_MMD3_SMARTEEE_CTL3,
+					 AT803X_MMD3_SMARTEEE_CTL3_LPI_EN);
+	} else {
+		if (priv->tx_lpi_on)
+			phy_clear_bits_mmd(phydev, MDIO_MMD_PCS,
+					   AT803X_MMD3_SMARTEEE_CTL3,
+					   AT803X_MMD3_SMARTEEE_CTL3_LPI_EN);
+	}
+}
+
 static int qca83xx_config_init(struct phy_device *phydev)
 {
 	u8 switch_revision;
@@ -2137,6 +2167,7 @@ static struct phy_driver at803x_driver[] = {
 	.cable_test_get_status	= at803x_cable_test_get_status,
 	.get_eee		= at803x_get_eee,
 	.set_eee		= at803x_set_eee,
+	.link_change_notify	= at8035_link_change_notify,
 }, {
 	/* Qualcomm Atheros AR8030 */
 	.phy_id			= ATH8030_PHY_ID,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 12/15] net: phy: add PHY specifica flag to signal SmartEEE support
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (10 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 11/15] net: phy: at803x: ar8035: fix EEE support for half duplex links Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 13/15] net: phy: at803x: add PHY_SMART_EEE flag to AR8035 Oleksij Rempel
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

Typical EEE support need cooperation of MAC and PHY, so both parts should
be able to do EEE. But, there also PHYs compatible with normal 802.3az
standard working with legacy MAC without EEE ability, acting as a complete
EEE power saving system.

To identify this PHYs we need a PHY specific flag. Since the PHY
specification implementing this functionality calls it SmartEEE, use
the same flag name - PHY_SMART_EEE.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/linux/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index f6619f4aaa4b..90d7b90fa1f4 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -85,6 +85,7 @@ extern const int phy_10gbit_features_array[1];
 #define PHY_IS_INTERNAL		0x00000001
 #define PHY_RST_AFTER_CLK_EN	0x00000002
 #define PHY_POLL_CABLE_TEST	0x00000004
+#define PHY_SMART_EEE		0x00000008 /* EEE done by PHY without MAC */
 #define MDIO_DEVICE_IS_PHY	0x80000000
 
 /**
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 13/15] net: phy: at803x: add PHY_SMART_EEE flag to AR8035
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (11 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 12/15] net: phy: add PHY specifica flag to signal SmartEEE support Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 14/15] net: phy: add phy_has_smarteee() helper Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 15/15] net: fec: add support for PHYs with SmartEEE support Oleksij Rempel
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

AR8035 is one of the PHYs with SmartEEE functionality. This flag will be
used by one of next patches on the i.MX FEC driver.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/at803x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 5ab43eb63581..94dbec0a992c 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -2147,7 +2147,7 @@ static struct phy_driver at803x_driver[] = {
 	/* Qualcomm Atheros AR8035 */
 	PHY_ID_MATCH_EXACT(ATH8035_PHY_ID),
 	.name			= "Qualcomm Atheros AR8035",
-	.flags			= PHY_POLL_CABLE_TEST,
+	.flags			= PHY_POLL_CABLE_TEST | PHY_SMART_EEE,
 	.probe			= at803x_probe,
 	.remove			= at803x_remove,
 	.config_aneg		= at803x_config_aneg,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 14/15] net: phy: add phy_has_smarteee() helper
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (12 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 13/15] net: phy: at803x: add PHY_SMART_EEE flag to AR8035 Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-30  8:07 ` [PATCH net-next v3 15/15] net: fec: add support for PHYs with SmartEEE support Oleksij Rempel
  14 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

Add helper to identify PHYs with SmartEEE support.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/linux/phy.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 90d7b90fa1f4..9891f384de30 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1415,6 +1415,15 @@ static inline bool phy_polling_mode(struct phy_device *phydev)
 	return phydev->irq == PHY_POLL;
 }
 
+/**
+ * phy_has_rxtstamp - Tests whether a PHY supports SmartEEE.
+ * @phydev: the phy_device struct
+ */
+static inline bool phy_has_smarteee(struct phy_device *phydev)
+{
+	return phydev && phydev->drv && !!(phydev->drv->flags & PHY_SMART_EEE);
+}
+
 /**
  * phy_has_hwtstamp - Tests whether a PHY time stamp configuration.
  * @phydev: the phy_device struct
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH net-next v3 15/15] net: fec: add support for PHYs with SmartEEE support
  2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
                   ` (13 preceding siblings ...)
  2023-01-30  8:07 ` [PATCH net-next v3 14/15] net: phy: add phy_has_smarteee() helper Oleksij Rempel
@ 2023-01-30  8:07 ` Oleksij Rempel
  2023-01-31 20:52   ` Vladimir Oltean
  14 siblings, 1 reply; 21+ messages in thread
From: Oleksij Rempel @ 2023-01-30  8:07 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, Arun.Ramadoss

Ethernet controller in i.MX6*/i.MX7* series do not provide EEE support.
But this chips are used sometimes in combinations with SmartEEE capable
PHYs.
So, instead of aborting get/set_eee access on MACs without EEE support,
ask PHY if it is able to do the EEE job by using SmartEEE.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/ethernet/freescale/fec_main.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e6238e53940d..25a2a9d860de 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3102,8 +3102,15 @@ fec_enet_get_eee(struct net_device *ndev, struct ethtool_eee *edata)
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	struct ethtool_eee *p = &fep->eee;
 
-	if (!(fep->quirks & FEC_QUIRK_HAS_EEE))
-		return -EOPNOTSUPP;
+	if (!(fep->quirks & FEC_QUIRK_HAS_EEE)) {
+		if (!netif_running(ndev))
+			return -ENETDOWN;
+
+		if (!phy_has_smarteee(ndev->phydev))
+			return -EOPNOTSUPP;
+
+		return phy_ethtool_get_eee(ndev->phydev, edata);
+	}
 
 	if (!netif_running(ndev))
 		return -ENETDOWN;
@@ -3123,8 +3130,15 @@ fec_enet_set_eee(struct net_device *ndev, struct ethtool_eee *edata)
 	struct ethtool_eee *p = &fep->eee;
 	int ret = 0;
 
-	if (!(fep->quirks & FEC_QUIRK_HAS_EEE))
-		return -EOPNOTSUPP;
+	if (!(fep->quirks & FEC_QUIRK_HAS_EEE)) {
+		if (!netif_running(ndev))
+			return -ENETDOWN;
+
+		if (!phy_has_smarteee(ndev->phydev))
+			return -EOPNOTSUPP;
+
+		return phy_ethtool_set_eee(ndev->phydev, edata);
+	}
 
 	if (!netif_running(ndev))
 		return -ENETDOWN;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v3 01/15] net: dsa: microchip: enable EEE support
  2023-01-30  8:07 ` [PATCH net-next v3 01/15] net: dsa: microchip: enable EEE support Oleksij Rempel
@ 2023-01-30 17:59   ` Vladimir Oltean
  0 siblings, 0 replies; 21+ messages in thread
From: Vladimir Oltean @ 2023-01-30 17:59 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, kernel, linux-kernel, netdev, Arun.Ramadoss

On Mon, Jan 30, 2023 at 09:07:00AM +0100, Oleksij Rempel wrote:
> @@ -3006,6 +3070,8 @@ static const struct dsa_switch_ops ksz_switch_ops = {
>  	.port_hwtstamp_set	= ksz_hwtstamp_set,
>  	.port_txtstamp		= ksz_port_txtstamp,
>  	.port_rxtstamp		= ksz_port_rxtstamp,
> +	.get_mac_eee		= ksz_get_mac_eee,
> +	.set_mac_eee		= ksz_set_mac_eee,
>  };

<<<<<<< head
	.port_setup_tc		= ksz_setup_tc,
=======
	.get_mac_eee		= ksz_get_mac_eee,
	.set_mac_eee		= ksz_set_mac_eee,
>>>>>>> net: dsa: microchip: enable eee support

with commit 71d7920fb2d1 ("net: dsa: microchip: add support for credit
based shaper").

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v3 02/15] net: phy: add genphy_c45_read_eee_abilities() function
  2023-01-30  8:07 ` [PATCH net-next v3 02/15] net: phy: add genphy_c45_read_eee_abilities() function Oleksij Rempel
@ 2023-01-30 21:55   ` Andrew Lunn
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2023-01-30 21:55 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Woojung Huh, UNGLinuxDriver, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, kernel, linux-kernel, netdev, Arun.Ramadoss

> +static inline void mii_eee_100_10000_adv_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);
> +}

This and mmd_eee_cap_to_ethtool_sup_t() are very similar. Could you
try to remove the duplication. Maybe
ethtool_convert_link_mode_to_legacy_u32() could be used?

	Andrew					  

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v3 05/15] net: phy: add genphy_c45_ethtool_get/set_eee() support
  2023-01-30  8:07 ` [PATCH net-next v3 05/15] net: phy: add genphy_c45_ethtool_get/set_eee() support Oleksij Rempel
@ 2023-01-30 22:06   ` Andrew Lunn
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Lunn @ 2023-01-30 22:06 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Woojung Huh, UNGLinuxDriver, Vivien Didelot, Florian Fainelli,
	Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, kernel, linux-kernel, netdev, Arun.Ramadoss

> +/**
> + * genphy_c45_write_eee_adv - read advertised EEE link modes

s/read/write

> + * @phydev: target phy_device struct
> + */
> +int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
> +	int val, changed;
> +
> +	linkmode_and(common, phydev->supported_eee, PHY_EEE_100_10000_FEATURES);
> +	if (!linkmode_empty(common)) {
> +		val = linkmode_adv_to_mii_eee_100_10000_adv_t(adv);
> +
> +		/* In eee_broken_modes are stored MDIO_AN_EEE_ADV specific raw
> +		 * register values.
> +		 */
> +		val &= ~phydev->eee_broken_modes;
> +
> +		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);
> +
> +		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
> + */
> +static int genphy_c45_read_eee_adv(struct phy_device *phydev,
> +				   unsigned long *adv)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
> +	int val;
> +
> +	linkmode_and(common, phydev->supported_eee, PHY_EEE_100_10000_FEATURES);
> +	if (!linkmode_empty(common)) {
> +		/* 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_100_10000_adv_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
> + */
> +static int genphy_c45_read_eee_lpa(struct phy_device *phydev,
> +				   unsigned long *lpa)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
> +	int val;
> +
> +	linkmode_and(common, phydev->supported_eee, PHY_EEE_100_10000_FEATURES);
> +	if (!linkmode_empty(common)) {
> +		/* 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_100_10000_adv_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_abilities - read supported EEE link modes
>   * @phydev: target phy_device struct
> @@ -1173,6 +1294,80 @@ int genphy_c45_plca_get_status(struct phy_device *phydev,
>  }
>  EXPORT_SYMBOL_GPL(genphy_c45_plca_get_status);
>  
> +/**
> + * genphy_c45_ethtool_get_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_get_eee(struct phy_device *phydev,
> +			       struct ethtool_eee *data)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(adv) = {};
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(lp) = {};
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
> +	int ret;
> +
> +	ret = genphy_c45_read_eee_adv(phydev, adv);
> +	if (ret)
> +		return ret;
> +
> +	ret = genphy_c45_read_eee_lpa(phydev, lp);
> +	if (ret)
> +		return ret;
> +
> +	data->eee_enabled = !linkmode_empty(adv);
> +	linkmode_and(common, adv, lp);
> +	if (data->eee_enabled && !linkmode_empty(common))
> +		data->eee_active = phy_check_valid(phydev->speed,
> +						   phydev->duplex, common);
> +	else
> +		data->eee_active = false;
> +
> +	/* FIXME: EEE ethtool interface currently do not support full set of
> +	 * possible EEE link modes.
> +	 */
> +	data->supported = phydev->supported_eee[0];
> +	data->advertised = adv[0];
> +	data->lp_advertised = lp[0];
> +
> +	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 ea4f7d08d1a6..231cf02671a7 100644
> --- a/include/linux/mdio.h
> +++ b/include/linux/mdio.h
> @@ -427,6 +427,42 @@ static inline void mii_eee_100_10000_adv_mod_linkmode_t(unsigned long *adv,
>  			 adv, val & MDIO_EEE_10GKR);
>  }
>  
> +static inline u32 linkmode_adv_to_mii_eee_100_10000_adv_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;

Please could you remove the duplication with ethtool_adv_to_mmd_eee_adv_t().

       Andrew

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v3 15/15] net: fec: add support for PHYs with SmartEEE support
  2023-01-30  8:07 ` [PATCH net-next v3 15/15] net: fec: add support for PHYs with SmartEEE support Oleksij Rempel
@ 2023-01-31 20:52   ` Vladimir Oltean
  2023-02-01 12:45     ` Oleksij Rempel
  0 siblings, 1 reply; 21+ messages in thread
From: Vladimir Oltean @ 2023-01-31 20:52 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, kernel, linux-kernel, netdev, Arun.Ramadoss

On Mon, Jan 30, 2023 at 09:07:14AM +0100, Oleksij Rempel wrote:
> Ethernet controller in i.MX6*/i.MX7* series do not provide EEE support.
> But this chips are used sometimes in combinations with SmartEEE capable
> PHYs.
> So, instead of aborting get/set_eee access on MACs without EEE support,
> ask PHY if it is able to do the EEE job by using SmartEEE.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index e6238e53940d..25a2a9d860de 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3102,8 +3102,15 @@ fec_enet_get_eee(struct net_device *ndev, struct ethtool_eee *edata)
>  	struct fec_enet_private *fep = netdev_priv(ndev);
>  	struct ethtool_eee *p = &fep->eee;
>  
> -	if (!(fep->quirks & FEC_QUIRK_HAS_EEE))
> -		return -EOPNOTSUPP;
> +	if (!(fep->quirks & FEC_QUIRK_HAS_EEE)) {
> +		if (!netif_running(ndev))
> +			return -ENETDOWN;
> +
> +		if (!phy_has_smarteee(ndev->phydev))
> +			return -EOPNOTSUPP;
> +
> +		return phy_ethtool_get_eee(ndev->phydev, edata);

I see many places in the fec driver guarding against a NULL
ndev->phydev, and TBH I don't completely understand why.
I guess it's because ndev->phydev is populated at fec_enet_open() time.

But then again, if the netif_running() check is sufficient to imply
presence of ndev->phydev as you suggest, then why does fec_enet_ioctl()
have this?

	if (!netif_running(ndev))
		return -EINVAL;

	if (!phydev)
		return -ENODEV;

Asking because phy_init_eee(), phy_ethtool_set_eee() and
phy_ethtool_get_eee() don't support being called with a NULL phydev.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH net-next v3 15/15] net: fec: add support for PHYs with SmartEEE support
  2023-01-31 20:52   ` Vladimir Oltean
@ 2023-02-01 12:45     ` Oleksij Rempel
  0 siblings, 0 replies; 21+ messages in thread
From: Oleksij Rempel @ 2023-02-01 12:45 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, kernel, linux-kernel, netdev, Arun.Ramadoss

On Tue, Jan 31, 2023 at 10:52:31PM +0200, Vladimir Oltean wrote:
> On Mon, Jan 30, 2023 at 09:07:14AM +0100, Oleksij Rempel wrote:
> > Ethernet controller in i.MX6*/i.MX7* series do not provide EEE support.
> > But this chips are used sometimes in combinations with SmartEEE capable
> > PHYs.
> > So, instead of aborting get/set_eee access on MACs without EEE support,
> > ask PHY if it is able to do the EEE job by using SmartEEE.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  drivers/net/ethernet/freescale/fec_main.c | 22 ++++++++++++++++++----
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> > index e6238e53940d..25a2a9d860de 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -3102,8 +3102,15 @@ fec_enet_get_eee(struct net_device *ndev, struct ethtool_eee *edata)
> >  	struct fec_enet_private *fep = netdev_priv(ndev);
> >  	struct ethtool_eee *p = &fep->eee;
> >  
> > -	if (!(fep->quirks & FEC_QUIRK_HAS_EEE))
> > -		return -EOPNOTSUPP;
> > +	if (!(fep->quirks & FEC_QUIRK_HAS_EEE)) {
> > +		if (!netif_running(ndev))
> > +			return -ENETDOWN;
> > +
> > +		if (!phy_has_smarteee(ndev->phydev))
> > +			return -EOPNOTSUPP;
> > +
> > +		return phy_ethtool_get_eee(ndev->phydev, edata);
> 
> I see many places in the fec driver guarding against a NULL
> ndev->phydev, and TBH I don't completely understand why.
> I guess it's because ndev->phydev is populated at fec_enet_open() time.
> 
> But then again, if the netif_running() check is sufficient to imply
> presence of ndev->phydev as you suggest, then why does fec_enet_ioctl()
> have this?
> 
> 	if (!netif_running(ndev))
> 		return -EINVAL;
> 
> 	if (!phydev)
> 		return -ENODEV;
> 
> Asking because phy_init_eee(), phy_ethtool_set_eee() and
> phy_ethtool_get_eee() don't support being called with a NULL phydev.

Hm..
phy_start() is protected against NULL phydev and it is used in
fec_enet_open().

Right now i do not know what is better way go. Any preferences?

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 |

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-02-01 12:45 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30  8:06 [PATCH net-next v3 00/15] net: add EEE support for KSZ9477 and AR8035 with i.MX6 Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 01/15] net: dsa: microchip: enable EEE support Oleksij Rempel
2023-01-30 17:59   ` Vladimir Oltean
2023-01-30  8:07 ` [PATCH net-next v3 02/15] net: phy: add genphy_c45_read_eee_abilities() function Oleksij Rempel
2023-01-30 21:55   ` Andrew Lunn
2023-01-30  8:07 ` [PATCH net-next v3 03/15] net: phy: micrel: add ksz9477_get_features() Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 04/15] net: phy: export phy_check_valid() function Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 05/15] net: phy: add genphy_c45_ethtool_get/set_eee() support Oleksij Rempel
2023-01-30 22:06   ` Andrew Lunn
2023-01-30  8:07 ` [PATCH net-next v3 06/15] net: phy: c22: migrate to genphy_c45_write_eee_adv() Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 07/15] net: phy: c45: " Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 08/15] net: phy: start using genphy_c45_ethtool_get/set_eee() Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 09/15] net: phy: add driver specific get/set_eee support Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 10/15] net: phy: at803x: implement ethtool access to SmartEEE functionality Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 11/15] net: phy: at803x: ar8035: fix EEE support for half duplex links Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 12/15] net: phy: add PHY specifica flag to signal SmartEEE support Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 13/15] net: phy: at803x: add PHY_SMART_EEE flag to AR8035 Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 14/15] net: phy: add phy_has_smarteee() helper Oleksij Rempel
2023-01-30  8:07 ` [PATCH net-next v3 15/15] net: fec: add support for PHYs with SmartEEE support Oleksij Rempel
2023-01-31 20:52   ` Vladimir Oltean
2023-02-01 12:45     ` Oleksij Rempel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.