* [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
@ 2009-08-28 5:58 Ajay Kumar Gupta
2009-08-28 9:23 ` Sergei Shtylyov
2009-08-28 14:24 ` Sergei Shtylyov
0 siblings, 2 replies; 16+ messages in thread
From: Ajay Kumar Gupta @ 2009-08-28 5:58 UTC (permalink / raw)
To: linux-usb
Cc: linux-omap, felipe.balbi, david-b, sshtylyov, Ajay Kumar Gupta,
Swaminathan S, Babu Ravi
Isochronous Tx DMA is getting programmed but never getting started
for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
Fixing it by starting DMAs using musb_h_tx_dma_start().
Signed-off-by: Swaminathan S <swami.iyer@ti.com>
Signed-off-by: Babu Ravi <ravibabu@ti.com>
Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
drivers/usb/musb/musb_host.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index cf94511..e3ab40a 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
return;
} else if (usb_pipeisoc(pipe) && dma) {
if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
- offset, length))
+ offset, length)) {
+ if (is_cppi_enabled() || tusb_dma_omap())
+ musb_h_tx_dma_start(hw_ep);
return;
+ }
} else if (tx_csr & MUSB_TXCSR_DMAENAB) {
DBG(1, "not complete, but DMA enabled?\n");
return;
--
1.6.2.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 5:58 [PATCH] musb: fix ISOC Tx programming for CPPI DMAs Ajay Kumar Gupta
@ 2009-08-28 9:23 ` Sergei Shtylyov
2009-08-28 9:29 ` Sergei Shtylyov
2009-08-28 9:44 ` Gupta, Ajay Kumar
2009-08-28 14:24 ` Sergei Shtylyov
1 sibling, 2 replies; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 9:23 UTC (permalink / raw)
To: Ajay Kumar Gupta
Cc: linux-usb, linux-omap, felipe.balbi, david-b, Swaminathan S, Babu Ravi
Hello.
Ajay Kumar Gupta wrote:
> Isochronous Tx DMA is getting programmed but never getting started
> for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
>
That's not true.
> Fixing it by starting DMAs using musb_h_tx_dma_start().
>
> Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> Signed-off-by: Babu Ravi <ravibabu@ti.com>
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
>
NAK.
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index cf94511..e3ab40a 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
> return;
> } else if (usb_pipeisoc(pipe) && dma) {
> if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
> - offset, length))
> + offset, length)) {
> + if (is_cppi_enabled() || tusb_dma_omap())
> + musb_h_tx_dma_start(hw_ep);
> return;
>
It will have been already started by this moment -- from
musb_start_urb() which is enough . Otherwise it wouldn't work, and I've
made sure it *does* work.
WBR, Sergei
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 9:23 ` Sergei Shtylyov
@ 2009-08-28 9:29 ` Sergei Shtylyov
2009-08-28 9:44 ` Gupta, Ajay Kumar
1 sibling, 0 replies; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 9:29 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Ajay Kumar Gupta, linux-usb, linux-omap, felipe.balbi, david-b,
Swaminathan S, Babu Ravi
Hello, I wrote:
>> Isochronous Tx DMA is getting programmed but never getting started
>> for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
>
> That's not true.
Well, this is only true iff URB_ISO_ASAP flag is *not* set for an
URB. In this case, PIO is also not being started, so you should fix this
too.
>> Signed-off-by: Swaminathan S <swami.iyer@ti.com>
>> Signed-off-by: Babu Ravi <ravibabu@ti.com>
>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
>>
> Fixing it by starting DMAs using musb_h_tx_dma_start().
>
> NAK.
Still NAK...
WBR, Sergei
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 9:23 ` Sergei Shtylyov
2009-08-28 9:29 ` Sergei Shtylyov
@ 2009-08-28 9:44 ` Gupta, Ajay Kumar
2009-08-28 9:46 ` Gupta, Ajay Kumar
2009-08-28 10:02 ` Sergei Shtylyov
1 sibling, 2 replies; 16+ messages in thread
From: Gupta, Ajay Kumar @ 2009-08-28 9:44 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: linux-usb, linux-omap, felipe.balbi, david-b, Subbrathnam,
Swaminathan, B, Ravi
> -----Original Message-----
> From: Sergei Shtylyov [mailto:sshtylyov@ru.mvista.com]
> Sent: Friday, August 28, 2009 2:53 PM
> To: Gupta, Ajay Kumar
> Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org;
> felipe.balbi@nokia.com; david-b@pacbell.net; Subbrathnam, Swaminathan; B,
> Ravi
> Subject: Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
>
> Hello.
>
> Ajay Kumar Gupta wrote:
>
> > Isochronous Tx DMA is getting programmed but never getting started
> > for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
> >
>
> That's not true.
We have tested and it doesn't work with current driver. Have you tested it and was it working for you ?
>
> > Fixing it by starting DMAs using musb_h_tx_dma_start().
> >
> > Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> > Signed-off-by: Babu Ravi <ravibabu@ti.com>
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> >
>
> NAK.
>
> > diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> > index cf94511..e3ab40a 100644
> > --- a/drivers/usb/musb/musb_host.c
> > +++ b/drivers/usb/musb/musb_host.c
> > @@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
> > return;
> > } else if (usb_pipeisoc(pipe) && dma) {
> > if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
> > - offset, length))
> > + offset, length)) {
> > + if (is_cppi_enabled() || tusb_dma_omap())
> > + musb_h_tx_dma_start(hw_ep);
> > return;
> >
>
> It will have been already started by this moment -- from
> musb_start_urb() which is enough . Otherwise it wouldn't work, and I've
> made sure it *does* work.
This part is being done at musb_host_rx() doing next packet programming within same urb and *not* starting next urb. Thus musb_start_urb() doesn't come into this path. So it wouldn't start the DMAs.
In case of PIO, it does load the FIFO and sets the TXPKTREADY.
-Ajay
>
> WBR, Sergei
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 9:44 ` Gupta, Ajay Kumar
@ 2009-08-28 9:46 ` Gupta, Ajay Kumar
2009-08-28 10:02 ` Sergei Shtylyov
1 sibling, 0 replies; 16+ messages in thread
From: Gupta, Ajay Kumar @ 2009-08-28 9:46 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: linux-usb, linux-omap, felipe.balbi, david-b, Subbrathnam,
Swaminathan, B, Ravi
> This part is being done at musb_host_rx()
musb_host_tx() ofcourse.
> doing next packet programming
> within same urb and *not* starting next urb. Thus musb_start_urb() doesn't
> come into this path. So it wouldn't start the DMAs.
>
> In case of PIO, it does load the FIFO and sets the TXPKTREADY.
>
> -Ajay
>
> >
> > WBR, Sergei
> >
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 9:44 ` Gupta, Ajay Kumar
2009-08-28 9:46 ` Gupta, Ajay Kumar
@ 2009-08-28 10:02 ` Sergei Shtylyov
[not found] ` <4A97AB42.4070008-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
2009-08-28 10:28 ` Gupta, Ajay Kumar
1 sibling, 2 replies; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 10:02 UTC (permalink / raw)
To: Gupta, Ajay Kumar
Cc: linux-usb, linux-omap, felipe.balbi, david-b, Subbrathnam,
Swaminathan, B, Ravi
Hello.
Gupta, Ajay Kumar wrote:
>> -----Original Message-----
>> From: Sergei Shtylyov [mailto:sshtylyov@ru.mvista.com]
>> Sent: Friday, August 28, 2009 2:53 PM
>> To: Gupta, Ajay Kumar
>> Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org;
>> felipe.balbi@nokia.com; david-b@pacbell.net; Subbrathnam, Swaminathan; B,
>> Ravi
>> Subject: Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
>>
>> Hello.
>>
>> Ajay Kumar Gupta wrote:
>>
>>
>>> Isochronous Tx DMA is getting programmed but never getting started
>>> for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
>>>
>>>
>> That's not true.
>>
>
> We have tested and it doesn't work with current driver. Have you tested it and was it working for you ?
>
Not with the current driver, I must admit. I'll try it today...
>>> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
>>> index cf94511..e3ab40a 100644
>>> --- a/drivers/usb/musb/musb_host.c
>>> +++ b/drivers/usb/musb/musb_host.c
>>> @@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
>>> return;
>>> } else if (usb_pipeisoc(pipe) && dma) {
>>> if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
>>> - offset, length))
>>> + offset, length)) {
>>> + if (is_cppi_enabled() || tusb_dma_omap())
>>> + musb_h_tx_dma_start(hw_ep);
>>> return;
>>>
>>>
>> It will have been already started by this moment -- from
>> musb_start_urb() which is enough . Otherwise it wouldn't work, and I've
>> made sure it *does* work.
>>
>
> This part is being done at musb_host_rx()
You're doing it in musb_host_tx() actually. Although musb_host_rx()
is also broken WRT the isochronous transfers.
> doing next packet programming within same urb and *not* starting next urb. Thus musb_start_urb() doesn't come into this path.
What? Read the code, please -- musb_start_urb() call should always
precede musb_host_tx() which handles the DMA interrupt. Unless something
clears DMAReqEnab after musb_start_urb() call, setting it only once
should work.
> So it wouldn't start the DMAs.
>
How musb_host_tx() can be called without musb_start_urb()?
> In case of PIO, it does load the FIFO and sets the TXPKTREADY.
>
Only is URB_ISO_ASAP is not set.
WBR, Sergei
> -Ajay
WBR, Sergei
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
[not found] ` <4A97AB42.4070008-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
@ 2009-08-28 10:11 ` Subbrathnam, Swaminathan
[not found] ` <FCCFB4CDC6E5564B9182F639FC35608702F9A45083-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2009-08-28 10:12 ` Sergei Shtylyov
1 sibling, 1 reply; 16+ messages in thread
From: Subbrathnam, Swaminathan @ 2009-08-28 10:11 UTC (permalink / raw)
To: Sergei Shtylyov, Gupta, Ajay Kumar
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
felipe.balbi-xNZwKgViW5gAvxtiuMwx3w,
david-b-yBeKhBN/0LDR7s880joybQ, B, Ravi
Sergei,
Pl. do the required testing with and without the patch on the current tree for ISO transfers in Tx direction. As Ajay indicated we have done the same and found it not working and hence the fix.
ISO Rx is also broken and the patch for fixing the same is on the way. Since that fix involves some bit of re-structuring it is delayed a bit.
We can discuss further if need be.
Regards
swami
-----Original Message-----
From: Sergei Shtylyov [mailto:sshtylyov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org]
Sent: Friday, August 28, 2009 3:33 PM
To: Gupta, Ajay Kumar
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org; david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org; Subbrathnam, Swaminathan; B, Ravi
Subject: Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
Hello.
Gupta, Ajay Kumar wrote:
>> -----Original Message-----
>> From: Sergei Shtylyov [mailto:sshtylyov-hkdhdckH98+B+jHODAdFcQ@public.gmane.org]
>> Sent: Friday, August 28, 2009 2:53 PM
>> To: Gupta, Ajay Kumar
>> Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
>> felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org; david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org; Subbrathnam, Swaminathan; B,
>> Ravi
>> Subject: Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
>>
>> Hello.
>>
>> Ajay Kumar Gupta wrote:
>>
>>
>>> Isochronous Tx DMA is getting programmed but never getting started
>>> for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
>>>
>>>
>> That's not true.
>>
>
> We have tested and it doesn't work with current driver. Have you tested it and was it working for you ?
>
Not with the current driver, I must admit. I'll try it today...
>>> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
>>> index cf94511..e3ab40a 100644
>>> --- a/drivers/usb/musb/musb_host.c
>>> +++ b/drivers/usb/musb/musb_host.c
>>> @@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
>>> return;
>>> } else if (usb_pipeisoc(pipe) && dma) {
>>> if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
>>> - offset, length))
>>> + offset, length)) {
>>> + if (is_cppi_enabled() || tusb_dma_omap())
>>> + musb_h_tx_dma_start(hw_ep);
>>> return;
>>>
>>>
>> It will have been already started by this moment -- from
>> musb_start_urb() which is enough . Otherwise it wouldn't work, and I've
>> made sure it *does* work.
>>
>
> This part is being done at musb_host_rx()
You're doing it in musb_host_tx() actually. Although musb_host_rx()
is also broken WRT the isochronous transfers.
> doing next packet programming within same urb and *not* starting next urb. Thus musb_start_urb() doesn't come into this path.
What? Read the code, please -- musb_start_urb() call should always
precede musb_host_tx() which handles the DMA interrupt. Unless something
clears DMAReqEnab after musb_start_urb() call, setting it only once
should work.
> So it wouldn't start the DMAs.
>
How musb_host_tx() can be called without musb_start_urb()?
> In case of PIO, it does load the FIFO and sets the TXPKTREADY.
>
Only is URB_ISO_ASAP is not set.
WBR, Sergei
> -Ajay
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
[not found] ` <4A97AB42.4070008-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
2009-08-28 10:11 ` Subbrathnam, Swaminathan
@ 2009-08-28 10:12 ` Sergei Shtylyov
1 sibling, 0 replies; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 10:12 UTC (permalink / raw)
To: Gupta, Ajay Kumar
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
felipe.balbi-xNZwKgViW5gAvxtiuMwx3w,
david-b-yBeKhBN/0LDR7s880joybQ, Subbrathnam, Swaminathan, B,
Ravi
Hello, I wrote:
>> In case of PIO, it does load the FIFO and sets the TXPKTREADY.
>>
>
> Only is URB_ISO_ASAP is not set.
This should read "only if URB_ISO_ASAP is set". :-/
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 10:02 ` Sergei Shtylyov
[not found] ` <4A97AB42.4070008-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
@ 2009-08-28 10:28 ` Gupta, Ajay Kumar
2009-08-28 10:35 ` Gupta, Ajay Kumar
2009-08-28 10:46 ` Sergei Shtylyov
1 sibling, 2 replies; 16+ messages in thread
From: Gupta, Ajay Kumar @ 2009-08-28 10:28 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: linux-usb, linux-omap, felipe.balbi, david-b, Subbrathnam,
Swaminathan, B, Ravi
> > We have tested and it doesn't work with current driver. Have you tested
> it and was it working for you ?
> >
>
> Not with the current driver, I must admit. I'll try it today...
>
> >>> diff --git a/drivers/usb/musb/musb_host.c
> b/drivers/usb/musb/musb_host.c
> >>> index cf94511..e3ab40a 100644
> >>> --- a/drivers/usb/musb/musb_host.c
> >>> +++ b/drivers/usb/musb/musb_host.c
> >>> @@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
> >>> return;
> >>> } else if (usb_pipeisoc(pipe) && dma) {
> >>> if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
> >>> - offset, length))
> >>> + offset, length)) {
> >>> + if (is_cppi_enabled() || tusb_dma_omap())
> >>> + musb_h_tx_dma_start(hw_ep);
> >>> return;
> >>>
> >>>
> >> It will have been already started by this moment -- from
> >> musb_start_urb() which is enough . Otherwise it wouldn't work, and I've
> >> made sure it *does* work.
> >>
> >
> > This part is being done at musb_host_rx()
>
> You're doing it in musb_host_tx() actually. Although musb_host_rx()
> is also broken WRT the isochronous transfers.
>
> > doing next packet programming within same urb and *not* starting next
> urb. Thus musb_start_urb() doesn't come into this path.
>
> What? Read the code, please -- musb_start_urb() call should always
> precede musb_host_tx() which handles the DMA interrupt. Unless something
> clears DMAReqEnab after musb_start_urb() call, setting it only once
> should work.
musb_start_urb() call does precede musb_host_tx() but when urb is *completed*.
check the 'done' flag and musb_advance_schedule getting called in the path.
if (done) {
/* set status */
urb->status = status;
urb->actual_length = qh->offset;
musb_advance_schedule(musb, urb, hw_ep, USB_DIR_OUT);
return;
} else if (usb_pipeisoc(pipe) && dma) {
if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
offset, length)) {
if (is_cppi_enabled() || tusb_dma_omap()
|| is_cppi41_enabled())
musb_h_tx_dma_start(hw_ep);
return;
}
>
> > So it wouldn't start the DMAs.
> >
>
> How musb_host_tx() can be called without musb_start_urb()?
>
> > In case of PIO, it does load the FIFO and sets the TXPKTREADY.
> >
>
> Only is URB_ISO_ASAP is not set.
>
> WBR, Sergei
>
> > -Ajay
>
> WBR, Sergei
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 10:28 ` Gupta, Ajay Kumar
@ 2009-08-28 10:35 ` Gupta, Ajay Kumar
2009-08-28 12:16 ` Sergei Shtylyov
2009-08-28 10:46 ` Sergei Shtylyov
1 sibling, 1 reply; 16+ messages in thread
From: Gupta, Ajay Kumar @ 2009-08-28 10:35 UTC (permalink / raw)
To: Gupta, Ajay Kumar, Sergei Shtylyov
Cc: linux-usb, linux-omap, felipe.balbi, david-b, Subbrathnam,
Swaminathan, B, Ravi
> > > This part is being done at musb_host_rx()
> >
> > You're doing it in musb_host_tx() actually. Although musb_host_rx()
> > is also broken WRT the isochronous transfers.
> >
> > > doing next packet programming within same urb and *not* starting next
> > urb. Thus musb_start_urb() doesn't come into this path.
> >
> > What? Read the code, please -- musb_start_urb() call should always
> > precede musb_host_tx() which handles the DMA interrupt. Unless something
> > clears DMAReqEnab after musb_start_urb() call, setting it only once
> > should work.
>
> musb_start_urb() call does precede musb_host_tx() but when urb is
> *completed*.
I think you are aware that there are multiple packets within same isochronous urb and musb_start_urb() programs only for first packet.
============================================================
case USB_ENDPOINT_XFER_ISOC:
qh->iso_idx = 0;
qh->frame = 0;
offset = urb->iso_frame_desc[0].offset;
len = urb->iso_frame_desc[0].length;
============================================================
-Ajay
>
> check the 'done' flag and musb_advance_schedule getting called in the
> path.
>
> if (done) {
> /* set status */
> urb->status = status;
> urb->actual_length = qh->offset;
> musb_advance_schedule(musb, urb, hw_ep, USB_DIR_OUT);
> return;
> } else if (usb_pipeisoc(pipe) && dma) {
> if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
> offset, length)) {
> if (is_cppi_enabled() || tusb_dma_omap()
> || is_cppi41_enabled())
> musb_h_tx_dma_start(hw_ep);
> return;
> }
>
>
> >
> > > So it wouldn't start the DMAs.
> > >
> >
> > How musb_host_tx() can be called without musb_start_urb()?
> >
> > > In case of PIO, it does load the FIFO and sets the TXPKTREADY.
> > >
> >
> > Only is URB_ISO_ASAP is not set.
> >
> > WBR, Sergei
> >
> > > -Ajay
> >
> > WBR, Sergei
> >
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 10:28 ` Gupta, Ajay Kumar
2009-08-28 10:35 ` Gupta, Ajay Kumar
@ 2009-08-28 10:46 ` Sergei Shtylyov
1 sibling, 0 replies; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 10:46 UTC (permalink / raw)
To: Gupta, Ajay Kumar
Cc: linux-usb, linux-omap, felipe.balbi, david-b, Subbrathnam,
Swaminathan, B, Ravi
Hello.
Gupta, Ajay Kumar wrote:
>>>>> diff --git a/drivers/usb/musb/musb_host.c
>>>>>
>> b/drivers/usb/musb/musb_host.c
>>
>>>>> index cf94511..e3ab40a 100644
>>>>> --- a/drivers/usb/musb/musb_host.c
>>>>> +++ b/drivers/usb/musb/musb_host.c
>>>>> @@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
>>>>> return;
>>>>> } else if (usb_pipeisoc(pipe) && dma) {
>>>>> if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
>>>>> - offset, length))
>>>>> + offset, length)) {
>>>>> + if (is_cppi_enabled() || tusb_dma_omap())
>>>>> + musb_h_tx_dma_start(hw_ep);
>>>>> return;
>>>>>
>>>>>
>>>>>
>>>> It will have been already started by this moment -- from
>>>> musb_start_urb() which is enough . Otherwise it wouldn't work, and I've
>>>> made sure it *does* work.
>>>>
>>>>
>>> This part is being done at musb_host_rx()
>>>
>> You're doing it in musb_host_tx() actually. Although musb_host_rx()
>> is also broken WRT the isochronous transfers.
>>
>>
>>> doing next packet programming within same urb and *not* starting next
>>>
>> urb. Thus musb_start_urb() doesn't come into this path.
>>
>> What? Read the code, please -- musb_start_urb() call should always
>> precede musb_host_tx() which handles the DMA interrupt. Unless something
>> clears DMAReqEnab after musb_start_urb() call, setting it only once
>> should work.
>>
>
> musb_start_urb() call does precede musb_host_tx() but when urb is *completed*.
>
> check the 'done' flag and musb_advance_schedule getting called in the path.
>
> if (done) {
> /* set status */
> urb->status = status;
> urb->actual_length = qh->offset;
> musb_advance_schedule(musb, urb, hw_ep, USB_DIR_OUT);
> return;
> } else if (usb_pipeisoc(pipe) && dma) {
> if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
> offset, length)) {
> if (is_cppi_enabled() || tusb_dma_omap()
> || is_cppi41_enabled())
> musb_h_tx_dma_start(hw_ep);
> return;
> }
>
Sigh, that will be musb_start_urb() for the *next* URB after a
completed one... Someone must have called it for the *current* URB when
starting an ISO transfer.
This call to musb_tx_dma_program() is only done for the 2nd and
subsequent data fragments of an URB -- the call for the 1st fragment
happens elsewhere, from musb_ep_program().
There are basically two Tx URB starting paths within the driver:
1) musb_urb_enqueue() -> musb_schedule() -> musb_start_urb() ->
musb_h_tx_dma_start();
2) musb_host_tx() -> musb_advance_schedule() -> musb_start_urb() ->
musb_h_tx_dma_start().
Transfer of the 1st fragment is started on either of these patch,
depending on whether there was URBs already queued at the time of
submitting the new URB; after that DMAReqMode/DMAReqEnab bits should
remain set after the first musb_h_tx_dma_start() call, so that calling
musb_tx_dma_program() should be enough for the subsequent fragments. So
your statement that "Isochronous Tx DMA is getting programmed but never
getting started for CPPI and TUSB DMAs" is an overstatement in any case
-- first fragment must be properly started.
WBR, Sergei
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 10:35 ` Gupta, Ajay Kumar
@ 2009-08-28 12:16 ` Sergei Shtylyov
[not found] ` <4A97CA80.7010600-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
0 siblings, 1 reply; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 12:16 UTC (permalink / raw)
To: Gupta, Ajay Kumar
Cc: linux-usb, linux-omap, felipe.balbi, david-b, Subbrathnam,
Swaminathan, B, Ravi
Gupta, Ajay Kumar wrote:
>>>>This part is being done at musb_host_rx()
>>> You're doing it in musb_host_tx() actually. Although musb_host_rx()
>>>is also broken WRT the isochronous transfers.
>>>> doing next packet programming within same urb and *not* starting next
>>>urb. Thus musb_start_urb() doesn't come into this path.
>>> What? Read the code, please -- musb_start_urb() call should always
>>>precede musb_host_tx() which handles the DMA interrupt. Unless something
>>>clears DMAReqEnab after musb_start_urb() call, setting it only once
>>>should work.
>>musb_start_urb() call does precede musb_host_tx() but when urb is
>>*completed*.
> I think you are aware that there are multiple packets within same isochronous urb and musb_start_urb() programs only for first packet.
>
> ============================================================
> case USB_ENDPOINT_XFER_ISOC:
> qh->iso_idx = 0;
> qh->frame = 0;
> offset = urb->iso_frame_desc[0].offset;
> len = urb->iso_frame_desc[0].length;
> ============================================================
Sure. What I'm still not aware of is where and how the TXCSR DMA bits
are cleared after the first fragment. Hopefully, testing will reveal this...
WBR, Sergei
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
[not found] ` <4A97CA80.7010600-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
@ 2009-08-28 13:06 ` Sergei Shtylyov
2009-08-28 13:23 ` Sergei Shtylyov
0 siblings, 1 reply; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 13:06 UTC (permalink / raw)
To: Gupta, Ajay Kumar
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
felipe.balbi-xNZwKgViW5gAvxtiuMwx3w,
david-b-yBeKhBN/0LDR7s880joybQ, Subbrathnam, Swaminathan, B,
Ravi
Hello, I wrote:
>>>>> This part is being done at musb_host_rx()
>>>> You're doing it in musb_host_tx() actually. Although musb_host_rx()
>>>> is also broken WRT the isochronous transfers.
>>>>> doing next packet programming within same urb and *not* starting next
>>>> urb. Thus musb_start_urb() doesn't come into this path.
>>>> What? Read the code, please -- musb_start_urb() call should always
>>>> precede musb_host_tx() which handles the DMA interrupt. Unless
>>>> something
>>>> clears DMAReqEnab after musb_start_urb() call, setting it only once
>>>> should work.
>>> musb_start_urb() call does precede musb_host_tx() but when urb is
>>> *completed*.
>> I think you are aware that there are multiple packets within same
>> isochronous urb and musb_start_urb() programs only for first packet.
>> ============================================================
>> case USB_ENDPOINT_XFER_ISOC:
>> qh->iso_idx = 0;
>> qh->frame = 0;
>> offset = urb->iso_frame_desc[0].offset;
>> len = urb->iso_frame_desc[0].length;
>> ============================================================
> Sure. What I'm still not aware of is where and how the TXCSR DMA bits
> are cleared after the first fragment. Hopefully, testing will reveal
> this...
I really should have stared at the code a bit more myself. Now that I
have the sad truth has dawned on me... :-/
My patch "USB: musb: bugfixes for multi-packet TXDMA support" (commit
c7bbc056a92476b3b3d70a8df7cc746ac5d56de7) inevitably causes the DMA bits to
be cleared because we're using DMA mode 1 regardless of whether this is
isochronous transfer or no. (We could really use mode 0 for isochronous
transfer and save the hassle when filtering out DMA completion interrupt in
musb_host_tx().)
So, I now have to ACK your patch (which could use a better description
though) and strew my head with ashes. All this also must mean that I have
managed to break ISO Tx DMA in the internal tree, where I added the
abovementioned patch after the one that fixed it (the order of the patches
in the community tree is reverse). I probably did not retest USB audio back
then...
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 13:06 ` Sergei Shtylyov
@ 2009-08-28 13:23 ` Sergei Shtylyov
0 siblings, 0 replies; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 13:23 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Gupta, Ajay Kumar, linux-usb, linux-omap, felipe.balbi, david-b,
Subbrathnam, Swaminathan, B, Ravi
Hello, I wrote:
>>>>> You're doing it in musb_host_tx() actually. Although musb_host_rx()
>>>>> is also broken WRT the isochronous transfers.
>>>>>> doing next packet programming within same urb and *not* starting next
>>>>> urb. Thus musb_start_urb() doesn't come into this path.
>>>>> What? Read the code, please -- musb_start_urb() call should always
>>>>> precede musb_host_tx() which handles the DMA interrupt. Unless
>>>>> something
>>>>> clears DMAReqEnab after musb_start_urb() call, setting it only once
>>>>> should work.
>>>> musb_start_urb() call does precede musb_host_tx() but when urb is
>>>> *completed*.
>>> I think you are aware that there are multiple packets within same
>>> isochronous urb and musb_start_urb() programs only for first packet.
>>> ============================================================
>>> case USB_ENDPOINT_XFER_ISOC:
>>> qh->iso_idx = 0;
>>> qh->frame = 0;
>>> offset = urb->iso_frame_desc[0].offset;
>>> len = urb->iso_frame_desc[0].length;
>>> ============================================================
>> Sure. What I'm still not aware of is where and how the TXCSR DMA
>> bits are cleared after the first fragment. Hopefully, testing will
>> reveal this...
> I really should have stared at the code a bit more myself. Now that I
> have the sad truth has dawned on me... :-/
> My patch "USB: musb: bugfixes for multi-packet TXDMA support" (commit
> c7bbc056a92476b3b3d70a8df7cc746ac5d56de7) inevitably causes the DMA bits
You can also refer to this patch'es brokenness in your patch description.
Looks like I now have the complete picture of what happened back then when
the patches were (re)submitted. The DaVinci case was competely broken when I
recast the patch "USB: musb: fix isochronous TXDMA (take 2)" to fix the
failure to transfer the whole ISO URB in DMA mode for the Mentor's own DMA
case; before that, DaVinci case could (likewise) complete the URB transfer
in PIO mode after transferring the first fragment via DMA...
> So, I now have to ACK your patch (which could use a better description though) and strew my head with ashes. All this also must mean that I have managed to break ISO Tx DMA in the internal tree, where I added the abovementioned patch after the one that fixed it (the order of the patches in the community tree is reverse). I probably did not retest USB audio back then...
No, looks like in the internal tree transfer is completed in PIO mode
since the internal version of patch lacks that Mentor DMA fix (luckily :-).
Should still check to make sure...
WBR, Sergei
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
[not found] ` <FCCFB4CDC6E5564B9182F639FC35608702F9A45083-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
@ 2009-08-28 13:27 ` Sergei Shtylyov
0 siblings, 0 replies; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 13:27 UTC (permalink / raw)
To: Subbrathnam, Swaminathan
Cc: Gupta, Ajay Kumar, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
felipe.balbi-xNZwKgViW5gAvxtiuMwx3w,
david-b-yBeKhBN/0LDR7s880joybQ, B, Ravi
Hello.
Subbrathnam, Swaminathan wrote:
> Sergei,
> Pl. do the required testing with and without the patch on the current tree for ISO transfers in Tx direction. As Ajay indicated we have done the same and found it not working and hence the fix.
Sigh, I'm now seeing it even witout testing... So, I'm sorry for all the
noise. It was a result of my 2 patches clashing.
> ISO Rx is also broken and the patch for fixing the same is on the way.
That's good to hear... musb_host_rx() was further broken WRT ISO
trabnsers while the Mentor DMA case was being fixed.
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] musb: fix ISOC Tx programming for CPPI DMAs
2009-08-28 5:58 [PATCH] musb: fix ISOC Tx programming for CPPI DMAs Ajay Kumar Gupta
2009-08-28 9:23 ` Sergei Shtylyov
@ 2009-08-28 14:24 ` Sergei Shtylyov
1 sibling, 0 replies; 16+ messages in thread
From: Sergei Shtylyov @ 2009-08-28 14:24 UTC (permalink / raw)
To: Ajay Kumar Gupta
Cc: linux-usb, linux-omap, felipe.balbi, david-b, Swaminathan S, Babu Ravi
Ajay Kumar Gupta wrote:
> Isochronous Tx DMA is getting programmed but never getting started
> for CPPI and TUSB DMAs and thus Isochronous Tx doesn't work.
> Fixing it by starting DMAs using musb_h_tx_dma_start().
> Signed-off-by: Swaminathan S <swami.iyer@ti.com>
> Signed-off-by: Babu Ravi <ravibabu@ti.com>
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index cf94511..e3ab40a 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -1301,8 +1301,11 @@ void musb_host_tx(struct musb *musb, u8 epnum)
> return;
> } else if (usb_pipeisoc(pipe) && dma) {
> if (musb_tx_dma_program(musb->dma_controller, hw_ep, qh, urb,
> - offset, length))
> + offset, length)) {
> + if (is_cppi_enabled() || tusb_dma_omap())
The latter check shouldn't really be needed, as in the TUSB DMA mode
only DMA mode 0 is used, in which case the DMA interrupt filtering code
above in this function shouldn't clear the TXCSR.DMAReEnab bit.
> + musb_h_tx_dma_start(hw_ep);
> return;
> + }
> } else if (tx_csr & MUSB_TXCSR_DMAENAB) {
> DBG(1, "not complete, but DMA enabled?\n");
> return;
WBR, Sergei
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2009-08-28 14:22 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-28 5:58 [PATCH] musb: fix ISOC Tx programming for CPPI DMAs Ajay Kumar Gupta
2009-08-28 9:23 ` Sergei Shtylyov
2009-08-28 9:29 ` Sergei Shtylyov
2009-08-28 9:44 ` Gupta, Ajay Kumar
2009-08-28 9:46 ` Gupta, Ajay Kumar
2009-08-28 10:02 ` Sergei Shtylyov
[not found] ` <4A97AB42.4070008-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
2009-08-28 10:11 ` Subbrathnam, Swaminathan
[not found] ` <FCCFB4CDC6E5564B9182F639FC35608702F9A45083-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2009-08-28 13:27 ` Sergei Shtylyov
2009-08-28 10:12 ` Sergei Shtylyov
2009-08-28 10:28 ` Gupta, Ajay Kumar
2009-08-28 10:35 ` Gupta, Ajay Kumar
2009-08-28 12:16 ` Sergei Shtylyov
[not found] ` <4A97CA80.7010600-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
2009-08-28 13:06 ` Sergei Shtylyov
2009-08-28 13:23 ` Sergei Shtylyov
2009-08-28 10:46 ` Sergei Shtylyov
2009-08-28 14:24 ` Sergei Shtylyov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.