| Message ID | 20250512074721.4118376-1-anton.nadezhdin@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [iwl-net,v2] ice/ptp: Fix reporting of crosstimestamp | expand |
[Cc: +Thomas] Dear Anton, Thank you for your patch. For the summary, I’d suggest something like: ice/ptp: Report crosstimestamp in ns Am 12.05.25 um 09:47 schrieb Anton Nadezhdin: > Set use_nsecs=true as timestamp is reported in ns. Lack of this result > in smaller timestamp error window which case error during phc2sys ca*u*se > execution on some platforms: > phc2sys[1768.256]: ioctl PTP_SYS_OFFSET_PRECISE: Invalid argument > > Previously function convert_art_to_tsc was converting ts to the cycles > instead of ns. > > Testing hints (ethX is PF netdev): > phc2sys -s ethX -c CLOCK_REALTIME -O 37 -m On what system with what hardware? Does the program output anything? > Fixes: d4bea547ebb57 (ice/ptp: Remove convert_art_to_tsc()) > Signed-off-by: Anton Nadezhdin <anton.nadezhdin@intel.com> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> > Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_ptp.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c > index 1fd1ae03eb90..11ed48a62b53 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -2307,6 +2307,7 @@ static int ice_capture_crosststamp(ktime_t *device, > ts = ((u64)ts_hi << 32) | ts_lo; > system->cycles = ts; > system->cs_id = CSID_X86_ART; > + system->use_nsecs = true; > > /* Read Device source clock time */ > ts_lo = rd32(hw, cfg->dev_time_l[tmr_idx]); Kind regards, Paul
[Remove lakshmi.sowjanya.d@intel.com again, 550 #5.1.0 Address rejected.] Am 12.05.25 um 10:10 schrieb Paul Menzel: > [Cc: +Thomas, Lakshmi] > > Dear Anton, > > > Thank you for your patch. For the summary, I’d suggest something like: > > ice/ptp: Report crosstimestamp in ns > > > Am 12.05.25 um 09:47 schrieb Anton Nadezhdin: >> Set use_nsecs=true as timestamp is reported in ns. Lack of this result >> in smaller timestamp error window which case error during phc2sys > > ca*u*se > >> execution on some platforms: >> phc2sys[1768.256]: ioctl PTP_SYS_OFFSET_PRECISE: Invalid argument >> >> Previously function convert_art_to_tsc was converting ts to the cycles >> instead of ns. >> >> Testing hints (ethX is PF netdev): >> phc2sys -s ethX -c CLOCK_REALTIME -O 37 -m > > On what system with what hardware? > > Does the program output anything? > >> Fixes: d4bea547ebb57 (ice/ptp: Remove convert_art_to_tsc()) >> Signed-off-by: Anton Nadezhdin <anton.nadezhdin@intel.com> >> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> >> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> >> --- >> drivers/net/ethernet/intel/ice/ice_ptp.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ >> ethernet/intel/ice/ice_ptp.c >> index 1fd1ae03eb90..11ed48a62b53 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c >> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c >> @@ -2307,6 +2307,7 @@ static int ice_capture_crosststamp(ktime_t *device, >> ts = ((u64)ts_hi << 32) | ts_lo; >> system->cycles = ts; >> system->cs_id = CSID_X86_ART; >> + system->use_nsecs = true; >> /* Read Device source clock time */ >> ts_lo = rd32(hw, cfg->dev_time_l[tmr_idx]); > > Kind regards, > > Paul
On 5/12/2025 1:10 AM, Paul Menzel wrote: > [Cc: +Thomas] > > Dear Anton, > > > Thank you for your patch. For the summary, I’d suggest something like: > > ice/ptp: Report crosstimestamp in ns > > IMHO this should still be "fix ...". This is only a bug because of the refactor to remove convert_art_to_tsc(). It was always intended to report the cross timestamp in nanoseconds, so a title of "report crosstimestamp in ns" gives the wrong impression. This is fixing a bug, not changing an intended behavior.
Agree what it should be fix... There was some general changes in kernel which introduce use_nsecs see: https://lore.kernel.org/r/20240513103813.5666-2-lakshmi.sowjanya.d@intel.com And during removal of convert_art_to_tsc() in a process of converting to new API the use_nsecs in the ICE driver was not set. -----Original Message----- From: Keller, Jacob E <jacob.e.keller@intel.com> Sent: Monday, May 12, 2025 11:01 PM To: Paul Menzel <pmenzel@molgen.mpg.de>; Nadezhdin, Anton <anton.nadezhdin@intel.com> Cc: intel-wired-lan@lists.osuosl.org; richardcochran@gmail.com; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Thomas Gleixner <tglx@linutronix.de> Subject: Re: [Intel-wired-lan] [PATCH iwl-net v2] ice/ptp: Fix reporting of crosstimestamp On 5/12/2025 1:10 AM, Paul Menzel wrote: > [Cc: +Thomas] > > Dear Anton, > > > Thank you for your patch. For the summary, I’d suggest something like: > > ice/ptp: Report crosstimestamp in ns > > IMHO this should still be "fix ...". This is only a bug because of the refactor to remove convert_art_to_tsc(). It was always intended to report the cross timestamp in nanoseconds, so a title of "report crosstimestamp in ns" gives the wrong impression. This is fixing a bug, not changing an intended behavior.
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c index 1fd1ae03eb90..11ed48a62b53 100644 --- a/drivers/net/ethernet/intel/ice/ice_ptp.c +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c @@ -2307,6 +2307,7 @@ static int ice_capture_crosststamp(ktime_t *device, ts = ((u64)ts_hi << 32) | ts_lo; system->cycles = ts; system->cs_id = CSID_X86_ART; + system->use_nsecs = true; /* Read Device source clock time */ ts_lo = rd32(hw, cfg->dev_time_l[tmr_idx]);