[PATCH RFC net-next 0/4] phylink EEE support

[PATCH RFC net-next 0/4] phylink EEE support

[Russell King (Oracle)]

Hi,

There has been some recent discussion on generalising EEE support so
that drivers implement it more consistently. This has mostly focused
around phylib, but there are other situations where EEE may be useful.

To illustrate this, in both USGMII and UXGMII communicate EEE through
the configuration word - bit 8 indicates EEE capability and bit 7
indicates the clock stop capability. The PHY may not be accessible.
Another case would be a PHY on a SFP module that may not be accessible,
signalling LPI to it may result in power savings if it has negotiated
with the link partner. My understanding is that signalling LPI when
negotiation hasn't agreed EEE support should be fine. In the classic
model, LPI informs the PHY that the MAC is idle, and gives it
permission to enter low power. It will only enter low power when the
MAC sends LPI and the remote PHY also agrees to enter low power.

mvneta has had EEE support for a while, but the implementation has its
quirks. This series implements EEE handling in phylink. To make use of
it, a MAC driver needs to fill in the default parameters for EEE, and
provide the enable and disable functions for LPI.

This series also adds EEE for mvpp2, which is only supported by the
GMAC (up to 1G) and not the XLG (5G,10G) MAC.

There is further work that needs to be considered - 802.3 has the
facility to negotiate the Tw (wake to data) parameter via packets.
Also, timing parameters are speed and media type specific, some
implementations need these parameters reprogrammed each time the speed
changes (e.g. mvneta and mvpp2.)

Patch 1 adds a structure to store the runtime EEE configuration  state,
a helper to decode the state to indicate whether LPI can be enabled (it
remains the responsibility of the user to determine whether EEE has
been negotiated.) A couple of helpers are provided to insert and
extract the EEE configuration from the ethtool EEE structure.

Patch 2 adds the phylink implementation.

Patch 3 converts mvneta to use phylink's implementation.

Patch 4 adds mvpp2 support.

This uses the current code from phylib, and is only functional when we
have a phylib PHY, but can be easily extended so that when we have a
SFP socket without a PHY, we can enable LPI signalling.

 drivers/net/ethernet/marvell/mvneta.c           | 95 ++++++++++++++++---------
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h      |  5 ++
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 85 ++++++++++++++++++++++
 drivers/net/phy/phylink.c                       | 82 +++++++++++++++++++--
 include/linux/phylink.h                         | 32 +++++++++
 include/net/eee.h                               | 38 ++++++++++
 6 files changed, 298 insertions(+), 39 deletions(-)
 create mode 100644 include/net/eee.h

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH RFC net-next 1/4] net: add helpers for EEE configuration

[Russell King (Oracle)]

Add helpers that phylib and phylink can use to manage EEE configuration
and determine whether the MAC should be permitted to use LPI based on
that configuration.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 include/net/eee.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 include/net/eee.h

diff --git a/include/net/eee.h b/include/net/eee.h
new file mode 100644
index 000000000000..d353b79ae79f
--- /dev/null
+++ b/include/net/eee.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _EEE_H
+#define _EEE_H
+
+#include <linux/types.h>
+
+struct eee_config {
+	u32 tx_lpi_timer;
+	bool tx_lpi_enabled;
+	bool eee_enabled;
+};
+
+static inline bool eeecfg_mac_can_tx_lpi(const struct eee_config *eeecfg)
+{
+	/* eee_enabled is the master on/off */
+	if (!eeecfg->eee_enabled || !eeecfg->tx_lpi_enabled)
+		return false;
+
+	return true;
+}
+
+static inline void eeecfg_to_eee(const struct eee_config *eeecfg,
+			  struct ethtool_eee *eee)
+{
+	eee->tx_lpi_timer = eeecfg->tx_lpi_timer;
+	eee->tx_lpi_enabled = eeecfg->tx_lpi_enabled;
+	eee->eee_enabled = eeecfg->eee_enabled;
+}
+
+static inline void eee_to_eeecfg(const struct ethtool_eee *eee,
+				 struct eee_config *eeecfg)
+{
+	eeecfg->tx_lpi_timer = eee->tx_lpi_timer;
+	eeecfg->tx_lpi_enabled = eee->tx_lpi_enabled;
+	eeecfg->eee_enabled = eee->eee_enabled;
+}
+
+#endif
-- 
2.30.2

[PATCH RFC net-next 2/4] net: phylink: add EEE management

[Russell King (Oracle)]

Add EEE management to phylink.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 82 ++++++++++++++++++++++++++++++++++++---
 include/linux/phylink.h   | 32 +++++++++++++++
 2 files changed, 109 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 1ae7868d2137..d0abae91ceb5 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -80,6 +80,9 @@ struct phylink {
 	DECLARE_PHY_INTERFACE_MASK(sfp_interfaces);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
 	u8 sfp_port;
+
+	struct eee_config eee_cfg;
+	bool eee_active;
 };
 
 #define phylink_printk(level, pl, fmt, ...) \
@@ -1253,6 +1256,24 @@ static const char *phylink_pause_to_str(int pause)
 	}
 }
 
