On Thu, Dec 03, 2020 at 05:26:10PM -0800, Richard Cochran wrote:
> On Fri, Dec 04, 2020 at 03:02:58AM +0200, Vladimir Oltean wrote:
> > On Thu, Dec 03, 2020 at 04:34:46PM -0800, Richard Cochran wrote:
> > > > Actually, for dpaa2, the one-step timestamping procedure just does:
> > > >
> > > > correctionField += t_TX - t_passed_inband
> > > >
> > > > where the driver populates:
> > > > t_passed_inband = originTimestamp = approximate PTP time of software 
> > > > packet delivery
> > >
> > > ptp4l sets to originTimestamp zero!
> >
> > And the driver happily overwrites it, I don't understand what the issue
> > is. If not the driver, the MAC would, anyway.
>
> huh?
>
> Doesn't it looks strang if the originTimestamp arrives at the client
> with zero.
>
> Or maybe I didn't understand what the dpaa2 driver/HW does?
>
> Does the driver populate the originTimestamp?

Too much confusion, why don't I just show you the code.

        if (skb->cb[0] == TX_TSTAMP_ONESTEP_SYNC) {
                if (dpaa2_eth_ptp_parse(skb, &msgtype, &twostep, &udp,
                                        &offset1, &offset2) ||                  
// offset1 points to correctionField, offset2 to originTimestamp
                    msgtype != PTP_MSGTYPE_SYNC || twostep) {
                        WARN_ONCE(1, "Bad packet for one-step timestamping\n");
                        return;
                }

                /* Mark the frame annotation status as valid */
                frc = dpaa2_fd_get_frc(fd);
                dpaa2_fd_set_frc(fd, frc | DPAA2_FD_FRC_FASV);

                /* Mark the PTP flag for one step timestamping */
                fas = dpaa2_get_fas(buf_start, true);
                fas->status = cpu_to_le32(DPAA2_FAS_PTP);

                dpaa2_ptp->caps.gettime64(&dpaa2_ptp->caps, &ts);               
// driver reads current PTP time here
                ns = dpaa2_get_ts(buf_start, true);
                *ns = cpu_to_le64(timespec64_to_ns(&ts) /                       
// and puts it into the frame annotation area,
                                  DPAA2_PTP_CLK_PERIOD_NS);                     
// a memory region that is passed in-band with the packet

                /* Update current time to PTP message originTimestamp field */
                ns_to_ptp_tstamp(&origin_timestamp, le64_to_cpup(ns));
                data = skb_mac_header(skb);
                *(__be16 *)(data + offset2) = htons(origin_timestamp.sec_msb);
                *(__be32 *)(data + offset2 + 2) =
                        htonl(origin_timestamp.sec_lsb);
                *(__be32 *)(data + offset2 + 6) = htonl(origin_timestamp.nsec); 
// it also puts the same value in the originTimestamp field,
                                                                                
// therefore making the two equal
                cfg.en = 1;
                cfg.ch_update = udp;
                cfg.offset = offset1;
                cfg.peer_delay = 0;

                if (dpni_set_single_step_cfg(priv->mc_io, 0, priv->mc_token,    
// then the MAC is told to apply one-step timestamping for this frame
                                             &cfg))
                        WARN_ONCE(1, "Failed to set single step register");
        }

Or, in short:

> > > > the driver populates:
> > > > t_passed_inband = originTimestamp = approximate PTP time of software 
> > > > packet delivery

This is like, pure C syntax.

When you write a = b = 1, what happens is that the "1" expression gets
written in the left-hand side variables a and b.


the t_passed_inband doesn't _have_ to be equal to the originTimestamp.
It's just that this is how the driver sets things up so that, in the
end, the sum between the originTimestamp and the correctionField
contains the precise t_TX, given the way that this MAC timestamps using
one-step.

The client sees just an approximate originTimestamp, the correctionField
is what makes it precise. Here's what the standard says:

9.5.9.3 One-step clocks
The originTimestamp field of the Sync message shall be an estimate no worse 
than ±1 s of the
<syncEventEgressTimestamp> excluding any fractional nanoseconds.


My entire point was that I hope we won't end up with two different ways
to request the one-step TX timestamping of a Sync message. One way for
the termination scenario, one for the forwarding scenario.


_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to