1. Patchew
  2. QEMU
  3. [PATCH 00/10] usb cleanups: remove usb_bus_find(), extract sysbus-ohci to a separate file
  4. View patch

[PATCH 05/10] hppa: do not require CONFIG_USB

Paolo Bonzini posted 10 patches 9 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, BALATON Zoltan <balaton@eik.bme.hu>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Magnus Damm <magnus.damm@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
[PATCH 05/10] hppa: do not require CONFIG_USB
Posted by Paolo Bonzini 9 months ago 
With --without-default-devices it is possible to build a binary that
does not include any USB host controller and therefore that does not
include the code guarded by CONFIG_USB.  While the simpler creation
functions such as usb_create_simple can be inlined, this is not true
of usb_bus_find().  Remove it, replacing it with a search of the single
USB bus on the machine.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/hppa/machine.c | 7 ++++---
 hw/hppa/Kconfig   | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 5fcaf5884be..11982d5776c 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -396,10 +396,11 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
     }
 
     /* create USB OHCI controller for USB keyboard & mouse on Astro machines */
-    if (!lasi_dev && machine->enable_graphics) {
+    if (!lasi_dev && machine->enable_graphics && defaults_enabled()) {
         pci_create_simple(pci_bus, -1, "pci-ohci");
-        usb_create_simple(usb_bus_find(-1), "usb-kbd");
-        usb_create_simple(usb_bus_find(-1), "usb-mouse");
+        Object *usb_bus = object_resolve_type_unambiguous(TYPE_USB_BUS, &error_abort);
+        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
+        usb_create_simple(USB_BUS(usb_bus), "usb-mouse");
     }
 
     /* register power switch emulation */
diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
index dff5df7f725..ee7ffd2bfb5 100644
--- a/hw/hppa/Kconfig
+++ b/hw/hppa/Kconfig
@@ -2,6 +2,7 @@ config HPPA_B160L
     bool
     imply PCI_DEVICES
     imply E1000_PCI
+    imply USB_OHCI_PCI
     imply VIRTIO_VGA
     select ASTRO
     select DINO
@@ -17,4 +18,3 @@ config HPPA_B160L
     select LASIPS2
     select PARALLEL
     select ARTIST
-    select USB_OHCI_PCI
-- 
2.43.0
Re: [PATCH 05/10] hppa: do not require CONFIG_USB
Posted by Thomas Huth 9 months ago 
On 23/02/2024 13.44, Paolo Bonzini wrote:
> With --without-default-devices it is possible to build a binary that
> does not include any USB host controller and therefore that does not
> include the code guarded by CONFIG_USB.  While the simpler creation
> functions such as usb_create_simple can be inlined, this is not true
> of usb_bus_find().  Remove it, replacing it with a search of the single
> USB bus on the machine.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/hppa/machine.c | 7 ++++---
>   hw/hppa/Kconfig   | 2 +-
>   2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index 5fcaf5884be..11982d5776c 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -396,10 +396,11 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
>       }
>   
>       /* create USB OHCI controller for USB keyboard & mouse on Astro machines */
> -    if (!lasi_dev && machine->enable_graphics) {
> +    if (!lasi_dev && machine->enable_graphics && defaults_enabled()) {

Do we need the defaults_enabled() here? Isn't enable_graphics already 
disabled if defaults_enabled() is not set?

  Thomas


>           pci_create_simple(pci_bus, -1, "pci-ohci");
> -        usb_create_simple(usb_bus_find(-1), "usb-kbd");
> -        usb_create_simple(usb_bus_find(-1), "usb-mouse");
> +        Object *usb_bus = object_resolve_type_unambiguous(TYPE_USB_BUS, &error_abort);
> +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
> +        usb_create_simple(USB_BUS(usb_bus), "usb-mouse");
>       }
Re: [PATCH 05/10] hppa: do not require CONFIG_USB
Posted by BALATON Zoltan 9 months ago 
On Fri, 23 Feb 2024, Thomas Huth wrote:
> On 23/02/2024 13.44, Paolo Bonzini wrote:
>> With --without-default-devices it is possible to build a binary that
>> does not include any USB host controller and therefore that does not
>> include the code guarded by CONFIG_USB.  While the simpler creation
>> functions such as usb_create_simple can be inlined, this is not true
>> of usb_bus_find().  Remove it, replacing it with a search of the single
>> USB bus on the machine.
>> 
>> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   hw/hppa/machine.c | 7 ++++---
>>   hw/hppa/Kconfig   | 2 +-
>>   2 files changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
>> index 5fcaf5884be..11982d5776c 100644
>> --- a/hw/hppa/machine.c
>> +++ b/hw/hppa/machine.c
>> @@ -396,10 +396,11 @@ static void machine_HP_common_init_tail(MachineState 
>> *machine, PCIBus *pci_bus,
>>       }
>>         /* create USB OHCI controller for USB keyboard & mouse on Astro 
>> machines */
>> -    if (!lasi_dev && machine->enable_graphics) {
>> +    if (!lasi_dev && machine->enable_graphics && defaults_enabled()) {
>
> Do we need the defaults_enabled() here? Isn't enable_graphics already 
> disabled if defaults_enabled() is not set?

Isn't enable_graphics controlled by -nographic and defaults_enabled 
controlled by -nodefaults? I think they are independent but maybe that 
does not answer the question if it's needed or not.

Regards,
BALATON Zoltan

> Thomas
>
>
>>           pci_create_simple(pci_bus, -1, "pci-ohci");
>> -        usb_create_simple(usb_bus_find(-1), "usb-kbd");
>> -        usb_create_simple(usb_bus_find(-1), "usb-mouse");
>> +        Object *usb_bus = object_resolve_type_unambiguous(TYPE_USB_BUS, 
>> &error_abort);
>> +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
>> +        usb_create_simple(USB_BUS(usb_bus), "usb-mouse");
>>       }
>
>
>
Re: [PATCH 05/10] hppa: do not require CONFIG_USB
Posted by Paolo Bonzini 9 months ago 
On Fri, Feb 23, 2024 at 8:56 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
> >> -    if (!lasi_dev && machine->enable_graphics) {
> >> +    if (!lasi_dev && machine->enable_graphics && defaults_enabled()) {
> >
> > Do we need the defaults_enabled() here? Isn't enable_graphics already
> > disabled if defaults_enabled() is not set?
>
> Isn't enable_graphics controlled by -nographic and defaults_enabled
> controlled by -nodefaults? I think they are independent but maybe that
> does not answer the question if it's needed or not.

Indeed they are different. The idea is that if graphics are disabled
you interact with a serial console so you don't need keyboard or
mouse; and if default devices are disabled of course you create as
little as possible and leave everything to the user.

Paolo
Re: [PATCH 05/10] hppa: do not require CONFIG_USB
Posted by Thomas Huth 9 months ago 
On 24/02/2024 23.38, Paolo Bonzini wrote:
> On Fri, Feb 23, 2024 at 8:56 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>>> -    if (!lasi_dev && machine->enable_graphics) {
>>>> +    if (!lasi_dev && machine->enable_graphics && defaults_enabled()) {
>>>
>>> Do we need the defaults_enabled() here? Isn't enable_graphics already
>>> disabled if defaults_enabled() is not set?
>>
>> Isn't enable_graphics controlled by -nographic and defaults_enabled
>> controlled by -nodefaults? I think they are independent but maybe that
>> does not answer the question if it's needed or not.
> 
> Indeed they are different. The idea is that if graphics are disabled
> you interact with a serial console so you don't need keyboard or
> mouse; and if default devices are disabled of course you create as
> little as possible and leave everything to the user.

Indeed, I just double-checked and they are independent, sorry for 
mis-remembering that.

  Thomas
Re: [PATCH 05/10] hppa: do not require CONFIG_USB
Posted by Philippe Mathieu-Daudé 9 months ago 
On 23/2/24 13:44, Paolo Bonzini wrote:
> With --without-default-devices it is possible to build a binary that
> does not include any USB host controller and therefore that does not
> include the code guarded by CONFIG_USB.  While the simpler creation
> functions such as usb_create_simple can be inlined, this is not true
> of usb_bus_find().  Remove it, replacing it with a search of the single
> USB bus on the machine.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/hppa/machine.c | 7 ++++---
>   hw/hppa/Kconfig   | 2 +-
>   2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index 5fcaf5884be..11982d5776c 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -396,10 +396,11 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
>       }
>   
>       /* create USB OHCI controller for USB keyboard & mouse on Astro machines */
> -    if (!lasi_dev && machine->enable_graphics) {
> +    if (!lasi_dev && machine->enable_graphics && defaults_enabled()) {
>           pci_create_simple(pci_bus, -1, "pci-ohci");
> -        usb_create_simple(usb_bus_find(-1), "usb-kbd");
> -        usb_create_simple(usb_bus_find(-1), "usb-mouse");
> +        Object *usb_bus = object_resolve_type_unambiguous(TYPE_USB_BUS, &error_abort);

Declare variable at begin of function; can be casted to USB_BUS once.
Otherwise:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
> +        usb_create_simple(USB_BUS(usb_bus), "usb-mouse");
>       }

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.