+static void phylink_disable_tx_lpi(struct phylink *pl)
+{
+	if (pl->mac_ops->mac_disable_tx_lpi)
+		pl->mac_ops->mac_disable_tx_lpi(pl->config);
+}
+
+static void phylink_enable_tx_lpi(struct phylink *pl)
+{
+	if (pl->mac_ops->mac_enable_tx_lpi)
+		pl->mac_ops->mac_enable_tx_lpi(pl->config,
+					       pl->eee_cfg.tx_lpi_timer);
+}
+
+static bool phylink_eee_is_active(struct phylink *pl)
+{
+	return phylink_init_eee(pl, pl->config->eee_clk_stop_enable) >= 0;
+}
+
 static void phylink_link_up(struct phylink *pl,
 			    struct phylink_link_state link_state)
 {
@@ -1294,6 +1315,12 @@ static void phylink_link_up(struct phylink *pl,
 				 pl->cur_interface, speed, duplex,
 				 !!(link_state.pause & MLO_PAUSE_TX), rx_pause);
 
+	if (eeecfg_mac_can_tx_lpi(&pl->eee_cfg)) {
+		pl->eee_active = phylink_eee_is_active(pl);
+		if (pl->eee_active)
+			phylink_enable_tx_lpi(pl);
+	}
+
 	if (ndev)
 		netif_carrier_on(ndev);
 
@@ -1310,25 +1337,32 @@ static void phylink_link_down(struct phylink *pl)
 
 	if (ndev)
 		netif_carrier_off(ndev);
+
+	if (eeecfg_mac_can_tx_lpi(&pl->eee_cfg) && pl->eee_active) {
+		pl->eee_active = false;
+		phylink_disable_tx_lpi(pl);
+	}
+
 	pl->mac_ops->mac_link_down(pl->config, pl->cur_link_an_mode,
 				   pl->cur_interface);
 	phylink_info(pl, "Link is Down\n");
 }
 
