| Message ID | 20240213155005.109954-9-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | mips: do not list individual devices from configs/ | expand |
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);
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);
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
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.
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);
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(-)