Re: [RFC 3/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 Sat, Apr 12, 2014 at 02:54:22AM +0800, Felipe Balbi wrote:
> Hi,
> 
> On Sat, Apr 12, 2014 at 12:04:27AM +0530, Pratyush Anand wrote:
> > >> +static u16 xhci_calculate_default_u1_timeout(struct usb_device *udev,
> > >> +             struct usb_endpoint_descriptor *desc)
> > >> +{
> > >> +     unsigned long long timeout_ns;
> > >> +
> > >> +     timeout_ns = udev->u1_params.sel;
> > >> +
> > >> +     /* 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. */
> > >> +     if (timeout_ns == USB3_LPM_DISABLED)
> > >
> > > will this *ever* happen ? you're using DIV_ROUND_UP...
> > 
> > Only when timeout_ns was zero before DIV_* operation.
> 
> but then I guess it's cheaper to just return 1 in that case and avoid
> doing a division at all:
> 
> if (timeout_ns == 0)
> 	return 1;
> 

OK..Moving this if statement above DIV will do.

Regards
Pratyush

> > >> +             timeout_ns++;
> > >> +
> > >> +     /* 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.
> > >> +      */
> > >> +     if (timeout_ns <= USB3_LPM_U1_MAX_TIMEOUT)
> > >> +             return timeout_ns;
> > >> +     dev_dbg(&udev->dev, "Hub-initiated U1 disabled "
> > >> +                     "due to long timeout %llu ms\n", timeout_ns);
> > >> +     return xhci_get_timeout_no_hub_lpm(udev, USB3_LPM_U1);
> > >> +}
> > >> +
> > >> +/* Returns the default hub-encoded U2 timeout value.
> > >> + */
> > >> +static u16 xhci_calculate_default_u2_timeout(struct usb_device *udev,
> > >> +             struct usb_endpoint_descriptor *desc)
> > >> +{
> > >> +     unsigned long long timeout_ns;
> > >> +
> > >> +     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
> > >> +      * USB 3.0 hub, we have to disable hub-initiated U2.
> > >> +      */
> > >> +     if (timeout_ns <= USB3_LPM_U2_MAX_TIMEOUT)
> > >> +             return timeout_ns;
> > >> +     dev_dbg(&udev->dev, "Hub-initiated U2 disabled "
> > >> +                     "due to long timeout %llu ms\n", timeout_ns);
> > >> +     return xhci_get_timeout_no_hub_lpm(udev, USB3_LPM_U2);
> > >
> > > this function is mostly duplicated with default_u1, perhaps refactor it
> > > a bit and pass one extra argument which will tell you if you're
> > > calculating U1 or U2 ?
> > 
> > Can be done.
> > Will wait for Mathias comments  and then will send another version.
> 
> alright.
> 
> -- 
> balbi


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