Message ID | 20201020074844.5304-5-kraxel@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series | microvm: add usb support | expand |
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
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
> > 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
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 --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
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(-)