Re: [PATCH net-next v6 2/2] net: ethernet: ti: am65-cpsw/ethtool: Enable RX HW timestamp only for PTP packets

[Chintan Vankar <c-vankar@ti.com>]

On 17/04/24 17:09, Chintan Vankar wrote:

Due to some issue in my email client other two patches of the series
are missing from this version. I am posting v7 as v6.

> In the current mechanism of timestamping, am65-cpsw-nuss driver
> enables hardware timestamping for all received packets by setting
> the TSTAMP_EN bit in CPTS_CONTROL register, which directs the CPTS
> module to timestamp all received packets, followed by passing
> timestamp via DMA descriptors. This mechanism causes CPSW Port to
> Lock up.
> 
> To prevent port lock up, don't enable rx packet timestamping by
> setting TSTAMP_EN bit in CPTS_CONTROL register. The workaround for
> timestamping received packets is to utilize the CPTS Event FIFO
> that records timestamps corresponding to certain events. The CPTS
> module is configured to generate timestamps for Multicast Ethernet,
> UDP/IPv4 and UDP/IPv6 PTP packets.
> 
> Update supported hwtstamp_rx_filters values for CPSW's timestamping
> capability.
> 
> Fixes: b1f66a5bee07 ("net: ethernet: ti: am65-cpsw-nuss: enable packet timestamping support")
> 
> Signed-off-by: Chintan Vankar <c-vankar@ti.com>
> ---
> 
> Link to v5:
> https://lore.kernel.org/r/20240402114405.219100-3-c-vankar@ti.com/
> 
> Changes from v5 to v6:
> - No changes in v6.
> 
>   drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 13 +++++-
>   drivers/net/ethernet/ti/am65-cpsw-nuss.c    | 51 ++++++++++-----------
>   drivers/net/ethernet/ti/am65-cpts.c         | 23 ----------
>   drivers/net/ethernet/ti/am65-cpts.h         |  5 --
>   4 files changed, 35 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
> index d6ce2c9f0a8d..a1d0935d1ebe 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-ethtool.c
> @@ -695,6 +695,17 @@ static int am65_cpsw_get_ethtool_ts_info(struct net_device *ndev,
>   					 struct ethtool_ts_info *info)
>   {
>   	struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
> +	unsigned int ptp_v2_filter;
> +
> +	ptp_v2_filter = BIT(HWTSTAMP_FILTER_PTP_V2_L4_EVENT)	 |
> +			BIT(HWTSTAMP_FILTER_PTP_V2_L4_SYNC)	 |
> +			BIT(HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ) |
> +			BIT(HWTSTAMP_FILTER_PTP_V2_L2_EVENT)	 |
> +			BIT(HWTSTAMP_FILTER_PTP_V2_L2_SYNC)	 |
> +			BIT(HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ) |
> +			BIT(HWTSTAMP_FILTER_PTP_V2_EVENT)	 |
> +			BIT(HWTSTAMP_FILTER_PTP_V2_SYNC)	 |
> +			BIT(HWTSTAMP_FILTER_PTP_V2_DELAY_REQ);
>   
>   	if (!IS_ENABLED(CONFIG_TI_K3_AM65_CPTS))
>   		return ethtool_op_get_ts_info(ndev, info);
> @@ -708,7 +719,7 @@ static int am65_cpsw_get_ethtool_ts_info(struct net_device *ndev,
>   		SOF_TIMESTAMPING_RAW_HARDWARE;
>   	info->phc_index = am65_cpts_phc_index(common->cpts);
>   	info->tx_types = BIT(HWTSTAMP_TX_OFF) | BIT(HWTSTAMP_TX_ON);
> -	info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) | BIT(HWTSTAMP_FILTER_ALL);
> +	info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) | ptp_v2_filter;
>   	return 0;
>   }
>   
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index bfba883d4fc4..305eda711e0c 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -103,6 +103,12 @@
>   #define AM65_CPSW_PN_TS_CTL_TX_HOST_TS_EN	BIT(11)
>   #define AM65_CPSW_PN_TS_CTL_MSG_TYPE_EN_SHIFT	16
>   
> +#define AM65_CPSW_PN_TS_CTL_RX_ANX_F_EN		BIT(0)
> +#define AM65_CPSW_PN_TS_CTL_RX_VLAN_LT1_EN	BIT(1)
> +#define AM65_CPSW_PN_TS_CTL_RX_VLAN_LT2_EN	BIT(2)
> +#define AM65_CPSW_PN_TS_CTL_RX_ANX_D_EN		BIT(3)
> +#define AM65_CPSW_PN_TS_CTL_RX_ANX_E_EN		BIT(9)
> +
>   /* AM65_CPSW_PORTN_REG_TS_SEQ_LTYPE_REG register fields */
>   #define AM65_CPSW_PN_TS_SEQ_ID_OFFSET_SHIFT	16
>   
> @@ -126,6 +132,11 @@
>   	 AM65_CPSW_PN_TS_CTL_TX_ANX_E_EN |	\
>   	 AM65_CPSW_PN_TS_CTL_TX_ANX_F_EN)
>   
> +#define AM65_CPSW_TS_RX_ANX_ALL_EN		\
> +	(AM65_CPSW_PN_TS_CTL_RX_ANX_D_EN |	\
> +	 AM65_CPSW_PN_TS_CTL_RX_ANX_E_EN |	\
> +	 AM65_CPSW_PN_TS_CTL_RX_ANX_F_EN)
> +
>   #define AM65_CPSW_ALE_AGEOUT_DEFAULT	30
>   /* Number of TX/RX descriptors */
>   #define AM65_CPSW_MAX_TX_DESC	500
> @@ -1044,18 +1055,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_common *common,
>   	return ret;
>   }
>   
> -static void am65_cpsw_nuss_rx_ts(struct sk_buff *skb, u32 *psdata)
> -{
> -	struct skb_shared_hwtstamps *ssh;
> -	u64 ns;
> -
> -	ns = ((u64)psdata[1] << 32) | psdata[0];
> -
> -	ssh = skb_hwtstamps(skb);
> -	memset(ssh, 0, sizeof(*ssh));
> -	ssh->hwtstamp = ns_to_ktime(ns);
> -}
> -
>   /* RX psdata[2] word format - checksum information */
>   #define AM65_CPSW_RX_PSD_CSUM_ADD	GENMASK(15, 0)
>   #define AM65_CPSW_RX_PSD_CSUM_ERR	BIT(16)
> @@ -1171,13 +1170,11 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_common *common,
>   		skb_reserve(skb, headroom);
>   	}
>   
> -	/* Pass skb to netstack if no XDP prog or returned XDP_PASS */
> -	if (port->rx_ts_enabled)
> -		am65_cpsw_nuss_rx_ts(skb, psdata);
> -
>   	ndev_priv = netdev_priv(ndev);
>   	am65_cpsw_nuss_set_offload_fwd_mark(skb, ndev_priv->offload_fwd_mark);
>   	skb_put(skb, pkt_len);
> +	if (port->rx_ts_enabled)
> +		am65_cpts_rx_timestamp(common->cpts, skb);
>   	skb_mark_for_recycle(skb);
>   	skb->protocol = eth_type_trans(skb, ndev);
>   	am65_cpsw_nuss_rx_csum(skb, csum_info);
> @@ -1730,7 +1727,6 @@ static int am65_cpsw_nuss_ndo_slave_set_mac_address(struct net_device *ndev,
>   static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
>   				       struct ifreq *ifr)
>   {
> -	struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
>   	struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
>   	u32 ts_ctrl, seq_id, ts_ctrl_ltype2, ts_vlan_ltype;
>   	struct hwtstamp_config cfg;
> @@ -1754,11 +1750,6 @@ static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
>   	case HWTSTAMP_FILTER_NONE:
>   		port->rx_ts_enabled = false;
>   		break;
> -	case HWTSTAMP_FILTER_ALL:
> -	case HWTSTAMP_FILTER_SOME:
> -	case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
> -	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> -	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
>   	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
>   	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
>   	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> @@ -1768,10 +1759,13 @@ static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
>   	case HWTSTAMP_FILTER_PTP_V2_EVENT:
>   	case HWTSTAMP_FILTER_PTP_V2_SYNC:
>   	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
> -	case HWTSTAMP_FILTER_NTP_ALL:
>   		port->rx_ts_enabled = true;
> -		cfg.rx_filter = HWTSTAMP_FILTER_ALL;
> +		cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
>   		break;
> +	case HWTSTAMP_FILTER_ALL:
> +	case HWTSTAMP_FILTER_SOME:
> +	case HWTSTAMP_FILTER_NTP_ALL:
> +		return -EOPNOTSUPP;
>   	default:
>   		return -ERANGE;
>   	}
> @@ -1801,6 +1795,10 @@ static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
>   		ts_ctrl |= AM65_CPSW_TS_TX_ANX_ALL_EN |
>   			   AM65_CPSW_PN_TS_CTL_TX_VLAN_LT1_EN;
>   
> +	if (port->rx_ts_enabled)
> +		ts_ctrl |= AM65_CPSW_TS_RX_ANX_ALL_EN |
> +			   AM65_CPSW_PN_TS_CTL_RX_VLAN_LT1_EN;
> +
>   	writel(seq_id, port->port_base + AM65_CPSW_PORTN_REG_TS_SEQ_LTYPE_REG);
>   	writel(ts_vlan_ltype, port->port_base +
>   	       AM65_CPSW_PORTN_REG_TS_VLAN_LTYPE_REG);
> @@ -1808,9 +1806,6 @@ static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
>   	       AM65_CPSW_PORTN_REG_TS_CTL_LTYPE2);
>   	writel(ts_ctrl, port->port_base + AM65_CPSW_PORTN_REG_TS_CTL);
>   
> -	/* en/dis RX timestamp */
> -	am65_cpts_rx_enable(common->cpts, port->rx_ts_enabled);
> -
>   	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
>   }
>   
> @@ -1827,7 +1822,7 @@ static int am65_cpsw_nuss_hwtstamp_get(struct net_device *ndev,
>   	cfg.tx_type = port->tx_ts_enabled ?
>   		      HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
>   	cfg.rx_filter = port->rx_ts_enabled ?
> -			HWTSTAMP_FILTER_ALL : HWTSTAMP_FILTER_NONE;
> +			HWTSTAMP_FILTER_PTP_V2_EVENT : HWTSTAMP_FILTER_NONE;
>   
>   	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
>   }
> diff --git a/drivers/net/ethernet/ti/am65-cpts.c b/drivers/net/ethernet/ti/am65-cpts.c
> index 599454c1d19f..68891a227dc8 100644
> --- a/drivers/net/ethernet/ti/am65-cpts.c
> +++ b/drivers/net/ethernet/ti/am65-cpts.c
> @@ -866,29 +866,6 @@ static long am65_cpts_ts_work(struct ptp_clock_info *ptp)
>   	return delay;
>   }
>   
> -/**
> - * am65_cpts_rx_enable - enable rx timestamping
> - * @cpts: cpts handle
> - * @en: enable
> - *
> - * This functions enables rx packets timestamping. The CPTS can timestamp all
> - * rx packets.
> - */
> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
> -{
> -	u32 val;
> -
> -	mutex_lock(&cpts->ptp_clk_lock);
> -	val = am65_cpts_read32(cpts, control);
> -	if (en)
> -		val |= AM65_CPTS_CONTROL_TSTAMP_EN;
> -	else
> -		val &= ~AM65_CPTS_CONTROL_TSTAMP_EN;
> -	am65_cpts_write32(cpts, val, control);
> -	mutex_unlock(&cpts->ptp_clk_lock);
> -}
> -EXPORT_SYMBOL_GPL(am65_cpts_rx_enable);
> -
>   static int am65_skb_get_mtype_seqid(struct sk_buff *skb, u32 *mtype_seqid)
>   {
>   	unsigned int ptp_class = ptp_classify_raw(skb);
> diff --git a/drivers/net/ethernet/ti/am65-cpts.h b/drivers/net/ethernet/ti/am65-cpts.h
> index 90296968a75c..6099d772799d 100644
> --- a/drivers/net/ethernet/ti/am65-cpts.h
> +++ b/drivers/net/ethernet/ti/am65-cpts.h
> @@ -25,7 +25,6 @@ int am65_cpts_phc_index(struct am65_cpts *cpts);
>   void am65_cpts_rx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>   void am65_cpts_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
>   void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts, struct sk_buff *skb);
> -void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en);
>   u64 am65_cpts_ns_gettime(struct am65_cpts *cpts);
>   int am65_cpts_estf_enable(struct am65_cpts *cpts, int idx,
>   			  struct am65_cpts_estf_cfg *cfg);
> @@ -64,10 +63,6 @@ static inline void am65_cpts_prep_tx_timestamp(struct am65_cpts *cpts,
>   {
>   }
>   
> -static inline void am65_cpts_rx_enable(struct am65_cpts *cpts, bool en)
> -{
> -}
> -
>   static inline s64 am65_cpts_ns_gettime(struct am65_cpts *cpts)
>   {
>   	return 0;