Commit 50aea6fca ("usb: musb: cppi41: fire hrtimer according to
programmed channel length") altered the hrtimer intervals and tried to
make it better for ISOC. This patch reverts those changes and here is
why:
- Daniel said that for his ISOC case a timer interval of 3 us would be
best. This should be handled now in the previous commit ("usb: musb:
musb_cppi41: recognize HS devices in hostmode") where we poll for up
to 25us before we setup a timer.
- the patch took total_len / 10 for the total delay. This isn't really
what should be done. According to my understanding (how the HW would
work) is that the DMA engine feeds packet_size bytes into MUSB and
then triggers the transfer. This is repeated over and over until
->total_len is transferred. That means upon DMA interrupt we need to
transfer the remaining 512 bytes (assuming that is our max packet)
in the worst case scenario.
With a USB Storage request size of 64KiB (not uncommon) the timer is
programmed with a delay of ~6.5ms while the completion of the transfer
could happen in as little as a few micro seconds.
Here is an example from the usb-storage:
|usb-storage 3459.798879: cppi41_dma_channel_program: len = 65536
|<idle>-0 3459.929518: cppi41_dma_callback:
The interrupt arrived ~131ms later.
|<idle>-0 3459.938388: cppi41_trans_done:
The transfer completed approx 9ms later.
The DMA transfer itself took ~131ms. USB could send the data quicker but
since the device on the other side was not fast enough it kept sending
NAKs. Now imagine, the DMA engine transferred the last 512 bytes and
raised the interrupt. Now MUSB would be pleased to send the remaining few
bytes but since the device is sending NAKs, it has to wait.
With a re-try of 50us we check three times that often for such "long
standing" conditions.
I've been testing with a USB-disk and I haven't needed the hrtimer at
all (the 25us was enough). The sdcards on the other hand needed the
timer more often. Here I noticed hardly a difference. Even with the
retry of 50us it jumped between 5.8 MiB/sec and 4.9MiB/sec.
Cc: Daniel Mack <zonque@xxxxxxxxx>
Cc: Sebastian Reimers <sebastian.reimers@xxxxxxxxxxxxxx>
Cc: Sven Neumann <neumann@xxxxxxxxx>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
---
drivers/usb/musb/musb_cppi41.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index 4aceb35dea4a..87b0ca0786d7 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -199,7 +199,7 @@ static enum hrtimer_restart cppi41_recheck_tx_req(struct hrtimer *timer)
if (!list_empty(&controller->early_tx_list)) {
ret = HRTIMER_RESTART;
hrtimer_forward_now(&controller->early_tx,
- ktime_set(0, 50 * NSEC_PER_USEC));
+ ktime_set(0, 150 * NSEC_PER_USEC));
}
spin_unlock_irqrestore(&musb->lock, flags);
@@ -282,9 +282,8 @@ static void cppi41_dma_callback(void *private_data)
&controller->early_tx_list);
if (!hrtimer_active(&controller->early_tx)) {
- unsigned long usecs = cppi41_channel->total_len / 10;
hrtimer_start_range_ns(&controller->early_tx,
- ktime_set(0, usecs * NSEC_PER_USEC),
+ ktime_set(0, 140 * NSEC_PER_USEC),
40 * NSEC_PER_USEC,
HRTIMER_MODE_REL);
}
--
2.1.1
---
drivers/usb/musb/musb_cppi41.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index d16b7561cce4..355f0dac9f0f 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -212,7 +212,7 @@ static enum hrtimer_restart cppi41_recheck_tx_req(struct hrtimer *timer)
if (!list_empty(&controller->early_tx_list)) {
ret = HRTIMER_RESTART;
hrtimer_forward_now(&controller->early_tx,
- ktime_set(0, 20 * NSEC_PER_USEC));
+ ktime_set(0, 150 * NSEC_PER_USEC));
}
spin_unlock_irqrestore(&musb->lock, flags);
@@ -293,14 +293,11 @@ static void cppi41_dma_callback(void *private_data)
}
list_add_tail(&cppi41_channel->tx_check,
&controller->early_tx_list);
- if (!hrtimer_is_queued(&controller->early_tx)) {
- unsigned long usecs = cppi41_channel->total_len / 10;
-
+ if (!hrtimer_is_queued(&controller->early_tx))
hrtimer_start_range_ns(&controller->early_tx,
- ktime_set(0, usecs * NSEC_PER_USEC),
+ ktime_set(0, 130 * NSEC_PER_USEC),
20 * NSEC_PER_USEC,
HRTIMER_MODE_REL);
- }
}
out:
spin_unlock_irqrestore(&musb->lock, flags);
--
2.1.1
--
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
