1. Patchew
  2. QEMU
  3. [PATCH v3 0/9] mips: do not list individual devices from configs/
  4. View patch

[PATCH v3 8/9] mips/loongson3_virt: do not require CONFIG_USB

Paolo Bonzini posted 9 patches 9 months, 2 weeks ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Gerd Hoffmann <kraxel@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>
[PATCH v3 8/9] mips/loongson3_virt: do not require CONFIG_USB
Posted by Paolo Bonzini 9 months, 2 weeks ago 
Once the Kconfig for hw/mips is cleaned up, it will be 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 created by loongson3_virt_devices_init().

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/loongson3_virt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index caedde2df00..bedd3d496bd 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -447,8 +447,9 @@ static inline void loongson3_virt_devices_init(MachineState *machine,
 
     if (defaults_enabled() && object_class_by_name("pci-ohci")) {
         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-tablet");
+        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS, NULL);
+        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
+        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
     }
 
     pci_init_nic_devices(pci_bus, mc->default_nic);
-- 
2.43.0
Re: [PATCH v3 8/9] mips/loongson3_virt: do not require CONFIG_USB
Posted by Philippe Mathieu-Daudé 9 months, 2 weeks ago 
On 13/2/24 16:50, Paolo Bonzini wrote:
> Once the Kconfig for hw/mips is cleaned up, it will be 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 created by loongson3_virt_devices_init().
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/mips/loongson3_virt.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
> index caedde2df00..bedd3d496bd 100644
> --- a/hw/mips/loongson3_virt.c
> +++ b/hw/mips/loongson3_virt.c
> @@ -447,8 +447,9 @@ static inline void loongson3_virt_devices_init(MachineState *machine,
>   
>       if (defaults_enabled() && object_class_by_name("pci-ohci")) {
>           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-tablet");
> +        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS, NULL);
> +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
> +        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
>       }
>   
>       pci_init_nic_devices(pci_bus, mc->default_nic);

Can we remove usb_bus_find() completely instead?

$ git grep -w usb_bus_find
hw/hppa/machine.c:401:        usb_create_simple(usb_bus_find(-1), 
"usb-kbd");
hw/hppa/machine.c:402:        usb_create_simple(usb_bus_find(-1), 
"usb-mouse");
hw/mips/loongson3_virt.c:450:        usb_create_simple(usb_bus_find(-1), 
"usb-kbd");
hw/mips/loongson3_virt.c:451:        usb_create_simple(usb_bus_find(-1), 
"usb-tablet");
hw/ppc/mac_newworld.c:434:            USBBus *usb_bus = usb_bus_find(-1);
hw/ppc/sam460ex.c:423:    usb_create_simple(usb_bus_find(-1), "usb-kbd");
hw/ppc/sam460ex.c:424:    usb_create_simple(usb_bus_find(-1), "usb-mouse");
hw/ppc/spapr.c:3027:            USBBus *usb_bus = usb_bus_find(-1);
hw/sh4/r2d.c:315:    usb_create_simple(usb_bus_find(-1), "usb-kbd");
hw/usb/bus.c:103:USBBus *usb_bus_find(int busnr)
hw/usb/bus.c:669:    USBBus *bus = usb_bus_find(-1 /* any */);
include/hw/usb.h:500:USBBus *usb_bus_find(int busnr);
Re: [PATCH v3 8/9] mips/loongson3_virt: do not require CONFIG_USB
Posted by BALATON Zoltan 9 months, 2 weeks ago 
On Thu, 15 Feb 2024, Philippe Mathieu-Daudé wrote:
> On 13/2/24 16:50, Paolo Bonzini wrote:
>> Once the Kconfig for hw/mips is cleaned up, it will be 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 created by loongson3_virt_devices_init().
>> 
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   hw/mips/loongson3_virt.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
>> index caedde2df00..bedd3d496bd 100644
>> --- a/hw/mips/loongson3_virt.c
>> +++ b/hw/mips/loongson3_virt.c
>> @@ -447,8 +447,9 @@ static inline void 
>> loongson3_virt_devices_init(MachineState *machine,
>>         if (defaults_enabled() && object_class_by_name("pci-ohci")) {
>>           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-tablet");
>> +        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS, 
>> NULL);
>> +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
>> +        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
>>       }
>>         pci_init_nic_devices(pci_bus, mc->default_nic);
>
> Can we remove usb_bus_find() completely instead?
>
> $ git grep -w usb_bus_find
> hw/hppa/machine.c:401:        usb_create_simple(usb_bus_find(-1), "usb-kbd");
> hw/hppa/machine.c:402:        usb_create_simple(usb_bus_find(-1), 
> "usb-mouse");
> hw/mips/loongson3_virt.c:450:        usb_create_simple(usb_bus_find(-1), 
> "usb-kbd");
> hw/mips/loongson3_virt.c:451:        usb_create_simple(usb_bus_find(-1), 
> "usb-tablet");
> hw/ppc/mac_newworld.c:434:            USBBus *usb_bus = usb_bus_find(-1);
> hw/ppc/sam460ex.c:423:    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> hw/ppc/sam460ex.c:424:    usb_create_simple(usb_bus_find(-1), "usb-mouse");
> hw/ppc/spapr.c:3027:            USBBus *usb_bus = usb_bus_find(-1);
> hw/sh4/r2d.c:315:    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> hw/usb/bus.c:103:USBBus *usb_bus_find(int busnr)
> hw/usb/bus.c:669:    USBBus *bus = usb_bus_find(-1 /* any */);
> include/hw/usb.h:500:USBBus *usb_bus_find(int busnr);

