>-----Original Message----- >From: Oliver Neukum [mailto:oneukum@xxxxxxx] >Sent: Wednesday, June 02, 2010 5:24 PM >To: Du, Alek >Cc: greg@xxxxxxxxx; david-b@xxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; Pan, >Jacob jun >Subject: Re: [PATCH 2/3] EHCI: EHCI 1.1 addendum: Basic LPM feature support >> >> +/* >> >> + * this function is called to put a link into L1 state. the steps are: >> >> + * - verify HC supports LPM >> >> + * - make sure all pipe idle on the link >> >> + * - shutdown all qh on the pipe >> >> + * - send LPM packet >> >> + * - confirm device ack >> >> + */ >> >> +static unsigned ehci_lpm_check(struct ehci_hcd *ehci, int port) >> >> +{ >> >[..] >> >> + if (retval != -ETIMEDOUT) { >> >> + ehci_dbg(ehci, "LPM: device ACK for LPM\n"); >> >> + val32 |= PORT_LPM; >> >> + /* >> >> + * now device should be in L1 sleep, let's wake up the device >> >> + * so that we can complete enumeration. >> >> + */ >> >> + ehci_writel(ehci, val32, portsc); >> >> + msleep(10); >> >> + val32 |= PORT_RESUME; >> >> + ehci_writel(ehci, val32, portsc); >> >> + } else { >> >> + ehci_dbg(ehci, "LPM: device does not ACK, disable LPM %d\n", >> >> + retval); >> >> + val32 &= ~PORT_LPM; >> >> + retval = -ETIMEDOUT; >> >> + ehci_writel(ehci, val32, portsc); >> >> + } >> > >> >This is bold. Do you have any experimental data how real devices react? >> >> For most current devices, of course, will go to the "LPM: device does not ACK" >way, and >> this has no side effects. We used the on board USB OTG port as USB device >that could >> support LPM to do the test. > >I am afraid that some devices might show side effects. Have you actually tested >on devices that don't support LPM? Yes, of course, we have tested a lot of "normal" USB devices. This behavior has been defined in USB 2.0 ECN: Link Power Management, Page 8: If the device does not understand the protocol extension transaction, no handshake is returned. Basically the LPM token is encapsulated in a standard token, while the device does not understand the extension token, nothing happens. >> >> + return retval; >> >> +} >> >> diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c >> >> index 11a79c4..0c39f49 100644 >> >> --- a/drivers/usb/host/ehci-q.c >> >> +++ b/drivers/usb/host/ehci-q.c >> >> @@ -643,6 +643,16 @@ qh_urb_transaction ( >> >> sizeof (struct usb_ctrlrequest), >> >> token | (2 /* "setup" */ << 8), 8); >> >> >> >> + if (((struct usb_ctrlrequest *)urb->setup_packet)->bRequest >> >> + == USB_REQ_SET_ADDRESS) { >> >> + /* for LPM capable HC, set up device address*/ >> >> + int dev_address = ((struct usb_ctrlrequest *) >> >> + (urb->setup_packet))->wValue; >> >> + if (ehci->has_lpm) >> >> + ehci_lpm_set_da(ehci, dev_address, >> >> + urb->dev->portnum); >> >> + } >> > >> >No. Please make an explicit hook for this in usbcore. >> >> Thanks, but I do not know if we can make it ehci specific? > >Does anything but a subset of EHCI support LPM? >I am not sure that I understand the question. What I mean is, I do not want to touch usb core code, since it is only a EHCI feature. If we could keep the patch only in EHCI part, it is good. BR, Alek -- 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
