Messages in this thread | | | Date | Tue, 27 Feb 2024 11:45:02 +0530 | Subject | Re: [PATCH net-next 2/2] net: ethernet: ti: am65-cpsw: Enable RX HW timestamp only for PTP packets | From | Chintan Vankar <> |
| |
On 19/02/24 16:29, Roger Quadros wrote: > Hi, > > On 15/02/2024 13:09, Chintan Vankar wrote: >> The CPSW peripherals on J7AHP, J7VCL, J7AEP, J7ES, AM64 SoCs have >> an errata i2401 "CPSW: Host Timestamps Cause CPSW Port to Lock up". > Does this affect all platforms using am65-cpsw?
Yes. It affects all platforms using am65-cpsw. I will update a
commit message for the same.
> If this affects only some platforms then you need to restrict the > workaround to those platforms only. > >> As a workaround, Disable timestamping on all RX packets and timestamp >> only PTP packets. > This does not exactly reflect what you are doing here. > > I think you need to clarify that you are disabling in-band RX timestamp > mechanism on all packets and using out of band mechanism to get the > RX timestamps only for certain type of packets. > >> Set Time Sync Receive bits in Time Sync control register so that >> packets can be determined as a valid Ethernet Receive Event for time >> synchronization. >> >> Update the RX filter configuration to indicate Hardware Timestamping >> support only for PTP packets. >> >> Replace "am65_cpsw_rx_ts()" function with "am65_cpts_rx_timestamp()" >> function which timestamps only PTP packets. >> >> Fixes: b1f66a5bee07 ("net: ethernet: ti: am65-cpsw-nuss: enable packet timestamping support") >> Signed-off-by: Chintan Vankar <c-vankar@ti.com> >> --- >> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 51 +++++++++++------------- >> 1 file changed, 24 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> index 9d2f4ac783e4..ab843fb64b93 100644 >> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> @@ -101,6 +101,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 >> >> @@ -124,6 +130,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 >> @@ -749,18 +760,6 @@ static int am65_cpsw_nuss_ndo_slave_open(struct net_device *ndev) >> 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) >> @@ -841,9 +840,6 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_common *common, >> skb->dev = ndev; >> >> psdata = cppi5_hdesc_get_psdata(desc_rx); >> - /* add RX timestamp */ >> - if (port->rx_ts_enabled) >> - am65_cpsw_nuss_rx_ts(skb, psdata); >> csum_info = psdata[2]; >> dev_dbg(dev, "%s rx csum_info:%#x\n", __func__, csum_info); >> >> @@ -856,6 +852,9 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_common *common, >> ndev_priv = netdev_priv(ndev); >> am65_cpsw_nuss_set_offload_fwd_mark(skb, ndev_priv->offload_fwd_mark); >> skb_put(skb, pkt_len); >> + skb_reset_mac_header(skb); > Why do you need to add skb_reset_mac_header here? > >> + if (port->rx_ts_enabled) >> + am65_cpts_rx_timestamp(common->cpts, skb); > The timestamp should be added before you do skb_put(). > >> skb->protocol = eth_type_trans(skb, ndev); >> am65_cpsw_nuss_rx_csum(skb, csum_info); >> napi_gro_receive(&common->napi_rx, skb); >> @@ -1334,7 +1333,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; >> @@ -1358,11 +1356,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: > Can't PTP_V1 be supported?
No. As per TRM:
https://www.ti.com/lit/ug/spruim2h/spruim2h.pdf,
section 12.2.1.4.7 Common Platform Time Sync(CPTS)
only IEEE 1588-2008 which is PTP v2 is supported.
> >> case HWTSTAMP_FILTER_PTP_V2_L4_EVENT: >> case HWTSTAMP_FILTER_PTP_V2_L4_SYNC: >> case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ: >> @@ -1372,10 +1365,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; >> } >> @@ -1405,6 +1401,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); >> @@ -1412,9 +1412,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; >> } >> >> @@ -1431,7 +1428,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; >> }
| |