Message-Id: <202512222355.10509.pisa@fel.cvut.cz>
Date: Mon, 22 Dec 2025 23:55:10 +0100
From: Pavel Pisa <pisa@....cvut.cz>
To: David Laight <david.laight.linux@...il.com>,
 Ondrej Ille <ondrej.ille@...il.com>
Cc: "Marc Kleine-Budde" <mkl@...gutronix.de>,
 Andrea Daoud <andreadaoud6@...il.com>,
 linux-can@...r.kernel.org,
 Wolfgang Grandegger <wg@...ndegger.com>,
 "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>,
 Christophe JAILLET <christophe.jaillet@...adoo.fr>,
 netdev@...r.kernel.org,
 Jan Altenberg <Jan.Altenberg@...dl.org>
Subject: Re: ctucanfd: possible coding error in ctucan_set_secondary_sample_point causing SSP not enabled

Dear David Laight, Ondrej Ille and others,

On Monday 22 of December 2025 19:22:11 David Laight wrote:
> On Mon, 22 Dec 2025 17:20:49 +0100
>
> Marc Kleine-Budde <mkl@...gutronix.de> wrote:
> > On 22.12.2025 16:51:07, Ondrej Ille wrote:
> > > yes, your thinking is correct, there is a bug there.
> > >
> > > This was pointed to by another user right in the CTU CAN FD repository
> > > where the Linux driver also lives:
> > > https://github.com/Blebowski/CTU-CAN-FD/pull/2
> > >
> > > It is as you say, it should be:
> > >
> > > -- ssp_cfg |= FIELD_PREP(REG_TRV_DELAY_SSP_SRC, 0x1);
> > > ++ ssp_cfg |= FIELD_PREP(REG_TRV_DELAY_SSP_SRC, 0x0);
> >
> > This statement has no effect, as 'ssp_cfg |= 0x0' is still 'ssp_cfg'.
>
> The compiler will optimise it away - so it is the same as a comment.
>
> > IMHO it's better to add a comment that says, why you don't set
> > REG_TRV_DELAY_SSP_SRC. Another option is to add create a define that
> > replaces 0x1 and 0x0 for REG_TRV_DELAY_SSP_SRC with a speaking name.
>
> Looking at the header, the 'field' is two bits wide.
> So what you really want the code to look like is:
> 	ssp_cfg |= REG_TRV_DELAY_SSP_SRC(n);
> There is nothing to stop working - it just needs the right defines.
> Sort of FIELD_PREP(GENMASK(25, 24), n) - but you can do a lot better than
> that. The inverse is also possible:
> 	val = GET_VAL(REG_TRV_DELAY_SSP_SRC, reg_val);
> #define GET_VAL(x, reg) ((reg & x(-1))/x(1))

I have no problem to prepare patch against 

git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git

and we have running daily testing at CTU for latest mainline and RT

https://canbus.pages.fel.cvut.cz/#can-bus-channels-mutual-latency-testing

and each CTU CAN FD master RTL and related driver is tested on FPGA
as well id the change in IP core repo is detected

https://gitlab.fel.cvut.cz/canbus/zynq/zynq-can-sja1000-top/-/pipelines

As for this specific case, I am aware of it for longer time
but the last time when we met with Ondrej Ille this part
as been the last one on the table and the firm confirmation
what is the best value have not been stated.

ssp_cfg |= FIELD_PREP(REG_TRV_DELAY_SSP_SRC, xxx);

There are three options

SSP_SRC Source of Secondary sampling point.
  - 0b00 - SSP_SRC_MEAS_N_OFFSET - SSP position = TRV_DELAY
    (Measured Transmitter delay) + SSP_OFFSET.
  - 0b01 - SSP_SRC_NO_SSP - SSP is not used. Transmitter uses
    regular Sampling Point during data bit rate.
  - 0b10 - SSP_SRC_OFFSET - SSP position = SSP_OFFSET. Measured
    Transmitter delay value is ignored.

there is filed for manual setting or adjustment

SSP_OFFSET Secondary sampling point offset. Value is given as multiple of minimal Time quanta.

As for the change, I can test the change in normal conditions
but I am afraid that there can be chance that automatic tuning
can lead to problems in some specific cases and I would like
to hear some confirmation from others there and may it be information
how it is solved in other drivers. SSP seems to be supported by

  usb/esd_usb.c  seems to be set to 0 only
  m_can/m_can.c for bitrate > 2500000

I am not sure if there is some option to (at least) switch is on and off
through some netlink, iproute2 or other API. It would worth to have
there some option to switch it off (for example through /sys) in case
that it causes some problems.

As for the driver, there are more ideas pending on my table.

