* [PATCH] can/peak_usb: fix timestamp wrapping
@ 2020-07-28 9:07 Stephane Grosjean
2020-09-02 11:43 ` TR: " Stéphane Grosjean
0 siblings, 1 reply; 3+ messages in thread
From: Stephane Grosjean @ 2020-07-28 9:07 UTC (permalink / raw)
To: linux-can Mailing List; +Cc: Stephane Grosjean
Fabian Inostroza <fabianinostrozap@gmail.com> has discovered a potential
problem in the hardware timestamp reporting from the PCAN-USB USB CAN
interface (only), related to the fact that a timestamp of an event may
precede the timestamp used for synchronization when both records are part
of the same USB packet. However, this case was used to detect the wrapping
of the time counter.
This patch details and fixes the two identified cases where this problem
can occur.
Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 51 ++++++++++++++++++--
1 file changed, 46 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index 0b7766b715fd..267ddc18ed51 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -130,14 +130,55 @@ void peak_usb_get_ts_time(struct peak_time_ref *time_ref, u32 ts, ktime_t *time)
/* protect from getting time before setting now */
if (ktime_to_ns(time_ref->tv_host)) {
u64 delta_us;
+ s64 delta_ts = 0;
+
+ /* General case: dev_ts_1 < dev_ts_2 < ts, with:
+ *
+ * - dev_ts_1 = previous sync timestamp
+ * - dev_ts_2 = last sync timestamp
+ * - ts = event timestamp
+ * - ts_period = known sync period (theoretical)
+ * ~ dev_ts2 - dev_ts1
+ * *but*:
+ *
+ * - time counters wrap (see adapter->ts_used_bits)
+ * - sometimes, dev_ts_1 < ts < dev_ts2
+ *
+ * "normal" case (sync time counters increase):
+ * must take into account case when ts wraps (tsw)
+ *
+ * < ts_period > < >
+ * | | |
+ * ---+--------+----+-------0-+--+-->
+ * ts_dev_1 | ts_dev_2 |
+ * ts tsw
+ */
+ if (time_ref->ts_dev_1 < time_ref->ts_dev_2) {
+ /* case when event time (tsw) wraps */
+ if (ts < time_ref->ts_dev_1)
+ delta_ts = 1 << time_ref->adapter->ts_used_bits;
+
+ /* Otherwise, sync time counter (ts_dev_2) has wrapped:
+ * handle case when event time (tsn) hasn't.
+ *
+ * < ts_period > < >
+ * | | |
+ * ---+--------+--0-+---------+--+-->
+ * ts_dev_1 | ts_dev_2 |
+ * tsn ts
+ */
+ } else if (time_ref->ts_dev_1 < ts) {
+ delta_ts = -(1 << time_ref->adapter->ts_used_bits);
+ }
- delta_us = ts - time_ref->ts_dev_2;
- if (ts < time_ref->ts_dev_2)
- delta_us &= (1 << time_ref->adapter->ts_used_bits) - 1;
+ /* add delay between last sync and event timestamps */
+ delta_ts += (signed int)(ts - time_ref->ts_dev_2);
- delta_us += time_ref->ts_total;
+ /* add time from beginning to last sync */
+ delta_ts += time_ref->ts_total;
- delta_us *= time_ref->adapter->us_per_ts_scale;
+ /* convert ticks number into microseconds */
+ delta_us = delta_ts * time_ref->adapter->us_per_ts_scale;
delta_us >>= time_ref->adapter->us_per_ts_shift;
*time = ktime_add_us(time_ref->tv_host_0, delta_us);
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* TR: [PATCH] can/peak_usb: fix timestamp wrapping
2020-07-28 9:07 [PATCH] can/peak_usb: fix timestamp wrapping Stephane Grosjean
@ 2020-09-02 11:43 ` Stéphane Grosjean
2020-09-30 23:48 ` Fabián Inostroza
0 siblings, 1 reply; 3+ messages in thread
From: Stéphane Grosjean @ 2020-09-02 11:43 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can
Hi Marc,
Any news about that patch?
---
Stéphane Grosjean
PEAK-System France
132, rue André Bisiaux
F-54320 MAXEVILLE
Tél : +(33) 9.72.54.51.97
De : Stephane Grosjean <s.grosjean@peak-system.com>
Envoyé : mardi 28 juillet 2020 11:07
À : linux-can Mailing List <linux-can@vger.kernel.org>
Cc : Stéphane Grosjean <s.grosjean@peak-system.com>
Objet : [PATCH] can/peak_usb: fix timestamp wrapping
Fabian Inostroza <fabianinostrozap@gmail.com> has discovered a potential
problem in the hardware timestamp reporting from the PCAN-USB USB CAN
interface (only), related to the fact that a timestamp of an event may
precede the timestamp used for synchronization when both records are part
of the same USB packet. However, this case was used to detect the wrapping
of the time counter.
This patch details and fixes the two identified cases where this problem
can occur.
Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 51 ++++++++++++++++++--
1 file changed, 46 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index 0b7766b715fd..267ddc18ed51 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -130,14 +130,55 @@ void peak_usb_get_ts_time(struct peak_time_ref *time_ref, u32 ts, ktime_t *time)
/* protect from getting time before setting now */
if (ktime_to_ns(time_ref->tv_host)) {
u64 delta_us;
+ s64 delta_ts = 0;
+
+ /* General case: dev_ts_1 < dev_ts_2 < ts, with:
+ *
+ * - dev_ts_1 = previous sync timestamp
+ * - dev_ts_2 = last sync timestamp
+ * - ts = event timestamp
+ * - ts_period = known sync period (theoretical)
+ * ~ dev_ts2 - dev_ts1
+ * *but*:
+ *
+ * - time counters wrap (see adapter->ts_used_bits)
+ * - sometimes, dev_ts_1 < ts < dev_ts2
+ *
+ * "normal" case (sync time counters increase):
+ * must take into account case when ts wraps (tsw)
+ *
+ * < ts_period > < >
+ * | | |
+ * ---+--------+----+-------0-+--+-->
+ * ts_dev_1 | ts_dev_2 |
+ * ts tsw
+ */
+ if (time_ref->ts_dev_1 < time_ref->ts_dev_2) {
+ /* case when event time (tsw) wraps */
+ if (ts < time_ref->ts_dev_1)
+ delta_ts = 1 << time_ref->adapter->ts_used_bits;
+
+ /* Otherwise, sync time counter (ts_dev_2) has wrapped:
+ * handle case when event time (tsn) hasn't.
+ *
+ * < ts_period > < >
+ * | | |
+ * ---+--------+--0-+---------+--+-->
+ * ts_dev_1 | ts_dev_2 |
+ * tsn ts
+ */
+ } else if (time_ref->ts_dev_1 < ts) {
+ delta_ts = -(1 << time_ref->adapter->ts_used_bits);
+ }
- delta_us = ts - time_ref->ts_dev_2;
- if (ts < time_ref->ts_dev_2)
- delta_us &= (1 << time_ref->adapter->ts_used_bits) - 1;
+ /* add delay between last sync and event timestamps */
+ delta_ts += (signed int)(ts - time_ref->ts_dev_2);
- delta_us += time_ref->ts_total;
+ /* add time from beginning to last sync */
+ delta_ts += time_ref->ts_total;
- delta_us *= time_ref->adapter->us_per_ts_scale;
+ /* convert ticks number into microseconds */
+ delta_us = delta_ts * time_ref->adapter->us_per_ts_scale;
delta_us >>= time_ref->adapter->us_per_ts_shift;
*time = ktime_add_us(time_ref->tv_host_0, delta_us);
--
2.25.1
--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
Unsere Datenschutzerklaerung mit wichtigen Hinweisen
zur Behandlung personenbezogener Daten finden Sie unter
www.peak-system.com/Datenschutz.483.0.html
--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
Unsere Datenschutzerklaerung mit wichtigen Hinweisen
zur Behandlung personenbezogener Daten finden Sie unter
www.peak-system.com/Datenschutz.483.0.html
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] can/peak_usb: fix timestamp wrapping
2020-09-02 11:43 ` TR: " Stéphane Grosjean
@ 2020-09-30 23:48 ` Fabián Inostroza
0 siblings, 0 replies; 3+ messages in thread
From: Fabián Inostroza @ 2020-09-30 23:48 UTC (permalink / raw)
To: Stéphane Grosjean; +Cc: Marc Kleine-Budde, linux-can
Ping
Fabián Antonio Inostroza Paredes
Ingeniero Civil Electrónico
Fono: +56962243036
On Wed, Sep 2, 2020 at 7:44 AM Stéphane Grosjean
<s.grosjean@peak-system.com> wrote:
>
> Hi Marc,
>
> Any news about that patch?
>
> ---
> Stéphane Grosjean
> PEAK-System France
> 132, rue André Bisiaux
> F-54320 MAXEVILLE
> Tél : +(33) 9.72.54.51.97
>
>
> De : Stephane Grosjean <s.grosjean@peak-system.com>
> Envoyé : mardi 28 juillet 2020 11:07
> À : linux-can Mailing List <linux-can@vger.kernel.org>
> Cc : Stéphane Grosjean <s.grosjean@peak-system.com>
> Objet : [PATCH] can/peak_usb: fix timestamp wrapping
>
> Fabian Inostroza <fabianinostrozap@gmail.com> has discovered a potential
> problem in the hardware timestamp reporting from the PCAN-USB USB CAN
> interface (only), related to the fact that a timestamp of an event may
> precede the timestamp used for synchronization when both records are part
> of the same USB packet. However, this case was used to detect the wrapping
> of the time counter.
>
> This patch details and fixes the two identified cases where this problem
> can occur.
>
> Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
> ---
> drivers/net/can/usb/peak_usb/pcan_usb_core.c | 51 ++++++++++++++++++--
> 1 file changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> index 0b7766b715fd..267ddc18ed51 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> @@ -130,14 +130,55 @@ void peak_usb_get_ts_time(struct peak_time_ref *time_ref, u32 ts, ktime_t *time)
> /* protect from getting time before setting now */
> if (ktime_to_ns(time_ref->tv_host)) {
> u64 delta_us;
> + s64 delta_ts = 0;
> +
> + /* General case: dev_ts_1 < dev_ts_2 < ts, with:
> + *
> + * - dev_ts_1 = previous sync timestamp
> + * - dev_ts_2 = last sync timestamp
> + * - ts = event timestamp
> + * - ts_period = known sync period (theoretical)
> + * ~ dev_ts2 - dev_ts1
> + * *but*:
> + *
> + * - time counters wrap (see adapter->ts_used_bits)
> + * - sometimes, dev_ts_1 < ts < dev_ts2
> + *
> + * "normal" case (sync time counters increase):
> + * must take into account case when ts wraps (tsw)
> + *
> + * < ts_period > < >
> + * | | |
> + * ---+--------+----+-------0-+--+-->
> + * ts_dev_1 | ts_dev_2 |
> + * ts tsw
> + */
> + if (time_ref->ts_dev_1 < time_ref->ts_dev_2) {
> + /* case when event time (tsw) wraps */
> + if (ts < time_ref->ts_dev_1)
> + delta_ts = 1 << time_ref->adapter->ts_used_bits;
> +
> + /* Otherwise, sync time counter (ts_dev_2) has wrapped:
> + * handle case when event time (tsn) hasn't.
> + *
> + * < ts_period > < >
> + * | | |
> + * ---+--------+--0-+---------+--+-->
> + * ts_dev_1 | ts_dev_2 |
> + * tsn ts
> + */
> + } else if (time_ref->ts_dev_1 < ts) {
> + delta_ts = -(1 << time_ref->adapter->ts_used_bits);
> + }
>
> - delta_us = ts - time_ref->ts_dev_2;
> - if (ts < time_ref->ts_dev_2)
> - delta_us &= (1 << time_ref->adapter->ts_used_bits) - 1;
> + /* add delay between last sync and event timestamps */
> + delta_ts += (signed int)(ts - time_ref->ts_dev_2);
>
> - delta_us += time_ref->ts_total;
> + /* add time from beginning to last sync */
> + delta_ts += time_ref->ts_total;
>
> - delta_us *= time_ref->adapter->us_per_ts_scale;
> + /* convert ticks number into microseconds */
> + delta_us = delta_ts * time_ref->adapter->us_per_ts_scale;
> delta_us >>= time_ref->adapter->us_per_ts_shift;
>
> *time = ktime_add_us(time_ref->tv_host_0, delta_us);
> --
> 2.25.1
>
>
> --
> PEAK-System Technik GmbH
> Sitz der Gesellschaft Darmstadt - HRB 9183
> Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
> Unsere Datenschutzerklaerung mit wichtigen Hinweisen
> zur Behandlung personenbezogener Daten finden Sie unter
> www.peak-system.com/Datenschutz.483.0.html
>
> --
> PEAK-System Technik GmbH
> Sitz der Gesellschaft Darmstadt - HRB 9183
> Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
> Unsere Datenschutzerklaerung mit wichtigen Hinweisen
> zur Behandlung personenbezogener Daten finden Sie unter
> www.peak-system.com/Datenschutz.483.0.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-30 23:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 9:07 [PATCH] can/peak_usb: fix timestamp wrapping Stephane Grosjean
2020-09-02 11:43 ` TR: " Stéphane Grosjean
2020-09-30 23:48 ` Fabián Inostroza
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).