Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] CAN and CAN FD bittiming/bitrate calculation consolidation and CAN drivers updates goals #15567

Open
1 task done
ppisa opened this issue Jan 15, 2025 · 6 comments
Open
1 task done
Labels
Type: Enhancement New feature or request

Comments

@ppisa
Copy link
Contributor

ppisa commented Jan 15, 2025

Is your feature request related to a problem? Please describe.

CAN/CAN FD bittiming calculation is spread in many diverging implementations and concepts in NuttX CAN drivers.

It is often solved by mapping to individual chip/controller timing parameters defined for discrete bitrate base, which makes controllers unusable for another chip variants or different clocks setups.

Describe the solution you'd like

Sound CAN/CAN FD subsystem where code duplication is seriously weighted against maintenance cost and problem to fix common or individual issues on many locations in the source tree. The Linux CAN subsystem has proven the broad scale reusability of the common controller codes. We have show such possibility in LinCAN even before SocketCAN and LinCAN base has been used for more system-less setups as well as ported to RTEMS recently. So there is space for incremental progress in proposed direction.

Describe alternatives you've considered

I would suggest to consider the solution which I have proposed more than 20 years ago for LinCAN and later updated and provided to SocketCAN

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/can/dev/calc_bittiming.c

The code which has been maintained for LinCAN, other our projects and updated for RTEMS recently has NuttX compatible license

https://gitlab.fel.cvut.cz/otrees/rtems/rtems-canfd/-/blob/master/lib/candrv/can-bittiming.c

I would like to know, what should be the prefix for NuttX to keep some namespace rules for single image linking with application. I propose nx_ prefix, that is rename rtems_can_bitrate2bittiming -> nx_can_bitrate2bittiming.

CTU CAN FD bittiming and changes

As for the CTU CAN FD driver, there is bittiming calculation missing completely in NuttX driver

https://github.com/apache/nuttx/blob/master/drivers/can/ctucanfd_pci.c

It is usable for test in QEMU emulated environment only for now.

The driver should be separated to common part and separate PCI mapping part. The common initialization function on RTEMS driver is

struct can_chip *ctucanfd_initialize(
  uint32_t addr,
  rtems_vector_number irq,
  int ntxbufs,
  rtems_option irq_option,
  unsigned long can_clk_rate
)

https://gitlab.fel.cvut.cz/otrees/rtems/rtems-canfd/-/blob/master/lib/candrv/ctucanfd/ctucanfd.c?ref_type=heads#L1491

Should be the prefix nx_ or some other applied there as well?

When the code is separated then the same base can be used for PCIe devices on real hardware and QEMU. And the same code could be reused for FPGA SoCs based systems.

I have experimental FPGA design with CTU CAN FD prepared for our LX_CPU (part of LX_RoCoN) board with NuttX support.

Our main CTU CAN FD development environment is XilinX Zynq, but that is not supported by NuttX.

But we are working on PolarFire (BeagleV Fire) CTU CAN FD support with RTEMS, Linux and it should be usable even for NuttX.

SJA1000

There is multiple implementations of the driver in the NuttX tree, some are fully based on our code, some probably only partially, i.e. new Espressif code which allows only limited set of discrete bitrates for limited input clocks frequencies and even for that it has lot of defines magic inside...

https://github.com/apache/nuttx/blob/master/drivers/can/kvaser_pci.c

https://github.com/apache/nuttx/blob/master/drivers/can/kvaser_pci.c

https://github.com/apache/nuttx/blob/master/arch/risc-v/src/esp32c3-legacy/esp32c3_twai.c

https://github.com/apache/nuttx/blob/master/arch/risc-v/src/common/espressif/esp_twai.c

https://github.com/apache/nuttx/blob/master/arch/xtensa/src/esp32/esp32_twai.c

This level of code duplication is nightmare and each copy has has some missing features and limitations. So long term goal should be to look for code reuse and ideally single central bittiming calculation and SJA1000 driver common, maintainable and durable functionality which allows mapping to different SoC integrations.

Verification

  • I have verified before submitting the report.
@ppisa ppisa added the Type: Enhancement New feature or request label Jan 15, 2025
@raiden00pl
Copy link
Member

+1 for common bittiming calculation interface. Names with nx_ prefix are OK. The only problem I see now is where to place such a common interface that must be shared across chardev and socketcan implementation ? We already had a PR that tries to generalize some of the code in CAN implementations, but it got stuck: #14759
I proposed there include/nuttx/can_cmn.h for common header but we need also place for common code, probably drivers/can/can_cmn.c will be enough.
Previously /drivers/can was reserved for chardev implementation, but since we
support PCI CAN cards in this directory that use both CAN interface - this is no longer valid. I don't see a better place for this kind of code.

CTU CAN FD bittiming and changes

I agree, but we need someone who could verify if the changes work on real HW. Personally, I don't have access to any CTU CAN FD hardware. Implementation without any verification on real device makes no sense to me.
Are there any commercially available PCI cards with this IP core? Playing with FPGA is out of the question for me at the moment :)

SJA1000

