On Wed, Aug 25, 2021 at 04:21:31PM +0530, Kishon Vijay Abraham I wrote:
> It has been observed with certain PCIe USB cards (like Inateck connected
> to AM64 EVM or J7200 EVM) that as soon as the primary roothub is
> registered, port status change is handled even before xHC is running
> leading to cold plug USB devices not detected. For such cases, registering
> both the root hubs along with the second HCD is required. Add support for
> deferring roothub registration in usb_add_hcd(), so that both primary and
> secondary roothubs are registered along with the second HCD.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
> Suggested-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/core/hcd.c | 28 +++++++++++++++++++++++-----
> include/linux/usb/hcd.h | 2 ++
> 2 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 775f0456f0ad..ba0493d22d13 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2795,6 +2795,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
> {
> int retval;
> struct usb_device *rhdev;
> + struct usb_hcd *shared_hcd;
>
> if (!hcd->skip_phy_initialization && usb_hcd_is_primary_hcd(hcd)) {
> hcd->phy_roothub = usb_phy_roothub_alloc(hcd->self.sysdev);
> @@ -2956,17 +2957,34 @@ int usb_add_hcd(struct usb_hcd *hcd,
> }
>
> /* starting here, usbcore will pay attention to this root hub */
That comment is now wrong. This is the point in the code where
usbcore will start paying attention to the _shared_ (primary) hcd's
roothub (if its HCD_DEFER_PRI_RH_REGISTER flag is set).
> - retval = register_root_hub(hcd);
> - if (retval != 0)
> - goto err_register_root_hub;
> + shared_hcd = hcd->shared_hcd;
> + if (!usb_hcd_is_primary_hcd(hcd) && shared_hcd &&
> + HCD_DEFER_RH_REGISTER(shared_hcd)) {
> + retval = register_root_hub(shared_hcd);
> + if (retval != 0)
> + goto err_register_shared_root_hub;
>
> - if (hcd->uses_new_polling && HCD_POLL_RH(hcd))
> - usb_hcd_poll_rh_status(hcd);
> + if (shared_hcd->uses_new_polling && HCD_POLL_RH(shared_hcd))
> + usb_hcd_poll_rh_status(shared_hcd);
> + }
> +
_This_ is where usbcore will start paying attention to the hcd's own
root hub (if its HCD_DEFER_PRI_RH_REGISTER flag isn't set).
> + if (!HCD_DEFER_RH_REGISTER(hcd)) {
> + retval = register_root_hub(hcd);
> + if (retval != 0)
> + goto err_register_root_hub;
> +
> + if (hcd->uses_new_polling && HCD_POLL_RH(hcd))
> + usb_hcd_poll_rh_status(hcd);
> + }
>
> return retval;
>
> err_register_root_hub:
> usb_stop_hcd(hcd);
> +err_register_shared_root_hub:
> + if (!usb_hcd_is_primary_hcd(hcd) && shared_hcd &&
> + shared_hcd->flags & HCD_FLAG_DEFER_PRI_RH_REGISTER)
> + usb_stop_hcd(shared_hcd);
> err_hcd_driver_start:
> if (usb_hcd_is_primary_hcd(hcd) && hcd->irq > 0)
> free_irq(irqnum, hcd);
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index 548a028f2dab..6a357ba72f5d 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -124,6 +124,7 @@ struct usb_hcd {
> #define HCD_FLAG_RH_RUNNING 5 /* root hub is running? */
> #define HCD_FLAG_DEAD 6 /* controller has died? */
> #define HCD_FLAG_INTF_AUTHORIZED 7 /* authorize interfaces? */
> +#define HCD_FLAG_DEFER_PRI_RH_REGISTER 8 /* Defer roothub registration */
>
> /* The flags can be tested using these macros; they are likely to
> * be slightly faster than test_bit().
> @@ -134,6 +135,7 @@ struct usb_hcd {
> #define HCD_WAKEUP_PENDING(hcd) ((hcd)->flags & (1U << HCD_FLAG_WAKEUP_PENDING))
> #define HCD_RH_RUNNING(hcd) ((hcd)->flags & (1U << HCD_FLAG_RH_RUNNING))
> #define HCD_DEAD(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEAD))
> +#define HCD_DEFER_RH_REGISTER(hcd) ((hcd)->flags & (1U << HCD_FLAG_DEFER_PRI_RH_REGISTER))
It's awkward to have slightly different names for the flag and the
test. How about getting rid of the "_PRI" part of the flag name? It
isn't really needed, because we obviously won't defer registering the
secondary hcd's root hub -- there's no place to defer it to.
Alan Stern
> * Specifies if interfaces are authorized by default
> --
> 2.17.1
>