+static bool phylink_link_is_up(struct phylink *pl)
+{
+	return pl->netdev ? netif_carrier_ok(pl->netdev) : pl->old_link_state;
+}
+
 static void phylink_resolve(struct work_struct *w)
 {
 	struct phylink *pl = container_of(w, struct phylink, resolve);
 	struct phylink_link_state link_state;
-	struct net_device *ndev = pl->netdev;
 	bool mac_config = false;
 	bool retrigger = false;
 	bool cur_link_state;
 
 	mutex_lock(&pl->state_mutex);
-	if (pl->netdev)
-		cur_link_state = netif_carrier_ok(ndev);
-	else
-		cur_link_state = pl->old_link_state;
+	cur_link_state = phylink_link_is_up(pl);
 
 	if (pl->phylink_disable_state) {
 		pl->mac_link_dropped = false;
@@ -1571,6 +1605,9 @@ struct phylink *phylink_create(struct phylink_config *config,
 	linkmode_copy(pl->link_config.advertising, pl->supported);
 	phylink_validate(pl, pl->supported, &pl->link_config);
 
+	/* Set the default EEE configuration */
+	pl->eee_cfg = pl->config->eee;
+
 	ret = phylink_parse_mode(pl, fwnode);
 	if (ret < 0) {
 		kfree(pl);
@@ -2589,6 +2626,12 @@ int phylink_ethtool_get_eee(struct phylink *pl, struct ethtool_eee *eee)
 	if (pl->phydev)
 		ret = phy_ethtool_get_eee(pl->phydev, eee);
 
+	if (!ret) {
+		/* Overwrite phylib's interpretation of configuration */
+		eeecfg_to_eee(&pl->eee_cfg, eee);
+		eee->eee_active = pl->eee_active;
+	}
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_get_eee);
@@ -2607,6 +2650,35 @@ int phylink_ethtool_set_eee(struct phylink *pl, struct ethtool_eee *eee)
 	if (pl->phydev)
 		ret = phy_ethtool_set_eee(pl->phydev, eee);
 
+	if (!ret) {
+		bool can_lpi, old_can_lpi;
+
+		mutex_lock(&pl->state_mutex);
+		old_can_lpi = eeecfg_mac_can_tx_lpi(&pl->eee_cfg);
+		eee_to_eeecfg(eee, &pl->eee_cfg);
+		can_lpi = eeecfg_mac_can_tx_lpi(&pl->eee_cfg);
+
+		/* IF the link is up, and the configuration changes the
+		 * LPI permissive state, deal with the change at the MAC.
+		 */
+		if (phylink_link_is_up(pl) && old_can_lpi != can_lpi) {
+			if (can_lpi) {
+				/* If LPI wasn't enabled, eee_active (result
+				 * of any AN) will be false. Update it here.
+				 * If the advertisement has been changed, the
+				 * link will cycle which will update this.
+				 */
+				pl->eee_active = phylink_eee_is_active(pl);
+				if (pl->eee_active)
+					phylink_enable_tx_lpi(pl);
+			} else {
+				phylink_disable_tx_lpi(pl);
+			}
+		}
+
+		mutex_unlock(&pl->state_mutex);
+	}
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_set_eee);
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 0cf07d7d11b8..6328a9f481ee 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -5,6 +5,8 @@
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 
+#include <net/eee.h>
+
 struct device_node;
 struct ethtool_cmd;
 struct fwnode_handle;
@@ -122,11 +124,13 @@ enum phylink_op_type {
  *		      if MAC link is at %MLO_AN_FIXED mode.
  * @mac_managed_pm: if true, indicate the MAC driver is responsible for PHY PM.
  * @ovr_an_inband: if true, override PCS to MLO_AN_INBAND
+ * @eee_clk_stop_enable: if true, PHY can stop the receive clock during LPI
  * @get_fixed_state: callback to execute to determine the fixed link state,
  *		     if MAC link is at %MLO_AN_FIXED mode.
  * @supported_interfaces: bitmap describing which PHY_INTERFACE_MODE_xxx
  *                        are supported by the MAC/PCS.
  * @mac_capabilities: MAC pause/speed/duplex capabilities.
+ * @eee: default EEE configuration.
  */
 struct phylink_config {
 	struct device *dev;
@@ -135,10 +139,12 @@ struct phylink_config {
 	bool poll_fixed_state;
 	bool mac_managed_pm;
 	bool ovr_an_inband;
+	bool eee_clk_stop_enable;
 	void (*get_fixed_state)(struct phylink_config *config,
 				struct phylink_link_state *state);
 	DECLARE_PHY_INTERFACE_MASK(supported_interfaces);
 	unsigned long mac_capabilities;
+	struct eee_config eee;
 };
 
 /**
@@ -152,6 +158,8 @@ struct phylink_config {
  * @mac_an_restart: restart 802.3z BaseX autonegotiation.
  * @mac_link_down: take the link down.
  * @mac_link_up: allow the link to come up.
+ * @mac_disable_tx_lpi: disable LPI.
+ * @mac_enable_tx_lpi: enable and configure LPI.
  *
  * The individual methods are described more fully below.
  */
@@ -176,6 +184,8 @@ struct phylink_mac_ops {
 			    struct phy_device *phy, unsigned int mode,
 			    phy_interface_t interface, int speed, int duplex,
 			    bool tx_pause, bool rx_pause);
+	void (*mac_disable_tx_lpi)(struct phylink_config *config);
+	void (*mac_enable_tx_lpi)(struct phylink_config *config, u32 timer);
 };
 
 #if 0 /* For kernel-doc purposes only. */
@@ -429,6 +439,28 @@ void mac_link_down(struct phylink_config *config, unsigned int mode,
 void mac_link_up(struct phylink_config *config, struct phy_device *phy,
 		 unsigned int mode, phy_interface_t interface,
 		 int speed, int duplex, bool tx_pause, bool rx_pause);
+
+/**
+ * mac_disable_tx_lpi() - disable LPI generation at the MAC
+ * @config: a pointer to a &struct phylink_config.
+ *
+ * Disable generation of LPI at the MAC, effectively preventing the MAC
+ * from indicating that it is idle.
+ */
+void mac_disable_tx_lpi(struct phylink_config *config);
+
+/**
+ * mac_enable_tx_lpi() - configure and enable LPI generation at the MAC
+ * @config: a pointer to a &struct phylink_config.
+ * @timer: LPI timeout in microseconds.
+ *
+ * Configure the LPI timeout accordingly. This will only be called when
+ * the link is already up, to cater for situations where the hardware
+ * needs to be programmed according to the link speed.
+ *
+ * Enable LPI generation at the MAC.
+ */
+void mac_enable_tx_lpi(struct phylink_config *config, u32 timer);
 #endif
 
 struct phylink_pcs_ops;
-- 
2.30.2

[PATCH RFC net-next 3/4] net: mvneta: convert to phylink EEE implementation

[Russell King (Oracle)]

Convert mvneta to use phylink's EEE implementation, which means we just
need to implement the two methods for LPI control, and adding the
initial configuration.

Disabling LPI requires clearing a single bit. Enabling LPI needs a full
configuration of several values, as the timer values are dependent on
the MAC operating speed.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvneta.c | 95 +++++++++++++++++----------
 1 file changed, 61 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index e2abc00d0472..c634ec5d3f9a 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -284,8 +284,10 @@
 					  MVNETA_TXQ_BUCKET_REFILL_PERIOD))
 
 #define MVNETA_LPI_CTRL_0                        0x2cc0
+#define      MVNETA_LPI_CTRL_0_TS                0xff << 8
 #define MVNETA_LPI_CTRL_1                        0x2cc4
 #define      MVNETA_LPI_REQUEST_ENABLE           BIT(0)
+#define      MVNETA_LPI_CTRL_1_TW                0xfff << 4
 #define MVNETA_LPI_CTRL_2                        0x2cc8
 #define MVNETA_LPI_STATUS                        0x2ccc
 
@@ -541,10 +543,6 @@ struct mvneta_port {
 	struct mvneta_bm_pool *pool_short;
 	int bm_win_id;
 
-	bool eee_enabled;
-	bool eee_active;
-	bool tx_lpi_enabled;
-
 	u64 ethtool_stats[ARRAY_SIZE(mvneta_statistics)];
 
 	u32 indir[MVNETA_RSS_LU_TABLE_SIZE];
@@ -4232,9 +4230,6 @@ static void mvneta_mac_link_down(struct phylink_config *config,
 		val |= MVNETA_GMAC_FORCE_LINK_DOWN;
 		mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
 	}
-
-	pp->eee_active = false;
-	mvneta_set_eee(pp, false);
 }
 
 static void mvneta_mac_link_up(struct phylink_config *config,
@@ -4283,11 +4278,57 @@ static void mvneta_mac_link_up(struct phylink_config *config,
 	}
 
 	mvneta_port_up(pp);
+}
+
+static void mvneta_mac_disable_tx_lpi(struct phylink_config *config)
+{
+	struct mvneta_port *pp = netdev_priv(to_net_dev(config->dev));
+	u32 lpi1;
+
+	lpi1 = mvreg_read(pp, MVNETA_LPI_CTRL_1);
+	lpi1 &= ~MVNETA_LPI_REQUEST_ENABLE;
+	mvreg_write(pp, MVNETA_LPI_CTRL_1, lpi1);
+}
 
-	if (phy && pp->eee_enabled) {
-		pp->eee_active = phy_init_eee(phy, false) >= 0;
-		mvneta_set_eee(pp, pp->eee_active && pp->tx_lpi_enabled);
+static void mvneta_mac_enable_tx_lpi(struct phylink_config *config, u32 timer)
+{
+	struct mvneta_port *pp = netdev_priv(to_net_dev(config->dev));
+	u32 ts, tw, lpi0, lpi1, status;
+
+	status = mvreg_read(pp, MVNETA_GMAC_STATUS);
+
+	if (status & MVNETA_GMAC_SPEED_1000) {
+		/* At 1G speeds, the timer resolution are 1us, and
+		 * 802.3 says tw is 16.5us. Round up to 17us.
+		 */
+		tw = 17;
+		ts = timer;
+	} else {
+		/* At 100M speeds, the timer resolutions are 10us, and
+		 * 802.3 says tw is 30us.
+		 */
+		tw = 3;
+		ts = DIV_ROUND_UP(timer, 10);
 	}
+
+	if (ts > 255)
+		ts = 255;
+
+	/* Ensure LPI generation is disabled */
+	lpi1 = mvreg_read(pp, MVNETA_LPI_CTRL_1);
+	mvreg_write(pp, MVNETA_LPI_CTRL_1, lpi1 & ~MVNETA_LPI_REQUEST_ENABLE);
+
+	/* Configure ts */
+	lpi0 = mvreg_read(pp, MVNETA_LPI_CTRL_0) & ~MVNETA_LPI_CTRL_0_TS;
+	lpi0 |= FIELD_PREP(MVNETA_LPI_CTRL_0_TS, ts);
+	mvreg_write(pp, MVNETA_LPI_CTRL_0, lpi0);
+
+	/* Configure tw */
+	lpi1 &= ~MVNETA_LPI_CTRL_1_TW;
+	lpi1 |= FIELD_PREP(MVNETA_LPI_CTRL_1_TW, tw);
+
+	/* Enable LPI generation */
+	mvreg_write(pp, MVNETA_LPI_CTRL_1, lpi1 | MVNETA_LPI_REQUEST_ENABLE);
 }
 
 static const struct phylink_mac_ops mvneta_phylink_ops = {
@@ -4297,6 +4338,8 @@ static const struct phylink_mac_ops mvneta_phylink_ops = {
 	.mac_finish = mvneta_mac_finish,
 	.mac_link_down = mvneta_mac_link_down,
 	.mac_link_up = mvneta_mac_link_up,
+	.mac_disable_tx_lpi = mvneta_mac_disable_tx_lpi,
+	.mac_enable_tx_lpi = mvneta_mac_enable_tx_lpi,
 };
 
 static int mvneta_mdio_probe(struct mvneta_port *pp)
@@ -5087,14 +5130,6 @@ static int mvneta_ethtool_get_eee(struct net_device *dev,
 				  struct ethtool_eee *eee)
 {
 	struct mvneta_port *pp = netdev_priv(dev);
-	u32 lpi_ctl0;
-
-	lpi_ctl0 = mvreg_read(pp, MVNETA_LPI_CTRL_0);
-
-	eee->eee_enabled = pp->eee_enabled;
-	eee->eee_active = pp->eee_active;
-	eee->tx_lpi_enabled = pp->tx_lpi_enabled;
-	eee->tx_lpi_timer = (lpi_ctl0) >> 8; // * scale;
 
 	return phylink_ethtool_get_eee(pp->phylink, eee);
 }
@@ -5103,23 +5138,10 @@ static int mvneta_ethtool_set_eee(struct net_device *dev,
 				  struct ethtool_eee *eee)
 {
 	struct mvneta_port *pp = netdev_priv(dev);
-	u32 lpi_ctl0;
 
-	/* The Armada 37x documents do not give limits for this other than
-	 * it being an 8-bit register.
-	 */
-	if (eee->tx_lpi_enabled && eee->tx_lpi_timer > 255)
-		return -EINVAL;
-
-	lpi_ctl0 = mvreg_read(pp, MVNETA_LPI_CTRL_0);
-	lpi_ctl0 &= ~(0xff << 8);
-	lpi_ctl0 |= eee->tx_lpi_timer << 8;
-	mvreg_write(pp, MVNETA_LPI_CTRL_0, lpi_ctl0);
-
-	pp->eee_enabled = eee->eee_enabled;
-	pp->tx_lpi_enabled = eee->tx_lpi_enabled;
-
-	mvneta_set_eee(pp, eee->tx_lpi_enabled && eee->eee_enabled);
+	/* clamp tx_lpi_timer */
+	if (eee->tx_lpi_timer > 255)
+		eee->tx_lpi_timer = 255;
 
 	return phylink_ethtool_set_eee(pp->phylink, eee);
 }
@@ -5550,6 +5572,11 @@ static int mvneta_probe(struct platform_device *pdev)
 			  pp->phylink_config.supported_interfaces);
 	}
 
+	/* Setup EEE.  Choose 250us idle. */
+	pp->phylink_config.eee.eee_enabled = true;
+	pp->phylink_config.eee.tx_lpi_enabled = true;
+	pp->phylink_config.eee.tx_lpi_timer = 250;
+
 	phylink = phylink_create(&pp->phylink_config, pdev->dev.fwnode,
 				 phy_mode, &mvneta_phylink_ops);
 	if (IS_ERR(phylink)) {
-- 
2.30.2

[PATCH RFC net-next 4/4] net: mvpp2: add EEE implementation

[Russell King (Oracle)]

Add EEE support for mvpp2, using phylink's EEE implementation, which
means we just need to implement the two methods for LPI control, and
with the initial configuration. Only the GMAC is supported, so only
100M, 1G and 2.5G speeds.

Disabling LPI requires clearing a single bit. Enabling LPI needs a full
configuration of several values, as the timer values are dependent on
the MAC operating speed.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  5 ++
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 85 +++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 11e603686a27..af31da9cc89a 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -481,6 +481,11 @@
 #define MVPP22_GMAC_INT_SUM_MASK		0xa4
 #define     MVPP22_GMAC_INT_SUM_MASK_LINK_STAT	BIT(1)
 #define	    MVPP22_GMAC_INT_SUM_MASK_PTP	BIT(2)
+#define MVPP2_GMAC_LPI_CTRL0			0xc0
+#define     MVPP2_GMAC_LPI_CTRL0_TS_MASK	GENMASK(8, 8)
+#define MVPP2_GMAC_LPI_CTRL1			0xc4
+#define     MVPP2_GMAC_LPI_CTRL1_REQ_EN		BIT(0)
+#define     MVPP2_GMAC_LPI_CTRL1_TW_MASK	GENMASK(15, 4)
 
 /* Per-port XGMAC registers. PPv2.2 and PPv2.3, only for GOP port 0,
  * relative to port->base.
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index adc953611913..a06c455b7180 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -5716,6 +5716,31 @@ static int mvpp2_ethtool_set_rxfh_context(struct net_device *dev,
 
 	return mvpp22_port_rss_ctx_indir_set(port, *rss_context, indir);
 }
+
+static int mvpp2_ethtool_get_eee(struct net_device *dev,
+				 struct ethtool_eee *eee)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+
+	if (!port->phylink)
+		return -ENOTSUPP;
+
+	return phylink_ethtool_get_eee(port->phylink, eee);
+}
+
+static int mvpp2_ethtool_set_eee(struct net_device *dev,
+				 struct ethtool_eee *eee)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+
+	if (!port->phylink)
+		return -ENOTSUPP;
+
+	if (eee->tx_lpi_timer > 255)
+		eee->tx_lpi_timer = 255;
+
+	return phylink_ethtool_set_eee(port->phylink, eee);
+}
 /* Device ops */
 
 static const struct net_device_ops mvpp2_netdev_ops = {
@@ -5759,6 +5784,8 @@ static const struct ethtool_ops mvpp2_eth_tool_ops = {
 	.set_rxfh		= mvpp2_ethtool_set_rxfh,
 	.get_rxfh_context	= mvpp2_ethtool_get_rxfh_context,
 	.set_rxfh_context	= mvpp2_ethtool_set_rxfh_context,
+	.get_eee		= mvpp2_ethtool_get_eee,
+	.set_eee		= mvpp2_ethtool_set_eee,
 };
 
 /* Used for PPv2.1, or PPv2.2 with the old Device Tree binding that
@@ -6623,6 +6650,57 @@ static void mvpp2_mac_link_down(struct phylink_config *config,
 	mvpp2_port_disable(port);
 }
 
+static void mvpp2_mac_disable_tx_lpi(struct phylink_config *config)
+{
+	struct mvpp2_port *port = mvpp2_phylink_to_port(config);
+
+	mvpp2_modify(port->base + MVPP2_GMAC_LPI_CTRL1,
+		     MVPP2_GMAC_LPI_CTRL1_REQ_EN, 0);
+}
+
+static void mvpp2_mac_enable_tx_lpi(struct phylink_config *config, u32 timer)
+{
+	struct mvpp2_port *port = mvpp2_phylink_to_port(config);
+	u32 ts, tw, lpi0, lpi1, status;
+
+	status = readl(port->base + MVPP2_GMAC_STATUS0);
+	if (status & MVPP2_GMAC_STATUS0_GMII_SPEED) {
+		/* At 1G speeds, the timer resolution are 1us, and
+		 * 802.3 says tw is 16.5us. Round up to 17us.
+		 */
+		tw = 17;
+		ts = timer;
+	} else {
+		/* At 100M speeds, the timer resolutions are 10us, and
+		 * 802.3 says tw is 30us.
+		 */
+		tw = 3;
+		ts = DIV_ROUND_UP(timer, 10);
+	}
+
+	if (ts > 255)
+		ts = 255;
+
+	/* Ensure LPI generation is disabled */
+	lpi1 = readl(port->base + MVPP2_GMAC_LPI_CTRL1);
+	writel(lpi1 & ~MVPP2_GMAC_LPI_CTRL1_REQ_EN,
+	       port->base + MVPP2_GMAC_LPI_CTRL1);
+
+	/* Configure ts */
+	lpi0 = readl(port->base + MVPP2_GMAC_LPI_CTRL0);
+	lpi0 &= ~MVPP2_GMAC_LPI_CTRL0_TS_MASK;
+	lpi0 |= FIELD_PREP(MVPP2_GMAC_LPI_CTRL0_TS_MASK, ts);
+	writel(lpi0, port->base + MVPP2_GMAC_LPI_CTRL0);
+
+	/* Configure tw */
+	lpi1 &= ~MVPP2_GMAC_LPI_CTRL1_TW_MASK;
+	lpi1 |= FIELD_PREP(MVPP2_GMAC_LPI_CTRL1_TW_MASK, tw);
+
+	/* Enable LPI generation */
+	writel(lpi1 | MVPP2_GMAC_LPI_CTRL1_REQ_EN,
+	       port->base + MVPP2_GMAC_LPI_CTRL1);
+}
+
 static const struct phylink_mac_ops mvpp2_phylink_ops = {
 	.mac_select_pcs = mvpp2_select_pcs,
 	.mac_prepare = mvpp2_mac_prepare,
@@ -6630,6 +6708,8 @@ static const struct phylink_mac_ops mvpp2_phylink_ops = {
 	.mac_finish = mvpp2_mac_finish,
 	.mac_link_up = mvpp2_mac_link_up,
 	.mac_link_down = mvpp2_mac_link_down,
+	.mac_enable_tx_lpi = mvpp2_mac_enable_tx_lpi,
+	.mac_disable_tx_lpi = mvpp2_mac_disable_tx_lpi,
 };
 
 /* Work-around for ACPI */
@@ -6968,6 +7048,11 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 				  port->phylink_config.supported_interfaces);
 		}
 
+		/* Setup EEE.  Choose 250us idle. */
+		port->phylink_config.eee.eee_enabled = true;
+		port->phylink_config.eee.tx_lpi_enabled = true;
+		port->phylink_config.eee.tx_lpi_timer = 250;
+
 		phylink = phylink_create(&port->phylink_config, port_fwnode,
 					 phy_mode, &mvpp2_phylink_ops);
 		if (IS_ERR(phylink)) {
-- 
2.30.2

Re: [PATCH RFC net-next 1/4] net: add helpers for EEE configuration

[Simon Horman]

On Fri, Jun 09, 2023 at 10:11:16AM +0100, Russell King (Oracle) wrote:
> Add helpers that phylib and phylink can use to manage EEE configuration
> and determine whether the MAC should be permitted to use LPI based on
> that configuration.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  include/net/eee.h | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 include/net/eee.h
> 
> diff --git a/include/net/eee.h b/include/net/eee.h
> new file mode 100644
> index 000000000000..d353b79ae79f
> --- /dev/null
> +++ b/include/net/eee.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef _EEE_H
> +#define _EEE_H
> +
> +#include <linux/types.h>
> +
> +struct eee_config {
> +	u32 tx_lpi_timer;
> +	bool tx_lpi_enabled;
> +	bool eee_enabled;
> +};
> +
> +static inline bool eeecfg_mac_can_tx_lpi(const struct eee_config *eeecfg)
> +{
> +	/* eee_enabled is the master on/off */
> +	if (!eeecfg->eee_enabled || !eeecfg->tx_lpi_enabled)
> +		return false;
> +
> +	return true;
> +}
> +
> +static inline void eeecfg_to_eee(const struct eee_config *eeecfg,
> +			  struct ethtool_eee *eee)

Hi Russell,

a minor nit from my side: the indentation of the line above is a bit off.

> +{
> +	eee->tx_lpi_timer = eeecfg->tx_lpi_timer;
> +	eee->tx_lpi_enabled = eeecfg->tx_lpi_enabled;
> +	eee->eee_enabled = eeecfg->eee_enabled;
> +}
> +
> +static inline void eee_to_eeecfg(const struct ethtool_eee *eee,
> +				 struct eee_config *eeecfg)
> +{
> +	eeecfg->tx_lpi_timer = eee->tx_lpi_timer;
> +	eeecfg->tx_lpi_enabled = eee->tx_lpi_enabled;
> +	eeecfg->eee_enabled = eee->eee_enabled;
> +}
> +
> +#endif
> -- 
> 2.30.2
> 
> 

Re: [PATCH RFC net-next 3/4] net: mvneta: convert to phylink EEE implementation

[Simon Horman]

On Fri, Jun 09, 2023 at 10:11:26AM +0100, Russell King (Oracle) wrote:
> Convert mvneta to use phylink's EEE implementation, which means we just
> need to implement the two methods for LPI control, and adding the
> initial configuration.
> 
> Disabling LPI requires clearing a single bit. Enabling LPI needs a full
> configuration of several values, as the timer values are dependent on
> the MAC operating speed.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 95 +++++++++++++++++----------
>  1 file changed, 61 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index e2abc00d0472..c634ec5d3f9a 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -284,8 +284,10 @@
>  					  MVNETA_TXQ_BUCKET_REFILL_PERIOD))
>  
>  #define MVNETA_LPI_CTRL_0                        0x2cc0
> +#define      MVNETA_LPI_CTRL_0_TS                0xff << 8

