| Message ID | dd539770e9c182125a9c3d87b9ca329121b11abc.1599168753.git.pisa@cmp.felk.cvut.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | CTU CAN FD core support | expand |
On 03/09/20 23:48, Pavel Pisa wrote: > The original CAN_PCI config option enables multiple SJA1000 PCI boards > emulation build. These boards bridge SJA1000 into I/O or memory > address space of the host CPU and depend on SJA1000 emulation. Can you explain how the mistake is related to the meson switch? The conversion seems good: diff --git a/hw/net/can/Makefile.objs b/hw/net/can/Makefile.objs deleted file mode 100644 index 9f0c4ee332..0000000000 --- a/hw/net/can/Makefile.objs +++ /dev/null @@ -1,4 +0,0 @@ -common-obj-$(CONFIG_CAN_SJA1000) += can_sja1000.o -common-obj-$(CONFIG_CAN_PCI) += can_kvaser_pci.o -common-obj-$(CONFIG_CAN_PCI) += can_pcm3680_pci.o -common-obj-$(CONFIG_CAN_PCI) += can_mioe3680_pci.o diff --git a/hw/net/can/meson.build b/hw/net/can/meson.build new file mode 100644 index 0000000000..c9cfeb7954 --- /dev/null +++ b/hw/net/can/meson.build @@ -0,0 +1,4 @@ +softmmu_ss.add(when: 'CONFIG_CAN_SJA1000', if_true: files('can_sja1000.c')) +softmmu_ss.add(when: 'CONFIG_CAN_PCI', if_true: files('can_kvaser_pci.c')) +softmmu_ss.add(when: 'CONFIG_CAN_PCI', if_true: files('can_pcm3680_pci.c')) +softmmu_ss.add(when: 'CONFIG_CAN_PCI', if_true: files('can_mioe3680_pci.c')) I queued the other six patches. Paolo > Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz> > --- > hw/net/Kconfig | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/hw/net/Kconfig b/hw/net/Kconfig > index 225d948841..6d795ec752 100644 > --- a/hw/net/Kconfig > +++ b/hw/net/Kconfig > @@ -132,16 +132,15 @@ config ROCKER > config CAN_BUS > bool > > -config CAN_PCI > +config CAN_SJA1000 > bool > default y if PCI_DEVICES > - depends on PCI > select CAN_BUS > > -config CAN_SJA1000 > +config CAN_PCI > bool > default y if PCI_DEVICES > - depends on PCI > + depends on PCI && CAN_SJA1000 > select CAN_BUS > > config CAN_CTUCANFD
Hello Paolo, On Wednesday 23 of September 2020 17:48:09 Paolo Bonzini wrote: > On 03/09/20 23:48, Pavel Pisa wrote: > > The original CAN_PCI config option enables multiple SJA1000 PCI boards > > emulation build. These boards bridge SJA1000 into I/O or memory > > address space of the host CPU and depend on SJA1000 emulation. > > Can you explain how the mistake is related to the meson switch? > > The conversion seems good: > > diff --git a/hw/net/can/Makefile.objs b/hw/net/can/Makefile.objs > deleted file mode 100644 > index 9f0c4ee332..0000000000 > --- a/hw/net/can/Makefile.objs > +++ /dev/null > @@ -1,4 +0,0 @@ > -common-obj-$(CONFIG_CAN_SJA1000) += can_sja1000.o > -common-obj-$(CONFIG_CAN_PCI) += can_kvaser_pci.o > -common-obj-$(CONFIG_CAN_PCI) += can_pcm3680_pci.o > -common-obj-$(CONFIG_CAN_PCI) += can_mioe3680_pci.o > diff --git a/hw/net/can/meson.build b/hw/net/can/meson.build > new file mode 100644 > index 0000000000..c9cfeb7954 > --- /dev/null > +++ b/hw/net/can/meson.build > @@ -0,0 +1,4 @@ > +softmmu_ss.add(when: 'CONFIG_CAN_SJA1000', if_true: > files('can_sja1000.c')) +softmmu_ss.add(when: 'CONFIG_CAN_PCI', if_true: > files('can_kvaser_pci.c')) +softmmu_ss.add(when: 'CONFIG_CAN_PCI', if_true: > files('can_pcm3680_pci.c')) +softmmu_ss.add(when: 'CONFIG_CAN_PCI', > if_true: files('can_mioe3680_pci.c')) I have analyzed history and potential problem source is older then the switch to meson build but I did not realized it before update to the meson build between v1 and v2 patches submission. There has not been described dependencies between options in the original QEMU code. There has been only single list what is compiled in for targets supporting PCI default-configs/pci.mak: CONFIG_CAN_BUS=y CONFIG_CAN_SJA1000=y CONFIG_CAN_PCI=y So good, all enabled together, so no problem with mutual dependencies. But when the QEMU switched to Kconfig build: convert pci.mak to Kconfig 1/23/19 7:56 AM then config looks like this hw/net/Kconfig: config CAN_PCI bool default y if PCI_DEVICES depends on PCI select CAN_BUS config CAN_SJA1000 bool default y if PCI_DEVICES depends on PCI select CAN_BUS There is a problem when some tool (kconfig-fronteds) is used to manually tune configuration because if the CAN_PCI is enabled but CAN_SJA1000 stays disabled then the build fails. That is no problem for default options combinations controlled by PCI option. So the problem existed there even before messon build switch but I have noticed it when I experienced clash of v1 patches with newer QEMU mainline. So my mistake is that I have not identified that switch to Kconfig started the problem. But in the CTU CAN FD v1 submission we (with Jan Charvat) have not distinguished between SJA1000 and CTU CAN FD emulation enable so I have not checked where option i enabled and how hw/net/can/Makefile.objs: common-obj-$(CONFIG_CAN_PCI) += can_kvaser_pci.o common-obj-$(CONFIG_CAN_PCI) += can_pcm3680_pci.o common-obj-$(CONFIG_CAN_PCI) += can_mioe3680_pci.o + +common-obj-$(CONFIG_CAN_PCI) += ctucan_core.o +common-obj-$(CONFIG_CAN_PCI) += ctucan_pci.o I have decided to separated SJA1000 PCI and CTU CAN FD support in the v2 when I have to reflect mainline change to meson anyway and I have found that there is potential problem with the dependencies. I have updated them. config CAN_SJA1000 bool default y if PCI_DEVICES select CAN_BUS config CAN_PCI bool default y if PCI_DEVICES depends on PCI && CAN_SJA1000 select CAN_BUS config CAN_CTUCANFD bool default y if PCI_DEVICES select CAN_BUS config CAN_CTUCANFD_PCI bool default y if PCI_DEVICES depends on PCI && CAN_CTUCANFD select CAN_BUS The SJA1000 and CAN_CTUCANFD controller sources should build even without PCI support because there can be and exists hardware options where cores are connected to platform-bus. These systems can be emulated even without PCI enabled in future if code for connection of these controllers to platform-bus/device-tree in implemented in future. By the way, you are replying to CTU CAN FD v2 series from 2020-09-03. But mainline moved forward and I have sent updated v3 at 2020-09-14 to reflect some bulk change to DECLARE_INSTANCE_CHECKER etc... If you have already pushed v2 and it does not cause build problems then I will provide update patch when code reaches mainline. If you have not pushed code to the mainline yet, consider v3 which should follow better actual mainline state. The list of updates to v3 follows. Thanks much for your time, Pavel +Patches v3 updates: + + - resend triggered by switch to DECLARE_INSTANCE_CHECKER + in mainline. I try to follow mainline as time allows. + + - SJA1000, CTU CAN FD and SocketCAN support retested + with QEMU mainline from 9/12/20 10:17 PM + + - Added Reviewed-by: Vikram Garhwal + to reviewed and tested patches which are used as common + CAN FD base at Xilinx + + - Added Vikram Garhwal to MAINTAINERS file as the second person + who has interrest in QEMU CAN (FD) support and would + like to be notified about changes and help with reviews.
On 23/09/20 19:44, Pavel Pisa wrote: > > If you have not pushed code to the mainline yet, > consider v3 which should follow better actual > mainline state. The list of updates to v3 follows. I actually queued v3 (I just use patchew to queue patches). Paolo
Hello Paolo, On Wednesday 23 of September 2020 20:11:08 Paolo Bonzini wrote: > On 23/09/20 19:44, Pavel Pisa wrote: > > If you have not pushed code to the mainline yet, > > consider v3 which should follow better actual > > mainline state. The list of updates to v3 follows. > > I actually queued v3 (I just use patchew to queue patches). That is great. Thanks, Pavel -- Pavel Pisa e-mail: pisa@cmp.felk.cvut.cz Department of Control Engineering FEE CVUT Karlovo namesti 13, 121 35, Prague 2 university: http://dce.fel.cvut.cz/ personal: http://cmp.felk.cvut.cz/~pisa projects: https://www.openhub.net/accounts/ppisa CAN related:http://canbus.pages.fel.cvut.cz/
Hello, 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 Sent from my iPhone Deniz Eren +61 405 194 317 > On 24 Sep 2020, at 4:15 am, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote: > > Hello Paolo, > >> On Wednesday 23 of September 2020 20:11:08 Paolo Bonzini wrote: >>> On 23/09/20 19:44, Pavel Pisa wrote: >>> If you have not pushed code to the mainline yet, >>> consider v3 which should follow better actual >>> mainline state. The list of updates to v3 follows. >> >> I actually queued v3 (I just use patchew to queue patches). > > That is great. > > Thanks, > > Pavel > > -- > Pavel Pisa > e-mail: pisa@cmp.felk.cvut.cz > Department of Control Engineering FEE CVUT > Karlovo namesti 13, 121 35, Prague 2 > university: http://dce.fel.cvut.cz/ > personal: http://cmp.felk.cvut.cz/~pisa > projects: https://www.openhub.net/accounts/ppisa > CAN related:http://canbus.pages.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
diff --git a/hw/net/Kconfig b/hw/net/Kconfig index 225d948841..6d795ec752 100644 --- a/hw/net/Kconfig +++ b/hw/net/Kconfig @@ -132,16 +132,15 @@ config ROCKER config CAN_BUS bool -config CAN_PCI +config CAN_SJA1000 bool default y if PCI_DEVICES - depends on PCI select CAN_BUS -config CAN_SJA1000 +config CAN_PCI bool default y if PCI_DEVICES - depends on PCI + depends on PCI && CAN_SJA1000 select CAN_BUS config CAN_CTUCANFD
The original CAN_PCI config option enables multiple SJA1000 PCI boards emulation build. These boards bridge SJA1000 into I/O or memory address space of the host CPU and depend on SJA1000 emulation. Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz> --- hw/net/Kconfig | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)