Dear Matyas Bobek and Bernhard Beschow, On Monday 15 of December 2025 21:03:09 Matyáš Bobek wrote: > This series adds emulation of the FlexCAN CAN controller, version 2, > found in NXP i.MX6 series SoCs. The controller is integrated into > fsl-imx6 and the Sabrelite ARM board.
First, thanks a lot to Matyas Bobek for finding the courage and sending the FlexCAN series, finally, after keeping it updated log time on his flexcan-series-XXX branches at his personal QEMU development repository https://gitlab.fel.cvut.cz/bobekmat/qemu-flexcan/-/branches Bernhard Beschow, thanks for expressing interrest in the project. Your intererst helped Matyas Bobek to collect courage to send. I have found that you have invested a lot in the CAN on your imx8mp-flexcan and can-cleanup branches, thanks again https://github.com/shentok/qemu/branches It is shame that long delay in sending of patches has lead to some divergence of the effort. I have gone through your changes and would be happy if the effort can be joined and integrated into mainline. I would prefer to help Matyas Bobek's series to be updated in state that it passes review and then the FlexCAN can be moved forward by you and others. In long term, the extension to support FlexCAN3 would be very usesfull. But that that is work for other thesis, GSoC and or company funded project or developers. I would be happy to provide my knowledge as the time allows or look for students, etc. As for the series and your (Bernhard Beschow) changes: I have seen that you suggest some reordering of some functions in the hw/net/can/flexcan.c file. If you think that it is the better, more readable order for QEMU developers, I would suggest and have plea to Matyas Bobek to proceed with reorder. Same with tracepoints and debug prints which could be updated into state that your or others follow-up patches would not cause massive code movement which complicates tracking and reading of the changes. On the other hand, I have some arguments against unification of memory FlexCAN access function and inlining register accesses into them. We have already discused with Matyas Bobek that for FlexCAN3 and other future changes it would worth to separate registers from memory part etc. So I would kept this separation. Making as much as possible of the core opaque for its external use is right from my view point on the other hand. As for the CAN core changes, again there are some which I see as good moves, some cleanup of long term unused original structures which have been planned for another integration into QEMU before simplification to pass review etc. On the other hand, I would keep client state without const and with destructors etc. Again, actual code is somehow usable in its actual form, but from the long term perspective, I see the need for back-pressure propagation, emulation of the highest priority message (the lowest ID) exchange the first, etc. and I have some plans for that. I do not think that CAN emulation is and will be some real performance bottleneck in QEMU use for embedded systems development so I would like to keep there space for future more precise emulation. Same with reset on the chip core level, I think that its redundant call does not cause any performance problems, but I would be happy if the controller codes are reusable for wide scenarios. I have written and used LinCAN with such controllers at time of ISA bus on PC. I would be happy if we have mechanism how to map them on SoCs with FPGAs. Unfortunately this valid and very usesfull feature ( for example for our RTEMS effort on Zynq https://docs.rtems.org/docs/main/bsp-howto/can.html and ideally even on PolarFire (as time allows) where even NuttX can be tested and CI run https://github.com/apache/nuttx/tree/master/drivers/can ) hit concrete wall in May, without any suggestion how to make that needed use of QEMU for CI acceptable. But our SJA1000 code is already used by Espressif in their QEMU fork https://github.com/espressif/qemu/tree/esp-develop/hw/net/can so there is more proven use out of PCI based cards. CTU CAN FD is used mostly on FPGAs but here are MCUs with it so again, even if the usability of mainline QEMU for FPGA development would stay blocked, there are standard, hopefully non problematic, regular machine code initiated uses of the CAN controllers which are part of QEMU. So I would be happy if we can thought about that wider use to check that it would not be more problematic in future if some code is optimized. On the other hand, it is right that even in esp32_twai.c case, the integration is based on RESETTABLE_CLASS and esp32_twai_reset() calls can_sja_hardware_reset() explicitly. So can_sja_hardware_reset() during can_sja_init() is not strictly necessary. Back to sabrelite FlexCAN support series. In general I agree with the patch series and I have consulted and reviewed it multiple times. So it can be considered to be approved by me that it is functionally OK as well as it respects copyright requirements etc. I add my Signed-off-by: Pavel Pisa <[email protected]> As for individual formatting and may be some debug prints, I would allow it to go in in its actual form and then reduce these latter that we have state with more, may it be even abundant, debug in mainline recorded. But I expect that there could be more request for style and details from more experienced QEMU developers. There is one unresolved patch check report about DEVICE_NATIVE_ENDIAN +static const struct MemoryRegionOps flexcan_ops = { + .read = flexcan_mem_read, + .write = flexcan_mem_write, + .endianness = DEVICE_NATIVE_ENDIAN, + .valid = { + .min_access_size = 1, + .max_access_size = 4, + .unaligned = true, + .accepts = flexcan_mem_accepts + }, + .impl = { + .min_access_size = 4, + .max_access_size = 4, + .unaligned = false + }, +}; But I do not know what I can suggest there. The device is infernally accessed by 32-bits words and should be mapped in native format because same core is used on little-endian and may it be even bi-endian ARMs[*1] and for sure on big-endian PowerPCs. We believe that native endianness with host is the best option in this case. Extending .valid.max_access_size to 8 is right and probably require for 64-bit targets as I understand from your patches. [*1] as I have used bi-endian ARMs they mapped peripherals often native way on 32-bit entities. So again, fixed DEVICE_BIG_ENDIAN or DEVICE_LITTLE_ENDIAN is incorrect in such case. Best wishes, Pavel Pavel Pisa phone: +420 603531357 e-mail: [email protected] 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
