diff mbox

[3.13.y.z,extended,stable] Patch "usb: musb: Ensure that cppi41 timer gets armed on premature DMA TX irq" has been added to staging queue

Message ID 1407358455-18749-1-git-send-email-kamal@canonical.com
State New
Headers show

Commit Message

Kamal Mostafa Aug. 6, 2014, 8:54 p.m. UTC
This is a note to let you know that I have just added a patch titled

    usb: musb: Ensure that cppi41 timer gets armed on premature DMA TX irq

to the linux-3.13.y-queue branch of the 3.13.y.z extended stable tree 
which can be found at:

 http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.13.y-queue

This patch is scheduled to be released in version 3.13.11.6.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.13.y.z tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Kamal

------

From 99b9d38ddcc0ee176175629dc9261a2eca4b687c Mon Sep 17 00:00:00 2001
From: Thomas Gleixner <tglx@linutronix.de>
Date: Fri, 20 Jun 2014 23:41:24 +0200
Subject: usb: musb: Ensure that cppi41 timer gets armed on premature DMA TX
 irq

commit c58d80f523ffc15ef4d062fc7aeb03793fe39701 upstream.

Some TI chips raise the DMA complete interrupt before the actual
transfer has been completed. The code tries to busy wait for a few
microseconds and if that fails it arms an hrtimer to recheck. So far
so good, but that has the following issue:

CPU 0					CPU1

start_next_transfer(RQ1);

DMA interrupt
  if (premature_irq(RQ1))
    if (!hrtimer_active(timer))
       hrtimer_start(timer);

hrtimer expires
  timer->state = CALLBACK_RUNNING;
  timer->fn()
    cppi41_recheck_tx_req()
      complete_request(RQ1);
      if (requests_pending())
        start_next_transfer(RQ2);

					DMA interrupt
					  if (premature_irq(RQ2))
					    if (!hrtimer_active(timer))
					       hrtimer_start(timer);
  timer->state = INACTIVE;

The premature interrupt of request2 on CPU1 does not arm the timer and
therefor the request completion never happens because it checks for
!hrtimer_active(). hrtimer_active() evaluates:

  timer->state != HRTIMER_STATE_INACTIVE

which of course evaluates to true in the above case as timer->state is
CALLBACK_RUNNING.

That's clearly documented:

 * A timer is active, when it is enqueued into the rbtree or the
 * callback function is running or it's in the state of being migrated
 * to another cpu.

But that's not what the code wants to check. The code wants to check
whether the timer is queued, i.e. whether its armed and waiting for
expiry.

We have a helper function for this: hrtimer_is_queued(). This
evaluates:

  timer->state & HRTIMER_STATE_QUEUED

So in the above case this evaluates to false and therefor forces the
DMA interrupt on CPU1 to call hrtimer_start().

Use hrtimer_is_queued() instead of hrtimer_active() and evrything is
good.

Reported-by: Torben Hohn <torbenh@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 drivers/usb/musb/musb_cppi41.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
1.9.1
diff mbox

Patch

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index a12bd30..4a5af5c 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -266,7 +266,7 @@  static void cppi41_dma_callback(void *private_data)
 		}
 		list_add_tail(&cppi41_channel->tx_check,
 				&controller->early_tx_list);
-		if (!hrtimer_active(&controller->early_tx)) {
+		if (!hrtimer_is_queued(&controller->early_tx)) {
 			hrtimer_start_range_ns(&controller->early_tx,
 				ktime_set(0, 140 * NSEC_PER_USEC),
 				40 * NSEC_PER_USEC,