diff mbox series

[v3,04/10] usb/xhci: fixup xhci kconfig deps

Message ID 20201020074844.5304-5-kraxel@redhat.com
State Superseded
Headers show
Series microvm: add usb support | expand

Commit Message

Gerd Hoffmann Oct. 20, 2020, 7:48 a.m. UTC
USB_XHCI does not depend on PCI any more.
USB_XHCI_SYSBUS must select USB_XHCI not USB.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/Kconfig | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Sai Pavan Boddu Oct. 20, 2020, 1:55 p.m. UTC | #1
Hi,

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, October 20, 2020 1:19 PM
> To: qemu-devel@nongnu.org
> Cc: Sergio Lopez <slp@redhat.com>; Sai Pavan Boddu <saipava@xilinx.com>;
> Igor Mammedov <imammedo@redhat.com>; Michael S. Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Thomas Huth <thuth@redhat.com>;
> Richard Henderson <rth@twiddle.net>; Gerd Hoffmann <kraxel@redhat.com>;
> Eduardo Habkost <ehabkost@redhat.com>; Laurent Vivier
> <lvivier@redhat.com>
> Subject: [PATCH v3 04/10] usb/xhci: fixup xhci kconfig deps
> 
> USB_XHCI does not depend on PCI any more.
> USB_XHCI_SYSBUS must select USB_XHCI not USB.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
[Sai Pavan Boddu] 
Looks clean now. I forgot to take care of this previously, this was one of our requirements initially.

Reviewed-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

Regards,
Sai Pavan

> ---
>  hw/usb/Kconfig | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig index
> 4dd2ba9630cb..a674ce4c542e 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -32,8 +32,6 @@ config USB_EHCI_SYSBUS
> 
>  config USB_XHCI
>      bool
> -    default y if PCI_DEVICES
> -    depends on PCI
>      select USB
> 
>  config USB_XHCI_PCI
> @@ -50,8 +48,8 @@ config USB_XHCI_NEC
> 
>  config USB_XHCI_SYSBUS
>      bool
> -    default y if USB_XHCI
> -    select USB
> +    default y
> +    select USB_XHCI
> 
>  config USB_MUSB
>      bool
> --
> 2.27.0
Bruce Rogers Nov. 20, 2020, 12:27 a.m. UTC | #2
On Tue, 2020-10-20 at 09:48 +0200, Gerd Hoffmann wrote:
> USB_XHCI does not depend on PCI any more.

> USB_XHCI_SYSBUS must select USB_XHCI not USB.

> 

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

> ---

>  hw/usb/Kconfig | 6 ++----

>  1 file changed, 2 insertions(+), 4 deletions(-)

> 

> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig

> index 4dd2ba9630cb..a674ce4c542e 100644

> --- a/hw/usb/Kconfig

> +++ b/hw/usb/Kconfig

> @@ -32,8 +32,6 @@ config USB_EHCI_SYSBUS

>  

>  config USB_XHCI

>      bool

> -    default y if PCI_DEVICES

> -    depends on PCI

>      select USB

>  

>  config USB_XHCI_PCI

> @@ -50,8 +48,8 @@ config USB_XHCI_NEC

>  

>  config USB_XHCI_SYSBUS

>      bool

> -    default y if USB_XHCI

> -    select USB

> +    default y

> +    select USB_XHCI

>  

>  config USB_MUSB

>      bool


I was reviewing what device changes are happening between v5.1.0 and
v5.2.0 for the QEMU arch's we support and noticed for s390x system
emulation that usb devices have appeared in the info qdm list in the
monitor.

I bisected the change to this patch, now commit 7114f6eac333d99b, which
does a 'default y' without any conditionals. I'm fairly sure that was
not the intent, though I do know what the proper conditionals should
be.

Can you take a look at it?

Thanks,

Bruce
Gerd Hoffmann Nov. 20, 2020, 6:55 a.m. UTC | #3
> >  config USB_XHCI

> > -    default y if PCI_DEVICES


> >  config USB_XHCI_SYSBUS

> > +    default y


> I was reviewing what device changes are happening between v5.1.0 and

> v5.2.0 for the QEMU arch's we support and noticed for s390x system

> emulation that usb devices have appeared in the info qdm list in the

> monitor.

> 

> I bisected the change to this patch, now commit 7114f6eac333d99b, which

> does a 'default y' without any conditionals. I'm fairly sure that was

> not the intent, though I do know what the proper conditionals should

> be.


I'm open to suggestions.  Depending on PCI_DEVICES doesn't work any more
because USB_XHCI_SYSBUS doesn't need PCI ...

take care,
  Gerd
Paolo Bonzini Nov. 20, 2020, 7:26 a.m. UTC | #4
On 20/11/20 01:27, Bruce Rogers wrote:
>>   config USB_XHCI_SYSBUS

>>       bool

>> -    default y if USB_XHCI

>> -    select USB

>> +    default y

>> +    select USB_XHCI

>>   

>>   config USB_MUSB

>>       bool

> I was reviewing what device changes are happening between v5.1.0 and

> v5.2.0 for the QEMU arch's we support and noticed for s390x system

> emulation that usb devices have appeared in the info qdm list in the

> monitor.

> 

> I bisected the change to this patch, now commit 7114f6eac333d99b, which

> does a 'default y' without any conditionals. I'm fairly sure that was

> not the intent, though I do know what the proper conditionals should

> be.

> 

> Can you take a look at it?


Generally, SYSBUS devices should never be "default y" because they're 
not user creatable.  Also kconfig.rst says

   Devices are usually ``default y`` if and only if they have at least
   one ``depends on``; the default could be conditional on a device
   group.

In this case, the right thing to do is to remove "default y" here; 
microvm already has a "select USB_XHCI_SYSBUS" so it would still work. 
That said, I would also change

     select PCI_EXPRESS_GENERIC_BRIDGE
     select USB_XHCI_SYSBUS

for microvm to "imply".  Again, according to kconfig.rst

   Boards specify their constituent devices using ``imply`` and
   ``select`` directives.  A device should be listed under ``select`` if
   the board cannot be started at all without it.  It should be listed
   under ``imply`` if (depending on the QEMU command line) the board may
   or may not be started without it.

Thanks,

Paolo
diff mbox series

Patch

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 4dd2ba9630cb..a674ce4c542e 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -32,8 +32,6 @@  config USB_EHCI_SYSBUS
 
 config USB_XHCI
     bool
-    default y if PCI_DEVICES
-    depends on PCI
     select USB
 
 config USB_XHCI_PCI
@@ -50,8 +48,8 @@  config USB_XHCI_NEC
 
 config USB_XHCI_SYSBUS
     bool
-    default y if USB_XHCI
-    select USB
+    default y
+    select USB_XHCI
 
 config USB_MUSB
     bool