> -----Original Message----- > From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] > Sent: Friday, December 10, 2010 7:24 AM > To: Xu, Andiry > Cc: linux-usb@xxxxxxxxxxxxxxx > Subject: [RFC] xhci: Rework port suspend structures for limited ports. > > The USB core only allows up to 31 (USB_MAXCHILDREN) ports under a roothub. > The xHCI driver keeps track of which ports are suspended, which ports have > a suspend change bit set, and what time the port will be done resuming. > It keeps track of the first two by setting a bit in a u32 variable, > suspended_ports or port_c_suspend. The xHCI driver currently assumes we > can have up to 256 ports under a roothub, so it allocates an array of 8 > u32 variables for both suspended_ports and port_c_suspend. It also > allocates a 256-element array to keep track of when the ports will be done > resuming. > > Since we can only have 31 roothub ports, we only need to use one u32 for > each of the suspend state and change variables. We simplify the bit math > that's trying to index into those arrays and set the correct bit, if we > assume wIndex never exceeds 30. (wIndex is zero-based after it's > decremented from the value passed in from the USB core.) Finally, we > change the resume_done array to only hold 31 elements. > > Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx> > --- > > Andiry, > > Can you take a look at this? The USB core relies on not having more > than USB_MAXCHILDREN ports in many different places, so I think this > simplification won't change any current behavior. Do you have any > objections? > The patch looks OK and it really simplifies the code, I've no objections. Please note there is a code in xhci_mem_init(): + for (i = 0; i < MAX_HC_PORTS; ++i) + xhci->resume_done[i] = 0; Please also change the MAX_HC_PORTS to USB_MAXCHILDREN, or you will access memory out of the array. Thanks, Andiry > > drivers/usb/host/xhci-hub.c | 28 ++++++++++------------------ > drivers/usb/host/xhci.h | 9 +++++---- > 2 files changed, 15 insertions(+), 22 deletions(-) > > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c > index eeba4b5..1dfac10 100644 > --- a/drivers/usb/host/xhci-hub.c > +++ b/drivers/usb/host/xhci-hub.c > @@ -452,20 +452,15 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 > typeReq, u16 wValue, > goto error; > } > xhci_ring_device(xhci, slot_id); > - xhci->port_c_suspend[wIndex >> 5] |= > - 1 << (wIndex & 31); > - xhci->suspended_ports[wIndex >> 5] &= > - ~(1 << (wIndex & 31)); > + xhci->port_c_suspend |= 1 << wIndex; > + xhci->suspended_ports &= ~(1 << wIndex); > } > } > if ((temp & PORT_PLS_MASK) == XDEV_U0 > && (temp & PORT_POWER) > - && (xhci->suspended_ports[wIndex >> 5] & > - (1 << (wIndex & 31)))) { > - xhci->suspended_ports[wIndex >> 5] &= > - ~(1 << (wIndex & 31)); > - xhci->port_c_suspend[wIndex >> 5] |= > - 1 << (wIndex & 31); > + && (xhci->suspended_ports & (1 << wIndex))) { > + xhci->suspended_ports &= ~(1 << wIndex); > + xhci->port_c_suspend |= 1 << wIndex; > } > if (temp & PORT_CONNECT) { > status |= USB_PORT_STAT_CONNECTION; > @@ -479,7 +474,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, > u16 wValue, > status |= USB_PORT_STAT_RESET; > if (temp & PORT_POWER) > status |= USB_PORT_STAT_POWER; > - if (xhci->port_c_suspend[wIndex >> 5] & (1 << (wIndex & 31))) > + if (xhci->port_c_suspend & (1 << wIndex)) > status |= 1 << USB_PORT_FEAT_C_SUSPEND; > xhci_dbg(xhci, "Get port status returned 0x%x\n", status); > put_unaligned(cpu_to_le32(status), (__le32 *) buf); > @@ -531,8 +526,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, > u16 wValue, > spin_lock_irqsave(&xhci->lock, flags); > > temp = xhci_readl(xhci, port_array[wIndex]); > - xhci->suspended_ports[wIndex >> 5] |= > - 1 << (wIndex & (31)); > + xhci->suspended_ports |= 1 << wIndex; > break; > case USB_PORT_FEAT_POWER: > /* > @@ -608,8 +602,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, > u16 wValue, > xhci_writel(xhci, temp, > port_array[wIndex]); > } > - xhci->port_c_suspend[wIndex >> 5] |= > - 1 << (wIndex & 31); > + xhci->port_c_suspend |= 1 << wIndex; > } > > slot_id = xhci_find_slot_id_by_port(hcd, xhci, > @@ -621,8 +614,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, > u16 wValue, > xhci_ring_device(xhci, slot_id); > break; > case USB_PORT_FEAT_C_SUSPEND: > - xhci->port_c_suspend[wIndex >> 5] &= > - ~(1 << (wIndex & 31)); > + xhci->port_c_suspend &= ~(1 << wIndex); > case USB_PORT_FEAT_C_RESET: > case USB_PORT_FEAT_C_CONNECTION: > case USB_PORT_FEAT_C_OVER_CURRENT: > @@ -689,7 +681,7 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char > *buf) > break; > } > if ((temp & mask) != 0 || > - (xhci->port_c_suspend[i >> 5] & 1 << (i & 31)) || > + (xhci->port_c_suspend & 1 << i) || > (xhci->resume_done[i] && time_after_eq( > jiffies, xhci->resume_done[i]))) { > buf[(i + 1) / 8] |= 1 << (i + 1) % 8; > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 29b8f10..d38f533 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -1248,10 +1248,11 @@ struct xhci_hcd { > #define XHCI_LINK_TRB_QUIRK (1 << 0) > #define XHCI_RESET_EP_QUIRK (1 << 1) > #define XHCI_NEC_HOST (1 << 2) > - u32 port_c_suspend[8]; /* port suspend change*/ > - u32 suspended_ports[8]; /* which ports are > - suspended */ > - unsigned long resume_done[MAX_HC_PORTS]; > + /* port suspend change*/ > + u32 port_c_suspend; > + /* which ports are suspended */ > + u32 suspended_ports; > + unsigned long resume_done[USB_MAXCHILDREN]; > /* Is each xHCI roothub port a USB 3.0, USB 2.0, or USB 1.1 port? */ > u8 *port_array; > /* Array of pointers to USB 3.0 PORTSC registers */ > -- > 1.6.3.3 > -- 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
