2023-08-10 15:45:55

by Roger Quadros

[permalink] [raw]
Subject: [PATCH v2] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support

Add driver support for viewing / changing the MAC Merge sublayer
parameters and seeing the verification state machine's current state
via ethtool.

As hardware does not support interrupt notification for verification
events we resort to polling on link up. On link up we try a couple of
times for verification success and if unsuccessful then give up.

The Frame Preemption feature is described in the Technical Reference
Manual [1] in section:
12.3.1.4.6.7 Intersperced Express Traffic (IET – P802.3br/D2.0)

Due to Silicon Errata i2208 [2] we set limit min IET fragment size to 124.

[1] AM62x TRM - https://www.ti.com/lit/ug/spruiv7a/spruiv7a.pdf
[2] AM62x Silicon Errata - https://www.ti.com/lit/er/sprz487c/sprz487c.pdf

Signed-off-by: Roger Quadros <[email protected]>
---

Hi,

Dropping the RFC tag as I now have MAC merge verification and frame
preemption working.

Changelog:
v2:
- Use proper control bits for PMAC enable (AM65_CPSW_PN_CTL_IET_PORT_EN)
and TX enable (AM65_CPSW_PN_IET_MAC_PENABLE)
- Common IET Enable (AM65_CPSW_CTL_IET_EN) is set if any port has
AM65_CPSW_PN_CTL_IET_PORT_EN set.
- Fix workaround for erratum i2208. i.e. Limit rx_min_frag_size to 124
- Fix am65_cpsw_iet_get_verify_timeout_ms() to default to timeout for
1G link if link is inactive.
- resize the RX FIFO based on pmac_enabled, not tx_enabled.

Test Procedure:

- 2 EVMs with AM65-CPSW network port connected to each other
- Run iet-setup.sh on both

#!/bin/sh
#iet-setup.sh

ifconfig eth0 down
ifconfig eth1 down
ethtool -L eth0 tx 2
ethtool --set-priv-flags eth0 p0-rx-ptype-rrobin off
ethtool --set-mm eth0 pmac-enabled on tx-enabled on verify-enabled on verify-time 10 tx-min-frag-size 124
ifconfig eth0 up
sleep 10

tc qdisc replace dev eth0 handle 8001: parent root stab overhead 24 taprio \
num_tc 2 \
map 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 \
queues 1@0 1@1 \
base-time 0 \
sched-entry S 01 1216 \
sched-entry S fe 12368 \
flags 0x2 \
fp P E E E E E E E

tc -g class show dev eth0
tc qdisc add dev eth0 clsact
tc filter add dev eth0 egress protocol ip prio 1 u32 match ip dport 5002 0xffff action skbedit priority 2
tc filter add dev eth0 egress protocol ip prio 1 u32 match ip dport 5003 0xffff action skbedit priority 3

- check that MAC merge verification has succeeded

ethtool --show-mm eth0

MAC Merge layer state for eth0:
pMAC enabled: on
TX enabled: on
TX active: on
TX minimum fragment size: 124
RX minimum fragment size: 124
Verify enabled: on
Verify time: 10
Max verify time: 134
Verification status: SUCCEEDED

- On receiver EVM run 2 iperf instances

iperf3 -s -i30 -p5002&
iperf3 -s -i30 -p5003&

- On sender EVM run 2 iperf instances

iperf3 -c 192.168.3.102 -u -b200M -l1472 -u -t5 -i30 -p5002&
iperf3 -c 192.168.3.102 -u -b50M -l1472 -u -t5 -i30 -p5003&

- Check IET stats on sender. Look for iet_tx_frag increments

ethtool -S eth0 | grep iet

iet_rx_assembly_err: 0
iet_rx_assembly_ok: 0
iet_rx_smd_err: 0
iet_rx_frag: 0
iet_tx_hold: 0
iet_tx_frag: 17307

- Check IET stats on receiver. Look for iet_rx_frag and iet_rx_assembly_*

ethtool -S eth0 | grep iet

iet_rx_assembly_err: 0
iet_rx_assembly_ok: 17302
iet_rx_smd_err: 0
iet_rx_frag: 17307
iet_tx_hold: 0
iet_tx_frag: 0

cheers,
-roger

drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 139 ++++++++++++
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 +
drivers/net/ethernet/ti/am65-cpsw-nuss.h | 5 +
drivers/net/ethernet/ti/am65-cpsw-qos.c | 234 ++++++++++++++++----
drivers/net/ethernet/ti/am65-cpsw-qos.h | 90 ++++++++
5 files changed, 433 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
index c51e2af91f69..8eb622ffac8e 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
@@ -11,6 +11,7 @@
#include <linux/pm_runtime.h>

#include "am65-cpsw-nuss.h"
+#include "am65-cpsw-qos.h"
#include "cpsw_ale.h"
#include "am65-cpts.h"

@@ -715,6 +716,142 @@ static int am65_cpsw_set_ethtool_priv_flags(struct net_device *ndev, u32 flags)
return 0;
}

