[PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[Bryan Wu]

Hi folks,

We discussed these 4 bugs before. Now we fixed them.
Please review following 2 patches.

Thanks
-Bryan

[PATCH 1/2] MUSB: fix bug - since 2.6.24 urb API changed, giveback function return wrong status

[Bryan Wu]

When MUSB got STALL, but reported "short read trasfer"
usb-storage: Command MODE_SENSE (6 bytes)
usb-storage:  1a 00 3f 00 c0 00
usb-storage: Bulk Command S 0x43425355 T 0x18 L 192 F 128 Trg 0 LUN 0 CL 6
usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
usb-storage: Status code 0; transferred 31/31
usb-storage: -- transfer complete
usb-storage: Bulk command transfer result=0
usb-storage: usb_stor_bulk_transfer_sglist: xfer 192 bytes, 1 entries
usb-storage: Status code -121; transferred 0/192
usb-storage: -- short read transfer
usb-storage: Bulk data transfer result 0x1
usb-storage: Attempting to get CSW...
usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
usb-storage: Status code 0; transferred 0/13
usb-storage: -- short transfer
usb-storage: Received 0-length CSW; retrying...
usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
usb-storage: Status code 0; transferred 0/13
usb-storage: -- short transfer
usb-storage: Bulk status result = 1
usb-storage: -- transport indicates error, resetting
usb-storage: storage_pre_reset


The right message is:

usb-storage: Command MODE_SENSE (6 bytes)
usb-storage:  1a 00 3f 00 c0 00
usb-storage: Bulk Command S 0x43425355 T 0x6 L 192 F 128 Trg 0 LUN 0 CL 6
usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
usb-storage: Status code 0; transferred 31/31
usb-storage: -- transfer complete
usb-storage: Bulk command transfer result=0
usb-storage: usb_stor_bulk_transfer_sglist: xfer 192 bytes, 1 entries
usb-storage: Status code -121; transferred 4/192
usb-storage: -- short read transfer
usb-storage: Bulk data transfer result 0x1
usb-storage: Attempting to get CSW...
usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
usb-storage: Status code -32; transferred 0/13
usb-storage: clearing endpoint halt for pipe 0xc0010280
usb-storage: usb_stor_control_msg: rq=01 rqtype=02 value=0000 index=82 len=0
usb-storage: usb_stor_clear_halt: result = 0
usb-storage: Attempting to get CSW (2nd try)...
usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
usb-storage: Status code 0; transferred 13/13
usb-storage: -- transfer complete

Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
 drivers/usb/musb/musb_host.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 3d4fc3e..535922d 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -422,7 +422,11 @@ musb_advance_schedule(struct musb *musb, struct urb *urb,
 		qh = hw_ep->in_qh;
 	else
 		qh = hw_ep->out_qh;
-	qh = musb_giveback(qh, urb, 0);
+
+	if (urb->status == -EINPROGRESS)
+		qh = musb_giveback(qh, urb, 0);
+	else
+		qh = musb_giveback(qh, urb, urb->status);
 
 	if (qh && qh->is_ready && !list_empty(&qh->hep->urb_list)) {
 		DBG(4, "... next ep%d %cX urb %p\n",
-- 
1.5.5

[PATCH 2/2] MUSB: Real fix bug 4 usb bugs found on Blackfin

[Bryan Wu]

 - USB-IDE
   http://blackfin.uclinux.org/gf/project/linux-kernel/tracker/?action=TrackerItemEdit&tracker_item_id=3789
 - 30G iPod
   http://blackfin.uclinux.org/gf/project/linux-kernel/tracker/?action=TrackerItemEdit&tracker_item_id=3806
 - USB to CF/SD card reader
   http://blackfin.uclinux.org/gf/project/linux-kernel/tracker/?action=TrackerItemEdit&tracker_item_id=3790
   http://blackfin.uclinux.org/gf/project/linux-kernel/tracker/?action=TrackerItemEdit&tracker_item_id=3807

Sonic found, when we got STALL, we should clear the toggle.

Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
Signed-off-by: Bryan Wu <cooloney@kernel.org>
---
 drivers/usb/musb/musb_host.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 535922d..806c479 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1473,7 +1473,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
 			(void) musb->dma_controller->channel_abort(dma);
 			xfer_len = dma->actual_len;
 		}
-		musb_h_flush_rxfifo(hw_ep, 0);
+		musb_h_flush_rxfifo(hw_ep, MUSB_RXCSR_CLRDATATOG);
 		musb_writeb(epio, MUSB_RXINTERVAL, 0);
 		done = true;
 		goto finish;
-- 
1.5.5

RE: [PATCH 2/2] MUSB: Real fix bug 4 usb bugs found on Blackfin

[Gadiyar, Anand]

Hi,

I am unable to visit any of these links. I get the following error "Unknown or invalid plugin: 'tracker'".

Regards,
Anand

> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org
> [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of Bryan Wu
> Sent: Saturday, May 17, 2008 4:31 PM
> To: me@felipebalbi.com; david-b@pacbell.net; tony@atomide.com
> Cc: linux-omap@vger.kernel.org; linux-usb@vger.kernel.org;
> linux-kernel@vger.kernel.org; Bryan Wu; Sonic Zhang
> Subject: [PATCH 2/2] MUSB: Real fix bug 4 usb bugs found on Blackfin
>
>  - USB-IDE
>
> <http://blackfin.uclinux.org/gf/project/linux-kernel/tracker/?action=TrackerItemEdit&tracker_item_id=3789>
>  - 30G iPod
>
> <http://blackfin.uclinux.org/gf/project/linux-kernel/tracker/?action=TrackerItemEdit&tracker_item_id=3806>
>  - USB to CF/SD card reader
>
> <http://blackfin.uclinux.org/gf/project/linux-kernel/tracker/?action=TrackerItemEdit&tracker_item_id=3790>
>
> <http://blackfin.uclinux.org/gf/project/linux-kernel/tracker/?action=TrackerItemEdit&tracker_item_id=3807>
>
> Sonic found, when we got STALL, we should clear the toggle.
>
> Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
> Signed-off-by: Bryan Wu <cooloney@kernel.org>
> ---
>  drivers/usb/musb/musb_host.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_host.c
> b/drivers/usb/musb/musb_host.c
> index 535922d..806c479 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -1473,7 +1473,7 @@ void musb_host_rx(struct musb *musb, u8 epnum)
>                         (void)
> musb->dma_controller->channel_abort(dma);
>                         xfer_len = dma->actual_len;
>                 }
> -               musb_h_flush_rxfifo(hw_ep, 0);
> +               musb_h_flush_rxfifo(hw_ep, MUSB_RXCSR_CLRDATATOG);
>                 musb_writeb(epio, MUSB_RXINTERVAL, 0);
>                 done = true;
>                 goto finish;
> --
> 1.5.5
>
> --
> 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
>

Re: [PATCH 2/2] MUSB: Real fix bug 4 usb bugs found on Blackfin

[Mike Frysinger]

On Mon, May 19, 2008 at 11:16 AM, Gadiyar, Anand <gadiyar@ti.com> wrote:
> I am unable to visit any of these links. I get the following error "Unknown or invalid plugin: 'tracker'".

change "linux-kernel" in the url to "uclinux-dist"
-mike

Re: [PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[David Brownell]

On Saturday 17 May 2008, Bryan Wu wrote:
> 
> We discussed these 4 bugs before. Now we fixed them.
> Please review following 2 patches.

They looked plausible to me ... though the urb->status
one is a bit of a band-aid, and when that field finally
vanishes a better fix will be needed.

- Dave

Re: [PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[Bryan Wu]

On Sat, May 17, 2008 at 10:37 PM, David Brownell <david-b@pacbell.net> wrote:
> On Saturday 17 May 2008, Bryan Wu wrote:
>>
>> We discussed these 4 bugs before. Now we fixed them.
>> Please review following 2 patches.
>
> They looked plausible to me ... though the urb->status
> one is a bit of a band-aid, and when that field finally
> vanishes a better fix will be needed.
>

OK, so is there any plan for merge the MUSB to mainline and finally to
remove the urb->status?

-Bryan

Re: [PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[Felipe Balbi]

On Mon, 19 May 2008 14:39:10 +0800, "Bryan Wu" <cooloney@kernel.org> wrote:
> On Sat, May 17, 2008 at 10:37 PM, David Brownell <david-b@pacbell.net>
> wrote:
>> On Saturday 17 May 2008, Bryan Wu wrote:
>>>
>>> We discussed these 4 bugs before. Now we fixed them.
>>> Please review following 2 patches.
>>
>> They looked plausible to me ... though the urb->status
>> one is a bit of a band-aid, and when that field finally
>> vanishes a better fix will be needed.
>>
> 
> OK, so is there any plan for merge the MUSB to mainline and finally to
> remove the urb->status?

I was planing to do this by the end of this year or as soon as I make all
usb if tests pass again. I'm out of tools for that right now :-s

But of course we can check this with more caution since we have this bug
to solve before sending it to mainline.

-- 
Best Regards,

Felipe Balbi
http://felipebalbi.com
me@felipebalbi.com

Re: [PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[Bryan Wu]

On Mon, May 19, 2008 at 3:49 PM, Felipe Balbi <me@felipebalbi.com> wrote:
>
>
> On Mon, 19 May 2008 14:39:10 +0800, "Bryan Wu" <cooloney@kernel.org> wrote:
>> On Sat, May 17, 2008 at 10:37 PM, David Brownell <david-b@pacbell.net>
>> wrote:
>>> On Saturday 17 May 2008, Bryan Wu wrote:
>>>>
>>>> We discussed these 4 bugs before. Now we fixed them.
>>>> Please review following 2 patches.
>>>
>>> They looked plausible to me ... though the urb->status
>>> one is a bit of a band-aid, and when that field finally
>>> vanishes a better fix will be needed.
>>>
>>
>> OK, so is there any plan for merge the MUSB to mainline and finally to
>> remove the urb->status?
>
> I was planing to do this by the end of this year or as soon as I make all
> usb if tests pass again. I'm out of tools for that right now :-s
>

Great, after I enable the DMA mode 1 on Blackfin and cleanup the code,
I will send
out the code before you sent them to upstream

Thanks
-Bryan

Re: [PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[Tony Lindgren]

* Bryan Wu <cooloney@kernel.org> [080519 01:06]:
> On Mon, May 19, 2008 at 3:49 PM, Felipe Balbi <me@felipebalbi.com> wrote:
> >
> >
> > On Mon, 19 May 2008 14:39:10 +0800, "Bryan Wu" <cooloney@kernel.org> wrote:
> >> On Sat, May 17, 2008 at 10:37 PM, David Brownell <david-b@pacbell.net>
> >> wrote:
> >>> On Saturday 17 May 2008, Bryan Wu wrote:
> >>>>
> >>>> We discussed these 4 bugs before. Now we fixed them.
> >>>> Please review following 2 patches.
> >>>
> >>> They looked plausible to me ... though the urb->status
> >>> one is a bit of a band-aid, and when that field finally
> >>> vanishes a better fix will be needed.
> >>>
> >>

I'll push these to linux-omap tree then.

> >> OK, so is there any plan for merge the MUSB to mainline and finally to
> >> remove the urb->status?
> >
> > I was planing to do this by the end of this year or as soon as I make all
> > usb if tests pass again. I'm out of tools for that right now :-s
> >
> 
> Great, after I enable the DMA mode 1 on Blackfin and cleanup the code,
> I will send
> out the code before you sent them to upstream

Maybe we should get the musb code to USB tree before that? It's been out
of the scope for linux-omap tree for quite a while now.

Tony

Re: [PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[David Brownell]

On Wednesday 21 May 2008, Tony Lindgren wrote:
> 
> > Great, after I enable the DMA mode 1 on Blackfin and cleanup the code,
> > I will send
> > out the code before you sent them to upstream
> 
> Maybe we should get the musb code to USB tree before that? It's been out
> of the scope for linux-omap tree for quite a while now.

I'm all for getting the musb_hdrc driver into the 2.6.27 queue...
I presume there are still some infrastructure changes in usbcore
that block that merge?  It'd be nice if we could merge musb_hdrc
without those changes (OTG related) and then update that stuff
separately.

- Dave

Re: [PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[Felipe Balbi]

On Wed, 21 May 2008 10:14:44 -0700, David Brownell <david-b@pacbell.net>
wrote:
> On Wednesday 21 May 2008, Tony Lindgren wrote:
>>
>> > Great, after I enable the DMA mode 1 on Blackfin and cleanup the code,
>> > I will send
>> > out the code before you sent them to upstream
>>
>> Maybe we should get the musb code to USB tree before that? It's been out
>> of the scope for linux-omap tree for quite a while now.
> 
> I'm all for getting the musb_hdrc driver into the 2.6.27 queue...
> I presume there are still some infrastructure changes in usbcore
> that block that merge?  It'd be nice if we could merge musb_hdrc
> without those changes (OTG related) and then update that stuff
> separately.

Ok then, I'll prepare the patches during the next week and post
here on l-o so people can comment.

Any otg related stuff will come later.

-- 
Best Regards,

Felipe Balbi
http://felipebalbi.com
me@felipebalbi.com

Re: [PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[David Brownell]

On Wednesday 21 May 2008, Felipe Balbi wrote:
> 
> > I'm all for getting the musb_hdrc driver into the 2.6.27 queue...
> > I presume there are still some infrastructure changes in usbcore
> > that block that merge?  It'd be nice if we could merge musb_hdrc
> > without those changes (OTG related) and then update that stuff
> > separately.
> 
> Ok then, I'll prepare the patches during the next week and post
> here on l-o so people can comment.

When it's a USB patch, please post to linux-usb.  Most of the
relevant reviewers won't be on Linux-OMAP.  ;)

That said, I'm looking forward to seeing the www.beagleboard.org
hardware [1] become more generally available.  That will make some
musb_hdrc hardware available in a more developer-friendly rig than,
say, an N810 tablet.  And it might well get more folk in a position
where they can use that driver!  (I think some Blackfin devboards
are available already, but they're more pricey.)

- Dave

[1] http://www.elinux.org/BeagleBoard

Re: [PATCH 0/2] MUSB: 2 patches to fix some bug found on Blackfin

[Mike Frysinger]

On Wed, May 21, 2008 at 4:04 PM, David Brownell wrote:
> That said, I'm looking forward to seeing the www.beagleboard.org
> hardware [1] become more generally available.  That will make some
> musb_hdrc hardware available in a more developer-friendly rig than,
> say, an N810 tablet.  And it might well get more folk in a position
> where they can use that driver!  (I think some Blackfin devboards
> are available already, but they're more pricey.)

unfortunately, that is the case.  the BF527 EZKit is ~$900 while the
BF548 EZKit is ~$1000.  i'm hoping us software guys keep complaining
enough for the hardware guys to put out a bare bone kit at a
reasonable price (sub $300).
-mike