1) the driver is fixed on 4 Tx Buffers synthesis value
for CTU CAN FD IP core. I am not aware about any other value
in real use (FPGA or silicon) but if the core is synthesized
with other value then driver would fail.  If more are used,
then current driver code stuck on Tx empty infinite interrupt,
if less, messages would be lost.
The option to obtain the number of Tx buffers from hardware
has been added into design and we have proper code in RTEMS
driver 

https://gitlab.rtems.org/rtems/rtos/rtems/-/blob/main/cpukit/dev/can/ctucanfd/ctucanfd.c?ref_type=heads#L1783

2) there is problem with bus monitor mode in pre 2.5.2 CTU CAN FD
IP core version. I have found as solution for some mass produced
silicon. It should be propagated into Linux driver. 

         mode_reg = (mode->flags & CAN_CTRLMODE_LISTENONLY) ?
                         (mode_reg | REG_MODE_BMM) :
                         (mode_reg & ~REG_MODE_BMM);

+         mode_reg = (mode->flags & CAN_CTRLMODE_LISTENONLY) ?
+                         (mode_reg | REG_MODE_ROM) :
+                         (mode_reg & ~REG_MODE_ROM);
+
-        mode_reg = (mode->flags & CAN_CTRLMODE_PRESUME_ACK) ?
+        mode_reg = (mode->flags & (CAN_CTRLMODE_PRESUME_ACK | CAN_CTRLMODE_LISTENONLY)) ?
                         (mode_reg | REG_MODE_ACF) :
                         (mode_reg & ~REG_MODE_ACF);

But it should be checked for which versions it is applicable.
There were more variants of the problem around listen only
mode for older core version.

3) there is long term prepared support for Rx timestamping
which is used for years in our latency tester, but it did not
pass review yet. It is result of
https://dspace.cvut.cz/bitstream/handle/10467/101450/F3-DP-2022-Vasilevski-Matej-vasilmat.pdf
and would worth to be cleaned up with help of reviewers feedback
and other help to pass. Waits for my or others time as well.

4) there is even option to implement HW Tx timestamps on later
core version which can feed Tx frames to Rx queue with timestamp
filled. But only latter core versions can report sending
Tx buffer for loopback frames in Rx queue

5) the multiqueue support. Again, it is tested in our RTEMS
complete CAN FD framework which will be extended by SJA1000
and LPC drivers soon. The theory
https://wiki.control.fel.cvut.cz/mediawiki/images/c/cc/Dp_2024_lenc_michal.pdf
CiA article
https://www.can-cia.org/fileadmin/cia/documents/proceedings/2024_lenc_pisa.pdf
and implementation in RTEMS driver
https://gitlab.rtems.org/rtems/rtos/rtems/-/tree/main/cpukit/dev/can/ctucanfd
https://docs.rtems.org/docs/main/bsp-howto/can.html

6) solving trimming issues of Linux kernel SocketCAN implementation.
Our RTEMS characted driver code has maximal latancies of 65 used even
with full BSD TCP/IP stack stress testing when CAN related stuff has
highest priorities. No matter what we done or Linux even with suggestions
of others, we experience multi-millisecond latencies on Linux even of
preempt RT kernels.

I have donated one system from CTU pocket (original Martin Jerabek's design
of Zynq based CAN latester) and one from my personal pocket (our Zynq based
MZ_APO design) to OSADL https://www.osadl.org/OSADL-QA-Farm-Real-time.linux-real-time.0.html
but I have not found time and external interest to start more work there. Again some
cooperation and interrest by these who work on the projects as SocketCAN and even
earn some money on it (not my case for 20+ years of work) would help. Some small
money or GSoC for example to find some studnet to work on project for example
for summer work could help. OSADL offers latency hunting services as well so
some cooperation and funding could help there. But no car marker or Automotive
Grade Linux funded people have expressed interrest yet. They sell Kubernetes
containers on the wheel, AI camera processing but that the commands
do not pass though CAN or other HW in time so save pedestrian life
seems to be unimportant detail.

Best wishes,

                Pavel Pisa
    phone:      +420 603531357
    e-mail:     pisa@....felk.cvut.cz
    Department of Control Engineering FEE CVUT
    Karlovo namesti 13, 121 35, Prague 2
    university: http://control.fel.cvut.cz/
    personal:   http://cmp.felk.cvut.cz/~pisa
    social:     https://social.kernel.org/ppisa
    projects:   https://www.openhub.net/accounts/ppisa
    CAN related:http://canbus.pages.fel.cvut.cz/
    RISC-V education: https://comparch.edu.cvut.cz/
    Open Technologies Research Education and Exchange Services
    https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home

Powered by blists - more mailing lists