STM32H7 FDCAN

[fvantienen]

In the FDCANv1 driver on the latest trunk there is a mistake with the RXGFC register.
For the STM32H7 this register is called GFC and not RXGFC. This could easily be fixed, since it is only set in can_lld_start.

[Giovanni]

Hi,

The driver is a non-complete state, probably there are even more problems hidden in there.

Giovanni

[fvantienen]

Ok thanks, I found the patches mentioned in viewtopic.php?f=3&t=5122&start=90. The patch from github(https://github.com/ChibiOS/ChibiOS/pull/38/files) 'works' but is not complete/fully correct.

I want to make a correct patch/help, but not sure what the best approach is. If you look at https://github.com/ChibiOS/ChibiOS/pull ... c0c0cfR304, you will see the RAM being allocated. The current setup exceeds the maximum of 2560 words, thus when more than 20 TX buffers are used it hangs. Also this doesn't work for multiple FDCAN interface since the offset from the previous interface is not added. This setup should be configurable by the user, depending on the needs. My question is what the best approach would be for this to implement this correctly?

My idea now is to give all the sizes as config options per interface and remove them from the registry. Then we can also check if it exceeds the 2560 words and it can be configured per interface differently. Do you think that is a good approach?

[Giovanni]

Hi,

I have no clear ideas about this driver, this is why it is not complete

In general things that are not variable, for example a memory size, should go in the registry rather than in configuration. Is there a point in making those parameters configurable? it increases complexity and moves checks at runtime.

Giovanni

[fvantienen]

There are some decisions to be made by the user I think. For example how many filters and if you want extended or normal filters. Next to that the buffer sizes for receiving and transmitting can be set to different sizes, depending if you would only use normal CAN or longer messages. Then depending on how many interfaces you enable you need to divide the 2560 available words.

I think there are a couple of options:
- Add the registers like SIDFC,XIDFC,RXF0C,RXF1C,RXBC,TXEFC,TXBC,TXESC,RXESC to the config structure such that the user can choose how to setup the RAM. The access to the buffers can then also be easily calculated by these config structure variables.
- Add options like the amount of messages and size of the buffer to the config structure and do the calculation in the driver for these registers. This is more complicated and is not the best solution I think
- Add options to the mcuconf.h for each of the can devices. The advantage is that everything is done during compile time, but you loose the ability of configuring it on the fly. The downside is that you add >9 configuration options in the mcuconf.h, which are all the defines in the registry + 2(or more optionally) for the different buffer sizes.

I don't see a general configuration which could work for every solution, because I think most people want to trade between filters and RX/TX buffers. For me I would prefer the last option, as it is the easiest to implement. But I'm not sure how happy you are with adding so many configuration options?

[Giovanni]

Then the best option is to place register-level settings in the config structure like other drivers do, mcuconf.h is not meant for complex configurations.

Note that the driver is meant to run on H7 and also on G4/newer devices, the FDCAN is slightly differently configured. In the registry there should be something telling which kind of FDCAN is present in the device.

Giovanni

[fvantienen]

I now got an updated branch see https://github.com/ChibiOS/ChibiOS/comp ... an_stm32h7 or https://github.com/ChibiOS/ChibiOS/comp ... 32h7.patch for the patch.

This is tested with an STM32H7 and works, I don't have an STM32G4 so I can't test with that. A very simple ram configuration for normal can using fdcan is for example:

Code: Select all

  // Configure the RAM
  uavcan1.can_cfg.RXF0C = (32 << FDCAN_RXF0C_F0S_Pos) | (0 << FDCAN_RXF0C_F0SA_Pos);
  uavcan1.can_cfg.RXF1C = (32 << FDCAN_RXF1C_F1S_Pos) | (128 << FDCAN_RXF1C_F1SA_Pos);
  uavcan1.can_cfg.TXBC  = (32 << FDCAN_TXBC_TFQS_Pos) | (256 << FDCAN_TXBC_TBSA_Pos);
  uavcan1.can_cfg.TXESC = 0x000; // 8 Byte mode only (4 words per message)
  uavcan1.can_cfg.RXESC = 0x000; // 8 Byte mode only (4 words per message)


I would be happy if something like this could be put in the latest trunk.