Re: ctucanfd: possible coding error in ctucan_set_secondary_sample_point causing SSP not enabled
From: Pavel Pisa <hidden>
Date: 2025-12-22 23:10:03
Also in:
linux-can
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 [off-list ref] wrote:quoted
On 22.12.2025 16:51:07, Ondrej Ille wrote:quoted
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.quoted
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@cmp.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