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

[BALATON Zoltan <balaton@eik.bme.hu>]

[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]

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