Hi Russell,

maybe GENMASK would be useful here. If not, perhaps (0xffUL << 8)

>  #define MVNETA_LPI_CTRL_1                        0x2cc4
>  #define      MVNETA_LPI_REQUEST_ENABLE           BIT(0)
> +#define      MVNETA_LPI_CTRL_1_TW                0xfff << 4

Likewise here.

>  #define MVNETA_LPI_CTRL_2                        0x2cc8
>  #define MVNETA_LPI_STATUS                        0x2ccc
>  
> @@ -541,10 +543,6 @@ struct mvneta_port {
>  	struct mvneta_bm_pool *pool_short;
>  	int bm_win_id;
>  
> -	bool eee_enabled;
> -	bool eee_active;
> -	bool tx_lpi_enabled;
> -
>  	u64 ethtool_stats[ARRAY_SIZE(mvneta_statistics)];
>  
>  	u32 indir[MVNETA_RSS_LU_TABLE_SIZE];
> @@ -4232,9 +4230,6 @@ static void mvneta_mac_link_down(struct phylink_config *config,
>  		val |= MVNETA_GMAC_FORCE_LINK_DOWN;
>  		mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val);
>  	}
> -
> -	pp->eee_active = false;
> -	mvneta_set_eee(pp, false);
>  }

mvneta_set_eee() seems unused after this patch.
Perhaps it can be removed?

...

Re: [PATCH RFC net-next 4/4] net: mvpp2: add EEE implementation

[Simon Horman]

On Fri, Jun 09, 2023 at 10:11:31AM +0100, Russell King (Oracle) wrote:
> Add EEE support for mvpp2, using phylink's EEE implementation, which
> means we just need to implement the two methods for LPI control, and
> with the initial configuration. Only the GMAC is supported, so only
> 100M, 1G and 2.5G speeds.
> 
> Disabling LPI requires clearing a single bit. Enabling LPI needs a full
> configuration of several values, as the timer values are dependent on
> the MAC operating speed.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

...

> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index adc953611913..a06c455b7180 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -5716,6 +5716,31 @@ static int mvpp2_ethtool_set_rxfh_context(struct net_device *dev,
>  
>  	return mvpp22_port_rss_ctx_indir_set(port, *rss_context, indir);
>  }
> +
> +static int mvpp2_ethtool_get_eee(struct net_device *dev,
> +				 struct ethtool_eee *eee)
> +{
> +	struct mvpp2_port *port = netdev_priv(dev);
> +
> +	if (!port->phylink)
> +		return -ENOTSUPP;

Hi Russell,

perhaps EOPNOTSUPP is more appropriate here.

> +
> +	return phylink_ethtool_get_eee(port->phylink, eee);
> +}
> +
> +static int mvpp2_ethtool_set_eee(struct net_device *dev,
> +				 struct ethtool_eee *eee)
> +{
> +	struct mvpp2_port *port = netdev_priv(dev);
> +
> +	if (!port->phylink)
> +		return -ENOTSUPP;

And here.

> +
> +	if (eee->tx_lpi_timer > 255)
> +		eee->tx_lpi_timer = 255;
> +
> +	return phylink_ethtool_set_eee(port->phylink, eee);
> +}
>  /* Device ops */
>  
>  static const struct net_device_ops mvpp2_netdev_ops = {

...

Re: [PATCH RFC net-next 3/4] net: mvneta: convert to phylink EEE implementation

[Russell King (Oracle)]

On Fri, Jun 09, 2023 at 04:02:09PM +0200, Simon Horman wrote:
> On Fri, Jun 09, 2023 at 10:11:26AM +0100, Russell King (Oracle) wrote:
> > Convert mvneta to use phylink's EEE implementation, which means we just
> > need to implement the two methods for LPI control, and adding the
> > initial configuration.
> > 
> > Disabling LPI requires clearing a single bit. Enabling LPI needs a full
> > configuration of several values, as the timer values are dependent on
> > the MAC operating speed.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/ethernet/marvell/mvneta.c | 95 +++++++++++++++++----------
> >  1 file changed, 61 insertions(+), 34 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index e2abc00d0472..c634ec5d3f9a 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -284,8 +284,10 @@
> >  					  MVNETA_TXQ_BUCKET_REFILL_PERIOD))
> >  
> >  #define MVNETA_LPI_CTRL_0                        0x2cc0
> > +#define      MVNETA_LPI_CTRL_0_TS                0xff << 8
> 
> Hi Russell,
> 
> maybe GENMASK would be useful here. If not, perhaps (0xffUL << 8)

