Re: [PATCH v5 net-next 3/4] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support

From: Roger Quadros
Date: Fri Oct 06 2023 - 08:31:02 EST




On 05/10/2023 10:57, Paolo Abeni wrote:
> On Wed, 2023-09-27 at 10:27 +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 <rogerq@xxxxxxxxxx>
>> ---
>> drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 150 ++++++++++++
>> 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 | 240 ++++++++++++++++----
>> drivers/net/ethernet/ti/am65-cpsw-qos.h | 104 +++++++++
>> 5 files changed, 454 insertions(+), 47 deletions(-)
>>
>> Changelog:
>> v5:
>> - No change
>>
>> v4:
>> - Rebase and include in the same series as mqprio support.
>>
>> v3:
>> - Rebase on top of v6.6-rc1 and mqprio support [1]
>> - Support ethtool_ops :: get_mm_stats()
>> - drop unused variables cmn_ctrl and verify_cnt
>> - make am65_cpsw_iet_link_state_update() and
>> am65_cpsw_iet_change_preemptible_tcs() static
>>
>> [1] https://lore.kernel.org/all/20230918075358.5878-1-rogerq@xxxxxxxxxx/
>>
>> 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-mqprio.sh on both
>>
>> #!/bin/sh
>> #iet-setup-mqprio.sh
>>
>> ifconfig eth0 down
>> ifconfig eth1 down
>> ethtool -L eth0 tx 4
>> 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 add dev eth0 handle 100: root mqprio \
>> num_tc 4 \
>> map 0 1 2 3 \
>> queues 1@0 1@1 1@2 1@3 \
>> hw 1 \
>> mode dcb \
>> fp P P P 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
>> ip addr add 192.168.3.102/24 dev eth0
>>
>> - 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 MACMergeFragCountTx: increments
>>
>> ethtool -I --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
>> Statistics:
>> MACMergeFrameAssErrorCount: 0
>> MACMergeFrameSmdErrorCount: 0
>> MACMergeFrameAssOkCount: 0
>> MACMergeFragCountRx: 0
>> MACMergeFragCountTx: 57824
>> MACMergeHoldCount: 0
>>
>> - Check IET stats on receiver. Look for MACMergeFragCountRx: and
>> MACMergeFrameAssOkCount:
>>
>> ethtool -I --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
>> Statistics:
>> MACMergeFrameAssErrorCount: 0
>> MACMergeFrameSmdErrorCount: 0
>> MACMergeFrameAssOkCount: 57018
>> MACMergeFragCountRx: 57824
>> MACMergeFragCountTx: 0
>> MACMergeHoldCount: 0
>>
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
>> index c51e2af91f69..f6b081b7e754 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,152 @@ 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_port *port = am65_ndev_to_port(ndev);
>> + struct am65_cpsw_ndev_priv *priv = netdev_priv(ndev);
>> + u32 port_ctrl, iet_ctrl, iet_status;
>> + u32 add_frag_size;
>> +
>> + mutex_lock(&priv->mm_lock);
>> +
>> + iet_ctrl = readl(port->port_base + AM65_CPSW_PN_REG_IET_CTRL);
>> + 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);
>> +
>> + 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;
>> +}
>> +
>> +static void am65_cpsw_get_mm_stats(struct net_device *ndev,
>> + struct ethtool_mm_stats *s)
>> +{
>> + struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
>> + void __iomem *base = port->stat_base;
>> +
>> + s->MACMergeFrameAssOkCount = readl(base + AM65_CPSW_STATN_IET_RX_ASSEMBLY_OK);
>> + s->MACMergeFrameAssErrorCount = readl(base + AM65_CPSW_STATN_IET_RX_ASSEMBLY_ERROR);
>> + s->MACMergeFrameSmdErrorCount = readl(base + AM65_CPSW_STATN_IET_RX_SMD_ERROR);
>> + s->MACMergeFragCountRx = readl(base + AM65_CPSW_STATN_IET_RX_FRAG);
>> + s->MACMergeFragCountTx = readl(base + AM65_CPSW_STATN_IET_TX_FRAG);
>> + s->MACMergeHoldCount = readl(base + AM65_CPSW_STATN_IET_TX_HOLD);
>> +}
>> +
>> const struct ethtool_ops am65_cpsw_ethtool_ops_slave = {
>> .begin = am65_cpsw_ethtool_op_begin,
>> .complete = am65_cpsw_ethtool_op_complete,
>> @@ -743,4 +890,7 @@ 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,
>> + .get_mm_stats = am65_cpsw_get_mm_stats,
>> };
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> index bea6fc0f324c..2004f6a020d3 100644
>> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> @@ -2161,6 +2161,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 f3dad2ab9828..1e4a045057fc 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 991221d8f148..ec2986e79edd 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,50 +17,6 @@
>> #include "am65-cpts.h"
>> #include "cpsw_ale.h"
>>
>> -#define AM65_CPSW_REG_CTL 0x004
>> -#define AM65_CPSW_P0_REG_TX_PRI_MAP 0x018
>> -#define AM65_CPSW_P0_REG_RX_PRI_MAP 0x020
>> -#define AM65_CPSW_P0_REG_FIFO_STATUS 0x050
>> -#define AM65_CPSW_P0_REG_PRI_CIR(pri) (0x140 + 4 * (pri))
>> -#define AM65_CPSW_P0_REG_PRI_EIR(pri) (0x160 + 4 * (pri))
>> -
>> -#define AM65_CPSW_PN_REG_CTL 0x004
>> -#define AM65_CPSW_PN_REG_TX_PRI_MAP 0x018
>> -#define AM65_CPSW_PN_REG_RX_PRI_MAP 0x020
>> -#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))
>> -#define AM65_CPSW_PN_REG_PRI_EIR(pri) (0x160 + 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
>> -
>> #define TO_MBPS(x) ((x) * 8 / 1000000)
>>
>> enum timer_act {
>> @@ -67,10 +25,193 @@ enum timer_act {
>> TACT_SKIP_PROG, /* just buffer can be updated */
>> };
>>
>> -/* number of priority queues per port FIFO */
>> -#define AM65_CPSW_PN_FIFO_PRIO_NUM 8
>> -#define AM65_CPSW_PN_TX_PRI_MAP_DEFAULT 0x76543210
>> +/* 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);
>> +}
>> +
>> +static 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);
>
> I'm sorry for the late feedback, but I was unable to notice the
> following before.
>
> It looks like am65_cpsw_iet_commit_preemptible_tcs() is always invoked
> with the priv->mm_lock lock held, except in the above call that can be
> reached without such lock held, via the am65_cpsw_qos_ndo_setup_tc().
>
> Is that safe? If so I think it deserves at least a comment in the code.

I think it is better to call it with the priv->mm_lock held. Thanks!

--
cheers,
-roger