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

2024-02-26 Thread BALATON Zoltan 

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()

2024-02-25 Thread Markus Armbruster 
Paolo Bonzini  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é 
> Signed-off-by: Paolo Bonzini 
> ---
>  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, _fatal);
>  sysbus_mmio_map(sbdev, 0, 0x4bffd);
>  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, 
> _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()

2024-02-25 Thread Markus Armbruster 
Please ignore this one; I replied to the wrong patch by accident.

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

2024-02-25 Thread Markus Armbruster 
Paolo Bonzini  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 
> ---
>  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, 
> _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, _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()

2024-02-25 Thread Thomas Huth 

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é 
Signed-off-by: Paolo Bonzini 
---
  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, _fatal);
  sysbus_mmio_map(sbdev, 0, 0x4bffd);
  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, 
_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