linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Stéphane Grosjean" <s.grosjean@peak-system.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: "linux-can@vger.kernel.org" <linux-can@vger.kernel.org>
Subject: TR: [PATCH] can/peak_usb: fix timestamp wrapping
Date: Wed, 2 Sep 2020 11:43:11 +0000	[thread overview]
Message-ID: <VI1PR03MB50536800A01EB17C97D9B6BDD62F0@VI1PR03MB5053.eurprd03.prod.outlook.com> (raw)
In-Reply-To: <20200728090736.7628-1-s.grosjean@peak-system.com>

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

  reply	other threads:[~2020-09-02 11:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-28  9:07 [PATCH] can/peak_usb: fix timestamp wrapping Stephane Grosjean
2020-09-02 11:43 ` Stéphane Grosjean [this message]
2020-09-30 23:48   ` Fabián Inostroza

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR03MB50536800A01EB17C97D9B6BDD62F0@VI1PR03MB5053.eurprd03.prod.outlook.com \
    --to=s.grosjean@peak-system.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).