| 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/
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(-)