| Message ID | e73bc0c1-e530-7419-0197-1b7de02c87c5@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: host: xhci-plat: omit shared hcd if either of the root hubs has no ports | expand |
On 4.3.2022 20.37, Heiner Kallweit wrote: > If either of the root hubs has no ports, then we can get rid of > overhead like the shared hcd. A major internal change is that now > the main hcd can be USB2 or USB3. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/usb/host/xhci-mem.c | 11 +++++------ > drivers/usb/host/xhci.c | 9 ++++++--- > 2 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > index a1a17713a..ced139583 100644 > --- a/drivers/usb/host/xhci-mem.c > +++ b/drivers/usb/host/xhci-mem.c > @@ -2362,12 +2362,11 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags) > xhci->usb2_rhub.num_ports = USB_MAXCHILDREN; > } > > - xhci->needs_shared_hcd = 1; > - > - /* > - * Note we could have all USB 3.0 ports, or all USB 2.0 ports. > - * Not sure how the USB core will handle a hub with no ports... > - */ > + if (xhci->usb2_rhub.num_ports && xhci->usb3_rhub.num_ports) > + xhci->needs_shared_hcd = 1; > + else > + xhci_info(xhci, "USB%u root hub has no ports\n", > + xhci->usb2_rhub.num_ports ? 3 : 2); This now works for xhci controllers using xhci-plat.c, but in all other cases the the secondary hcd will still be added. Would it make sense to instead of setting xhci->needs_shared_hcd, we set a xhci->allow_single_roothub flag in the .reset override function? In the xhci-plat.c case this would be in xhci_plat_setup() We would only add the flag if the respective probe supports one roothub. Add a helper function to check if we if really should set up just one hcd in probe, and should call xhci_run_finished() already the he first time xhci_run() is called (like you do in patch 3/5). Something like: bool xhci_has_one_roothub(struct xhci_hcd *xhci) { return xhci->allow_single_roothub && (!xhci->usb2_rhub.num_ports != !xhci->usb3_rhub.num_ports); } Thanks -Mathias
On 08.03.2022 15:55, Mathias Nyman wrote: > On 4.3.2022 20.37, Heiner Kallweit wrote: >> If either of the root hubs has no ports, then we can get rid of >> overhead like the shared hcd. A major internal change is that now >> the main hcd can be USB2 or USB3. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/usb/host/xhci-mem.c | 11 +++++------ >> drivers/usb/host/xhci.c | 9 ++++++--- >> 2 files changed, 11 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c >> index a1a17713a..ced139583 100644 >> --- a/drivers/usb/host/xhci-mem.c >> +++ b/drivers/usb/host/xhci-mem.c >> @@ -2362,12 +2362,11 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags) >> xhci->usb2_rhub.num_ports = USB_MAXCHILDREN; >> } >> >> - xhci->needs_shared_hcd = 1; >> - >> - /* >> - * Note we could have all USB 3.0 ports, or all USB 2.0 ports. >> - * Not sure how the USB core will handle a hub with no ports... >> - */ >> + if (xhci->usb2_rhub.num_ports && xhci->usb3_rhub.num_ports) >> + xhci->needs_shared_hcd = 1; >> + else >> + xhci_info(xhci, "USB%u root hub has no ports\n", >> + xhci->usb2_rhub.num_ports ? 3 : 2); > > This now works for xhci controllers using xhci-plat.c, but in all other cases > the the secondary hcd will still be added. > > Would it make sense to instead of setting xhci->needs_shared_hcd, we > set a xhci->allow_single_roothub flag in the .reset override function? > In the xhci-plat.c case this would be in xhci_plat_setup() > > We would only add the flag if the respective probe supports one roothub. > > Add a helper function to check if we if really should set up just one > hcd in probe, and should call xhci_run_finished() already the he first time > xhci_run() is called (like you do in patch 3/5). > > Something like: > > bool xhci_has_one_roothub(struct xhci_hcd *xhci) > { > return xhci->allow_single_roothub && (!xhci->usb2_rhub.num_ports != !xhci->usb3_rhub.num_ports); > } Looks (very) good to me. I'll incorporate this and reorder the patches, then I'll send a v2. > > Thanks > -Mathias Heiner
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index a1a17713a..ced139583 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2362,12 +2362,11 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags) xhci->usb2_rhub.num_ports = USB_MAXCHILDREN; } - xhci->needs_shared_hcd = 1; - - /* - * Note we could have all USB 3.0 ports, or all USB 2.0 ports. - * Not sure how the USB core will handle a hub with no ports... - */ + if (xhci->usb2_rhub.num_ports && xhci->usb3_rhub.num_ports) + xhci->needs_shared_hcd = 1; + else + xhci_info(xhci, "USB%u root hub has no ports\n", + xhci->usb2_rhub.num_ports ? 3 : 2); xhci_create_rhub_port_array(xhci, &xhci->usb2_rhub, flags); xhci_create_rhub_port_array(xhci, &xhci->usb3_rhub, flags); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 83a54a566..623901890 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -5279,9 +5279,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) xhci = hcd_to_xhci(hcd); - if (usb_hcd_is_primary_hcd(hcd)) { - xhci_hcd_init_usb2_data(xhci, hcd); - } else { + if (!usb_hcd_is_primary_hcd(hcd)) { xhci_hcd_init_usb3_data(xhci, hcd); return 0; } @@ -5363,6 +5361,11 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) return retval; xhci_dbg(xhci, "Called HCD init\n"); + if (xhci_hcd_is_usb3(hcd)) + xhci_hcd_init_usb3_data(xhci, hcd); + else + xhci_hcd_init_usb2_data(xhci, hcd); + xhci_info(xhci, "hcc params 0x%08x hci version 0x%x quirks 0x%016llx\n", xhci->hcc_params, xhci->hci_version, xhci->quirks);
If either of the root hubs has no ports, then we can get rid of overhead like the shared hcd. A major internal change is that now the main hcd can be USB2 or USB3. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/usb/host/xhci-mem.c | 11 +++++------ drivers/usb/host/xhci.c | 9 ++++++--- 2 files changed, 11 insertions(+), 9 deletions(-)