On Mon, Aug 16, 2021 at 02:11:19PM +0200, Arnd Bergmann wrote:
> On Mon, Aug 16, 2021 at 12:22 PM Cristian Marussi
> <cristian.marussi@arm.com> 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
> 

Hi Arnd,

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

Thanks a lot for the feedback/explanation.

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

I'll go for b) and post a fix.

Thanks,
Cristian