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

[Enrico Weigelt, metux IT consult]

On 05.12.20 20:32, Michael S. Tsirkin wrote:

Hi,

> 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.

if VIRTIO just enables something that could be seen as library
functions, then select should be right, IMHO.

> The text near it says:
> 
> # SPDX-License-Identifier: GPL-2.0-only
> config VIRTIO
>         tristate

oh, wait, doesn't have an menu text, so we can't even explicitly enable
it (not shown in menu) - only implicitly. Which means that some other
option must select it, in order to become availe at all, and in order
to make others depending on it becoming available.

IMHO, therefore select is the correct approach.


>         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.

See above: NAK. because it can't even be enabled directly (by the user).
If it wasn't meant otherwise, we'd have to add an menu text.

> 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 ...

When using other transports ?
In my current project, eg. I'm using mmio - my kernel has pci completely
disabled.

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

I don't thinkt that would be good - instead everybody should just select
VIRTIO, never depend on it (maybe depend on VIRTIO_MENU instead)


--mtx

-- 
---
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