+static void am65_cpsw_port_iet_rx_enable(struct am65_cpsw_port *port, bool enable)
+{
+ u32 val;
+
+ val = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
+ if (enable)
+ val |= AM65_CPSW_PN_CTL_IET_PORT_EN;
+ else
+ val &= ~AM65_CPSW_PN_CTL_IET_PORT_EN;
+
+ writel(val, port->port_base + AM65_CPSW_PN_REG_CTL);
+ am65_cpsw_iet_common_enable(port->common);
+}
+
+static void am65_cpsw_port_iet_tx_enable(struct am65_cpsw_port *port, bool enable)
+{
+ u32 val;
+
+ val = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+ if (enable)
+ val |= AM65_CPSW_PN_IET_MAC_PENABLE;
+ else
+ val &= ~AM65_CPSW_PN_IET_MAC_PENABLE;
+
+ writel(val, port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+}
+
+static int am65_cpsw_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
+{
+ struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
+ u32 port_ctrl, cmn_ctrl, iet_ctrl, iet_status, verify_cnt;
+ struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
+ struct am65_cpsw_ndev_priv *priv = netdev_priv(ndev);
+ u32 add_frag_size;
+
+ mutex_lock(&priv->mm_lock);
+
+ iet_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+ cmn_ctrl = readl(common->cpsw_base + AM65_CPSW_REG_CTL);
+ port_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
+
+ state->tx_enabled = !!(iet_ctrl & AM65_CPSW_PN_IET_MAC_PENABLE);
+ state->pmac_enabled = !!(port_ctrl & AM65_CPSW_PN_CTL_IET_PORT_EN);
+
+ iet_status = readl(port->port_base + AM65_CPSW_PN_REG_IET_STATUS);
+
+ if (iet_ctrl & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY)
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
+ else if (iet_status & AM65_CPSW_PN_MAC_VERIFIED)
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
+ else if (iet_status & AM65_CPSW_PN_MAC_VERIFY_FAIL)
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_FAILED;
+ else
+ state->verify_status = ETHTOOL_MM_VERIFY_STATUS_UNKNOWN;
+
+ add_frag_size = AM65_CPSW_PN_IET_MAC_GET_ADDFRAGSIZE(iet_ctrl);
+ state->tx_min_frag_size = ethtool_mm_frag_size_add_to_min(add_frag_size);
+
+ /* Errata i2208: RX min fragment size cannot be less than 124 */
+ state->rx_min_frag_size = 124;
+
+ /* FPE active if common tx_enabled and verification success or disabled (forced) */
+ state->tx_active = state->tx_enabled &&
+ (state->verify_status == ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ||
+ state->verify_status == ETHTOOL_MM_VERIFY_STATUS_DISABLED);
+ state->verify_enabled = !(iet_ctrl & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY);
+
+ verify_cnt = AM65_CPSW_PN_MAC_GET_VERIFY_CNT(readl(port->port_base +
+ AM65_CPSW_PN_REG_IET_VERIFY));
+ state->verify_time = port->qos.iet.verify_time_ms;
+ state->max_verify_time = am65_cpsw_iet_get_verify_timeout_ms(AM65_CPSW_PN_MAC_VERIFY_CNT_MASK,
+ port);
+ mutex_unlock(&priv->mm_lock);
+
+ return 0;
+}
+
+static int am65_cpsw_set_mm(struct net_device *ndev, struct ethtool_mm_cfg *cfg,
+ struct netlink_ext_ack *extack)
+{
+ struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
+ struct am65_cpsw_ndev_priv *priv = netdev_priv(ndev);
+ struct am65_cpsw_iet *iet = &port->qos.iet;
+ u32 val, add_frag_size;
+ int err;
+
+ err = ethtool_mm_frag_size_min_to_add(cfg->tx_min_frag_size, &add_frag_size, extack);
+ if (err)
+ return err;
+
+ mutex_lock(&priv->mm_lock);
+
+ if (cfg->pmac_enabled) {
+ /* change TX & RX FIFO MAX_BLKS as per TRM recommendation */
+ if (!iet->original_max_blks)
+ iet->original_max_blks = readl(port->port_base + AM65_CPSW_PN_REG_MAX_BLKS);
+
+ writel(AM65_CPSW_PN_TX_RX_MAX_BLKS_IET,
+ port->port_base + AM65_CPSW_PN_REG_MAX_BLKS);
+ } else {
+ /* restore RX & TX FIFO MAX_BLKS */
+ if (iet->original_max_blks) {
+ writel(iet->original_max_blks,
+ port->port_base + AM65_CPSW_PN_REG_MAX_BLKS);
+ }
+ }
+
+ am65_cpsw_port_iet_rx_enable(port, cfg->pmac_enabled);
+ am65_cpsw_port_iet_tx_enable(port, cfg->tx_enabled);
+
+ val = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+ if (cfg->verify_enabled) {
+ val &= ~AM65_CPSW_PN_IET_MAC_DISABLEVERIFY;
+ /* Reset Verify state machine. Verification won't start here.
+ * Verification will be done once link-up.
+ */
+ val |= AM65_CPSW_PN_IET_MAC_LINKFAIL;
+ } else {
+ val |= AM65_CPSW_PN_IET_MAC_DISABLEVERIFY;
+ }
+
+ val &= ~AM65_CPSW_PN_IET_MAC_MAC_ADDFRAGSIZE_MASK;
+ val |= AM65_CPSW_PN_IET_MAC_SET_ADDFRAGSIZE(add_frag_size);
+ writel(val, port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+
+ /* verify_timeout_count can only be set at valid link */
+ port->qos.iet.verify_time_ms = cfg->verify_time;
+
+ /* enable/disable pre-emption based on link status */
+ am65_cpsw_iet_commit_preemptible_tcs(port);
+
+ mutex_unlock(&priv->mm_lock);
+
+ return 0;
+}
+
const struct ethtool_ops am65_cpsw_ethtool_ops_slave = {
.begin = am65_cpsw_ethtool_op_begin,
.complete = am65_cpsw_ethtool_op_complete,
@@ -743,4 +880,6 @@ const struct ethtool_ops am65_cpsw_ethtool_ops_slave = {
.get_eee = am65_cpsw_get_eee,
.set_eee = am65_cpsw_set_eee,
.nway_reset = am65_cpsw_nway_reset,
+ .get_mm = am65_cpsw_get_mm,
+ .set_mm = am65_cpsw_set_mm,
};
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index bebcfd5e6b57..b0e2d6773543 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2160,6 +2160,8 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx)
ndev_priv = netdev_priv(port->ndev);
ndev_priv->port = port;
ndev_priv->msg_enable = AM65_CPSW_DEBUG;
+ mutex_init(&ndev_priv->mm_lock);
+ port->qos.link_speed = SPEED_UNKNOWN;
SET_NETDEV_DEV(port->ndev, dev);

eth_hw_addr_set(port->ndev, port->slave.mac_addr);
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.h b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
index bf40c88fbd9b..a7751e2b5b6c 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.h
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
@@ -145,6 +145,7 @@ struct am65_cpsw_common {
bool pf_p0_rx_ptype_rrobin;
struct am65_cpts *cpts;
int est_enabled;
+ bool iet_enabled;

bool is_emac_mode;
u16 br_members;
@@ -170,6 +171,10 @@ struct am65_cpsw_ndev_priv {
struct am65_cpsw_port *port;
struct am65_cpsw_ndev_stats __percpu *stats;
bool offload_fwd_mark;
+ /* Serialize access to MAC Merge state between ethtool requests
+ * and link state updates
+ */
+ struct mutex mm_lock;
};

#define am65_ndev_to_priv(ndev) \
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c b/drivers/net/ethernet/ti/am65-cpsw-qos.c
index eced87fa261c..29d6cb4bbecb 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
@@ -4,9 +4,11 @@
*
* quality of service module includes:
* Enhanced Scheduler Traffic (EST - P802.1Qbv/D2.2)
+ * Interspersed Express Traffic (IET - P802.3br/D2.0)
*/

#include <linux/pm_runtime.h>
+#include <linux/units.h>
#include <linux/time.h>
#include <net/pkt_cls.h>

@@ -15,47 +17,199 @@
#include "am65-cpts.h"
#include "cpsw_ale.h"

-#define AM65_CPSW_REG_CTL 0x004
-#define AM65_CPSW_PN_REG_CTL 0x004
-#define AM65_CPSW_PN_REG_FIFO_STATUS 0x050
-#define AM65_CPSW_PN_REG_EST_CTL 0x060
-#define AM65_CPSW_PN_REG_PRI_CIR(pri) (0x140 + 4 * (pri))
-
-/* AM65_CPSW_REG_CTL register fields */
-#define AM65_CPSW_CTL_EST_EN BIT(18)
-
-/* AM65_CPSW_PN_REG_CTL register fields */
-#define AM65_CPSW_PN_CTL_EST_PORT_EN BIT(17)
-
-/* AM65_CPSW_PN_REG_EST_CTL register fields */
-#define AM65_CPSW_PN_EST_ONEBUF BIT(0)
-#define AM65_CPSW_PN_EST_BUFSEL BIT(1)
-#define AM65_CPSW_PN_EST_TS_EN BIT(2)
-#define AM65_CPSW_PN_EST_TS_FIRST BIT(3)
-#define AM65_CPSW_PN_EST_ONEPRI BIT(4)
-#define AM65_CPSW_PN_EST_TS_PRI_MSK GENMASK(7, 5)
-
-/* AM65_CPSW_PN_REG_FIFO_STATUS register fields */
-#define AM65_CPSW_PN_FST_TX_PRI_ACTIVE_MSK GENMASK(7, 0)
-#define AM65_CPSW_PN_FST_TX_E_MAC_ALLOW_MSK GENMASK(15, 8)
-#define AM65_CPSW_PN_FST_EST_CNT_ERR BIT(16)
-#define AM65_CPSW_PN_FST_EST_ADD_ERR BIT(17)
-#define AM65_CPSW_PN_FST_EST_BUFACT BIT(18)
-
-/* EST FETCH COMMAND RAM */
-#define AM65_CPSW_FETCH_RAM_CMD_NUM 0x80
-#define AM65_CPSW_FETCH_CNT_MSK GENMASK(21, 8)
-#define AM65_CPSW_FETCH_CNT_MAX (AM65_CPSW_FETCH_CNT_MSK >> 8)
-#define AM65_CPSW_FETCH_CNT_OFFSET 8
-#define AM65_CPSW_FETCH_ALLOW_MSK GENMASK(7, 0)
-#define AM65_CPSW_FETCH_ALLOW_MAX AM65_CPSW_FETCH_ALLOW_MSK
-
enum timer_act {
TACT_PROG, /* need program timer */
TACT_NEED_STOP, /* need stop first */
TACT_SKIP_PROG, /* just buffer can be updated */
};

+/* IET */
+static int am65_cpsw_iet_set_verify_timeout_count(struct am65_cpsw_port *port)
+{
+ int verify_time_ms = port->qos.iet.verify_time_ms;
+ int link_speed = port->qos.link_speed;
+ u32 val;
+
+ if (WARN_ON(link_speed == SPEED_UNKNOWN))
+ return -ENODEV;
+
+ /* The number of wireside clocks contained in the verify
+ * timeout counter. The default is 0x1312d0
+ * (10ms at 125Mhz in 1G mode).
+ */
+ val = 125 * HZ_PER_MHZ; /* assuming 125MHz wireside clock */
+
+ val /= MILLIHZ_PER_HZ; /* count per ms timeout */
+ val *= verify_time_ms; /* count for timeout ms */
+ if (link_speed < SPEED_1000)
+ val <<= 1; /* FIXME: Is this correct? */
+
+ if (val > AM65_CPSW_PN_MAC_VERIFY_CNT_MASK)
+ return -EINVAL;
+
+ writel(val, port->port_base + AM65_CPSW_PN_REG_IET_VERIFY);
+
+ return 0;
+}
+
+unsigned int am65_cpsw_iet_get_verify_timeout_ms(u32 count, struct am65_cpsw_port *port)
+{
+ int link_speed = port->qos.link_speed;
+ u32 val = 125 * HZ_PER_MHZ; /* assuming 125MHz wireside clock */
+ unsigned int timeout_ms;
+
+ if (link_speed == SPEED_UNKNOWN)
+ link_speed = SPEED_1000;
+
+ val /= MILLIHZ_PER_HZ; /* count per ms timeout */
+
+ timeout_ms = count / val;
+
+ if (link_speed < SPEED_1000)
+ timeout_ms >>= 1; /* FIXME: Is this correct? */
+
+ return timeout_ms;
+}
+
+static int am65_cpsw_iet_verify_wait(struct am65_cpsw_port *port)
+{
+ u32 ctrl, status;
+ int try;
+
+ try = 20;
+ do {
+ /* Clear MAC_LINKFAIL bit to start Verify. */
+ ctrl = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+ ctrl &= ~AM65_CPSW_PN_IET_MAC_LINKFAIL;
+ writel(ctrl, port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+
+ msleep(port->qos.iet.verify_time_ms);
+
+ status = readl(port->port_base + AM65_CPSW_PN_REG_IET_STATUS);
+ if (status & AM65_CPSW_PN_MAC_VERIFIED)
+ return 0;
+
+ if (status & AM65_CPSW_PN_MAC_VERIFY_FAIL) {
+ netdev_dbg(port->ndev,
+ "MAC Merge verify failed, trying again");
+ /* Reset the verify state machine by writing 1
+ * to LINKFAIL
+ */
+ ctrl = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+ ctrl |= AM65_CPSW_PN_IET_MAC_LINKFAIL;
+ writel(ctrl, port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+ continue;
+ }
+
+ if (status & AM65_CPSW_PN_MAC_RESPOND_ERR) {
+ netdev_dbg(port->ndev, "MAC Merge respond error");
+ return -ENODEV;
+ }
+
+ if (status & AM65_CPSW_PN_MAC_VERIFY_ERR) {
+ netdev_dbg(port->ndev, "MAC Merge verify error");
+ return -ENODEV;
+ }
+ } while (try-- > 0);
+
+ netdev_dbg(port->ndev, "MAC Merge verify timeout");
+ return -ETIMEDOUT;
+}
+
+static void am65_cpsw_iet_set_preempt_mask(struct am65_cpsw_port *port, u8 preemptible_tcs)
+{
+ u32 val;
+
+ val = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+ val &= ~AM65_CPSW_PN_IET_MAC_PREMPT_MASK;
+ val |= AM65_CPSW_PN_IET_MAC_SET_PREEMPT(preemptible_tcs);
+ writel(val, port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+}
+
+/* enable common IET_ENABLE only if at least 1 port has rx IET enabled.
+ * UAPI doesn't allow tx enable without rx enable.
+ */
+void am65_cpsw_iet_common_enable(struct am65_cpsw_common *common)
+{
+ struct am65_cpsw_port *port;
+ bool rx_enable = false;
+ u32 val;
+ int i;
+
+ for (i = 0; i < common->port_num; i++) {
+ port = &common->ports[i];
+ val = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
+ rx_enable = !!(val & AM65_CPSW_PN_CTL_IET_PORT_EN);
+ if (rx_enable)
+ break;
+ }
+
+ val = readl(common->cpsw_base + AM65_CPSW_REG_CTL);
+
+ if (rx_enable)
+ val |= AM65_CPSW_CTL_IET_EN;
+ else
+ val &= ~AM65_CPSW_CTL_IET_EN;
+
+ writel(val, common->cpsw_base + AM65_CPSW_REG_CTL);
+ common->iet_enabled = rx_enable;
+}
+
+/* CPSW does not have an IRQ to notify changes to the MAC Merge TX status
+ * (active/inactive), but the preemptible traffic classes should only be
+ * committed to hardware once TX is active. Resort to polling.
+ */
+void am65_cpsw_iet_commit_preemptible_tcs(struct am65_cpsw_port *port)
+{
+ u8 preemptible_tcs = 0;
+ int err;
+ u32 val;
+
+ if (port->qos.link_speed == SPEED_UNKNOWN)
+ return;
+
+ val = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
+ if (!(val & AM65_CPSW_PN_CTL_IET_PORT_EN))
+ return;
+
+ /* update common IET enable */
+ am65_cpsw_iet_common_enable(port->common);
+
+ /* update verify count */
+ err = am65_cpsw_iet_set_verify_timeout_count(port);
+ if (err) {
+ netdev_err(port->ndev, "couldn't set verify count: %d\n", err);
+ return;
+ }
+
+ val = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
+ if (!(val & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY)) {
+ err = am65_cpsw_iet_verify_wait(port);
+ if (err)
+ return;
+ }
+
+ preemptible_tcs = port->qos.iet.preemptible_tcs;
+ am65_cpsw_iet_set_preempt_mask(port, preemptible_tcs);
+}
+
+void am65_cpsw_iet_change_preemptible_tcs(struct am65_cpsw_port *port, u8 preemptible_tcs)
+{
+ port->qos.iet.preemptible_tcs = preemptible_tcs;
+ am65_cpsw_iet_commit_preemptible_tcs(port);
+}
+
+void am65_cpsw_iet_link_state_update(struct net_device *ndev)
+{
+ struct am65_cpsw_ndev_priv *priv = am65_ndev_to_priv(ndev);
+ struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
+
+ mutex_lock(&priv->mm_lock);
+ am65_cpsw_iet_commit_preemptible_tcs(port);
+ mutex_unlock(&priv->mm_lock);
+}
+
+/* EST */
static int am65_cpsw_port_est_enabled(struct am65_cpsw_port *port)
{
return port->qos.est_oper || port->qos.est_admin;
@@ -541,7 +695,6 @@ static void am65_cpsw_est_link_up(struct net_device *ndev, int link_speed)
ktime_t cur_time;
s64 delta;

- port->qos.link_speed = link_speed;
if (!am65_cpsw_port_est_enabled(port))
return;

@@ -588,6 +741,8 @@ static int am65_cpsw_setup_taprio(struct net_device *ndev, void *type_data)
if (port->qos.link_speed == SPEED_UNKNOWN)
return -ENOLINK;

+ am65_cpsw_iet_change_preemptible_tcs(port, taprio->mqprio.preemptible_tcs);
+
return am65_cpsw_set_taprio(ndev, type_data);
}

@@ -806,6 +961,9 @@ void am65_cpsw_qos_link_up(struct net_device *ndev, int link_speed)
{
struct am65_cpsw_port *port = am65_ndev_to_port(ndev);

+ port->qos.link_speed = link_speed;
+ am65_cpsw_iet_link_state_update(ndev);
+
if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
return;

@@ -817,13 +975,15 @@ void am65_cpsw_qos_link_down(struct net_device *ndev)
{
struct am65_cpsw_port *port = am65_ndev_to_port(ndev);

+ port->qos.link_speed = SPEED_UNKNOWN;
+ am65_cpsw_iet_link_state_update(ndev);
+
if (!IS_ENABLED(CONFIG_TI_AM65_CPSW_TAS))
return;

if (!port->qos.link_down_time)
port->qos.link_down_time = ktime_get();

- port->qos.link_speed = SPEED_UNKNOWN;
}

static u32
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.h b/drivers/net/ethernet/ti/am65-cpsw-qos.h
index 0cc2a3b3d7f9..8045ca1519eb 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.h
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.h
@@ -9,6 +9,7 @@
#include <net/pkt_sched.h>

struct am65_cpsw_common;
+struct am65_cpsw_port;

struct am65_cpsw_est {
int buf;
@@ -16,6 +17,12 @@ struct am65_cpsw_est {
struct tc_taprio_qopt_offload taprio;
};

+struct am65_cpsw_iet {
+ u8 preemptible_tcs;
+ u32 original_max_blks;
+ int verify_time_ms;
+};
+
struct am65_cpsw_ale_ratelimit {
unsigned long cookie;
u64 rate_packet_ps;
@@ -26,6 +33,7 @@ struct am65_cpsw_qos {
struct am65_cpsw_est *est_oper;
ktime_t link_down_time;
int link_speed;
+ struct am65_cpsw_iet iet;

struct am65_cpsw_ale_ratelimit ale_bc_ratelimit;
struct am65_cpsw_ale_ratelimit ale_mc_ratelimit;
@@ -38,4 +46,86 @@ void am65_cpsw_qos_link_down(struct net_device *ndev);
int am65_cpsw_qos_ndo_tx_p0_set_maxrate(struct net_device *ndev, int queue, u32 rate_mbps);
void am65_cpsw_qos_tx_p0_rate_init(struct am65_cpsw_common *common);

+void am65_cpsw_iet_commit_preemptible_tcs(struct am65_cpsw_port *port);
+unsigned int am65_cpsw_iet_get_verify_timeout_ms(u32 count, struct am65_cpsw_port *port);
+void am65_cpsw_iet_common_enable(struct am65_cpsw_common *common);
+
+#define AM65_CPSW_REG_CTL 0x004
+#define AM65_CPSW_PN_REG_CTL 0x004
+#define AM65_CPSW_PN_REG_MAX_BLKS 0x008
+#define AM65_CPSW_PN_REG_IET_CTRL 0x040
+#define AM65_CPSW_PN_REG_IET_STATUS 0x044
+#define AM65_CPSW_PN_REG_IET_VERIFY 0x048
+#define AM65_CPSW_PN_REG_FIFO_STATUS 0x050
+#define AM65_CPSW_PN_REG_EST_CTL 0x060
+#define AM65_CPSW_PN_REG_PRI_CIR(pri) (0x140 + 4 * (pri))
+
+/* AM65_CPSW_REG_CTL register fields */
+#define AM65_CPSW_CTL_IET_EN BIT(17)
+#define AM65_CPSW_CTL_EST_EN BIT(18)
+
+/* AM65_CPSW_PN_REG_CTL register fields */
+#define AM65_CPSW_PN_CTL_IET_PORT_EN BIT(16)
+#define AM65_CPSW_PN_CTL_EST_PORT_EN BIT(17)
+
+/* AM65_CPSW_PN_REG_EST_CTL register fields */
+#define AM65_CPSW_PN_EST_ONEBUF BIT(0)
+#define AM65_CPSW_PN_EST_BUFSEL BIT(1)
+#define AM65_CPSW_PN_EST_TS_EN BIT(2)
+#define AM65_CPSW_PN_EST_TS_FIRST BIT(3)
+#define AM65_CPSW_PN_EST_ONEPRI BIT(4)
+#define AM65_CPSW_PN_EST_TS_PRI_MSK GENMASK(7, 5)
+
+/* AM65_CPSW_PN_REG_IET_CTRL register fields */
+#define AM65_CPSW_PN_IET_MAC_PENABLE BIT(0)
+#define AM65_CPSW_PN_IET_MAC_DISABLEVERIFY BIT(2)
+#define AM65_CPSW_PN_IET_MAC_LINKFAIL BIT(3)
+#define AM65_CPSW_PN_IET_MAC_MAC_ADDFRAGSIZE_MASK GENMASK(10, 8)
+#define AM65_CPSW_PN_IET_MAC_MAC_ADDFRAGSIZE_OFFSET 8
+#define AM65_CPSW_PN_IET_MAC_PREMPT_MASK GENMASK(23, 16)
+#define AM65_CPSW_PN_IET_MAC_PREMPT_OFFSET 16
+
+#define AM65_CPSW_PN_IET_MAC_SET_ADDFRAGSIZE(n) (((n) << AM65_CPSW_PN_IET_MAC_MAC_ADDFRAGSIZE_OFFSET) & \
+ AM65_CPSW_PN_IET_MAC_MAC_ADDFRAGSIZE_MASK)
+#define AM65_CPSW_PN_IET_MAC_GET_ADDFRAGSIZE(n) (((n) & AM65_CPSW_PN_IET_MAC_MAC_ADDFRAGSIZE_MASK) >> \
+ AM65_CPSW_PN_IET_MAC_MAC_ADDFRAGSIZE_OFFSET)
+#define AM65_CPSW_PN_IET_MAC_SET_PREEMPT(n) (((n) << AM65_CPSW_PN_IET_MAC_PREMPT_OFFSET) & \
+ AM65_CPSW_PN_IET_MAC_PREMPT_MASK)
+#define AM65_CPSW_PN_IET_MAC_GET_PREEMPT(n) (((n) & AM65_CPSW_PN_IET_MAC_PREMPT_MASK) >> \
+ AM65_CPSW_PN_IET_MAC_PREMPT_OFFSET)
+
+/* AM65_CPSW_PN_REG_IET_STATUS register fields */
+#define AM65_CPSW_PN_MAC_STATUS GENMASK(3, 0)
+#define AM65_CPSW_PN_MAC_VERIFIED BIT(0)
+#define AM65_CPSW_PN_MAC_VERIFY_FAIL BIT(1)
+#define AM65_CPSW_PN_MAC_RESPOND_ERR BIT(2)
+#define AM65_CPSW_PN_MAC_VERIFY_ERR BIT(3)
+
+/* AM65_CPSW_PN_REG_IET_VERIFY register fields */
+#define AM65_CPSW_PN_MAC_VERIFY_CNT_MASK GENMASK(23, 0)
+#define AM65_CPSW_PN_MAC_GET_VERIFY_CNT(n) ((n) & AM65_CPSW_PN_MAC_VERIFY_CNT_MASK)
+/* 10 msec converted to NSEC */
+#define AM65_CPSW_IET_VERIFY_CNT_MS (10)
+#define AM65_CPSW_IET_VERIFY_CNT_NS (AM65_CPSW_IET_VERIFY_CNT_MS * \
+ NSEC_PER_MSEC)
+
+/* AM65_CPSW_PN_REG_FIFO_STATUS register fields */
+#define AM65_CPSW_PN_FST_TX_PRI_ACTIVE_MSK GENMASK(7, 0)
+#define AM65_CPSW_PN_FST_TX_E_MAC_ALLOW_MSK GENMASK(15, 8)
+#define AM65_CPSW_PN_FST_EST_CNT_ERR BIT(16)
+#define AM65_CPSW_PN_FST_EST_ADD_ERR BIT(17)
+#define AM65_CPSW_PN_FST_EST_BUFACT BIT(18)
+
+/* EST FETCH COMMAND RAM */
+#define AM65_CPSW_FETCH_RAM_CMD_NUM 0x80
+#define AM65_CPSW_FETCH_CNT_MSK GENMASK(21, 8)
+#define AM65_CPSW_FETCH_CNT_MAX (AM65_CPSW_FETCH_CNT_MSK >> 8)
+#define AM65_CPSW_FETCH_CNT_OFFSET 8
+#define AM65_CPSW_FETCH_ALLOW_MSK GENMASK(7, 0)
+#define AM65_CPSW_FETCH_ALLOW_MAX AM65_CPSW_FETCH_ALLOW_MSK
+
+/* AM65_CPSW_PN_REG_MAX_BLKS fields for IET and No IET cases */
+/* 7 blocks for pn_rx_max_blks, 13 for pn_tx_max_blks*/
+#define AM65_CPSW_PN_TX_RX_MAX_BLKS_IET 0xD07
+
#endif /* AM65_CPSW_QOS_H_ */
--
2.34.1



2023-08-10 17:19:36

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support

Hi Roger,

On Thu, Aug 10, 2023 at 06:25:38PM +0300, Roger Quadros wrote:
> Add driver support for viewing / changing the MAC Merge sublayer
> parameters and seeing the verification state machine's current state
> via ethtool.
>
> As hardware does not support interrupt notification for verification
> events we resort to polling on link up. On link up we try a couple of
> times for verification success and if unsuccessful then give up.
>
> The Frame Preemption feature is described in the Technical Reference
> Manual [1] in section:
> 12.3.1.4.6.7 Intersperced Express Traffic (IET – P802.3br/D2.0)
>
> Due to Silicon Errata i2208 [2] we set limit min IET fragment size to 124.
>
> [1] AM62x TRM - https://www.ti.com/lit/ug/spruiv7a/spruiv7a.pdf
> [2] AM62x Silicon Errata - https://www.ti.com/lit/er/sprz487c/sprz487c.pdf
>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
>
> Hi,
>
> Dropping the RFC tag as I now have MAC merge verification and frame
> preemption working.
>
> Changelog:
> v2:
> - Use proper control bits for PMAC enable (AM65_CPSW_PN_CTL_IET_PORT_EN)
> and TX enable (AM65_CPSW_PN_IET_MAC_PENABLE)
> - Common IET Enable (AM65_CPSW_CTL_IET_EN) is set if any port has
> AM65_CPSW_PN_CTL_IET_PORT_EN set.
> - Fix workaround for erratum i2208. i.e. Limit rx_min_frag_size to 124
> - Fix am65_cpsw_iet_get_verify_timeout_ms() to default to timeout for
> 1G link if link is inactive.
> - resize the RX FIFO based on pmac_enabled, not tx_enabled.
>
> Test Procedure:
>
> - 2 EVMs with AM65-CPSW network port connected to each other
> - Run iet-setup.sh on both
>
> #!/bin/sh
> #iet-setup.sh
>
> ifconfig eth0 down
> ifconfig eth1 down
> ethtool -L eth0 tx 2
> ethtool --set-priv-flags eth0 p0-rx-ptype-rrobin off
> ethtool --set-mm eth0 pmac-enabled on tx-enabled on verify-enabled on verify-time 10 tx-min-frag-size 124
> ifconfig eth0 up
> sleep 10
>
> tc qdisc replace dev eth0 handle 8001: parent root stab overhead 24 taprio \
> num_tc 2 \
> map 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 \
> queues 1@0 1@1 \
> base-time 0 \
> sched-entry S 01 1216 \
> sched-entry S fe 12368 \
> flags 0x2 \
> fp P E E E E E E E
>
> tc -g class show dev eth0
> tc qdisc add dev eth0 clsact
> tc filter add dev eth0 egress protocol ip prio 1 u32 match ip dport 5002 0xffff action skbedit priority 2
> tc filter add dev eth0 egress protocol ip prio 1 u32 match ip dport 5003 0xffff action skbedit priority 3
>
> - check that MAC merge verification has succeeded
>
> ethtool --show-mm eth0
>
> MAC Merge layer state for eth0:
> pMAC enabled: on
> TX enabled: on
> TX active: on
> TX minimum fragment size: 124
> RX minimum fragment size: 124
> Verify enabled: on
> Verify time: 10
> Max verify time: 134
> Verification status: SUCCEEDED

Do you also plan to add support for mqprio, to present the same feature
set as everyone, and permit IET to be used without EST?

What else would need to change in order for the am65 cpsw to
successfully run the existing tools/testing/selftests/net/forwarding/ethtool_mm.sh?
Just the extra driver-specific ethtool --set-priv-flags? If so, maybe
you could create a wrapper over the generic selftest, and put it in
tools/testing/selftests/drivers/net/. For DSA we also symlink a lot of
bridge selftests, for example.

> - On receiver EVM run 2 iperf instances
>
> iperf3 -s -i30 -p5002&
> iperf3 -s -i30 -p5003&
>
> - On sender EVM run 2 iperf instances
>
> iperf3 -c 192.168.3.102 -u -b200M -l1472 -u -t5 -i30 -p5002&
> iperf3 -c 192.168.3.102 -u -b50M -l1472 -u -t5 -i30 -p5003&
>
> - Check IET stats on sender. Look for iet_tx_frag increments
>
> ethtool -S eth0 | grep iet
>
> iet_rx_assembly_err: 0
> iet_rx_assembly_ok: 0
> iet_rx_smd_err: 0
> iet_rx_frag: 0
> iet_tx_hold: 0
> iet_tx_frag: 17307

Can you please also implement the standardized ethtool_ops :: get_mm_stats()?
You can see these with ethtool -I --show-mm eth0.

>
> - Check IET stats on receiver. Look for iet_rx_frag and iet_rx_assembly_*
>
> ethtool -S eth0 | grep iet
>
> iet_rx_assembly_err: 0

This would be struct ethtool_mm_stats :: MACMergeFrameAssErrorCount

> iet_rx_assembly_ok: 17302

This would be struct ethtool_mm_stats :: MACMergeFrameAssOkCount

> iet_rx_smd_err: 0

This would be struct ethtool_mm_stats :: MACMergeFrameSmdErrorCount

> iet_rx_frag: 17307

This would be struct ethtool_mm_stats :: MACMergeFragCountRx

> iet_tx_hold: 0

This would be struct ethtool_mm_stats :: MACMergeHoldCount

> iet_tx_frag: 0

This would be struct ethtool_mm_stats :: MACMergeFragCountTx

Also, I opened the AM62x datasheet again and I didn't find this, but I
still need to ask. The CPSW doesn't show the Ethernet counters broken
down by eMAC/pMAC, no? If it does, you should also add support for the
following:

ethtool -I --show-pause eth0 --src pmac
ethtool -S eth0 --groups eth-mac eth-phy eth-ctrl rmon -- --src pmac

2023-08-11 10:09:28

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v2] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support

On Thu, Aug 10, 2023 at 06:25:38PM +0300, Roger Quadros wrote:
> Add driver support for viewing / changing the MAC Merge sublayer
> parameters and seeing the verification state machine's current state
> via ethtool.
>
> As hardware does not support interrupt notification for verification
> events we resort to polling on link up. On link up we try a couple of
> times for verification success and if unsuccessful then give up.
>
> The Frame Preemption feature is described in the Technical Reference
> Manual [1] in section:
> 12.3.1.4.6.7 Intersperced Express Traffic (IET – P802.3br/D2.0)
>
> Due to Silicon Errata i2208 [2] we set limit min IET fragment size to 124.
>
> [1] AM62x TRM - https://www.ti.com/lit/ug/spruiv7a/spruiv7a.pdf
> [2] AM62x Silicon Errata - https://www.ti.com/lit/er/sprz487c/sprz487c.pdf
>
> Signed-off-by: Roger Quadros <[email protected]>

Hi Roger,

some minor feedback from my side.

...

> +static int am65_cpsw_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
> +{
> + struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
> + u32 port_ctrl, cmn_ctrl, iet_ctrl, iet_status, verify_cnt;
> + struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
> + struct am65_cpsw_ndev_priv *priv = netdev_priv(ndev);
> + u32 add_frag_size;
> +
> + mutex_lock(&priv->mm_lock);
> +
> + iet_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
> + cmn_ctrl = readl(common->cpsw_base + AM65_CPSW_REG_CTL);

cmn_ctrl appears to be set but not used.
Is this intentional?

> + port_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
> +
> + state->tx_enabled = !!(iet_ctrl & AM65_CPSW_PN_IET_MAC_PENABLE);
> + state->pmac_enabled = !!(port_ctrl & AM65_CPSW_PN_CTL_IET_PORT_EN);
> +
> + iet_status = readl(port->port_base + AM65_CPSW_PN_REG_IET_STATUS);
> +
> + if (iet_ctrl & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY)
> + state->verify_status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
> + else if (iet_status & AM65_CPSW_PN_MAC_VERIFIED)
> + state->verify_status = ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
> + else if (iet_status & AM65_CPSW_PN_MAC_VERIFY_FAIL)
> + state->verify_status = ETHTOOL_MM_VERIFY_STATUS_FAILED;
> + else
> + state->verify_status = ETHTOOL_MM_VERIFY_STATUS_UNKNOWN;
> +
> + add_frag_size = AM65_CPSW_PN_IET_MAC_GET_ADDFRAGSIZE(iet_ctrl);
> + state->tx_min_frag_size = ethtool_mm_frag_size_add_to_min(add_frag_size);
> +
> + /* Errata i2208: RX min fragment size cannot be less than 124 */
> + state->rx_min_frag_size = 124;
> +
> + /* FPE active if common tx_enabled and verification success or disabled (forced) */
> + state->tx_active = state->tx_enabled &&
> + (state->verify_status == ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ||
> + state->verify_status == ETHTOOL_MM_VERIFY_STATUS_DISABLED);
> + state->verify_enabled = !(iet_ctrl & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY);
> +
> + verify_cnt = AM65_CPSW_PN_MAC_GET_VERIFY_CNT(readl(port->port_base +
> + AM65_CPSW_PN_REG_IET_VERIFY));

Likewise, verify_cnt appears to be set but not used.

> + state->verify_time = port->qos.iet.verify_time_ms;
> + state->max_verify_time = am65_cpsw_iet_get_verify_timeout_ms(AM65_CPSW_PN_MAC_VERIFY_CNT_MASK,
> + port);
> + mutex_unlock(&priv->mm_lock);
> +
> + return 0;
> +}

...

> +void am65_cpsw_iet_change_preemptible_tcs(struct am65_cpsw_port *port, u8 preemptible_tcs)

nit: should this function be static?

> +{
> + port->qos.iet.preemptible_tcs = preemptible_tcs;
> + am65_cpsw_iet_commit_preemptible_tcs(port);
> +}
> +
> +void am65_cpsw_iet_link_state_update(struct net_device *ndev)

Ditto

> +{
> + struct am65_cpsw_ndev_priv *priv = am65_ndev_to_priv(ndev);
> + struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
> +
> + mutex_lock(&priv->mm_lock);
> + am65_cpsw_iet_commit_preemptible_tcs(port);
> + mutex_unlock(&priv->mm_lock);
> +}

...

2023-08-11 15:51:48

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support

Hi Vladimir,

On 10/08/2023 18:47, Vladimir Oltean wrote:
> Hi Roger,
>
> On Thu, Aug 10, 2023 at 06:25:38PM +0300, Roger Quadros wrote:
>> Add driver support for viewing / changing the MAC Merge sublayer
>> parameters and seeing the verification state machine's current state
>> via ethtool.
>>
>> As hardware does not support interrupt notification for verification
>> events we resort to polling on link up. On link up we try a couple of
>> times for verification success and if unsuccessful then give up.
>>
>> The Frame Preemption feature is described in the Technical Reference
>> Manual [1] in section:
>> 12.3.1.4.6.7 Intersperced Express Traffic (IET – P802.3br/D2.0)
>>
>> Due to Silicon Errata i2208 [2] we set limit min IET fragment size to 124.
>>
>> [1] AM62x TRM - https://www.ti.com/lit/ug/spruiv7a/spruiv7a.pdf
>> [2] AM62x Silicon Errata - https://www.ti.com/lit/er/sprz487c/sprz487c.pdf
>>
>> Signed-off-by: Roger Quadros <[email protected]>
>> ---
>>
>> Hi,
>>
>> Dropping the RFC tag as I now have MAC merge verification and frame
>> preemption working.
>>
>> Changelog:
>> v2:
>> - Use proper control bits for PMAC enable (AM65_CPSW_PN_CTL_IET_PORT_EN)
>> and TX enable (AM65_CPSW_PN_IET_MAC_PENABLE)
>> - Common IET Enable (AM65_CPSW_CTL_IET_EN) is set if any port has
>> AM65_CPSW_PN_CTL_IET_PORT_EN set.
>> - Fix workaround for erratum i2208. i.e. Limit rx_min_frag_size to 124
>> - Fix am65_cpsw_iet_get_verify_timeout_ms() to default to timeout for
>> 1G link if link is inactive.
>> - resize the RX FIFO based on pmac_enabled, not tx_enabled.
>>
>> Test Procedure:
>>
>> - 2 EVMs with AM65-CPSW network port connected to each other
>> - Run iet-setup.sh on both
>>
>> #!/bin/sh
>> #iet-setup.sh
>>
>> ifconfig eth0 down
>> ifconfig eth1 down
>> ethtool -L eth0 tx 2
>> ethtool --set-priv-flags eth0 p0-rx-ptype-rrobin off
>> ethtool --set-mm eth0 pmac-enabled on tx-enabled on verify-enabled on verify-time 10 tx-min-frag-size 124
>> ifconfig eth0 up
>> sleep 10
>>
>> tc qdisc replace dev eth0 handle 8001: parent root stab overhead 24 taprio \
>> num_tc 2 \
>> map 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 \
>> queues 1@0 1@1 \
>> base-time 0 \
>> sched-entry S 01 1216 \
>> sched-entry S fe 12368 \
>> flags 0x2 \
>> fp P E E E E E E E
>>
>> tc -g class show dev eth0
>> tc qdisc add dev eth0 clsact
>> tc filter add dev eth0 egress protocol ip prio 1 u32 match ip dport 5002 0xffff action skbedit priority 2
>> tc filter add dev eth0 egress protocol ip prio 1 u32 match ip dport 5003 0xffff action skbedit priority 3
>>
>> - check that MAC merge verification has succeeded
>>
>> ethtool --show-mm eth0
>>
>> MAC Merge layer state for eth0:
>> pMAC enabled: on
>> TX enabled: on
>> TX active: on
>> TX minimum fragment size: 124
>> RX minimum fragment size: 124
>> Verify enabled: on
>> Verify time: 10
>> Max verify time: 134
>> Verification status: SUCCEEDED
>
> Do you also plan to add support for mqprio, to present the same feature
> set as everyone, and permit IET to be used without EST?

Yes. I'll try to include it in next spin.

>
> What else would need to change in order for the am65 cpsw to
> successfully run the existing tools/testing/selftests/net/forwarding/ethtool_mm.sh?
> Just the extra driver-specific ethtool --set-priv-flags? If so, maybe
> you could create a wrapper over the generic selftest, and put it in
> tools/testing/selftests/drivers/net/. For DSA we also symlink a lot of
> bridge selftests, for example.

OK. I'll give it a try.

>
>> - On receiver EVM run 2 iperf instances
>>
>> iperf3 -s -i30 -p5002&
>> iperf3 -s -i30 -p5003&
>>
>> - On sender EVM run 2 iperf instances
>>
>> iperf3 -c 192.168.3.102 -u -b200M -l1472 -u -t5 -i30 -p5002&
>> iperf3 -c 192.168.3.102 -u -b50M -l1472 -u -t5 -i30 -p5003&
>>
>> - Check IET stats on sender. Look for iet_tx_frag increments
>>
>> ethtool -S eth0 | grep iet
>>
>> iet_rx_assembly_err: 0
>> iet_rx_assembly_ok: 0
>> iet_rx_smd_err: 0
>> iet_rx_frag: 0
>> iet_tx_hold: 0
>> iet_tx_frag: 17307
>
> Can you please also implement the standardized ethtool_ops :: get_mm_stats()?
> You can see these with ethtool -I --show-mm eth0.

Yes.

>
>>
>> - Check IET stats on receiver. Look for iet_rx_frag and iet_rx_assembly_*
>>
>> ethtool -S eth0 | grep iet
>>
>> iet_rx_assembly_err: 0
>
> This would be struct ethtool_mm_stats :: MACMergeFrameAssErrorCount
>
>> iet_rx_assembly_ok: 17302
>
> This would be struct ethtool_mm_stats :: MACMergeFrameAssOkCount
>
>> iet_rx_smd_err: 0
>
> This would be struct ethtool_mm_stats :: MACMergeFrameSmdErrorCount
>
>> iet_rx_frag: 17307
>
> This would be struct ethtool_mm_stats :: MACMergeFragCountRx
>
>> iet_tx_hold: 0
>
> This would be struct ethtool_mm_stats :: MACMergeHoldCount
>
>> iet_tx_frag: 0
>
> This would be struct ethtool_mm_stats :: MACMergeFragCountTx
>

Thanks for the hints :)

> Also, I opened the AM62x datasheet again and I didn't find this, but I
> still need to ask. The CPSW doesn't show the Ethernet counters broken
> down by eMAC/pMAC, no? If it does, you should also add support for the
> following:

I don't think it does.

>
> ethtool -I --show-pause eth0 --src pmac
> ethtool -S eth0 --groups eth-mac eth-phy eth-ctrl rmon -- --src pmac

--
cheers,
-roger

2023-08-11 16:22:16

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support

Hi Simon,

On 11/08/2023 12:46, Simon Horman wrote:
> On Thu, Aug 10, 2023 at 06:25:38PM +0300, Roger Quadros wrote:
>> Add driver support for viewing / changing the MAC Merge sublayer
>> parameters and seeing the verification state machine's current state
>> via ethtool.
>>
>> As hardware does not support interrupt notification for verification
>> events we resort to polling on link up. On link up we try a couple of
>> times for verification success and if unsuccessful then give up.
>>
>> The Frame Preemption feature is described in the Technical Reference
>> Manual [1] in section:
>> 12.3.1.4.6.7 Intersperced Express Traffic (IET – P802.3br/D2.0)
>>
>> Due to Silicon Errata i2208 [2] we set limit min IET fragment size to 124.
>>
>> [1] AM62x TRM - https://www.ti.com/lit/ug/spruiv7a/spruiv7a.pdf
>> [2] AM62x Silicon Errata - https://www.ti.com/lit/er/sprz487c/sprz487c.pdf
>>
>> Signed-off-by: Roger Quadros <[email protected]>
>
> Hi Roger,
>
> some minor feedback from my side.
>
> ...
>
>> +static int am65_cpsw_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
>> +{
>> + struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
>> + u32 port_ctrl, cmn_ctrl, iet_ctrl, iet_status, verify_cnt;
>> + struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
>> + struct am65_cpsw_ndev_priv *priv = netdev_priv(ndev);
>> + u32 add_frag_size;
>> +
>> + mutex_lock(&priv->mm_lock);
>> +
>> + iet_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
>> + cmn_ctrl = readl(common->cpsw_base + AM65_CPSW_REG_CTL);
>
> cmn_ctrl appears to be set but not used.
> Is this intentional?

No. It is stale code.
>
>> + port_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
>> +
>> + state->tx_enabled = !!(iet_ctrl & AM65_CPSW_PN_IET_MAC_PENABLE);
>> + state->pmac_enabled = !!(port_ctrl & AM65_CPSW_PN_CTL_IET_PORT_EN);
>> +
>> + iet_status = readl(port->port_base + AM65_CPSW_PN_REG_IET_STATUS);
>> +
>> + if (iet_ctrl & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY)
>> + state->verify_status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
>> + else if (iet_status & AM65_CPSW_PN_MAC_VERIFIED)
>> + state->verify_status = ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
>> + else if (iet_status & AM65_CPSW_PN_MAC_VERIFY_FAIL)
>> + state->verify_status = ETHTOOL_MM_VERIFY_STATUS_FAILED;
>> + else
>> + state->verify_status = ETHTOOL_MM_VERIFY_STATUS_UNKNOWN;
>> +
>> + add_frag_size = AM65_CPSW_PN_IET_MAC_GET_ADDFRAGSIZE(iet_ctrl);
>> + state->tx_min_frag_size = ethtool_mm_frag_size_add_to_min(add_frag_size);
>> +
>> + /* Errata i2208: RX min fragment size cannot be less than 124 */
>> + state->rx_min_frag_size = 124;
>> +
>> + /* FPE active if common tx_enabled and verification success or disabled (forced) */
>> + state->tx_active = state->tx_enabled &&
>> + (state->verify_status == ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ||
>> + state->verify_status == ETHTOOL_MM_VERIFY_STATUS_DISABLED);
>> + state->verify_enabled = !(iet_ctrl & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY);
>> +
>> + verify_cnt = AM65_CPSW_PN_MAC_GET_VERIFY_CNT(readl(port->port_base +
>> + AM65_CPSW_PN_REG_IET_VERIFY));
>
> Likewise, verify_cnt appears to be set but not used.

Will remove it.
>
>> + state->verify_time = port->qos.iet.verify_time_ms;
>> + state->max_verify_time = am65_cpsw_iet_get_verify_timeout_ms(AM65_CPSW_PN_MAC_VERIFY_CNT_MASK,
>> + port);
>> + mutex_unlock(&priv->mm_lock);
>> +
>> + return 0;
>> +}
>
> ...
>
>> +void am65_cpsw_iet_change_preemptible_tcs(struct am65_cpsw_port *port, u8 preemptible_tcs)
>
> nit: should this function be static?
>
>> +{
>> + port->qos.iet.preemptible_tcs = preemptible_tcs;
>> + am65_cpsw_iet_commit_preemptible_tcs(port);
>> +}
>> +
>> +void am65_cpsw_iet_link_state_update(struct net_device *ndev)
>
> Ditto

Yes, both need to be static.
Thanks for review!

--
cheers,
-roger

2023-08-13 10:06:53

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support

Hi Roger,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]
[also build test WARNING on net/main linus/master v6.5-rc5 next-20230809]
[cannot apply to horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Roger-Quadros/net-ethernet-ti-am65-cpsw-qos-Add-Frame-Preemption-MAC-Merge-support/20230810-232714
base: net-next/main
patch link: https://lore.kernel.org/r/20230810152538.138718-1-rogerq%40kernel.org
patch subject: [PATCH v2] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20230813/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230813/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

drivers/net/ethernet/ti/am65-cpsw-ethtool.c: In function 'am65_cpsw_get_mm':
>> drivers/net/ethernet/ti/am65-cpsw-ethtool.c:749:56: warning: variable 'verify_cnt' set but not used [-Wunused-but-set-variable]
749 | u32 port_ctrl, cmn_ctrl, iet_ctrl, iet_status, verify_cnt;
| ^~~~~~~~~~
>> drivers/net/ethernet/ti/am65-cpsw-ethtool.c:749:24: warning: variable 'cmn_ctrl' set but not used [-Wunused-but-set-variable]
749 | u32 port_ctrl, cmn_ctrl, iet_ctrl, iet_status, verify_cnt;
| ^~~~~~~~
--
>> drivers/net/ethernet/ti/am65-cpsw-qos.c:196:6: warning: no previous prototype for 'am65_cpsw_iet_change_preemptible_tcs' [-Wmissing-prototypes]
196 | void am65_cpsw_iet_change_preemptible_tcs(struct am65_cpsw_port *port, u8 preemptible_tcs)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/ti/am65-cpsw-qos.c:202:6: warning: no previous prototype for 'am65_cpsw_iet_link_state_update' [-Wmissing-prototypes]
202 | void am65_cpsw_iet_link_state_update(struct net_device *ndev)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/verify_cnt +749 drivers/net/ethernet/ti/am65-cpsw-ethtool.c

745
746 static int am65_cpsw_get_mm(struct net_device *ndev, struct ethtool_mm_state *state)
747 {
748 struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
> 749 u32 port_ctrl, cmn_ctrl, iet_ctrl, iet_status, verify_cnt;
750 struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
751 struct am65_cpsw_ndev_priv *priv = netdev_priv(ndev);
752 u32 add_frag_size;
753
754 mutex_lock(&priv->mm_lock);
755
756 iet_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
757 cmn_ctrl = readl(common->cpsw_base + AM65_CPSW_REG_CTL);
758 port_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_CTL);
759
760 state->tx_enabled = !!(iet_ctrl & AM65_CPSW_PN_IET_MAC_PENABLE);
761 state->pmac_enabled = !!(port_ctrl & AM65_CPSW_PN_CTL_IET_PORT_EN);
762
763 iet_status = readl(port->port_base + AM65_CPSW_PN_REG_IET_STATUS);
764
765 if (iet_ctrl & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY)
766 state->verify_status = ETHTOOL_MM_VERIFY_STATUS_DISABLED;
767 else if (iet_status & AM65_CPSW_PN_MAC_VERIFIED)
768 state->verify_status = ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED;
769 else if (iet_status & AM65_CPSW_PN_MAC_VERIFY_FAIL)
770 state->verify_status = ETHTOOL_MM_VERIFY_STATUS_FAILED;
771 else
772 state->verify_status = ETHTOOL_MM_VERIFY_STATUS_UNKNOWN;
773
774 add_frag_size = AM65_CPSW_PN_IET_MAC_GET_ADDFRAGSIZE(iet_ctrl);
775 state->tx_min_frag_size = ethtool_mm_frag_size_add_to_min(add_frag_size);
776
777 /* Errata i2208: RX min fragment size cannot be less than 124 */
778 state->rx_min_frag_size = 124;
779
780 /* FPE active if common tx_enabled and verification success or disabled (forced) */
781 state->tx_active = state->tx_enabled &&
782 (state->verify_status == ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ||
783 state->verify_status == ETHTOOL_MM_VERIFY_STATUS_DISABLED);
784 state->verify_enabled = !(iet_ctrl & AM65_CPSW_PN_IET_MAC_DISABLEVERIFY);
785
786 verify_cnt = AM65_CPSW_PN_MAC_GET_VERIFY_CNT(readl(port->port_base +
787 AM65_CPSW_PN_REG_IET_VERIFY));
788 state->verify_time = port->qos.iet.verify_time_ms;
789 state->max_verify_time = am65_cpsw_iet_get_verify_timeout_ms(AM65_CPSW_PN_MAC_VERIFY_CNT_MASK,
790 port);
791 mutex_unlock(&priv->mm_lock);
792
793 return 0;
794 }
795

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki