public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/2] Add Frame Preemption MAC Merge support for ICSSG
@ 2026-02-24 12:48 Meghana Malladi
  2026-02-24 12:48 ` [PATCH net-next v4 1/2] net: ti: icssg-prueth: Add Frame Preemption MAC Merge support Meghana Malladi
  2026-02-24 12:48 ` [PATCH net-next v4 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge Meghana Malladi
  0 siblings, 2 replies; 6+ messages in thread
From: Meghana Malladi @ 2026-02-24 12:48 UTC (permalink / raw)
  To: haokexin, vadim.fedorenko, jacob.e.keller, m-malladi, horms,
	basharath, parvathi, afd, vladimir.oltean, rogerq, danishanwar,
	pabeni, kuba, edumazet, davem, andrew+netdev
  Cc: linux-arm-kernel, netdev, linux-kernel, srk, Vignesh Raghavendra

This patch series adds QoS support to the ICSSG PRUETH driver.
The first patch implements mqprio qdisc handling and TC offload hooks
so userspace can request TC mappings and queue counts.

It also integrates a driver-side mechanism to program the firmware
with the IET/FPE preemption mask and to kick the firmware verify state
machine when frame preemption is enabled. The second patch adds ethtool
perations for the MAC Merge (Frame Preemption) sublayer, exposing .get_mm,
.set_mm and .get_mm_stats so admins can view and change MAC Merge
parameters and retrieve preemption statistics.

v3: https://lore.kernel.org/all/20260205184747.2451816-1-m-malladi@ti.com/

MD Danish Anwar (2):
  net: ti: icssg-prueth: Add Frame Preemption MAC Merge support
  net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge

 drivers/net/ethernet/ti/Makefile              |   2 +-
 drivers/net/ethernet/ti/icssg/icssg_config.h  |   9 -
 drivers/net/ethernet/ti/icssg/icssg_ethtool.c |  98 +++++++-
 drivers/net/ethernet/ti/icssg/icssg_prueth.c  |  10 +
 drivers/net/ethernet/ti/icssg/icssg_prueth.h  |   7 +-
 drivers/net/ethernet/ti/icssg/icssg_qos.c     | 223 ++++++++++++++++++
 drivers/net/ethernet/ti/icssg/icssg_qos.h     |  80 +++++++
 drivers/net/ethernet/ti/icssg/icssg_stats.c   |   1 -
 drivers/net/ethernet/ti/icssg/icssg_stats.h   |   5 +
 .../net/ethernet/ti/icssg/icssg_switch_map.h  |   5 +
 10 files changed, 426 insertions(+), 14 deletions(-)
 create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.c
 create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.h


base-commit: 8bf22c33e7a172fbc72464f4cc484d23a6b412ba
-- 
2.43.0


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

* [PATCH net-next v4 1/2] net: ti: icssg-prueth: Add Frame Preemption MAC Merge support
  2026-02-24 12:48 [PATCH net-next v4 0/2] Add Frame Preemption MAC Merge support for ICSSG Meghana Malladi
@ 2026-02-24 12:48 ` Meghana Malladi
  2026-02-26 16:19   ` Vladimir Oltean
  2026-02-24 12:48 ` [PATCH net-next v4 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge Meghana Malladi
  1 sibling, 1 reply; 6+ messages in thread
From: Meghana Malladi @ 2026-02-24 12:48 UTC (permalink / raw)
  To: haokexin, vadim.fedorenko, jacob.e.keller, m-malladi, horms,
	basharath, parvathi, afd, vladimir.oltean, rogerq, danishanwar,
	pabeni, kuba, edumazet, davem, andrew+netdev
  Cc: linux-arm-kernel, netdev, linux-kernel, srk, Vignesh Raghavendra

From: MD Danish Anwar <danishanwar@ti.com>

This patch introduces qos support for the icssg driver. This
includes adding support to configure mqprio qdisc and IET FPE.
By default all the queues are marked as express which can be
overwritten by the mqprio tc mask passed by tc qdisc.

icssg_config_ietfpe() work thread takes care of configuring
IET FPE in the firmware and triggering the verify state machine
based on the MAC Merge sublayer parameters set by the ethtool.
The firmware handles the cleanup after successful mac verification.
And in case the remote peer fails to respond to verify command
before the timeout (5secs), then FPE is disabled by firmware.

During link up/down, verify state machine gets triggered again
based on the state of fpe_enabled and fpe_active.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---

v4-v3:
- Cancel work_sync during interface down and module removal time as suggested
  by Paolo <pabeni@redhat.com>
- Added INIT_WORK() inside prueth_netdev_init() instead of emac_ndo_open() as
  flagged by AI-generated review.
- Add mutex protection to serialize FPE configuration access.

 drivers/net/ethernet/ti/Makefile             |   2 +-
 drivers/net/ethernet/ti/icssg/icssg_config.h |   9 -
 drivers/net/ethernet/ti/icssg/icssg_prueth.c |  10 +
 drivers/net/ethernet/ti/icssg/icssg_prueth.h |   2 +
 drivers/net/ethernet/ti/icssg/icssg_qos.c    | 223 +++++++++++++++++++
 drivers/net/ethernet/ti/icssg/icssg_qos.h    |  60 +++++
 6 files changed, 296 insertions(+), 10 deletions(-)
 create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.c
 create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.h

diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
index 6da50f4b7c2e..6893baf47d46 100644
--- a/drivers/net/ethernet/ti/Makefile
+++ b/drivers/net/ethernet/ti/Makefile
@@ -35,7 +35,7 @@ ti-am65-cpsw-nuss-$(CONFIG_TI_K3_AM65_CPSW_SWITCHDEV) += am65-cpsw-switchdev.o
 obj-$(CONFIG_TI_K3_AM65_CPTS) += am65-cpts.o
 
 obj-$(CONFIG_TI_ICSSG_PRUETH) += icssg-prueth.o icssg.o
-icssg-prueth-y := icssg/icssg_prueth.o icssg/icssg_switchdev.o
+icssg-prueth-y := icssg/icssg_prueth.o icssg/icssg_switchdev.o icssg/icssg_qos.o
 
 obj-$(CONFIG_TI_ICSSG_PRUETH_SR1) += icssg-prueth-sr1.o icssg.o
 icssg-prueth-sr1-y := icssg/icssg_prueth_sr1.o
diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
index 60d69744ffae..1ac202f855ed 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_config.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
@@ -323,13 +323,4 @@ struct prueth_fdb_slot {
 	u8 fid;
 	u8 fid_c2;
 } __packed;
-
-enum icssg_ietfpe_verify_states {
-	ICSSG_IETFPE_STATE_UNKNOWN = 0,
-	ICSSG_IETFPE_STATE_INITIAL,
-	ICSSG_IETFPE_STATE_VERIFYING,
-	ICSSG_IETFPE_STATE_SUCCEEDED,
-	ICSSG_IETFPE_STATE_FAILED,
-	ICSSG_IETFPE_STATE_DISABLED
-};
 #endif /* __NET_TI_ICSSG_CONFIG_H */
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 0939994c932f..fc44beda86a5 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -374,9 +374,11 @@ static void emac_adjust_link(struct net_device *ndev)
 			spin_unlock_irqrestore(&emac->lock, flags);
 			icssg_config_set_speed(emac);
 			icssg_set_port_state(emac, ICSSG_EMAC_PORT_FORWARD);
+			icssg_qos_link_up(ndev);
 
 		} else {
 			icssg_set_port_state(emac, ICSSG_EMAC_PORT_DISABLE);
+			icssg_qos_link_down(ndev);
 		}
 	}
 
@@ -1024,6 +1026,7 @@ static int emac_ndo_stop(struct net_device *ndev)
 	prueth_destroy_rxq(emac);
 
 	cancel_work_sync(&emac->rx_mode_work);
+	cancel_work_sync(&emac->qos.iet.fpe_config_task);
 
 	/* Destroying the queued work in ndo_stop() */
 	cancel_delayed_work_sync(&emac->stats_work);
@@ -1421,6 +1424,7 @@ static const struct net_device_ops emac_netdev_ops = {
 	.ndo_hwtstamp_get = icssg_ndo_get_ts_config,
 	.ndo_hwtstamp_set = icssg_ndo_set_ts_config,
 	.ndo_xsk_wakeup = prueth_xsk_wakeup,
+	.ndo_setup_tc = icssg_qos_ndo_setup_tc,
 };
 
 static int prueth_netdev_init(struct prueth *prueth,
@@ -1455,6 +1459,8 @@ static int prueth_netdev_init(struct prueth *prueth,
 
 	INIT_DELAYED_WORK(&emac->stats_work, icssg_stats_work_handler);
 
+	icssg_qos_init(ndev);
+
 	ret = pruss_request_mem_region(prueth->pruss,
 				       port == PRUETH_PORT_MII0 ?
 				       PRUSS_MEM_DRAM0 : PRUSS_MEM_DRAM1,
@@ -2230,6 +2236,8 @@ static int prueth_probe(struct platform_device *pdev)
 		}
 		unregister_netdev(prueth->registered_netdevs[i]);
 		disable_work_sync(&prueth->emac[i]->rx_mode_work);
+		disable_work_sync(&prueth->emac[i]->qos.iet.fpe_config_task);
+		mutex_destroy(&prueth->emac[i]->qos.iet.fpe_lock);
 	}
 
 netdev_exit:
@@ -2290,6 +2298,8 @@ static void prueth_remove(struct platform_device *pdev)
 		prueth->emac[i]->ndev->phydev = NULL;
 		unregister_netdev(prueth->registered_netdevs[i]);
 		disable_work_sync(&prueth->emac[i]->rx_mode_work);
+		disable_work_sync(&prueth->emac[i]->qos.iet.fpe_config_task);
+		mutex_destroy(&prueth->emac[i]->qos.iet.fpe_lock);
 	}
 
 	for (i = 0; i < PRUETH_NUM_MACS; i++) {
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 3d94fa5a7ac1..37de534e4d43 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -44,6 +44,7 @@
 #include "icssg_config.h"
 #include "icss_iep.h"
 #include "icssg_switch_map.h"
+#include "icssg_qos.h"
 
 #define PRUETH_MAX_MTU          (2000 - ETH_HLEN - ETH_FCS_LEN)
 #define PRUETH_MIN_PKT_SIZE     (VLAN_ETH_ZLEN)
@@ -254,6 +255,7 @@ struct prueth_emac {
 	struct bpf_prog *xdp_prog;
 	struct xdp_attachment_info xdpi;
 	int xsk_qid;
+	struct prueth_qos qos;
 };
 
 /* The buf includes headroom compatible with both skb and xdpf */
diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.c b/drivers/net/ethernet/ti/icssg/icssg_qos.c
new file mode 100644
index 000000000000..388dfcea426b
--- /dev/null
+++ b/drivers/net/ethernet/ti/icssg/icssg_qos.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Texas Instruments ICSSG PRUETH QoS submodule
+ * Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+#include "icssg_prueth.h"
+#include "icssg_switch_map.h"
+
+static void icssg_iet_set_preempt_mask(struct prueth_emac *emac, u8 preemptible_tcs)
+{
+	void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
+	struct prueth_qos_mqprio *p_mqprio = &emac->qos.mqprio;
+	struct tc_mqprio_qopt *qopt = &p_mqprio->mqprio.qopt;
+	int prempt_mask = 0, i;
+	u8 tc;
+
+	/* Configure the queues based on the preemptible tc map set by the user */
+	for (tc = 0; tc < p_mqprio->mqprio.qopt.num_tc; tc++) {
+		/* check if the tc is preemptive or not */
+		if (preemptible_tcs & BIT(tc)) {
+			for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
+				/* Set all the queues in this tc as preemptive queues */
+				writeb(BIT(4), config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
+				prempt_mask &= ~BIT(i);
+			}
+		} else {
+			/* Set all the queues in this tc as express queues */
+			for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
+				writeb(0, config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
+				prempt_mask |= BIT(i);
+			}
+		}
+		netdev_set_tc_queue(emac->ndev, tc, qopt->count[tc], qopt->offset[tc]);
+	}
+	writeb(prempt_mask, config + EXPRESS_PRE_EMPTIVE_Q_MASK);
+}
+
+static void icssg_config_ietfpe(struct work_struct *work)
+{
+	struct prueth_qos_iet *iet =
+		container_of(work, struct prueth_qos_iet, fpe_config_task);
+	void __iomem *config = iet->emac->dram.va + ICSSG_CONFIG_OFFSET;
+	struct prueth_qos_mqprio *p_mqprio =  &iet->emac->qos.mqprio;
+	bool enable = !!atomic_read(&iet->enable_fpe_config);
+	int ret;
+	u8 val;
+
+	if (!netif_running(iet->emac->ndev))
+		return;
+
+	mutex_lock(&iet->fpe_lock);
+
+	/* Update FPE Tx enable bit (PRE_EMPTION_ENABLE_TX) if
+	 * fpe_enabled is set to enable MM in Tx direction
+	 */
+	writeb(enable ? 1 : 0, config + PRE_EMPTION_ENABLE_TX);
+
+	/* If FPE is to be enabled, first configure MAC Verify state
+	 * machine in firmware as firmware kicks the Verify process
+	 * as soon as ICSSG_EMAC_PORT_PREMPT_TX_ENABLE command is
+	 * received.
+	 */
+	if (enable && iet->mac_verify_configure) {
+		writeb(1, config + PRE_EMPTION_ENABLE_VERIFY);
+		writew(iet->tx_min_frag_size, config + PRE_EMPTION_ADD_FRAG_SIZE_LOCAL);
+		writel(iet->verify_time_ms, config + PRE_EMPTION_VERIFY_TIME);
+	}
+
+	/* Send command to enable FPE Tx side. Rx is always enabled */
+	ret = icssg_set_port_state(iet->emac,
+				   enable ? ICSSG_EMAC_PORT_PREMPT_TX_ENABLE :
+					    ICSSG_EMAC_PORT_PREMPT_TX_DISABLE);
+	if (ret) {
+		netdev_err(iet->emac->ndev, "TX preempt %s command failed\n",
+			   str_enable_disable(enable));
+		writeb(0, config + PRE_EMPTION_ENABLE_VERIFY);
+		iet->verify_status = ICSSG_IETFPE_STATE_DISABLED;
+		goto unlock;
+	}
+
+	if (enable && iet->mac_verify_configure) {
+		ret = readb_poll_timeout(config + PRE_EMPTION_VERIFY_STATUS, iet->verify_status,
+					 (iet->verify_status == ICSSG_IETFPE_STATE_SUCCEEDED),
+					 USEC_PER_MSEC, 5 * USEC_PER_SEC);
+		if (ret) {
+			iet->verify_status = ICSSG_IETFPE_STATE_FAILED;
+			netdev_err(iet->emac->ndev,
+				   "timeout for MAC Verify: status %x\n",
+				   iet->verify_status);
+			goto unlock;
+		}
+	} else if (enable) {
+		/* Give f/w some time to update PRE_EMPTION_ACTIVE_TX state */
+		usleep_range(100, 200);
+	}
+
+	if (enable) {
+		val = readb(config + PRE_EMPTION_ACTIVE_TX);
+		if (val != 1) {
+			netdev_err(iet->emac->ndev,
+				   "F/w fails to activate IET/FPE\n");
+			goto unlock;
+		}
+		iet->fpe_active = true;
+	} else {
+		iet->fpe_active = false;
+	}
+
+	netdev_info(iet->emac->ndev, "IET FPE %s successfully\n",
+		    str_enable_disable(iet->fpe_active));
+	icssg_iet_set_preempt_mask(iet->emac, p_mqprio->preemptible_tcs);
+
+unlock:
+	mutex_unlock(&iet->fpe_lock);
+}
+
+void icssg_qos_init(struct net_device *ndev)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct prueth_qos_iet *iet = &emac->qos.iet;
+
+	/* Init work queue for IET MAC verify process */
+	iet->emac = emac;
+	INIT_WORK(&iet->fpe_config_task, icssg_config_ietfpe);
+	mutex_init(&iet->fpe_lock);
+}
+
+static int emac_tc_query_caps(struct net_device *ndev, void *type_data)
+{
+	struct tc_query_caps_base *base = type_data;
+
+	switch (base->type) {
+	case TC_SETUP_QDISC_MQPRIO: {
+		struct tc_mqprio_caps *caps = base->caps;
+
+		caps->validate_queue_counts = true;
+		return 0;
+	}
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int emac_tc_setup_mqprio(struct net_device *ndev, void *type_data)
+{
+	struct tc_mqprio_qopt_offload *mqprio = type_data;
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
+	struct prueth_qos_mqprio *p_mqprio;
+	u8 num_tc = mqprio->qopt.num_tc;
+	int tc, offset, count;
+
+	p_mqprio = &emac->qos.mqprio;
+
+	if (!num_tc) {
+		netdev_reset_tc(ndev);
+		p_mqprio->preemptible_tcs = 0;
+		goto reset_tcs;
+	}
+
+	memcpy(&p_mqprio->mqprio, mqprio, sizeof(*mqprio));
+	p_mqprio->preemptible_tcs = mqprio->preemptible_tcs;
+	netdev_set_num_tc(ndev, mqprio->qopt.num_tc);
+
+	for (tc = 0; tc < num_tc; tc++) {
+		count = qopt->count[tc];
+		offset = qopt->offset[tc];
+		netdev_set_tc_queue(ndev, tc, count, offset);
+	}
+
+reset_tcs:
+	mutex_lock(&emac->qos.iet.fpe_lock);
+	icssg_iet_set_preempt_mask(emac, p_mqprio->preemptible_tcs);
+	mutex_unlock(&emac->qos.iet.fpe_lock);
+
+	return 0;
+}
+
+int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
+			   void *type_data)
+{
+	switch (type) {
+	case TC_QUERY_CAPS:
+		return emac_tc_query_caps(ndev, type_data);
+	case TC_SETUP_QDISC_MQPRIO:
+		return emac_tc_setup_mqprio(ndev, type_data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+void icssg_qos_link_up(struct net_device *ndev)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct prueth_qos_iet *iet = &emac->qos.iet;
+
+	/* Enable FPE if not active but fpe_enabled is true
+	 * and disable FPE if active but fpe_enabled is false
+	 */
+	if (!iet->fpe_active && iet->fpe_enabled) {
+		/* Schedule IET FPE enable */
+		atomic_set(&iet->enable_fpe_config, 1);
+	} else if (iet->fpe_active && !iet->fpe_enabled) {
+		/* Schedule IET FPE disable */
+		atomic_set(&iet->enable_fpe_config, 0);
+	} else {
+		return;
+	}
+	schedule_work(&iet->fpe_config_task);
+}
+
+void icssg_qos_link_down(struct net_device *ndev)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct prueth_qos_iet *iet = &emac->qos.iet;
+
+	/* disable FPE if active during link down */
+	if (iet->fpe_active) {
+		/* Schedule IET FPE disable */
+		atomic_set(&iet->enable_fpe_config, 0);
+		schedule_work(&iet->fpe_config_task);
+	}
+}
diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.h b/drivers/net/ethernet/ti/icssg/icssg_qos.h
new file mode 100644
index 000000000000..653dbb57791d
--- /dev/null
+++ b/drivers/net/ethernet/ti/icssg/icssg_qos.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/
+ */
+
+#ifndef __NET_TI_ICSSG_QOS_H
+#define __NET_TI_ICSSG_QOS_H
+
+#include <linux/atomic.h>
+#include <linux/netdevice.h>
+#include <net/pkt_sched.h>
+
+enum icssg_ietfpe_verify_states {
+	ICSSG_IETFPE_STATE_UNKNOWN = 0,
+	ICSSG_IETFPE_STATE_INITIAL,
+	ICSSG_IETFPE_STATE_VERIFYING,
+	ICSSG_IETFPE_STATE_SUCCEEDED,
+	ICSSG_IETFPE_STATE_FAILED,
+	ICSSG_IETFPE_STATE_DISABLED
+};
+
+struct prueth_qos_mqprio {
+	struct tc_mqprio_qopt_offload mqprio;
+	u8 preemptible_tcs;
+};
+
+struct prueth_qos_iet {
+	struct work_struct fpe_config_task;
+	struct prueth_emac *emac;
+	atomic_t enable_fpe_config;
+	/* Set when IET frame preemption is enabled via ethtool */
+	bool fpe_enabled;
+	/* Set when the IET MAC Verify state machine is enabled
+	 * via ethtool
+	 */
+	bool mac_verify_configure;
+	/* Min TX fragment size, set via ethtool */
+	u32 tx_min_frag_size;
+	/* wait time between verification attempts in ms (according to clause
+	 * 30.14.1.6 aMACMergeVerifyTime), set via ethtool
+	 */
+	u32 verify_time_ms;
+	/* Set if IET FPE is active */
+	bool fpe_active;
+	/* State of verification state machine */
+	enum icssg_ietfpe_verify_states verify_status;
+	/* Mutex to serialize FPE configuration access */
+	struct mutex fpe_lock;
+};
+
+struct prueth_qos {
+	struct prueth_qos_iet iet;
+	struct prueth_qos_mqprio mqprio;
+};
+
+void icssg_qos_init(struct net_device *ndev);
+void icssg_qos_link_up(struct net_device *ndev);
+void icssg_qos_link_down(struct net_device *ndev);
+int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
+			   void *type_data);
+#endif /* __NET_TI_ICSSG_QOS_H */
-- 
2.43.0


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

* [PATCH net-next v4 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge
  2026-02-24 12:48 [PATCH net-next v4 0/2] Add Frame Preemption MAC Merge support for ICSSG Meghana Malladi
  2026-02-24 12:48 ` [PATCH net-next v4 1/2] net: ti: icssg-prueth: Add Frame Preemption MAC Merge support Meghana Malladi
@ 2026-02-24 12:48 ` Meghana Malladi
  2026-02-26 18:08   ` Vladimir Oltean
  2026-03-01 15:10   ` [net-next,v4,2/2] " Simon Horman
  1 sibling, 2 replies; 6+ messages in thread
From: Meghana Malladi @ 2026-02-24 12:48 UTC (permalink / raw)
  To: haokexin, vadim.fedorenko, jacob.e.keller, m-malladi, horms,
	basharath, parvathi, afd, vladimir.oltean, rogerq, danishanwar,
	pabeni, kuba, edumazet, davem, andrew+netdev
  Cc: linux-arm-kernel, netdev, linux-kernel, srk, Vignesh Raghavendra

From: MD Danish Anwar <danishanwar@ti.com>

Add driver support for viewing / changing the MAC Merge sublayer
parameters and dump the Mac Merge stats via ethtool ops: .set_mm(),
.get_mm() and .get_mm_stats().

The minimum size of non-final mPacket fragments supported by the firmware
without leading errors is 64 Bytes (in octets). Add a check to ensure
user passed tx_min_frag_size argument via ethtool, honors this .
Add pa stats registers to check statistics for preemption, which can be
dumped using ethtool ops.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
Signed-off-by: Meghana Malladi <m-malladi@ti.com>
---

v4-v3:
- Added pa-stats availability check inside emac_get_mm_stats() as flagged
  by AI-generated review.
- Return -EOPNOTSUPP for SR1 devices inside ethtool_*_mm() functions

 drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 98 ++++++++++++++++++-
 drivers/net/ethernet/ti/icssg/icssg_prueth.h  |  5 +-
 drivers/net/ethernet/ti/icssg/icssg_qos.h     | 20 ++++
 drivers/net/ethernet/ti/icssg/icssg_stats.c   |  1 -
 drivers/net/ethernet/ti/icssg/icssg_stats.h   |  5 +
 .../net/ethernet/ti/icssg/icssg_switch_map.h  |  5 +
 6 files changed, 130 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
index b715af21d23a..2176536a0989 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
@@ -6,7 +6,6 @@
  */
 
 #include "icssg_prueth.h"
-#include "icssg_stats.h"
 
 static void emac_get_drvinfo(struct net_device *ndev,
 			     struct ethtool_drvinfo *info)
@@ -294,6 +293,100 @@ static int emac_set_per_queue_coalesce(struct net_device *ndev, u32 queue,
 	return 0;
 }
 
+static int emac_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct prueth_qos_iet *iet = &emac->qos.iet;
+
+	if (emac->is_sr1)
+		return -EOPNOTSUPP;
+
+	state->tx_enabled = iet->fpe_enabled;
+	state->pmac_enabled = true;
+	state->tx_min_frag_size = iet->tx_min_frag_size;
+	/* 64Bytes is the minimum fragment size supported
+	 * by the firmware. <64B leads to min frame errors
+	 */
+	state->rx_min_frag_size = 64;
+	state->tx_active = iet->fpe_active;
+	state->verify_enabled = iet->mac_verify_configure;
+	state->verify_time = iet->verify_time_ms;
+
+	switch (iet->verify_status) {
+	case ICSSG_IETFPE_STATE_DISABLED:
+		state->verify_status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
+		break;
+	case ICSSG_IETFPE_STATE_SUCCEEDED:
+		state->verify_status = ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
+		break;
+	case ICSSG_IETFPE_STATE_FAILED:
+		state->verify_status = ETHTOOL_MM_VERIFY_STATUS_FAILED;
+		break;
+	default:
+		state->verify_status = ETHTOOL_MM_VERIFY_STATUS_UNKNOWN;
+		break;
+	}
+
+	/* 802.3-2018 clause 30.14.1.6, says that the aMACMergeVerifyTime
+	 * variable has a range between 1 and 128 ms inclusive. Limit to that.
+	 */
+	state->max_verify_time = ETHTOOL_MM_MAX_VERIFY_TIME_MS;
+
+	return 0;
+}
+
+static int emac_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg,
+		       struct netlink_ext_ack *extack)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+	struct prueth_qos_iet *iet = &emac->qos.iet;
+	int err;
+
+	if (emac->is_sr1)
+		return -EOPNOTSUPP;
+
+	if (!cfg->pmac_enabled)
+		NL_SET_ERR_MSG_MOD(extack, "preemptible MAC is always enabled");
+
+	err = icssg_qos_frag_size_min_to_add(cfg->tx_min_frag_size, extack);
+	if (err)
+		return err;
+
+	iet->verify_time_ms = cfg->verify_time;
+	iet->tx_min_frag_size = cfg->tx_min_frag_size;
+
+	iet->fpe_enabled = cfg->tx_enabled;
+	iet->mac_verify_configure = cfg->verify_enabled;
+
+	/* Re-trigger the state machine to incorporate the updated configuration */
+	if (iet->fpe_enabled)
+		atomic_set(&iet->enable_fpe_config, 1);
+	else
+		atomic_set(&iet->enable_fpe_config, 0);
+
+	schedule_work(&iet->fpe_config_task);
+
+	return 0;
+}
+
+static void emac_get_mm_stats(struct net_device *ndev,
+			      struct ethtool_mm_stats *s)
+{
+	struct prueth_emac *emac = netdev_priv(ndev);
+
+	if (emac->is_sr1)
+		return;
+
+	if (!emac->prueth->pa_stats)
+		return;
+
+	s->MACMergeFrameAssOkCount = emac_get_stat_by_name(emac, "FW_PREEMPT_ASSEMBLY_OK");
+	s->MACMergeFrameAssErrorCount = emac_get_stat_by_name(emac, "FW_PREEMPT_ASSEMBLY_ERR");
+	s->MACMergeFragCountRx = emac_get_stat_by_name(emac, "FW_PREEMPT_FRAG_CNT_RX");
+	s->MACMergeFragCountTx = emac_get_stat_by_name(emac, "FW_PREEMPT_FRAG_CNT_TX");
+	s->MACMergeFrameSmdErrorCount = emac_get_stat_by_name(emac, "FW_PREEMPT_BAD_FRAG");
+}
+
 const struct ethtool_ops icssg_ethtool_ops = {
 	.get_drvinfo = emac_get_drvinfo,
 	.get_msglevel = emac_get_msglevel,
@@ -317,5 +410,8 @@ const struct ethtool_ops icssg_ethtool_ops = {
 	.set_eee = emac_set_eee,
 	.nway_reset = emac_nway_reset,
 	.get_rmon_stats = emac_get_rmon_stats,
+	.get_mm = emac_get_mm,
+	.set_mm = emac_set_mm,
+	.get_mm_stats = emac_get_mm_stats,
 };
 EXPORT_SYMBOL_GPL(icssg_ethtool_ops);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index 37de534e4d43..d9f95d6edc8d 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -45,6 +45,7 @@
 #include "icss_iep.h"
 #include "icssg_switch_map.h"
 #include "icssg_qos.h"
+#include "icssg_stats.h"
 
 #define PRUETH_MAX_MTU          (2000 - ETH_HLEN - ETH_FCS_LEN)
 #define PRUETH_MIN_PKT_SIZE     (VLAN_ETH_ZLEN)
@@ -58,8 +59,8 @@
 
 #define ICSSG_MAX_RFLOWS	8	/* per slice */
 
-#define ICSSG_NUM_PA_STATS	32
-#define ICSSG_NUM_MIIG_STATS	60
+#define ICSSG_NUM_PA_STATS	ARRAY_SIZE(icssg_all_pa_stats)
+#define ICSSG_NUM_MIIG_STATS	ARRAY_SIZE(icssg_all_miig_stats)
 /* Number of ICSSG related stats */
 #define ICSSG_NUM_STATS (ICSSG_NUM_MIIG_STATS + ICSSG_NUM_PA_STATS)
 #define ICSSG_NUM_STANDARD_STATS 31
diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.h b/drivers/net/ethernet/ti/icssg/icssg_qos.h
index 653dbb57791d..bf84cc1b8282 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_qos.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_qos.h
@@ -57,4 +57,24 @@ void icssg_qos_link_up(struct net_device *ndev);
 void icssg_qos_link_down(struct net_device *ndev);
 int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
 			   void *type_data);
+static inline int icssg_qos_frag_size_min_to_add(u32 min_frag_size,
+						 struct netlink_ext_ack *extack)
+{
+	/* The minimum size of the non-final mPacket supported
+	 * by the firmware is 64B and multiples of 64B.
+	 */
+	if (min_frag_size < 64) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "tx_min_frag_size must be at least 64 bytes");
+		return -EINVAL;
+	}
+
+	if (min_frag_size % (ETH_ZLEN + ETH_FCS_LEN)) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "tx_min_frag_size must be a multiple of 64 bytes");
+		return -EINVAL;
+	}
+
+	return 0;
+}
 #endif /* __NET_TI_ICSSG_QOS_H */
diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.c b/drivers/net/ethernet/ti/icssg/icssg_stats.c
index 7159baa0155c..d27e1c48976f 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_stats.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_stats.c
@@ -6,7 +6,6 @@
  */
 
 #include "icssg_prueth.h"
-#include "icssg_stats.h"
 #include <linux/regmap.h>
 
 #define ICSSG_TX_PACKET_OFFSET	0xA0
diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
index 5ec0b38e0c67..f35ae1b4f846 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
@@ -189,6 +189,11 @@ static const struct icssg_pa_stats icssg_all_pa_stats[] = {
 	ICSSG_PA_STATS(FW_INF_DROP_PRIOTAGGED),
 	ICSSG_PA_STATS(FW_INF_DROP_NOTAG),
 	ICSSG_PA_STATS(FW_INF_DROP_NOTMEMBER),
+	ICSSG_PA_STATS(FW_PREEMPT_BAD_FRAG),
+	ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_ERR),
+	ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_TX),
+	ICSSG_PA_STATS(FW_PREEMPT_ASSEMBLY_OK),
+	ICSSG_PA_STATS(FW_PREEMPT_FRAG_CNT_RX),
 	ICSSG_PA_STATS(FW_RX_EOF_SHORT_FRMERR),
 	ICSSG_PA_STATS(FW_RX_B0_DROP_EARLY_EOF),
 	ICSSG_PA_STATS(FW_TX_JUMBO_FRM_CUTOFF),
diff --git a/drivers/net/ethernet/ti/icssg/icssg_switch_map.h b/drivers/net/ethernet/ti/icssg/icssg_switch_map.h
index 7e053b8af3ec..855fd4ed0b3f 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_switch_map.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_switch_map.h
@@ -256,6 +256,11 @@
 #define FW_INF_DROP_PRIOTAGGED		0x0148
 #define FW_INF_DROP_NOTAG		0x0150
 #define FW_INF_DROP_NOTMEMBER		0x0158
+#define FW_PREEMPT_BAD_FRAG		0x0160
+#define FW_PREEMPT_ASSEMBLY_ERR		0x0168
+#define FW_PREEMPT_FRAG_CNT_TX		0x0170
+#define FW_PREEMPT_ASSEMBLY_OK		0x0178
+#define FW_PREEMPT_FRAG_CNT_RX		0x0180
 #define FW_RX_EOF_SHORT_FRMERR		0x0188
 #define FW_RX_B0_DROP_EARLY_EOF		0x0190
 #define FW_TX_JUMBO_FRM_CUTOFF		0x0198
-- 
2.43.0


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

* Re: [PATCH net-next v4 1/2] net: ti: icssg-prueth: Add Frame Preemption MAC Merge support
  2026-02-24 12:48 ` [PATCH net-next v4 1/2] net: ti: icssg-prueth: Add Frame Preemption MAC Merge support Meghana Malladi
@ 2026-02-26 16:19   ` Vladimir Oltean
  0 siblings, 0 replies; 6+ messages in thread
From: Vladimir Oltean @ 2026-02-26 16:19 UTC (permalink / raw)
  To: Meghana Malladi
  Cc: haokexin, vadim.fedorenko, jacob.e.keller, horms, basharath,
	parvathi, afd, rogerq, danishanwar, pabeni, kuba, edumazet, davem,
	andrew+netdev, linux-arm-kernel, netdev, linux-kernel, srk,
	Vignesh Raghavendra

Hi Meghana,

On Tue, Feb 24, 2026 at 06:18:02PM +0530, Meghana Malladi wrote:
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.c b/drivers/net/ethernet/ti/icssg/icssg_qos.c
> new file mode 100644
> index 000000000000..388dfcea426b
> --- /dev/null
> +++ b/drivers/net/ethernet/ti/icssg/icssg_qos.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Texas Instruments ICSSG PRUETH QoS submodule
> + * Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +
> +#include "icssg_prueth.h"
> +#include "icssg_switch_map.h"
> +
> +static void icssg_iet_set_preempt_mask(struct prueth_emac *emac, u8 preemptible_tcs)
> +{
> +	void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
> +	struct prueth_qos_mqprio *p_mqprio = &emac->qos.mqprio;
> +	struct tc_mqprio_qopt *qopt = &p_mqprio->mqprio.qopt;
> +	int prempt_mask = 0, i;
> +	u8 tc;
> +
> +	/* Configure the queues based on the preemptible tc map set by the user */
> +	for (tc = 0; tc < p_mqprio->mqprio.qopt.num_tc; tc++) {
> +		/* check if the tc is preemptive or not */
> +		if (preemptible_tcs & BIT(tc)) {
> +			for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
> +				/* Set all the queues in this tc as preemptive queues */
> +				writeb(BIT(4), config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
> +				prempt_mask &= ~BIT(i);
> +			}
> +		} else {
> +			/* Set all the queues in this tc as express queues */
> +			for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
> +				writeb(0, config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
> +				prempt_mask |= BIT(i);
> +			}
> +		}
> +		netdev_set_tc_queue(emac->ndev, tc, qopt->count[tc], qopt->offset[tc]);
> +	}
> +	writeb(prempt_mask, config + EXPRESS_PRE_EMPTIVE_Q_MASK);
> +}

Shouldn't the preemptible TCs be committed to hardware only if FPE is
active? The callers pay absolutely no regard to that.

> +
> +static void icssg_config_ietfpe(struct work_struct *work)
> +{
> +	struct prueth_qos_iet *iet =
> +		container_of(work, struct prueth_qos_iet, fpe_config_task);
> +	void __iomem *config = iet->emac->dram.va + ICSSG_CONFIG_OFFSET;
> +	struct prueth_qos_mqprio *p_mqprio =  &iet->emac->qos.mqprio;
> +	bool enable = !!atomic_read(&iet->enable_fpe_config);
> +	int ret;
> +	u8 val;
> +
> +	if (!netif_running(iet->emac->ndev))
> +		return;
> +
> +	mutex_lock(&iet->fpe_lock);
> +
> +	/* Update FPE Tx enable bit (PRE_EMPTION_ENABLE_TX) if
> +	 * fpe_enabled is set to enable MM in Tx direction
> +	 */
> +	writeb(enable ? 1 : 0, config + PRE_EMPTION_ENABLE_TX);
> +
> +	/* If FPE is to be enabled, first configure MAC Verify state
> +	 * machine in firmware as firmware kicks the Verify process
> +	 * as soon as ICSSG_EMAC_PORT_PREMPT_TX_ENABLE command is
> +	 * received.
> +	 */
> +	if (enable && iet->mac_verify_configure) {
> +		writeb(1, config + PRE_EMPTION_ENABLE_VERIFY);
> +		writew(iet->tx_min_frag_size, config + PRE_EMPTION_ADD_FRAG_SIZE_LOCAL);
> +		writel(iet->verify_time_ms, config + PRE_EMPTION_VERIFY_TIME);
> +	}
> +
> +	/* Send command to enable FPE Tx side. Rx is always enabled */
> +	ret = icssg_set_port_state(iet->emac,
> +				   enable ? ICSSG_EMAC_PORT_PREMPT_TX_ENABLE :
> +					    ICSSG_EMAC_PORT_PREMPT_TX_DISABLE);
> +	if (ret) {
> +		netdev_err(iet->emac->ndev, "TX preempt %s command failed\n",
> +			   str_enable_disable(enable));
> +		writeb(0, config + PRE_EMPTION_ENABLE_VERIFY);
> +		iet->verify_status = ICSSG_IETFPE_STATE_DISABLED;
> +		goto unlock;
> +	}
> +
> +	if (enable && iet->mac_verify_configure) {
> +		ret = readb_poll_timeout(config + PRE_EMPTION_VERIFY_STATUS, iet->verify_status,
> +					 (iet->verify_status == ICSSG_IETFPE_STATE_SUCCEEDED),
> +					 USEC_PER_MSEC, 5 * USEC_PER_SEC);

You are sleeping up to 5 seconds in the system_percpu_wq kernel-wide
workqueue, blocking the kernel from making any sort of progress with
other items in this workqueue. As include/linux/workqueue.h puts it:
"Don't queue works which can run for too long.".

I guess you should allocate a driver-private workqueue on which you can
sleep as much as you want. Or make the icssg_config_ietfpe task smarter,
to be stateful and reschedule itself until the PRE_EMPTION_VERIFY_STATUS
changes, or a timeout expires. But that's more complicated.

Side note: I had this question on my mind - all contexts from which you
call schedule_work(&iet->fpe_config_task) are sleepable, so why not just
invoke icssg_config_ietfpe() via a direct function call instead?
I guess that's why - it takes too long to reasonably wait for it from
call sites like ethtool, phylink etc. I would make sure this design
decision is part of the commit message.

But let's not lie to ourselves. Having a deferred fpe_config_task
creates its own problems which you are not handling well.

Consider:
- iet->tx_min_frag_size
- iet->verify_time_ms

Writer is emac_set_mm(), reader is icssg_config_ietfpe(). But the reader
can run concurrently with the writer. This means the reader can pick up
and send to firmware settings in an inconsistent state (old tx_min_frag_size
with new verify_time_ms). Configuration which was never requested by the user.

You have a mutex &iet->fpe_lock. Does it help? No.
You have an atomic &iet->enable_fpe_config. Does it help? Also no.

Please try to think of a synchronization pattern where the config
writer, emac_set_mm(), stops or otherwise blocks out the deferred reader
while it's making changes.

In addition, schedule_work() while the work is already scheduled will do
nothing. So if the fpe_config_task takes close to 5 seconds and the user
sends multiple ethtool --set-mm requests in that time, they will be
ignored or incorrectly processed.

Also, iet->fpe_active is problematic too. It has emac_get_mm(),
icssg_qos_link_up() and icssg_qos_link_down() as readers, and
icssg_config_ietfpe() as writer. But it's not obvious what the correct
access pattern is. These things rarely work correctly by chance :(

I'm sorry that I don't have more time to untangle everything and see
what would work best. As a result, the comments above are just "some"
observations. Please try to be more deliberate with the synchronization
procedures, explain them and I am more than happy to double-check their
sanity. It's just that not much effort seems to have been put into the
current proposal.

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

* Re: [PATCH net-next v4 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge
  2026-02-24 12:48 ` [PATCH net-next v4 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge Meghana Malladi
@ 2026-02-26 18:08   ` Vladimir Oltean
  2026-03-01 15:10   ` [net-next,v4,2/2] " Simon Horman
  1 sibling, 0 replies; 6+ messages in thread
From: Vladimir Oltean @ 2026-02-26 18:08 UTC (permalink / raw)
  To: Meghana Malladi
  Cc: haokexin, vadim.fedorenko, jacob.e.keller, horms, basharath,
	parvathi, afd, rogerq, danishanwar, pabeni, kuba, edumazet, davem,
	andrew+netdev, linux-arm-kernel, netdev, linux-kernel, srk,
	Vignesh Raghavendra

On Tue, Feb 24, 2026 at 06:18:03PM +0530, Meghana Malladi wrote:
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.h b/drivers/net/ethernet/ti/icssg/icssg_qos.h
> index 653dbb57791d..bf84cc1b8282 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_qos.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_qos.h
> @@ -57,4 +57,24 @@ void icssg_qos_link_up(struct net_device *ndev);
>  void icssg_qos_link_down(struct net_device *ndev);
>  int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
>  			   void *type_data);
> +static inline int icssg_qos_frag_size_min_to_add(u32 min_frag_size,
> +						 struct netlink_ext_ack *extack)

This function is poorly named.

It draws obvious inspiration from ethtool_mm_frag_size_min_to_add(),
which had the following meaning: "convert min_frag_size (input) to
addFragSize (output)". That's where the "min *to* add" comes from.

Your function does _not_ do that (nor is it used like that). It returns
negative failure, or 0. But that 0 is confusingly not an addFragSize
value, just "success".

> +{
> +	/* The minimum size of the non-final mPacket supported
> +	 * by the firmware is 64B and multiples of 64B.
> +	 */
> +	if (min_frag_size < 64) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "tx_min_frag_size must be at least 64 bytes");
> +		return -EINVAL;
> +	}
> +
> +	if (min_frag_size % (ETH_ZLEN + ETH_FCS_LEN)) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "tx_min_frag_size must be a multiple of 64 bytes");

The min_frag_size as passed by user space represents a value that
doesn't include the FCS length (this is what the "4" in the addFragSize
formula excludes: "64 x (1 + addFragSize) - 4").

If your firmware takes a value including FCS, make the adjustment
privately to the driver, but accept values from user space that are 4
octets larger than what is programmed to the firmware. This allows the
same user space commands to have the same meaning on your hardware.

> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

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

* Re: [net-next,v4,2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge
  2026-02-24 12:48 ` [PATCH net-next v4 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge Meghana Malladi
  2026-02-26 18:08   ` Vladimir Oltean
@ 2026-03-01 15:10   ` Simon Horman
  1 sibling, 0 replies; 6+ messages in thread
From: Simon Horman @ 2026-03-01 15:10 UTC (permalink / raw)
  To: m-malladi
  Cc: Simon Horman, vladimir.oltean, haokexin, pabeni, rogerq, parvathi,
	edumazet, vigneshr, basharath, andrew+netdev, linux-arm-kernel,
	danishanwar, jacob.e.keller, linux-kernel, netdev, srk, kuba, afd,
	vadim.fedorenko, davem

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge

This commit adds driver support for viewing and changing the MAC Merge
sublayer parameters and dumping MAC Merge statistics via ethtool
operations.

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> index b715af21d23a..2176536a0989 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_ethtool.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_ethtool.c

[ ... ]

> @@ -294,6 +293,100 @@ static int emac_set_per_queue_coalesce(struct net_device *ndev, u32 queue,
>  	return 0;
>  }
>
> +static int emac_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
> +{
> +	struct prueth_emac *emac = netdev_priv(ndev);
> +	struct prueth_qos_iet *iet = &emac->qos.iet;
> +
> +	if (emac->is_sr1)
> +		return -EOPNOTSUPP;
> +
> +	state->tx_enabled = iet->fpe_enabled;
> +	state->pmac_enabled = true;
> +	state->tx_min_frag_size = iet->tx_min_frag_size;
> +	/* 64Bytes is the minimum fragment size supported
> +	 * by the firmware. <64B leads to min frame errors
> +	 */
> +	state->rx_min_frag_size = 64;
> +	state->tx_active = iet->fpe_active;
> +	state->verify_enabled = iet->mac_verify_configure;
> +	state->verify_time = iet->verify_time_ms;
> +
> +	switch (iet->verify_status) {
> +	case ICSSG_IETFPE_STATE_DISABLED:
> +		state->verify_status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
> +		break;
> +	case ICSSG_IETFPE_STATE_SUCCEEDED:
> +		state->verify_status = ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
> +		break;
> +	case ICSSG_IETFPE_STATE_FAILED:
> +		state->verify_status = ETHTOOL_MM_VERIFY_STATUS_FAILED;
> +		break;
> +	default:
> +		state->verify_status = ETHTOOL_MM_VERIFY_STATUS_UNKNOWN;
> +		break;
> +	}

Does this switch statement handle all valid verify_status values?

The switch only explicitly handles ICSSG_IETFPE_STATE_DISABLED,
ICSSG_IETFPE_STATE_SUCCEEDED, and ICSSG_IETFPE_STATE_FAILED, falling
through to ETHTOOL_MM_VERIFY_STATUS_UNKNOWN for any other states.

Looking at the icssg_ietfpe_verify_states enum, there are also
ICSSG_IETFPE_STATE_INITIAL and ICSSG_IETFPE_STATE_VERIFYING states. The
ethtool UAPI defines ETHTOOL_MM_VERIFY_STATUS_INITIAL and
ETHTOOL_MM_VERIFY_STATUS_VERIFYING which directly correspond to these
states.

When the MAC Verify process is actively running, userspace will see
'unknown' instead of the more accurate 'verifying' status. This means
information about the ongoing state machine transition is lost.

Should the switch include explicit cases for ICSSG_IETFPE_STATE_INITIAL
and ICSSG_IETFPE_STATE_VERIFYING to provide more accurate status
reporting to userspace?

> +
> +	/* 802.3-2018 clause 30.14.1.6, says that the aMACMergeVerifyTime
> +	 * variable has a range between 1 and 128 ms inclusive. Limit to that.
> +	 */
> +	state->max_verify_time = ETHTOOL_MM_MAX_VERIFY_TIME_MS;
> +
> +	return 0;
> +}

[ ... ]

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

end of thread, other threads:[~2026-03-01 15:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-24 12:48 [PATCH net-next v4 0/2] Add Frame Preemption MAC Merge support for ICSSG Meghana Malladi
2026-02-24 12:48 ` [PATCH net-next v4 1/2] net: ti: icssg-prueth: Add Frame Preemption MAC Merge support Meghana Malladi
2026-02-26 16:19   ` Vladimir Oltean
2026-02-24 12:48 ` [PATCH net-next v4 2/2] net: ti: icssg-prueth: Add ethtool ops for Frame Preemption MAC Merge Meghana Malladi
2026-02-26 18:08   ` Vladimir Oltean
2026-03-01 15:10   ` [net-next,v4,2/2] " Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox