Re: [PATCH v2 7/7] hw/net/can: Correct Kconfig dependencies after switch to meson build.

[Pavel Pisa]

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.