Am Mittwoch, 2. Juni 2010 05:17:42 schrieb alek.du@xxxxxxxxx:
> From: Alek Du <alek.du@xxxxxxxxx>
>
> With this patch, the LPM capable EHCI host controller can put device
> into L1 sleep state which is a mode that can enter/exit quickly, and
> reduce power consumption.
Cool feature.
>
> @@ -855,7 +873,16 @@ static int ehci_urb_enqueue (
> default:
> if (!qh_urb_transaction (ehci, urb, &qtd_list, mem_flags))
> return -ENOMEM;
> - return submit_async(ehci, urb, &qtd_list, mem_flags);
> + status = submit_async(ehci, urb, &qtd_list, mem_flags);
> +
> + /* check device LPM cap after set address */
> + if (usb_pipecontrol(urb->pipe)) {
> + if (((struct usb_ctrlrequest *)urb->setup_packet)
> + ->bRequest == USB_REQ_SET_ADDRESS &&
> + ehci->has_lpm)
> + ehci_lpm_check(ehci, urb->dev->portnum);
> + }
> + return status;
This is very ugly. The check should not be here.
> +/*
> + * 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?
> + 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.
Regards
Oliver
--
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
