On Mon, Aug 16, 2021 at 12:22 PM Cristian Marussi
<cristian.marussi@xxxxxxx> 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.
Arnd
