Message-ID: <7e696ab339e423f987b3fe1d8c1fc9231dd4e92c.camel@redhat.com>
Date:   Thu, 05 Oct 2023 09:57:44 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Roger Quadros <rogerq@...nel.org>, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, vladimir.oltean@....com
Cc:     horms@...nel.org, s-vadapalli@...com, srk@...com, vigneshr@...com,
        p-varis@...com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 net-next 3/4] net: ethernet: ti: am65-cpsw-qos: Add
 Frame Preemption MAC Merge support

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@...nel.org>
> ---
>  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@kernel.org/
> 
> 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.

Cheers,

Paolo

Powered by blists - more mailing lists