[kbuild-all] Re: [linux-next:master 5053/7963] virtio.c:undefined reference to `virtio_check_driver_offered_feature'

On Mon, Aug 16, 2021 at 12:22 PM Cristian Marussi <cristian.marussi(a)arm.com&gt; wrote: > On Sun, Aug 15, 2021 at 12:54:38PM +0800, kernel test robot wrote: > > diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig > index 7f4d2435503b..daa349615e91 100644 > --- a/drivers/firmware/arm_scmi/Kconfig > +++ b/drivers/firmware/arm_scmi/Kconfig > @@ -4,6 +4,7 @@ menu "ARM System Control and Management Interface Protocol" > config ARM_SCMI_PROTOCOL > tristate "ARM System Control and Management Interface (SCMI) Message Protocol" > depends on ARM || ARM64 || COMPILE_TEST > + select VIRTIO if ARM_SCMI_TRANSPORT_VIRTIO > help > ARM System Control and Management Interface (SCMI) protocol is a > set of operating system-independent software interfaces that are > @@ -68,7 +69,6 @@ config ARM_SCMI_TRANSPORT_SMC > > config ARM_SCMI_TRANSPORT_VIRTIO > bool "SCMI transport based on VirtIO" > - depends on VIRTIO > select ARM_SCMI_HAVE_TRANSPORT > select ARM_SCMI_HAVE_MSG > help

Generally speaking it's bad to have both 'select' and 'depends on' for drivers using the same subsystem. This can lead to both circular dependencies, and to general confusion when users are surprised by this.

My recommendation is to stick with 'depends on' in general, as that is usually what you want. In this case, there are two things you could do that work better to solve this problem: a) add 'depends on VIRTIO || !VIRTIO' under ARM_SCMI_PROTOCOL, to prevent it from being built-in when virtio is a loadable module b) change the dependency in ARM_SCMI_TRANSPORT_VIRTIO to list the correct thing, as in 'depends on VIRTIO=y || VIRTIO=ARM_SCMI_PROTOCOL'. I don't have a strong preference between those two, as neither of them is perfect: either you and up not seeing the virtio transport option, or you force ARM_SCMI_PROTOCOL to be non-built-in when virtio is a module even when the virtio transport is disabled.