[PATCH 02/10] ppc: sam460ex: do not use usb_bus_find()

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 02/10] ppc: sam460ex: do not use usb_bus_find()
Posted by Paolo Bonzini 9 months ago
usb_bus_find() is always used with argument -1; it can be replaced 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/ppc/sam460ex.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 1e615b8d355..4d5655ab6b4 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -273,6 +273,7 @@ static void sam460ex_init(MachineState *machine)
     DeviceState *uic[4];
     int i;
     PCIBus *pci_bus;
+    USBBus *usb_bus;
     PowerPCCPU *cpu;
     CPUPPCState *env;
     I2CBus *i2c;
@@ -420,8 +421,9 @@ static void sam460ex_init(MachineState *machine)
     sysbus_realize_and_unref(sbdev, &error_fatal);
     sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
     sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(uic[2], 30));
-    usb_create_simple(usb_bus_find(-1), "usb-kbd");
-    usb_create_simple(usb_bus_find(-1), "usb-mouse");
+    usb_bus = USB_BUS(object_resolve_type_unambiguous(TYPE_USB_BUS, &error_abort));
+    usb_create_simple(usb_bus, "usb-kbd");
+    usb_create_simple(usb_bus, "usb-mouse");
 
     /* PCIe buses */
     dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
-- 
2.43.0


Re: [PATCH 02/10] ppc: sam460ex: do not use usb_bus_find()
Posted by Markus Armbruster 9 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> usb_bus_find() is always used with argument -1; it can be replaced 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/ppc/sam460ex.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 1e615b8d355..4d5655ab6b4 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -273,6 +273,7 @@ static void sam460ex_init(MachineState *machine)
>      DeviceState *uic[4];
>      int i;
>      PCIBus *pci_bus;
> +    USBBus *usb_bus;
>      PowerPCCPU *cpu;
>      CPUPPCState *env;
>      I2CBus *i2c;
> @@ -420,8 +421,9 @@ static void sam460ex_init(MachineState *machine)
>      sysbus_realize_and_unref(sbdev, &error_fatal);
>      sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
>      sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(uic[2], 30));
> -    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> -    usb_create_simple(usb_bus_find(-1), "usb-mouse");
> +    usb_bus = USB_BUS(object_resolve_type_unambiguous(TYPE_USB_BUS, &error_abort));

This long line is really easy to break.

> +    usb_create_simple(usb_bus, "usb-kbd");
> +    usb_create_simple(usb_bus, "usb-mouse");
>  
>      /* PCIe buses */
>      dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
Re: [PATCH 02/10] ppc: sam460ex: do not use usb_bus_find()
Posted by Markus Armbruster 9 months ago
Paolo Bonzini <pbonzini@redhat.com> writes:

> 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..b2a8b22b4ea 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_type_unambiguous(TYPE_USB_BUS, &error_abort);
> +        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
> +        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");

In the previous patches, you cast just once, like this:

           USBBus *usb_bus = USB_BUS(object_resolve_type_unambiguous(TYPE_USB_BUS, &error_abort));
           usb_create_simple(usb_bus, "usb-kbd");
           usb_create_simple(usb_bus, "usb-tablet");

Could you do that here, too?

Same for the next few patches.

>      }
>  
>      pci_init_nic_devices(pci_bus, mc->default_nic);
Re: [PATCH 02/10] ppc: sam460ex: do not use usb_bus_find()
Posted by Markus Armbruster 9 months ago
Please ignore this one; I replied to the wrong patch by accident.
Re: [PATCH 02/10] ppc: sam460ex: do not use usb_bus_find()
Posted by BALATON Zoltan 9 months ago
On Mon, 26 Feb 2024, Markus Armbruster wrote:
> Please ignore this one; I replied to the wrong patch by accident.

Your comment is still valid for that patch so no need to ignore it.

Regards,
BALATON Zoltan
Re: [PATCH 02/10] ppc: sam460ex: do not use usb_bus_find()
Posted by Thomas Huth 9 months ago
On 23/02/2024 13.43, Paolo Bonzini wrote:
> usb_bus_find() is always used with argument -1; it can be replaced 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/ppc/sam460ex.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 1e615b8d355..4d5655ab6b4 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -273,6 +273,7 @@ static void sam460ex_init(MachineState *machine)
>       DeviceState *uic[4];
>       int i;
>       PCIBus *pci_bus;
> +    USBBus *usb_bus;
>       PowerPCCPU *cpu;
>       CPUPPCState *env;
>       I2CBus *i2c;
> @@ -420,8 +421,9 @@ static void sam460ex_init(MachineState *machine)
>       sysbus_realize_and_unref(sbdev, &error_fatal);
>       sysbus_mmio_map(sbdev, 0, 0x4bffd0000);
>       sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(uic[2], 30));
> -    usb_create_simple(usb_bus_find(-1), "usb-kbd");
> -    usb_create_simple(usb_bus_find(-1), "usb-mouse");
> +    usb_bus = USB_BUS(object_resolve_type_unambiguous(TYPE_USB_BUS, &error_abort));
> +    usb_create_simple(usb_bus, "usb-kbd");
> +    usb_create_simple(usb_bus, "usb-mouse");
>   
>       /* PCIe buses */
>       dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);

Reviewed-by: Thomas Huth <thuth@redhat.com>