netdev - Re: CAN-FD Transmitter Delay Compensation (TDC) on mcp2518fd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMZ6RqJCZB6Q79JYfxD7PGboPwMndDQRKsuUEk5Q34fj2vOcYg@mail.gmail.com>
Date:   Fri, 18 Jun 2021 23:27:52 +0900
From:   Vincent MAILHOL <mailhol.vincent@...adoo.fr>
To:     Marc Kleine-Budde <mkl@...gutronix.de>
Cc:     linux-can <linux-can@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        Oliver Hartkopp <socketcan@...tkopp.net>,
        Thomas Kopp <thomas.kopp@...rochip.com>
Subject: Re: CAN-FD Transmitter Delay Compensation (TDC) on mcp2518fd

On Fri. 18 Jun 2021 at 21:44, Marc Kleine-Budde <mkl@...gutronix.de> wrote:
> On 18.06.2021 20:17:51, Vincent MAILHOL wrote:
> > > > I just noticed in the mcp2518fd data sheet:
> > > >
> > > > | bit 14-8 TDCO[6:0]: Transmitter Delay Compensation Offset bits;
> > > > | Secondary Sample Point (SSP) Two’s complement; offset can be positive,
> > > > | zero, or negative.
> > > > |
> > > > | 011 1111 = 63 x TSYSCLK
> > > > | ...
> > > > | 000 0000 = 0 x TSYSCLK
> > > > | ...
> > > > | 111 1111 = –64 x TSYSCLK
> > > >
> > > > Have you takes this into account?
> > >
> > > I have not. And I fail to understand what would be the physical
> > > meaning if TDCO is zero or negative.
>
> The mcp25xxfd family data sheet says:
>
> | SSP = TDCV + TDCO
>
> > > TDCV indicates the position of the bit start on the RX pin.
>
> If I understand correctly in automatic mode TDCV is measured by the CAN
> controller and reflects the transceiver delay.

Yes. I phrased it poorly but this is what I wanted to say. It is
the delay to propagate from the TX pin to the RX pin.

If TDCO = 0 then SSP = TDCV + 0 = TDCV thus the measurement
occurs at the bit start on the RX pin.

> I don't know why you want
> to subtract a time from that....
>
> The rest of the relevant registers:
>
> | TDCMOD[1:0]: Transmitter Delay Compensation Mode bits; Secondary Sample Point (SSP)
> | 10-11 = Auto; measure delay and add TDCO.
> | 01 = Manual; Do not measure, use TDCV + TDCO from register
> | 00 = TDC Disabled
> |
> | TDCO[6:0]: Transmitter Delay Compensation Offset bits; Secondary Sample Point (SSP)
> | Two’s complement; offset can be positive, zero, or negative.
> | 011 1111 = 63 x TSYSCLK
> | ...
> | 000 0000 = 0 x TSYSCLK
> | ...
> | 111 1111 = –64 x TSYSCLK
> |
> | TDCV[5:0]: Transmitter Delay Compensation Value bits; Secondary Sample Point (SSP)
> | 11 1111 = 63 x TSYSCLK
> | ...
> | 00 0000 = 0 x TSYSCLK

Aside from the negative TDCO, the rest is standard stuff. We can
note the absence of the TDCF but that's not a blocker.

> > > If TDCO is zero, the measurement occurs on the bit start when all
> > > the ringing occurs. That is a really bad choice to do the
> > > measurement. If it is negative, it means that you are measuring the
> > > previous bit o_O !?
>
> I don't know...
>
> > > Maybe I am missing something but I just do not get it.
> > >
> > > I believe you started to implement the mcp2518fd.
>
> No I've just looked into the register description.

OK. For your information, the ETAS ES58x FD devices do not allow
the use of manual mode for TDCV. The microcontroller from
Microchip supports it but ETAS firmware only exposes the
automatic TDCV mode. So it can not be used to test what would
occur if SSP = 0.

I will prepare a patch to allow zero value for both TDCV and
TDCO (I am a bit sad because I prefer the current design, but if
ISO allows it, I feel like I have no choice).  However, I refuse
to allow the negative TDCO value unless someone is able to
explain the rationale.


Yours sincerely,
Vincent