Re: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed channel length
On 06/19/2014 05:53 PM, David Laight wrote: > From: Daniel Mack [mailto:dan...@zonque.org] >> On 06/19/2014 05:07 PM, Felipe Balbi wrote: >>> On Wed, Jun 18, 2014 at 11:36:43AM +0200, Daniel Mack wrote: On 06/18/2014 11:32 AM, David Laight wrote: > You can't really mean nanoseconds? Microseconds of course. Thanks for the heads-up. Fixed locally. >>> >>> Are you sending another one or want me to fix the commit log when >>> applying here ? >> >> I was actually waiting for feedback on the patch before resending >> another version. In particular, I'd like to know whether George's test >> case still works with this patch applied. >> >> If nobody has objections, you fixing the commit log would of course be >> easiest, yes :) > > IIRC there was a reference to nanoseconds inside the patch as well. > Possibly a variable name. Indeed. Nevermind, I'll send another version soon. Thanks! Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed channel length
From: Daniel Mack [mailto:dan...@zonque.org] > On 06/19/2014 05:07 PM, Felipe Balbi wrote: > > On Wed, Jun 18, 2014 at 11:36:43AM +0200, Daniel Mack wrote: > >> On 06/18/2014 11:32 AM, David Laight wrote: > >>> From: Of Daniel Mack > >>>> Sent: 18 June 2014 10:28 > >>>> To: ba...@ti.com; george.cher...@ti.com; bige...@linutronix.de > >>>> Cc: sebastian.reim...@googlemail.com; linux-usb@vger.kernel.org; Daniel > >>>> Mack > >>>> Subject: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to > >>>> programmed channel length > >>>> > >>>> The musb/cppi41 code installs a hrtimer to work around DMA completion > >>>> interrupts that have fired too early on AM335x hardware. This timer > >>>> is currently programmed to first fire 140 nanoseconds after the DMA > >>>> completion callback. According to the commit which introduced it > >>>> (a655f481d83, "usb: musb: musb_cppi41: handle pre-mature TX complete > >>>> interrupt"), that value is is considered a 'rule of thumb' that worked > >>>> well with the test case described in the commit log. > >>>> > >>>> Test show, however, that for USB audio devices and much smaller packet > >>>> sizes, the timer has to fire earlier in order to correctly handle the > >>>> audio > >>>> stream. The original test case had output transfer sizes of 1514 bytes, > >>>> and > >>>> a delay of 140 nanoseconds. For audio devices with 24 bytes channel > >>>> size, 3 > >>>> nanoseconds seem to work well. > >>> > >>> You can't really mean nanoseconds? > >> > >> Microseconds of course. Thanks for the heads-up. Fixed locally. > > > > Are you sending another one or want me to fix the commit log when > > applying here ? > > I was actually waiting for feedback on the patch before resending > another version. In particular, I'd like to know whether George's test > case still works with this patch applied. > > If nobody has objections, you fixing the commit log would of course be > easiest, yes :) IIRC there was a reference to nanoseconds inside the patch as well. Possibly a variable name. David
Re: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed channel length
On 06/19/2014 05:07 PM, Felipe Balbi wrote: > On Wed, Jun 18, 2014 at 11:36:43AM +0200, Daniel Mack wrote: >> On 06/18/2014 11:32 AM, David Laight wrote: >>> From: Of Daniel Mack >>>> Sent: 18 June 2014 10:28 >>>> To: ba...@ti.com; george.cher...@ti.com; bige...@linutronix.de >>>> Cc: sebastian.reim...@googlemail.com; linux-usb@vger.kernel.org; Daniel >>>> Mack >>>> Subject: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to >>>> programmed channel length >>>> >>>> The musb/cppi41 code installs a hrtimer to work around DMA completion >>>> interrupts that have fired too early on AM335x hardware. This timer >>>> is currently programmed to first fire 140 nanoseconds after the DMA >>>> completion callback. According to the commit which introduced it >>>> (a655f481d83, "usb: musb: musb_cppi41: handle pre-mature TX complete >>>> interrupt"), that value is is considered a 'rule of thumb' that worked >>>> well with the test case described in the commit log. >>>> >>>> Test show, however, that for USB audio devices and much smaller packet >>>> sizes, the timer has to fire earlier in order to correctly handle the audio >>>> stream. The original test case had output transfer sizes of 1514 bytes, and >>>> a delay of 140 nanoseconds. For audio devices with 24 bytes channel size, 3 >>>> nanoseconds seem to work well. >>> >>> You can't really mean nanoseconds? >> >> Microseconds of course. Thanks for the heads-up. Fixed locally. > > Are you sending another one or want me to fix the commit log when > applying here ? I was actually waiting for feedback on the patch before resending another version. In particular, I'd like to know whether George's test case still works with this patch applied. If nobody has objections, you fixing the commit log would of course be easiest, yes :) Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed channel length
On Wed, Jun 18, 2014 at 11:36:43AM +0200, Daniel Mack wrote: > On 06/18/2014 11:32 AM, David Laight wrote: > > From: Of Daniel Mack > >> Sent: 18 June 2014 10:28 > >> To: ba...@ti.com; george.cher...@ti.com; bige...@linutronix.de > >> Cc: sebastian.reim...@googlemail.com; linux-usb@vger.kernel.org; Daniel > >> Mack > >> Subject: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to > >> programmed channel length > >> > >> The musb/cppi41 code installs a hrtimer to work around DMA completion > >> interrupts that have fired too early on AM335x hardware. This timer > >> is currently programmed to first fire 140 nanoseconds after the DMA > >> completion callback. According to the commit which introduced it > >> (a655f481d83, "usb: musb: musb_cppi41: handle pre-mature TX complete > >> interrupt"), that value is is considered a 'rule of thumb' that worked > >> well with the test case described in the commit log. > >> > >> Test show, however, that for USB audio devices and much smaller packet > >> sizes, the timer has to fire earlier in order to correctly handle the audio > >> stream. The original test case had output transfer sizes of 1514 bytes, and > >> a delay of 140 nanoseconds. For audio devices with 24 bytes channel size, 3 > >> nanoseconds seem to work well. > > > > You can't really mean nanoseconds? > > Microseconds of course. Thanks for the heads-up. Fixed locally. Are you sending another one or want me to fix the commit log when applying here ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed channel length
On 06/18/2014 11:32 AM, David Laight wrote: > From: Of Daniel Mack >> Sent: 18 June 2014 10:28 >> To: ba...@ti.com; george.cher...@ti.com; bige...@linutronix.de >> Cc: sebastian.reim...@googlemail.com; linux-usb@vger.kernel.org; Daniel Mack >> Subject: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed >> channel length >> >> The musb/cppi41 code installs a hrtimer to work around DMA completion >> interrupts that have fired too early on AM335x hardware. This timer >> is currently programmed to first fire 140 nanoseconds after the DMA >> completion callback. According to the commit which introduced it >> (a655f481d83, "usb: musb: musb_cppi41: handle pre-mature TX complete >> interrupt"), that value is is considered a 'rule of thumb' that worked >> well with the test case described in the commit log. >> >> Test show, however, that for USB audio devices and much smaller packet >> sizes, the timer has to fire earlier in order to correctly handle the audio >> stream. The original test case had output transfer sizes of 1514 bytes, and >> a delay of 140 nanoseconds. For audio devices with 24 bytes channel size, 3 >> nanoseconds seem to work well. > > You can't really mean nanoseconds? Microseconds of course. Thanks for the heads-up. Fixed locally. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed channel length
From: Of Daniel Mack > Sent: 18 June 2014 10:28 > To: ba...@ti.com; george.cher...@ti.com; bige...@linutronix.de > Cc: sebastian.reim...@googlemail.com; linux-usb@vger.kernel.org; Daniel Mack > Subject: [PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed > channel length > > The musb/cppi41 code installs a hrtimer to work around DMA completion > interrupts that have fired too early on AM335x hardware. This timer > is currently programmed to first fire 140 nanoseconds after the DMA > completion callback. According to the commit which introduced it > (a655f481d83, "usb: musb: musb_cppi41: handle pre-mature TX complete > interrupt"), that value is is considered a 'rule of thumb' that worked > well with the test case described in the commit log. > > Test show, however, that for USB audio devices and much smaller packet > sizes, the timer has to fire earlier in order to correctly handle the audio > stream. The original test case had output transfer sizes of 1514 bytes, and > a delay of 140 nanoseconds. For audio devices with 24 bytes channel size, 3 > nanoseconds seem to work well. You can't really mean nanoseconds? David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: musb: cppi41: fire hrtimer according to programmed channel length
The musb/cppi41 code installs a hrtimer to work around DMA completion
interrupts that have fired too early on AM335x hardware. This timer
is currently programmed to first fire 140 nanoseconds after the DMA
completion callback. According to the commit which introduced it
(a655f481d83, "usb: musb: musb_cppi41: handle pre-mature TX complete
interrupt"), that value is is considered a 'rule of thumb' that worked
well with the test case described in the commit log.
Test show, however, that for USB audio devices and much smaller packet
sizes, the timer has to fire earlier in order to correctly handle the audio
stream. The original test case had output transfer sizes of 1514 bytes, and
a delay of 140 nanoseconds. For audio devices with 24 bytes channel size, 3
nanoseconds seem to work well.
Hence, let's assume that the time it takes to clear the bit correlates with
the number of bytes transferred. The referenced commit log mentions such a
suspicion as well. Let the timer fire in cppi41_channel->total_len/10
nanoseconds to correctly handle both cases.
Also, shorten the interval in which the timer fires again in case of
a non-empty early_tx list.
With these changes in place, both FS and HS audio devices appear to work
well on AM335x hardware.
Signed-off-by: Daniel Mack
Reported-by: Sebastian Reimers
---
drivers/usb/musb/musb_cppi41.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index a2c4456..ce918eb 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -200,7 +200,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, 150 * NSEC_PER_USEC));
+ ktime_set(0, 50 * NSEC_PER_USEC));
}
spin_unlock_irqrestore(&musb->lock, flags);
@@ -274,8 +274,10 @@ 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)) {
+ unsigned long nsecs = cppi41_channel->total_len / 10;
+
hrtimer_start_range_ns(&controller->early_tx,
- ktime_set(0, 140 * NSEC_PER_USEC),
+ ktime_set(0, nsecs * NSEC_PER_USEC),
40 * NSEC_PER_USEC,
HRTIMER_MODE_REL);
}
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
