> From: Sarah Sharp [sarah.a.sharp@xxxxxxxxxxxxxxx]
> Sent: Saturday, April 14, 2012 12:13 AM
> To: Andiry Xu
> Cc: linux-usb@xxxxxxxxxxxxxxx; Xu, Andiry; Alan Stern
> Subject: Re: [RFC] xHCI: keep track of ports being resumed and indicate in hub_status_data
> On Sat, Apr 14, 2012 at 02:54:30AM +0800, Andiry Xu wrote:
> > This commit adds a bit-array to xhci bus_state for keeping track of
> > which ports are undergoing a resume transition. If any of the bits
> > are set when xhci_hub_status_data() is called, the routine will return
> > a non-zero value even if no ports have any status changes pending.
> > This will allow usbcore to handle races between root-hub suspend and
> > port wakeup.
> Hi Andiry,
> Thanks for doing this! The patch looks fine, but I have one question:
> > --- a/drivers/usb/host/xhci.h
> > +++ b/drivers/usb/host/xhci.h
> > @@ -1362,6 +1362,8 @@ struct xhci_bus_state {
> > u32 suspended_ports;
> > u32 port_remote_wakeup;
> > unsigned long resume_done[USB_MAXCHILDREN];
> > + /* which ports have started to resume */
> > + unsigned long resuming_ports;
> Is there a particular reason why you chose unsigned long for this when
> the other two bitmasks for port suspend states (suspended_ports and
> port_remote_wakeup) are u32?
No particular reason, this line is copied from Alan's patch. But you are right,
u32 is better: it shows the hub supports up to 31 ports, and comply with
port_c_suspend and suspended_ports variable type. I'll modify it.
> I think the USB core limits the number of max ports on a hub to 31, so there's
> no sense using an unsigned long. Just use u32. Then if we find an xHCI host
> with more than 31 ports and we have to expand the USB core USB_MAXCHILDREN
> variable, we can easily search for u32 bit fields.
> We should probably add a warning to the xhci-mem.c:xhci_mem_init() and
> fail if we find there's more than 31 ports. We print a debug message,
> but that won't show up unless CONFIG_USB_XHCI_HCD_DEBUGGING is turned
> on:
> static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
> {
> ...
> if (xhci->num_usb2_ports > USB_MAXCHILDREN) {
> xhci_dbg(xhci, "Limiting USB 2.0 roothub ports to %u.\n",
> USB_MAXCHILDREN);
> xhci->num_usb2_ports = USB_MAXCHILDREN;
> }
OK, I'll do this.
Thanks,
Andiry
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
