Re: [PATCH 1/4] xhci: A default implementation for Ux timeout calculation and tier policy check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 24, 2014 at 11:01:58PM +0800, Mathias Nyman wrote:
> On 04/22/2014 07:27 AM, Pratyush Anand wrote:
> > As best case, a host controller should support U0 to U1 switching for
> > the devices connected below any tier of hub level supported by usb
> > specification. Therefore xhci_check_tier_policy should always return
> > success as default implementation.
> >
> > A host should be able to issue LGO_Ux after the timeout calculated as
> > per definition of system exit latency defined in C.1.5.2. Therefore
> > xhci_calculate_ux_timeout returns ux_params.sel as the default
> > implementation.
> >
> > Use default calculation in absence of any vendor specific limitations.
> >
> > Signed-off-by: Pratyush Anand <pratyush.anand@xxxxxx>
> > Tested-by: Aymen Bouattay <aymen.bouattay@xxxxxx>
> > ---
> >   drivers/usb/host/xhci.c | 66 +++++++++++++++++++++++++++++++++++--------------
> >   1 file changed, 48 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 6cc58fe..f5ac76a 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -4140,8 +4140,7 @@ static u16 xhci_get_timeout_no_hub_lpm(struct usb_device *udev,
> >   	return USB3_LPM_DISABLED;
> >   }
> >
> > -/* Returns the hub-encoded U1 timeout value.
> > - * The U1 timeout should be the maximum of the following values:
> > +/* The U1 timeout should be the maximum of the following values:
> >    *  - For control endpoints, U1 system exit latency (SEL) * 3
> >    *  - For bulk endpoints, U1 SEL * 5
> >    *  - For interrupt endpoints:
> > @@ -4149,7 +4148,8 @@ static u16 xhci_get_timeout_no_hub_lpm(struct usb_device *udev,
> >    *    - Periodic EPs, max(105% of bInterval, U1 SEL * 2)
> >    *  - For isochronous endpoints, max(105% of bInterval, U1 SEL * 2)
> >    */
> > -static u16 xhci_calculate_intel_u1_timeout(struct usb_device *udev,
> > +static unsigned long long xhci_calculate_intel_u1_timeout(
> > +		struct usb_device *udev,
> >   		struct usb_endpoint_descriptor *desc)
> >   {
> >   	unsigned long long timeout_ns;
> > @@ -4181,11 +4181,28 @@ static u16 xhci_calculate_intel_u1_timeout(struct usb_device *udev,
> >   		return 0;
> >   	}
> >
> > -	/* The U1 timeout is encoded in 1us intervals. */
> > -	timeout_ns = DIV_ROUND_UP_ULL(timeout_ns, 1000);
> > -	/* Don't return a timeout of zero, because that's USB3_LPM_DISABLED. */
> > +	return timeout_ns;
> > +}
> > +
> > +/* Returns the hub-encoded U1 timeout value. */
> > +static u16 xhci_calculate_u1_timeout(struct xhci_hcd *xhci,
> > +		struct usb_device *udev,
> > +		struct usb_endpoint_descriptor *desc)
> > +{
> > +	unsigned long long timeout_ns;
> > +
> > +	if (xhci->quirks & XHCI_INTEL_HOST)
> > +		timeout_ns = xhci_calculate_intel_u1_timeout(udev, desc);
> > +	else
> > +		timeout_ns = udev->u1_params.sel;
> > +
> > +	/* The U1 timeout is encoded in 1us intervals.
> > +	 * Don't return a timeout of zero, because that's USB3_LPM_DISABLED.
> > +	 */
> >   	if (timeout_ns == USB3_LPM_DISABLED)
> > -		timeout_ns++;
> > +		timeout_ns = 1;
> > +	else
> > +		timeout_ns = DIV_ROUND_UP_ULL(timeout_ns, 1000);
> >
> >   	/* If the necessary timeout value is bigger than what we can set in the
> >   	 * USB 3.0 hub, we have to disable hub-initiated U1.
> > @@ -4197,14 +4214,14 @@ static u16 xhci_calculate_intel_u1_timeout(struct usb_device *udev,
> >   	return xhci_get_timeout_no_hub_lpm(udev, USB3_LPM_U1);
> >   }
> >
> > -/* Returns the hub-encoded U2 timeout value.
> > - * The U2 timeout should be the maximum of:
> > +/* The U2 timeout should be the maximum of:
> >    *  - 10 ms (to avoid the bandwidth impact on the scheduler)
> >    *  - largest bInterval of any active periodic endpoint (to avoid going
> >    *    into lower power link states between intervals).
> >    *  - the U2 Exit Latency of the device
> >    */
> > -static u16 xhci_calculate_intel_u2_timeout(struct usb_device *udev,
> > +static unsigned long long xhci_calculate_intel_u2_timeout(
> > +		struct usb_device *udev,
> >   		struct usb_endpoint_descriptor *desc)
> >   {
> >   	unsigned long long timeout_ns;
> > @@ -4220,6 +4237,21 @@ static u16 xhci_calculate_intel_u2_timeout(struct usb_device *udev,
> >   	if (u2_del_ns > timeout_ns)
> >   		timeout_ns = u2_del_ns;
> >
> > +	return timeout_ns;
> > +}
> > +
> > +/* Returns the hub-encoded U2 timeout value. */
> > +static u16 xhci_calculate_u2_timeout(struct xhci_hcd *xhci,
> > +		struct usb_device *udev,
> > +		struct usb_endpoint_descriptor *desc)
> > +{
> > +	unsigned long long timeout_ns;
> > +
> > +	if (xhci->quirks & XHCI_INTEL_HOST)
> > +		timeout_ns = xhci_calculate_intel_u2_timeout(udev, desc);
> > +	else
> > +		timeout_ns = udev->u2_params.sel;
> > +
> >   	/* The U2 timeout is encoded in 256us intervals */
> >   	timeout_ns = DIV_ROUND_UP_ULL(timeout_ns, 256 * 1000);
> >   	/* If the necessary timeout value is bigger than what we can set in the
> > @@ -4238,13 +4270,10 @@ static u16 xhci_call_host_update_timeout_for_endpoint(struct xhci_hcd *xhci,
> >   		enum usb3_link_state state,
> >   		u16 *timeout)
> >   {
> > -	if (state == USB3_LPM_U1) {
> > -		if (xhci->quirks & XHCI_INTEL_HOST)
> > -			return xhci_calculate_intel_u1_timeout(udev, desc);
> > -	} else {
> > -		if (xhci->quirks & XHCI_INTEL_HOST)
> > -			return xhci_calculate_intel_u2_timeout(udev, desc);
> > -	}
> > +	if (state == USB3_LPM_U1)
> > +		return xhci_calculate_u1_timeout(xhci, udev, desc);
> > +	else
> > +		return xhci_calculate_u2_timeout(xhci, udev, desc);
> >
> >   	return USB3_LPM_DISABLED;
> 
> Tiny nitpick, but this last return doesn't do much here.
> 
> either:
> if (state == USB3_LPM_U1)
>      return xhci_calculate_u1..
> 
> return xhci_calculate_u2...
> 
> or then:
> 
> if (state == USB3_LPM_U1)
>      return xhci_claculate_u1...
> else if (state == USB3_LPM_U2)   /* or just if (state == USB_LPM_U2) */
>      return xhci_calculate_u2..
> return USB3_LPM_DISABLED

This seems better.. Will modify in next version.

Pratyush

> 
> -Mathias
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux