Re: [PATCH 11/21] hw/usb: Add QOM parentship relation with hub devices

[Zhao Liu <zhao1.liu@intel.com>]

Hi Philippe,

On Fri, Feb 16, 2024 at 12:03:02PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Fri, 16 Feb 2024 12:03:02 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH 11/21] hw/usb: Add QOM parentship relation with hub devices
> X-Mailer: git-send-email 2.41.0
> 
> QDev objects created with qdev_*new() need to manually add
> their parent relationship with object_property_add_child().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/usb/bus.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index a599e2552b..baad04f466 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -439,6 +439,7 @@ void usb_claim_port(USBDevice *dev, Error **errp)
>              /* Create a new hub and chain it on */
>              hub = USB_DEVICE(qdev_try_new("usb-hub"));

One additional question comes to mind, should we use the qdev_new()
here?

The difference between qdev_try_new() and qdev_new() is the latter
uses assert() to ensure the passed type exists.

So if we know that type parameter is correct, we should just use
qdev_new().

Only when the caller is not sure whether the type is valid,
qdev_try_new() should be preferred.

Am I understand correctly? ;-)

>              if (hub) {
> +                object_property_add_child(OBJECT(dev), "hub", OBJECT(hub));

From the comment above the code:

/* Create a new hub and chain it on */

this only creates a new usb-hub, should the new usb-hub become the child
object of the original usb-hub?

>                  usb_realize_and_unref(hub, bus, NULL);
>              }
>          }
> -- 
> 2.41.0
> 
>