Why "unsigned long" when the variable we use it with is u32, which is
defined as "unsigned int" ?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC net-next 3/4] net: mvneta: convert to phylink EEE implementation

[Simon Horman]

On Fri, Jun 09, 2023 at 03:31:28PM +0100, Russell King (Oracle) wrote:
> On Fri, Jun 09, 2023 at 04:02:09PM +0200, Simon Horman wrote:
> > On Fri, Jun 09, 2023 at 10:11:26AM +0100, Russell King (Oracle) wrote:
> > > Convert mvneta to use phylink's EEE implementation, which means we just
> > > need to implement the two methods for LPI control, and adding the
> > > initial configuration.
> > > 
> > > Disabling LPI requires clearing a single bit. Enabling LPI needs a full
> > > configuration of several values, as the timer values are dependent on
> > > the MAC operating speed.
> > > 
> > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > ---
> > >  drivers/net/ethernet/marvell/mvneta.c | 95 +++++++++++++++++----------
> > >  1 file changed, 61 insertions(+), 34 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > > index e2abc00d0472..c634ec5d3f9a 100644
> > > --- a/drivers/net/ethernet/marvell/mvneta.c
> > > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > > @@ -284,8 +284,10 @@
> > >  					  MVNETA_TXQ_BUCKET_REFILL_PERIOD))
> > >  
> > >  #define MVNETA_LPI_CTRL_0                        0x2cc0
> > > +#define      MVNETA_LPI_CTRL_0_TS                0xff << 8
> > 
> > Hi Russell,
> > 
> > maybe GENMASK would be useful here. If not, perhaps (0xffUL << 8)
> 
> Why "unsigned long" when the variable we use it with is u32, which is
> defined as "unsigned int" ?