These are all the machines that add devices to a USB bus, there's no other 
example to do it in a different way currently. We could change this to get 
the usb bus in a different way but how? We could either peek into the 
object that owns the usb_bus or try using qdev_get_child_bus() but that 
needs the name of the bus which might change if other usb hosts are added 
so neither of these options seem better than this function. What would it 
bring to remove this function other than more complex or uglier code? I 
don't mind if you remove it just don't see the benefit in that.

Regards,
BALATON Zoltan
Re: [PATCH v3 8/9] mips/loongson3_virt: do not require CONFIG_USB
Posted by Paolo Bonzini 9 months, 2 weeks ago 
On Thu, Feb 15, 2024 at 3:27 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Thu, 15 Feb 2024, Philippe Mathieu-Daudé wrote:
> > On 13/2/24 16:50, Paolo Bonzini wrote:
> >> Once the Kconfig for hw/mips is cleaned up, it will be 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 created by loongson3_virt_devices_init().
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >>   hw/mips/loongson3_virt.c | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
> >> index caedde2df00..bedd3d496bd 100644
> >> --- a/hw/mips/loongson3_virt.c
> >> +++ b/hw/mips/loongson3_virt.c
> >> @@ -447,8 +447,9 @@ static inline void
> >> loongson3_virt_devices_init(MachineState *machine,
> >>         if (defaults_enabled() && object_class_by_name("pci-ohci")) {
> >>           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-tablet");
> >> +        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS,
> >> NULL);
> >> +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
> >> +        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
> >>       }
> >>         pci_init_nic_devices(pci_bus, mc->default_nic);
> >
> > Can we remove usb_bus_find() completely instead?
> >
> > $ git grep -w usb_bus_find
> > hw/hppa/machine.c:401:        usb_create_simple(usb_bus_find(-1), "usb-kbd");
> > hw/hppa/machine.c:402:        usb_create_simple(usb_bus_find(-1),
> > "usb-mouse");
> > hw/mips/loongson3_virt.c:450:        usb_create_simple(usb_bus_find(-1),
> > "usb-kbd");
> > hw/mips/loongson3_virt.c:451:        usb_create_simple(usb_bus_find(-1),
> > "usb-tablet");
> > hw/ppc/mac_newworld.c:434:            USBBus *usb_bus = usb_bus_find(-1);
> > hw/ppc/sam460ex.c:423:    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> > hw/ppc/sam460ex.c:424:    usb_create_simple(usb_bus_find(-1), "usb-mouse");
> > hw/ppc/spapr.c:3027:            USBBus *usb_bus = usb_bus_find(-1);
> > hw/sh4/r2d.c:315:    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> > hw/usb/bus.c:103:USBBus *usb_bus_find(int busnr)
> > hw/usb/bus.c:669:    USBBus *bus = usb_bus_find(-1 /* any */);
> > include/hw/usb.h:500:USBBus *usb_bus_find(int busnr);
>
> These are all the machines that add devices to a USB bus, there's no other
> example to do it in a different way currently. We could change this to get
> the usb bus in a different way but how?

We can move object_resolve_type_unambiguous out of
hw/i386/acpi-build.c to common code and use it, it's overall a
self-explanatory function.
Re: [PATCH v3 8/9] mips/loongson3_virt: do not require CONFIG_USB
Posted by Paolo Bonzini 9 months, 2 weeks ago 
On Thu, Feb 15, 2024 at 8:55 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
> >       if (defaults_enabled() && object_class_by_name("pci-ohci")) {
> >           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-tablet");
> > +        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS, NULL);
> > +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
> > +        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
> >       }
> >
> >       pci_init_nic_devices(pci_bus, mc->default_nic);
>
> Can we remove usb_bus_find() completely instead?

s/instead/in fact/

Yes, we can, but this would be just one patch in that series...

Paolo

> $ git grep -w usb_bus_find
> hw/hppa/machine.c:401:        usb_create_simple(usb_bus_find(-1),
> "usb-kbd");
> hw/hppa/machine.c:402:        usb_create_simple(usb_bus_find(-1),
> "usb-mouse");
> hw/mips/loongson3_virt.c:450:        usb_create_simple(usb_bus_find(-1),
> "usb-kbd");
> hw/mips/loongson3_virt.c:451:        usb_create_simple(usb_bus_find(-1),
> "usb-tablet");
> hw/ppc/mac_newworld.c:434:            USBBus *usb_bus = usb_bus_find(-1);
> hw/ppc/sam460ex.c:423:    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> hw/ppc/sam460ex.c:424:    usb_create_simple(usb_bus_find(-1), "usb-mouse");
> hw/ppc/spapr.c:3027:            USBBus *usb_bus = usb_bus_find(-1);
> hw/sh4/r2d.c:315:    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> hw/usb/bus.c:103:USBBus *usb_bus_find(int busnr)
> hw/usb/bus.c:669:    USBBus *bus = usb_bus_find(-1 /* any */);
> include/hw/usb.h:500:USBBus *usb_bus_find(int busnr);

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.