On 03/01/2025 at 03:58, Stefan Mätje wrote: > This patch adds support for CAN-FD in the existing esd_402_pci > driver for the PCI based PCIe/402 CAN interface family from > esd GmbH. > > The patch is sent as RFC because the driver needs some > extensions in the netlink interface and user API. > > @Vincent: I see that it conflicts with your changes for > CAN-XL support in netlink.h but I send it as RFC > to show what I have now. No problem. Whoever is merged in second will rebase, shouldn't be hard though :) > The esdACC CAN controller has only a single bitrate prescaler > for both the nominal and data bitrate. The hardware was > designed this way to follow CiA recommendations for the > bitrate configuration. > > The detailed recommendations of the CiA can be found here: > https://www.can-cia.org/fileadmin/cia/documents/publications/cnlm/march_2018/18-1_p28_recommendation_for_the_canfd_bit-timing_holger_zeltwanger_cia.pdf >> The current bit rate calculation in the kernel is done independently > for the nominal and the data bitrate. This can lead to the fact > that the selected nominal (NBRP) and data bitrate prescalers (DBRP) > differ. This kind of configuration is not supported by the esdACC. > > In a first step to avoid this I propose to add a new control mode > CAN_CTRLMODE_FD_COMMON_BRP that tells the bitrate check logic > to reject bitrate settings with different NBRP and DBRP values. Do you think that we may have controllers with shared Classical CAN and CAN FD BRP but different CAN XL BRP? If not, I would suggest to make this "CAN XL ready" by using a CAN FD agnostic naming, e.g. CAN_CTRLMODE_COMMON_BRP or CAN_CTRLMODE_SHARED_BRP. > This CAN_CTRLMODE_FD_COMMON_BRP is set statically by the driver > to tell that it needs a common BRP (see patch 0005). > > The netlink part of this is done in the patches marked with > "DO NOT MERGE" in this patchset. > > In a second step the bitrate calculation should be changed to > prefer to use a common BRP at least if CAN_CTRLMODE_FD_COMMON_BRP > is set. This could be based on the bitrate calculation > algorithm of my colleague Oliver Thimm that I will present > in a follow up email. This email will then also discuss the > advantages of this approach in more detail. > > Unfortunately I have no patches yet. > > I will send a patch set for the iproute2 tool to add the > capability to set the CAN_CTRLMODE_FD_COMMON_BRP using > "fd-common-brp" with the ip command. Looking at the paper you linked, I understand that it is recommended to use a single BRP. But this raises a follow-up question: is there any use case in which we do *not* want a common BRP? If such use case does not exist, then I am not convinced that the CAN_CTRLMODE_FD_COMMON_BRP should be exposed in the netlink interface. If common BRP is simply better, wouldn't it be then simpler to just change the bitrate calculation so that everyone uses common BRP? That said, maybe there is one edge case I am not aware of. If under some circumstances it is not possible achieve common BRP, then that new control mode makes sense. And I guess that if the calculation fails to select a common BRP, then this control mode will be used in the new algorithm to decide if the calculation should stop (shared BRP) or continue (different BRP allowed). Yours sincerely, Vincent Mailhol
