Re: hw/net/can: PCI MSI suport for SJA1000 based addon card Was: [PATCH] hw/net/can: ... to meson build.

[Pavel Pisa <pisa@fel.cvut.cz>]

Hello Deniz,

On Saturday 15 of February 2025 14:30:58 Deniz Eren wrote:
> I have implemented support for PCI MSI capability CANbus card support;
> fully tested using QNX operating system guest image. How can I go about
> contributing this to the main repo:
>
> https://github.com/Deniz-Eren/qemu/blob/feature/can-sja100-pci-msi-support/
>hw/net/can/can_pcm26d2ca_pci.c

the first thanks for information and work done.

I am replying to all addressees of your e-mail but if the others
do not respond, I suggest to limit and would limit the recipients
list only to me, other QEMU CAN subsystem maintainers:

  Francisco Iglesias <francisco.iglesias@amd.com>
  Vikram Garhwal <vikram.garhwal@bytedance.com>

QEMU list

  qemu-devel@nongnu.org

and some QEMU core developers who could accept the code
for mainline, my previous changes have been accepted
for mainline by one of the following developers

  Peter Maydell <peter.maydell@linaro.org>
  Michael Tokarev<mjt@tls.msk.ru>
  Philippe Mathieu-Daudé <philmd@linaro.org>
  Paolo Bonzini <pbonzini@redhat.com>

The patches should be send in plain text format (git format-patch)
to qemu-devel@nongnu.org and me and other QEMU CAN maintainers.

As for the format and other details, look at 

  https://www.qemu.org/docs/master/devel/submitting-a-patch.html

Check the source formating and basic requirements by

  scripts/checkpatch.pl

As for the actual changes you propose, I have looked
into your repository. I would like to discuss a little
changes to generic SJA1000 code

  
https://github.com/Deniz-Eren/qemu/commit/a2f593f21946328821f8456274c9c688d5f1c4de

Because my understanding is that the emulated board is some
Advantech board PCM-26D2CA

  
https://www.advantech.com/en/products/14263729-aaa3-4552-b990-99d16cdfee24/pcm-26d2ca/mod_9a1e9dbf-e22d-4770-a896-cecf40607084

which has PCIe MSI capable interface. The datasheet states
NXP SJA-1000 used as the controller. But I do not expect
that this chip allows distinguish interrupt source directly
to map it to PCIe MSI signal. So one option is complete controller
logic in FPGA, another is somehow read and map interrupt PeliCAN
register to MSI messages. But that mapping can be specific
to different boards and SJA1000 compatible controllers
implementations.

I am not sure if your code is prepared to make mapping
so generic.

In the fact, I am not sure if whole PCIe MSI details
should be pushed to the bare SJA1000 controller implementation.

I would suggest to think about option to register callback
for interrupt state update which would receive interrupt
register state as argument and move actual mapping to PCIe
MSI writes outside of the core SJA1000 implementation.

This way it could be mapped even to other bus technologies
which allows multiple even boxes per single addon
card/controller source. I can imagine VME for example
even that it probably not common today but even some
SoC with SJA1000 compatible CAN controller which maps
it to multiple interrupts.

We should discuse this probably within smaller group, me
and Francisco Iglesias and Vikram Garhwal with CC to
qemu-devel@nongnu.org and when we agree on the patche
series in the review process we should ask some
other QEMU developers to accept result for mainline. 


Best wishes,

                Pavel
--
                Pavel Pisa
    phone:      +420 603531357
    e-mail:     pisa@cmp.felk.cvut.cz
    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