Fair point. What I actually wanted to ask is if the logic
that uses this works without parentheses. Probably I'm missing
something obvious and it does. But it looked odd to me.

Re: [PATCH RFC net-next 2/4] net: phylink: add EEE management

[Andrew Lunn]

On Fri, Jun 09, 2023 at 10:11:21AM +0100, Russell King (Oracle) wrote:
> Add EEE management to phylink.

Hi Russell

I've been working on my EEE patches. I have all pure phylib drivers
converted. I've incorporated these four patches as well, and make use
of the first patch in phylib.

Looking at this patch, i don't see a way for the MAC to indicate it
actually does support EEE. Am i missing it?

What i proposed last time is to add another bit to
pl->config->mac_capabilities. Does that work for you?

Do you have time to respin these patches to address the comments? It
is probably easier for my series to wait until these are merged.

	Andrew

Re: [PATCH RFC net-next 2/4] net: phylink: add EEE management

[Russell King (Oracle)]

On Sun, Jun 11, 2023 at 11:28:32PM +0200, Andrew Lunn wrote:
> On Fri, Jun 09, 2023 at 10:11:21AM +0100, Russell King (Oracle) wrote:
> > Add EEE management to phylink.
> 
> Hi Russell
> 
> I've been working on my EEE patches. I have all pure phylib drivers
> converted. I've incorporated these four patches as well, and make use
> of the first patch in phylib.
> 
> Looking at this patch, i don't see a way for the MAC to indicate it
> actually does support EEE. Am i missing it?

If a MAC doesn't support EEE, it won't have the necessary calls to
phylink_*_eee() in its ethtool ops. As can be seen from the mvpp2
patch, mvpp2_ethtool_get_eee() and mvpp2_ethtool_set_eee() are
needed to call the phylink methods.

Given that a MAC has to provide those hooks, why would we need a
capability for EEE? Are you thinking that it may be optional for
some MACs?

Thinking of the future (not having done a lot of research though) it
may be appropriate to have a bitmap of... I was going to say ethtool
modes but that doesn't really work... phy interface modes that the MAC
can support EEE. I'm thinking of devices such as mvpp2 where EEE is
supported by the GMAC (for <=2.5G) but not XLG (for >= 5G).

If we use phy interface modes, we somehow need to turn that into
ethtool link modes for the media side, which is e.g. PHY dependent.
For example, the Aquantia PHYs doing rate adaption to 10G plugged
into mvpp2 (which probably doesn't work too well due to the lack
of pause support in mvpp2 hardware) won't be able to do EEE at any
speed because it'll be only using the XLG, but a PHY that uses SGMII
connected to mvpp2 can because that will use GMAC.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC net-next 2/4] net: phylink: add EEE management

[Andrew Lunn]

On Sun, Jun 11, 2023 at 10:37:55PM +0100, Russell King (Oracle) wrote:
> On Sun, Jun 11, 2023 at 11:28:32PM +0200, Andrew Lunn wrote:
> > On Fri, Jun 09, 2023 at 10:11:21AM +0100, Russell King (Oracle) wrote:
> > > Add EEE management to phylink.
> > 
> > Hi Russell
> > 
> > I've been working on my EEE patches. I have all pure phylib drivers
> > converted. I've incorporated these four patches as well, and make use
> > of the first patch in phylib.
> > 
> > Looking at this patch, i don't see a way for the MAC to indicate it
> > actually does support EEE. Am i missing it?
> 
> If a MAC doesn't support EEE, it won't have the necessary calls to
> phylink_*_eee() in its ethtool ops. As can be seen from the mvpp2
> patch, mvpp2_ethtool_get_eee() and mvpp2_ethtool_set_eee() are
> needed to call the phylink methods.

