RE: [PATCH v3 net-next 11/15] net: enetc: extract enetc_update_ptp_sync_msg() to handle PTP Sync packets

Wei Fang
2025-08-13T02:04:10Z

> On Tue, Aug 12, 2025 at 05:46:30PM +0800, Wei Fang wrote:
> > Move PTP Sync packet processing from enetc_map_tx_buffs() to a new
> > helper function enetc_update_ptp_sync_msg() to simplify the original
> function.
> > Prepare for upcoming ENETC v4 one-step support.
>
> Add "no functional change".
>
> >
> > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> >
> > ---
> > v2: no changes
> > v3: Change the subject and improve the commit message
> > ---
> > drivers/net/ethernet/freescale/enetc/enetc.c | 129 ++++++++++--------
> > .../net/ethernet/freescale/enetc/enetc_hw.h | 1 +
> > 2 files changed, 71 insertions(+), 59 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> > b/drivers/net/ethernet/freescale/enetc/enetc.c
> > index 54ccd7c57961..ef002ed2fdb9 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > @@ -221,12 +221,79 @@ static void enetc_unwind_tx_frame(struct
> enetc_bdr *tx_ring, int count, int i)
> > }
> > }
> >
> > +static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
> > + struct sk_buff *skb)
> > +{
> > + struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
> > + u16 tstamp_off = enetc_cb->origin_tstamp_off;
> > + u16 corr_off = enetc_cb->correction_off;
> > + struct enetc_si *si = priv->si;
> > + struct enetc_hw *hw = &si->hw;
> > + __be32 new_sec_l, new_nsec;
> > + __be16 new_sec_h;
> > + u32 lo, hi, nsec;
> > + u8 *data;
> > + u64 sec;
> > + u32 val;
> > +
> > + lo = enetc_rd_hot(hw, ENETC_SICTR0);
> > + hi = enetc_rd_hot(hw, ENETC_SICTR1);
> > + sec = (u64)hi << 32 | lo;
> > + nsec = do_div(sec, 1000000000);
> > +
> > + /* Update originTimestamp field of Sync packet
> > + * - 48 bits seconds field
> > + * - 32 bits nanseconds field
> > + *
> > + * In addition, the UDP checksum needs to be updated
> > + * by software after updating originTimestamp field,
> > + * otherwise the hardware will calculate the wrong
> > + * checksum when updating the correction field and
> > + * update it to the packet.
> > + */
> > +
> > + data = skb_mac_header(skb);
> > + new_sec_h = htons((sec >> 32) & 0xffff);
> > + new_sec_l = htonl(sec & 0xffffffff);
> > + new_nsec = htonl(nsec);
> > + if (enetc_cb->udp) {
> > + struct udphdr *uh = udp_hdr(skb);
> > + __be32 old_sec_l, old_nsec;
> > + __be16 old_sec_h;
> > +
> > + old_sec_h = *(__be16 *)(data + tstamp_off);
> > + inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
> > + new_sec_h, false);
> > +
> > + old_sec_l = *(__be32 *)(data + tstamp_off + 2);
> > + inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
> > + new_sec_l, false);
> > +
> > + old_nsec = *(__be32 *)(data + tstamp_off + 6);
> > + inet_proto_csum_replace4(&uh->check, skb, old_nsec,
> > + new_nsec, false);
> > + }
> > +
> > + *(__be16 *)(data + tstamp_off) = new_sec_h;
> > + *(__be32 *)(data + tstamp_off + 2) = new_sec_l;
> > + *(__be32 *)(data + tstamp_off + 6) = new_nsec;
> > +
> > + /* Configure single-step register */
> > + val = ENETC_PM0_SINGLE_STEP_EN;
> > + val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
> > + if (enetc_cb->udp)
> > + val |= ENETC_PM0_SINGLE_STEP_CH;
> > +
> > + enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP, val);
> > +
> > + return lo & ENETC_TXBD_TSTAMP;
> > +}
> > +
> > static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct
> > sk_buff *skb) {
> > bool do_vlan, do_onestep_tstamp = false, do_twostep_tstamp = false;
> > struct enetc_ndev_priv *priv = netdev_priv(tx_ring->ndev);
> > struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
> > - struct enetc_hw *hw = &priv->si->hw;
> > struct enetc_tx_swbd *tx_swbd;
> > int len = skb_headlen(skb);
> > union enetc_tx_bd temp_bd;
> > @@ -326,67 +393,11 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> > }
> >
> > if (do_onestep_tstamp) {
> > - u16 tstamp_off = enetc_cb->origin_tstamp_off;
> > - u16 corr_off = enetc_cb->correction_off;
> > - __be32 new_sec_l, new_nsec;
> > - u32 lo, hi, nsec, val;
> > - __be16 new_sec_h;
> > - u8 *data;
> > - u64 sec;
> > -
> > - lo = enetc_rd_hot(hw, ENETC_SICTR0);
> > - hi = enetc_rd_hot(hw, ENETC_SICTR1);
> > - sec = (u64)hi << 32 | lo;
> > - nsec = do_div(sec, 1000000000);
> > + u32 tstamp = enetc_update_ptp_sync_msg(priv, skb);
> >
> > /* Configure extension BD */
> > - temp_bd.ext.tstamp = cpu_to_le32(lo & 0x3fffffff);
> > + temp_bd.ext.tstamp = cpu_to_le32(tstamp);
> > e_flags |= ENETC_TXBD_E_FLAGS_ONE_STEP_PTP;
> > -
> > - /* Update originTimestamp field of Sync packet
> > - * - 48 bits seconds field
> > - * - 32 bits nanseconds field
> > - *
> > - * In addition, the UDP checksum needs to be updated
> > - * by software after updating originTimestamp field,
> > - * otherwise the hardware will calculate the wrong
> > - * checksum when updating the correction field and
> > - * update it to the packet.
> > - */
> > - data = skb_mac_header(skb);
> > - new_sec_h = htons((sec >> 32) & 0xffff);
> > - new_sec_l = htonl(sec & 0xffffffff);
> > - new_nsec = htonl(nsec);
> > - if (enetc_cb->udp) {
> > - struct udphdr *uh = udp_hdr(skb);
> > - __be32 old_sec_l, old_nsec;
> > - __be16 old_sec_h;
> > -
> > - old_sec_h = *(__be16 *)(data + tstamp_off);
> > - inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
> > - new_sec_h, false);
> > -
> > - old_sec_l = *(__be32 *)(data + tstamp_off + 2);
> > - inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
> > - new_sec_l, false);
> > -
> > - old_nsec = *(__be32 *)(data + tstamp_off + 6);
> > - inet_proto_csum_replace4(&uh->check, skb, old_nsec,
> > - new_nsec, false);
> > - }
> > -
> > - *(__be16 *)(data + tstamp_off) = new_sec_h;
> > - *(__be32 *)(data + tstamp_off + 2) = new_sec_l;
> > - *(__be32 *)(data + tstamp_off + 6) = new_nsec;
> > -
> > - /* Configure single-step register */
> > - val = ENETC_PM0_SINGLE_STEP_EN;
> > - val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
> > - if (enetc_cb->udp)
> > - val |= ENETC_PM0_SINGLE_STEP_CH;
> > -
> > - enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP,
> > - val);
> > } else if (do_twostep_tstamp) {
> > skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> > e_flags |= ENETC_TXBD_E_FLAGS_TWO_STEP_PTP; diff --git
> > a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > index 73763e8f4879..377c96325814 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > @@ -614,6 +614,7 @@ enum enetc_txbd_flags {
> > #define ENETC_TXBD_STATS_WIN BIT(7)
> > #define ENETC_TXBD_TXSTART_MASK GENMASK(24, 0) #define
> > ENETC_TXBD_FLAGS_OFFSET 24
> > +#define ENETC_TXBD_TSTAMP GENMASK(29, 0)
>
> Suppose this type patch no any addtional change to reduce review efforts.
>
> Or you need say replace 0x3fffffff with ENETC_TXBD_TSTAMP.
>

Okay, I will describe this change in the commit message, thanks

>
> >
> > static inline __le32 enetc_txbd_set_tx_start(u64 tx_start, u8 flags)
> > {
> > --
> > 2.34.1
> >