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;
> >> + 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
Attachment:
signature.asc
Description: Digital signature
