[PATCH] usb: musb: fix bug in musbhsdma programming

[PATCH] usb: musb: fix bug in musbhsdma programming

[Ajay Kumar Gupta]

Mode bit should be set based on function parameter "mode" of
configure_channel() function.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/musb/musbhsdma.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
index a23d005..d87c6c7 100644
--- a/drivers/usb/musb/musbhsdma.c
+++ b/drivers/usb/musb/musbhsdma.c
@@ -225,13 +225,14 @@ static void configure_channel(struct dma_channel *channel,
 			csr |= MUSB_HSDMA_BURSTMODE_INCR4;
 
 		csr |= (musb_channel->epnum << MUSB_HSDMA_ENDPOINT_SHIFT)
-			| MUSB_HSDMA_MODE1
 			| MUSB_HSDMA_ENABLE
 			| MUSB_HSDMA_IRQENABLE
 			| (musb_channel->transmit
 					? MUSB_HSDMA_TRANSMIT
 					: 0);
 
+		if (mode)
+			csr |= MUSB_HSDMA_MODE1;
 		/* address/count */
 		musb_write_hsdma_addr(mbase, bchannel, dma_addr);
 		musb_write_hsdma_count(mbase, bchannel, len);
-- 
1.5.6

Re: [PATCH] usb: musb: fix bug in musbhsdma programming

[Felipe Balbi]

On Thu, Jan 08, 2009 at 06:01:36PM +0530, Ajay Kumar Gupta wrote:
> Mode bit should be set based on function parameter "mode" of
> configure_channel() function.
> 
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>

Acked-by: Felipe Balbi <felipe.balbi@nokia.com>

> ---
>  drivers/usb/musb/musbhsdma.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
> index a23d005..d87c6c7 100644
> --- a/drivers/usb/musb/musbhsdma.c
> +++ b/drivers/usb/musb/musbhsdma.c
> @@ -225,13 +225,14 @@ static void configure_channel(struct dma_channel *channel,
>  			csr |= MUSB_HSDMA_BURSTMODE_INCR4;
>  
>  		csr |= (musb_channel->epnum << MUSB_HSDMA_ENDPOINT_SHIFT)
> -			| MUSB_HSDMA_MODE1
>  			| MUSB_HSDMA_ENABLE
>  			| MUSB_HSDMA_IRQENABLE
>  			| (musb_channel->transmit
>  					? MUSB_HSDMA_TRANSMIT
>  					: 0);
>  
> +		if (mode)
> +			csr |= MUSB_HSDMA_MODE1;
>  		/* address/count */
>  		musb_write_hsdma_addr(mbase, bchannel, dma_addr);
>  		musb_write_hsdma_count(mbase, bchannel, len);
> -- 
> 1.5.6

-- 
balbi

Re: [PATCH] usb: musb: fix bug in musbhsdma programming

[David Brownell]

On Friday 23 January 2009, Felipe Balbi wrote:
> On Thu, Jan 08, 2009 at 06:01:36PM +0530, Ajay Kumar Gupta wrote:
> > Mode bit should be set based on function parameter "mode" of
> > configure_channel() function.
> > 
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
> 
> Acked-by: Felipe Balbi <felipe.balbi@nokia.com>

NAK -- interpretation of the MODE parameter is correct
(though it's not clear to me that the callers set it right),
but this won't apply to current GIT since the code differs.

I don't know what this is *against* but it's not mainline
or the OMAP tree...

        csr |= (musb_channel->epnum << MUSB_HSDMA_ENDPOINT_SHIFT)
                | (1 << MUSB_HSDMA_ENABLE_SHIFT)
                | (1 << MUSB_HSDMA_IRQENABLE_SHIFT)
                | (musb_channel->transmit
                                ? (1 << MUSB_HSDMA_TRANSMIT_SHIFT)
                                : 0);

Ajay, if this needs to merge to mainline (e.g. so it can be
pulled down to omap-git and fix DMA there) please re-issue.

- Dave

> 
> > ---
> >  drivers/usb/musb/musbhsdma.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
> > index a23d005..d87c6c7 100644
> > --- a/drivers/usb/musb/musbhsdma.c
> > +++ b/drivers/usb/musb/musbhsdma.c
> > @@ -225,13 +225,14 @@ static void configure_channel(struct dma_channel *channel,
> >  			csr |= MUSB_HSDMA_BURSTMODE_INCR4;
> >  
> >  		csr |= (musb_channel->epnum << MUSB_HSDMA_ENDPOINT_SHIFT)
> > -			| MUSB_HSDMA_MODE1
> >  			| MUSB_HSDMA_ENABLE
> >  			| MUSB_HSDMA_IRQENABLE
> >  			| (musb_channel->transmit
> >  					? MUSB_HSDMA_TRANSMIT
> >  					: 0);
> >  
> > +		if (mode)
> > +			csr |= MUSB_HSDMA_MODE1;
> >  		/* address/count */
> >  		musb_write_hsdma_addr(mbase, bchannel, dma_addr);
> >  		musb_write_hsdma_count(mbase, bchannel, len);
> > -- 
> > 1.5.6
> 
> -- 
> balbi
> 
> 

Re: [PATCH] usb: musb: fix bug in musbhsdma programming

[Felipe Balbi]

On Fri, Jan 23, 2009 at 10:36:04AM -0800, David Brownell wrote:
> On Friday 23 January 2009, Felipe Balbi wrote:
> > On Thu, Jan 08, 2009 at 06:01:36PM +0530, Ajay Kumar Gupta wrote:
> > > Mode bit should be set based on function parameter "mode" of
> > > configure_channel() function.
> > > 
> > > Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
> > 
> > Acked-by: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> 
> NAK -- interpretation of the MODE parameter is correct
> (though it's not clear to me that the callers set it right),
> but this won't apply to current GIT since the code differs.

hmmm.. don't get what you mean. If mode parameter in channel_program()
is true, that means channel->desired_mode should be true and we're
trying to kick mode1 dma, isn't it ??

But true, should be re-issued

-- 
balbi
--
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

Re: [PATCH] usb: musb: fix bug in musbhsdma programming

[David Brownell]

On Friday 23 January 2009, Felipe Balbi wrote:
> 
> > NAK -- interpretation of the MODE parameter is correct
> > (though it's not clear to me that the callers set it right),
> > but this won't apply to current GIT since the code differs.
> 
> hmmm.. don't get what you mean. If mode parameter in channel_program()
> is true, that means channel->desired_mode should be true and we're
> trying to kick mode1 dma, isn't it ??

What I mean is:  you're working on "mode1 DMA"
(multiple packets per request, the only sane mode,
when it works) ... and I know you've had a hard time
getting it all to behave.  Ergo it's not clear to me
that Ajay's patch could be sufficient.  If it were,
then you've been spending a LOT of time on something
very simple ... ;)

The "desired_mode" thing is confusing too; it's
TX vs RX, unrelated to "mode1" or "mode0", but
that's a separate naming issue.

- Dave



> 
> But true, should be re-issued
> 


--
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

RE: [PATCH] usb: musb: fix bug in musbhsdma programming

[Gupta, Ajay Kumar]

> On Thu, Jan 08, 2009 at 06:01:36PM +0530, Ajay Kumar Gupta wrote:
> > Mode bit should be set based on function parameter "mode" of
> > configure_channel() function.
> >
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
>
> Acked-by: Felipe Balbi <felipe.balbi@nokia.com>

> NAK -- interpretation of the MODE parameter is correct
> (t hough it's not clear to me that the callers set it right),
> but this won't apply to current GIT since the code differs.

> I don't know what this is *against* but it's not mainline
> or the OMAP tree...

>        csr |= (musb_channel->epnum << MUSB_HSDMA_ENDPOINT_SHIFT)
>                | (1 << MUSB_HSDMA_ENABLE_SHIFT)
>                | (1 << MUSB_HSDMA_IRQENABLE_SHIFT)
>                | (musb_channel->transmit
>                                ? (1 << MUSB_HSDMA_TRANSMIT_SHIFT)
>                                : 0);

>Ajay, if this needs to merge to mainline (e.g. so it can be
>pulled down to omap-git and fix DMA there) please re-issue.

This patch is not for mainline. 
It is intended for MV's OMAP GIT to solve the bug introduced by [1] below
and is already applied at [2], the modified version of this patch. 

As [1] is not yet gone to mainline so this patch is also not required in mainline.

[1] usb: musb: musbhsdma cleanup
http://source.mvista.com/git/gitweb.cgi?p=linux-omap-2.6-historic.git;a=commitdiff;h=cd66af4a8d9ea5923e9e935a9d6fb7bded6e1655
[2]usb: musb: fix bug in musbhsdma programming
http://source.mvista.com/git/gitweb.cgi?p=linux-omap-2.6-historic.git;a=commitdiff;h=15f75b6226c2d3b82062bb721e7cb9a1d6f35efd

-Ajay
>- Dave

>
> > ---
> >  drivers/usb/musb/musbhsdma.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
> > index a23d005..d87c6c7 100644
> > --- a/drivers/usb/musb/musbhsdma.c
> > +++ b/drivers/usb/musb/musbhsdma.c
> > @@ -225,13 +225,14 @@ static void configure_channel(struct dma_channel *channel,
> >                     csr |= MUSB_HSDMA_BURSTMODE_INCR4;
> >
> >             csr |= (musb_channel->epnum << MUSB_HSDMA_ENDPOINT_SHIFT)
> > -                   | MUSB_HSDMA_MODE1
> >                     | MUSB_HSDMA_ENABLE
> >                     | MUSB_HSDMA_IRQENABLE
> >                     | (musb_channel->transmit
> >                                     ? MUSB_HSDMA_TRANSMIT
> >                                     : 0);
> >
> > +           if (mode)
> > +                   csr |= MUSB_HSDMA_MODE1;
> >             /* address/count */
> >             musb_write_hsdma_addr(mbase, bchannel, dma_addr);
> >             musb_write_hsdma_count(mbase, bchannel, len);
> > --
> > 1.5.6
>
> --
> balbi
>
>

Re: [PATCH] usb: musb: fix bug in musbhsdma programming

[David Brownell]

On Friday 23 January 2009, Gupta, Ajay Kumar wrote:
> This patch is not for mainline. 
> It is intended for MV's OMAP GIT

Ah, I see.  Thanks for clarifying.

However ... that's not MontaVista's GIT tree, exactly,
except in that they've hosted the OMAP community tree.
And since that tree is now obsolete, and that patch 
isn't in the main branch ... I'll just ignore this.  :)

- Dave


> to solve the bug introduced by [1] below 
> and is already applied at [2], the modified version of this patch. 
> 
> As [1] is not yet gone to mainline so this patch is also not required in mainline.


--
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