Also agree, but again - we need someone who could verify all changes on different hardware with SAJ1000: 1. ESP implementation, 2. FPGA implementation, 3. PCI implementation. This is quite difficult for one person and also difficult to coordinate several people in the project who could help.

@ppisa
Copy link
Contributor Author

ppisa commented Jan 24, 2025

+1 for common bittiming calculation interface. Names with nx_ prefix are OK.

Great, I will discuss that with @michallenc and we would reuse knowledge or code (license is OK for some time already) from our RTEMS project.

The only problem I see now is where to place such a common interface that must be shared across chardev and socketcan implementation ? We already had a PR that tries to generalize some of the code in CAN implementations, but it got stuck: #14759 I proposed there include/nuttx/can_cmn.h for common header but we need also place for common code, probably drivers/can/can_cmn.c will be enough. Previously /drivers/can was reserved for chardev implementation, but since we support PCI CAN cards in this directory that use both CAN interface - this is no longer valid. I don't see a better place for this kind of code.

I would propose drivers/can/can-bittiming.c and include/nuttx/can-bittiming.h, which can be shared easily. The bittiming code in Linux kernel SocketCAN has been moved to separated files during its evolution. So it would not be a problem.

CTU CAN FD bittiming and changes

I agree, but we need someone who could verify if the changes work on real HW. Personally, I don't have access to any CTU CAN FD hardware. Implementation without any verification on real device makes no sense to me. Are there any commercially available PCI cards with this IP core? Playing with FPGA is out of the question for me at the moment :)

CTU CAN FD has been run on multiple FPGAs, Altera (for some time Intel now), Xilinx, Lattice etc. The easiest it is on XilinX Zynq, but there is problem with missing NuttX support. I have that experiment build on our NXP LPC4088 + Xilinx XC6SLX9 based LX?RoCoN which has NuttX support. It not for real use, because FPGA is small and CTU CAN FD barely fits and nothing else, but I have tested that it can communicate with LPC CAN interface on this system from my system-less LinCAN port.

SJA1000

Also agree, but again - we need someone who could verify all changes on different hardware with SAJ1000: 1. ESP implementation, 2. FPGA implementation, 3. PCI implementation. This is quite difficult for one person and also difficult to coordinate several people in the project who could help.

The ESP is probably easiest, the best is ESP32C3 legacy because there is TWAI driver implemented by us, Jan Charvat's bachelor thesis solved under my mentorship which is based on LinCAN and includes proposed bittiming algorithms inlined, simplified in the driver code. So they could be removed and replaced by common ones and tested easily. The later Espressif non legacy ESP32Cx HAL based TWAI driver made me to weep when I have seen the defines and bitrates, baserates ifdefs mess.

@ppisa
Copy link
Contributor Author

ppisa commented Jan 24, 2025

In addition, NuttX seems to have reasonable support for Microchip's PolarFire SoCs. I have BeagleV-Fire SBCs based on the chip. I tested Linux and RTEMS on it, plan NuttX too and plan to have there CTU CAN FD tested as well. Even one student has selected that topic in September, but it seems that has not started work on the project yet. But in longer time, I hope to have the port and may it be find my time for it because there is some chance that my proposal to a flight computer on the rad-hard PoalrFire version for next Czech satellite mission would be heard. RTEMS would be main target in such case, but I would be happy to test NuttX and even use it for other project which do not require certification.

@ppisa
Copy link
Contributor Author

ppisa commented Jan 29, 2025

For your information (@raiden00pl, @acassis, @radek-pesina, @tmedicci ), the CAN FD stack developed for RTEMS by @michallenc has undergone review (hundreds minor style, naming etc. details solved during review) and has been mainlined.

I propose use bittiming from that driver for NuttX. See dev/can/can-bittiming.h and dev/can/can-bittiming.c. It is same model which I have proposed for SocketCAN 20 years ago and which is used by Linux kernel till today - include/uapi/linux/can/netlink.h and drivers/net/can/dev/calc_bittiming.c. The code originates from LinCAN and that basic part used for RTEMS has been maintained whole time outside Linux source tree and that is why we have had full right to use following license

BSD-2-Clause OR Apache-2.0 OR GPL-2.0-or-later

So the code should be properly licensed for NuttX.

I am not sure when I or somebody from people from the groups connected to me get to that but this is the plan, goal for future incremental step.

Then we can test the approach on CTU CAN FD, I have tested @raiden00pl CTU CAN FD NuttX driver against x86 QEMU. I have tested our complete RTEMS stack on our LPC40xx LX_CPU board with hacked CTU CAN FD instance in FPGA in addition to our standard Zynq setup and I have NuttX running on LX_CPU as well, so adapting the driver to run on real hardware should not take much effort after required bittiming support is available in NuttX.

@acassis
Copy link
Contributor

acassis commented Jan 29, 2025

Hi Pavel,
I think this is a great achievement! Kudos to @michallenc and you for working to integrate it on RTEMS. I hope it be integrated on NuttX as well!

@raiden00pl
Copy link
Member

I am not sure when I or somebody from people from the groups connected to me get to that but this is the plan, goal for future incremental step.

@ppisa if someone starts working on this issue, please let us know here. So that we don't duplicate work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants