On Thu, Mar 10, 2011 at 09:42:55PM -0800, Dmitry Torokhov wrote:
> Hi Sarah,
>
> On Mon, Mar 07, 2011 at 02:11:54PM -0800, Sarah Sharp wrote:
> > The xhci_bus_suspend() and xhci_bus_resume() functions are a bit hard to
> > read, because they have an ambiguously named variable "port". Rename it
> > to "port_index". Introduce a new temporary variable, "max_ports" that
> > holds the maximum number of roothub ports the host controller supports.
> > This will reduce the number of register reads, and make it easy to change
> > the maximum number of ports when there are two roothubs.
> >
> > Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
> > ---
> > drivers/usb/host/xhci-hub.c | 41 ++++++++++++++++++++++-------------------
> > drivers/usb/host/xhci-ring.c | 6 +++---
> > 2 files changed, 25 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> > index 50e250c..004a465 100644
> > --- a/drivers/usb/host/xhci-hub.c
> > +++ b/drivers/usb/host/xhci-hub.c
> > @@ -568,42 +568,44 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
> > int xhci_bus_suspend(struct usb_hcd *hcd)
> > {
> > struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> > - int port;
> > + int max_ports, port_index;
> > unsigned long flags;
> >
> > xhci_dbg(xhci, "suspend root hub\n");
> > + max_ports = HCS_MAX_PORTS(xhci->hcs_params1);
> >
> > spin_lock_irqsave(&xhci->lock, flags);
> >
> > if (hcd->self.root_hub->do_remote_wakeup) {
> > - port = HCS_MAX_PORTS(xhci->hcs_params1);
> > - while (port--) {
> > - if (xhci->resume_done[port] != 0) {
> > + port_index = max_ports;
> > + while (port_index--) {
>
> I do not believe that iterating from highest to lowest port is
> significant here so I would go with:
>
> for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
> ...
> }
>
> or
>
> for (i = 0; i < max_ports; i++) {
> ...
> }
I don't really care either way; it was Andiry's original code.
Can you please submit a clean up patch later after this series lands? I
don't think that particular cleanup should go in this renaming patch.
Sarah Sharp
--
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
