RE: [PATCH net-next v6 06/15] net: ethernet: oa_tc6: Support for hardware timestamp

[Selvamani Rajagopal]

> -----Original Message-----
> From: Jerry.Ray@xxxxxxxxxxxxx <Jerry.Ray@xxxxxxxxxxxxx>
> Subject: RE: [PATCH net-next v6 06/15] net: ethernet: oa_tc6: Support for hardware
> timestamp
> 
> The receive path here unconditionally consumes a 64-bit (OA_TC6_TSTAMP_SZ = 8
> byte) frame timestamp: it is gated only on the footer RTSA bit, always copies
> two 32-bit words, and pulls sizeof(ts) from the skb. The RX buffer is likewise
> sized with a fixed + OA_TC6_TSTAMP_SZ. Nothing consults the configured timestamp
> width.
> 
> But oa_tc6_set_hwtstamp_settings() only sets CONFIG0.FTSE:
> 


I do remember our conversation on this subject.

For this comment and the comment at the bottom, I believe the feedback is to set 
CONFIG0_FTSS_64BIT bit In OA TC6 framework. Will do.


> > +}
> > +/* Tx timestamp capture register A (high) */
> > +#define OA_TC6_REG_TTSCA_HIGH (0x1010)
> > +
> 
> Please fix the value of OA_TC6_REG_TTSCA_HIGH to 0x0010 in patch 6 where it is
> introduced rather than correcting it in patch 12.


I didn't realize that. Let me check and fix it. Thanks


> 
> > /* Control command header */
> >
> > + cfg0 &= ~CONFIG0_FTSE_ENABLE;
> 
> It never sets the 64-bit frame-timestamp-select bit (CONFIG0 bit 6). So the
> framework enables timestamping for 32-bit bitwidth while the
> receive path strips 8 bytes.
> 
> This happens to work for the S2500 only because the S2500 driver forces 64-bit
> independently in its own SPI config (patch 12/15):
> 
> Please review my feedback against your v3 patch series on 5-Jun.
> (CONFIG0_FTSE_ENABLE | CONFIG0_FTSS_64BIT)
>