On 10/29/2015 10:08 PM, Mathias Nyman wrote:
On 29.10.2015 14:58, Lu, Baolu wrote:
On 10/29/2015 08:51 PM, Sergei Shtylyov wrote:
Hello.
On 10/29/2015 5:46 AM, Lu Baolu wrote:
Function ep_ring_is_processing() checks the dequeue pointer
in endpoint context to know whether an endpoint is busy with
processing TRBs. This is not correct since dequeue pointer
field in an endpoint context is only valid when the endpoint
is in Halted or Stopped states. This buggy code causes audio
noise when playing sound with USB headset connected to host
controllers which support CFC (one of xhci 1.1 features).
This patch should exist in stable kernel since v4.3.
Reported-and-tested-by: YD Tseng <yd_tseng@xxxxxxxxxxxxxx>
Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
---
v1->v2:
Implement the logic in xhci_queue_isoc_tx_prepare() instead of
a seperated function as suggested by Mathias.
---
drivers/usb/host/xhci-ring.c | 32 ++++++--------------------------
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c
b/drivers/usb/host/xhci-ring.c
index fa83625..8edc286 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
[...]
@@ -3983,10 +3961,12 @@ int xhci_queue_isoc_tx_prepare(struct
xhci_hcd *xhci, gfp_t mem_flags,
}
/* Calculate the start frame and put it in urb->start_frame. */
- if (HCC_CFC(xhci->hcc_params) &&
- ep_ring_is_processing(xhci, slot_id, ep_index)) {
- urb->start_frame = xep->next_frame_id;
- goto skip_start_over;
+ if (HCC_CFC(xhci->hcc_params)) {
+ if ((le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK)
+ == EP_STATE_RUNNING &&
+ !list_empty(&ep_ring->td_list))
+ urb->start_frame = xep->next_frame_id;
+ goto skip_start_over;
Forgot {}?
Oh, I am sorry. I am wondering how it passed my test.
I will send v3 patch soon any way.
If you are anyway making a v3 then maybe one more change,
just for readability, no (real) functional change:
if (HCC_CFC(xhci->hcc_params) && !list_empty(&ep_ring->td_list)) {
if (le32_to_cpu(ep_...
Done.
While thinking about code cleanup I also think we should use a local
variable
u32 ep_info = le32_to_cpu(ep_ctx->ep_info) as it's used several times
in xhci_queue_isoc_tx_preapare(),
causing a lot of line splitting.
It should be ok as we are under the same spinlock so ep_ctx should not
change.
But that is not a fix sent to a rc and stable, I can make a separate
cleanup patch for it later.
-Mathias
--
To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
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
