Am 16. Dezember 2025 00:44:54 UTC schrieb Pavel Pisa <[email protected]>:
>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.
Nice! Yeah, contributing to an OSS project can be intimidating at first. But
seeing one's work accepted is encuraging which is the real beauty of OSS.
>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.
Well, the imx8mp-flexcan branch is a moving target and currently exists as a
proof of concept. Once this series is merged, I would like to integrate this
series' FlexCAN functionality into imx8mp, even if the real controller has more
features. So this branch just allows me to look into the future which helps my
review.
>
>As for the series and your (Bernhard Beschow) changes:
Let's keep the discussion on the list. While there is some truth to my branches
I'd prefer to stick to the review process and only discuss topics raised here.
I'll provide my feedback as soon as I find time for it.
Best regards,
Bernhard
>
>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
