On 16.11.2016 07:01, OGAWA Hirofumi wrote:
Now, xhci_abort_cmd_ring() is sleepable. So no reason to use busy loop
anymore.
- Convert udelay(1000) => msleep(1).
Sounds good.
- Add xhci_handshake_sleep(), and use it.
This seems a but overkill, I'd rather don't have xhci_handshake(), xhci_handshake_sleep()
and __xhci_handshake() to maintain.
As we now can sleep in xhci_abort_cmd_ring() I would rather just first wait for the completion
and then maybe handshake check the register. At that point it shouldn't need to busyloop anymore,
one read should do it.
But this is all just optimizing an error path, I'm actually fine with taking just
patch 1/3 and 2/3 v3, and skipping 3/3. the udelay(1000) to msleep(1) I can add myself
As related change, current xhci_handshake() is strange behavior,
E.g. xhci_handshake(ptr, mask, done, 1) does
result = readl(ptr);
/* check result */
udelay(1); <= meaningless delay
return -ETIMEDOUT;
Only in the timeout case, so if we after 3 or 5 million register reads + udelays(1)
still don't get the value we want then there is an unnecessary udelay(1).
Not worth putting much effort on it now.
Sorry about the review delay, I got my hands quite full at the moment
-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
