Re: [PATCH v2 2/2] drivers: gpio: add virtio-gpio guest driver

From: Jason Wang
Date: Sun Dec 06 2020 - 22:14:38 EST


On 2020/12/6 上午3:32, Michael S. Tsirkin wrote:
On Sat, Dec 05, 2020 at 08:59:55AM +0100, Enrico Weigelt, metux IT consult wrote:
On 04.12.20 04:35, Jason Wang wrote:

--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1615,6 +1615,15 @@ config GPIO_MOCKUP
        tools/testing/selftests/gpio/gpio-mockup.sh. Reference the
usage in
        it.
  +config GPIO_VIRTIO
+    tristate "VirtIO GPIO support"
+    depends on VIRTIO

Let's use select, since there's no prompt for VIRTIO and it doesn't have
any dependencies.
whoops, it's not that simple:

make: Entering directory '/home/nekrad/src/apu2-dev/pkg/kernel.apu2.git'
make[1]: Entering directory
'/home/nekrad/src/dk/DistroKit/platform-x86_64/build-target/linux-5.8.9-build'
GEN Makefile
drivers/gpu/drm/Kconfig:74:error: recursive dependency detected!
drivers/gpu/drm/Kconfig:74: symbol DRM_KMS_HELPER is selected by
DRM_VIRTIO_GPU
drivers/gpu/drm/virtio/Kconfig:2: symbol DRM_VIRTIO_GPU depends on VIRTIO
drivers/virtio/Kconfig:2: symbol VIRTIO is selected by GPIO_VIRTIO
drivers/gpio/Kconfig:1618: symbol GPIO_VIRTIO depends on GPIOLIB
drivers/gpio/Kconfig:14: symbol GPIOLIB is selected by I2C_MUX_LTC4306
drivers/i2c/muxes/Kconfig:47: symbol I2C_MUX_LTC4306 depends on I2C
drivers/i2c/Kconfig:8: symbol I2C is selected by FB_DDC
drivers/video/fbdev/Kconfig:63: symbol FB_DDC depends on FB
drivers/video/fbdev/Kconfig:12: symbol FB is selected by DRM_KMS_FB_HELPER
drivers/gpu/drm/Kconfig:80: symbol DRM_KMS_FB_HELPER depends on
DRM_KMS_HELPER

Seems that we can only depend on or select some symbol - we run into
huge trouble if thats mixed. Just changed DRM_VIRTIO_GPU to just select
VIRIO instead of depending on it, and now it works.

I've posted another patch for fixing drivers/gpu/drm/virtio/Kconfig
to use 'select' instead of 'depends on'.
It seems a bit of a mess, at this point I'm not entirely sure when
should drivers select VIRTIO and when depend on it.

The text near it says:

# SPDX-License-Identifier: GPL-2.0-only
config VIRTIO
tristate
help
This option is selected by any driver which implements the virtio
bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_RPMSG
or CONFIG_S390_GUEST.

Which seems clear enough and would indicate drivers for devices *behind*
the bus should not select VIRTIO and thus presumably should "depend on" it.
This is violated in virtio console and virtio fs drivers.

For console it says:

commit 9f30eb29c514589e16f2999ea070598583d1f6ec
Author: Michal Suchanek <msuchanek@xxxxxxx>
Date: Mon Aug 31 18:58:50 2020 +0200

char: virtio: Select VIRTIO from VIRTIO_CONSOLE.
Make it possible to have virtio console built-in when
other virtio drivers are modular.
Signed-off-by: Michal Suchanek <msuchanek@xxxxxxx>
Reviewed-by: Amit Shah <amit@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20200831165850.26163-1-msuchanek@xxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

which seems kind of bogus - why do we care about allowing a builtin
virtio console driver if the pci virtio bus driver is a module?
There won't be any devices on the bus to attach to ...


For testing like switching bus from pci to MMIO?


And for virtio fs it was like this from the beginning.

I am inclined to fix console and virtio fs to depend on VIRTIO:
select is harder to use correctly ...

Jason?


I think it works, but we need a prompt for VIRTIO otherwise there's no way to enable it.

Thanks




--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@xxxxxxxxx -- +49-151-27565287