This is a bit messy for stmmac. Some versions of stmmac have EEE
support, some don't. Same is true for r8169, but that is a phylib
driver. I expect there are other examples.

We also have the same problem with DSA. There is currently one
phylink_mac_ops structure which has generic methods for all the
callbacks which then call into the switch driver if the switch driver
implements the call. And at least with the mv88e6xxx driver, when we
get around to adding EEE support, some switches will support it, some
won't, so will we need two different switch op structures?

Adding to the mac_capabilities just seems easier.

       Andrew

Re: [PATCH RFC net-next 2/4] net: phylink: add EEE management

[Russell King (Oracle)]

On Mon, Jun 12, 2023 at 12:25:29AM +0200, Andrew Lunn wrote:
> On Sun, Jun 11, 2023 at 10:37:55PM +0100, Russell King (Oracle) wrote:
> > On Sun, Jun 11, 2023 at 11:28:32PM +0200, Andrew Lunn wrote:
> > > On Fri, Jun 09, 2023 at 10:11:21AM +0100, Russell King (Oracle) wrote:
> > > > Add EEE management to phylink.
> > > 
> > > Hi Russell
> > > 
> > > I've been working on my EEE patches. I have all pure phylib drivers
> > > converted. I've incorporated these four patches as well, and make use
> > > of the first patch in phylib.
> > > 
> > > Looking at this patch, i don't see a way for the MAC to indicate it
> > > actually does support EEE. Am i missing it?
> > 
> > If a MAC doesn't support EEE, it won't have the necessary calls to
> > phylink_*_eee() in its ethtool ops. As can be seen from the mvpp2
> > patch, mvpp2_ethtool_get_eee() and mvpp2_ethtool_set_eee() are
> > needed to call the phylink methods.
> 
> This is a bit messy for stmmac. Some versions of stmmac have EEE
> support, some don't. Same is true for r8169, but that is a phylib
> driver. I expect there are other examples.

I see what you mean, but I think really what's going on there is
whether the MAC supports LPI or not coupled with whether the PHY has
any "smartEEE" feature meaning overall the system doesn't support
any kind of EEE.

The reason I pick that level of detail is when one comes to a setup
like i.MX6 FEC coupled with an AR8035 with it's SmartEEE, the system
as a whole supports EEE but the MAC doesn't. So, while the FEC would
have a MAC_CAP_EEE flag clear, we don't actually want to disable EEE
support with that setup.

> We also have the same problem with DSA. There is currently one
> phylink_mac_ops structure which has generic methods for all the
> callbacks which then call into the switch driver if the switch driver
> implements the call. And at least with the mv88e6xxx driver, when we
> get around to adding EEE support, some switches will support it, some
> won't, so will we need two different switch op structures?

I'd rather not end up with multiple mac_ops.

> Adding to the mac_capabilities just seems easier.

It does, but I suspect MAC_CAP_EEE is not sufficient because:

a) the issue of PHYs with SmartEEE like features such as AR803x.
b) an interface which has multiple MACs that it switches between
   depending on speed may have differing levels of LPI support,
   so a single property doesn't seem to be suitable.
c) rate adapting PHYs that may connect only with e.g. a 10G MAC that
   doesn't support LPI, but the MAC setup does have a 1G MAC that
   does.

I'm wondering if, rather than adding a bit to mac_capabilities, whether
instead:

1) add eee_capabilities and re-use the existing MAC_CAP_* definitions
   to indicate what speeds the MAC supports LPI. This doesn't seem to
   solve (c).
2) add a phy interface bitmap indicating which interface modes support
   LPI generation.

Phylib already has similar with its supported_eee link mode bitmap,
which presumably MACs can knock out link modes that they know they
wouldn't support.

Hmm.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC net-next 2/4] net: phylink: add EEE management

[Andrew Lunn]

> I'm wondering if, rather than adding a bit to mac_capabilities, whether
> instead:
> 
> 1) add eee_capabilities and re-use the existing MAC_CAP_* definitions
>    to indicate what speeds the MAC supports LPI. This doesn't seem to
>    solve (c).
> 2) add a phy interface bitmap indicating which interface modes support
>    LPI generation.
> 
> Phylib already has similar with its supported_eee link mode bitmap,
> which presumably MACs can knock out link modes that they know they
> wouldn't support.

O.K, I can probably make that work. None of the MAC drivers i've
looked at need this flexibility yet, but we can add it now.

I do however wounder if it should be called lpi_capabilities, not
eee_capabilities. These patches are all about making the core deal
with 99% of EEE. All the MAC driver needs to do is enable/disable
sending LPI and set the timer value. So we are really talking about
the MACs LPI capabilities.

	Andrew

Re: [PATCH RFC net-next 2/4] net: phylink: add EEE management

[Russell King (Oracle)]

On Tue, Jun 13, 2023 at 02:26:22PM +0200, Andrew Lunn wrote:
> > I'm wondering if, rather than adding a bit to mac_capabilities, whether
> > instead:
> > 
> > 1) add eee_capabilities and re-use the existing MAC_CAP_* definitions
> >    to indicate what speeds the MAC supports LPI. This doesn't seem to
> >    solve (c).
> > 2) add a phy interface bitmap indicating which interface modes support
> >    LPI generation.
> > 
> > Phylib already has similar with its supported_eee link mode bitmap,
> > which presumably MACs can knock out link modes that they know they
> > wouldn't support.
> 
> O.K, I can probably make that work. None of the MAC drivers i've
> looked at need this flexibility yet, but we can add it now.
> 
> I do however wounder if it should be called lpi_capabilities, not
> eee_capabilities. These patches are all about making the core deal
> with 99% of EEE. All the MAC driver needs to do is enable/disable
> sending LPI and set the timer value. So we are really talking about
> the MACs LPI capabilities.

No problem with calling it lpi_capabilities.

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!