* [PATCH 0/2] usb: musb bug fixing patches @ 2008-08-25 9:13 Bryan Wu 2008-08-25 9:13 ` [PATCH 1/2] usb: musb: fix bug - don't mess up count number and CSR0 register value Bryan Wu 2008-08-25 9:13 ` [PATCH 2/2] usb: musb: fix bug - should call usb_hcd_unlink_urb_from_ep before usb_hcd_giveback_urb Bryan Wu 0 siblings, 2 replies; 7+ messages in thread From: Bryan Wu @ 2008-08-25 9:13 UTC (permalink / raw) To: felipe.balbi, linux-usb; +Cc: linux-kernel Hi Felipe, I found some MUSB bugs on Blackfin, here are 2 patches to fix them. Thanks -Bryan ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] usb: musb: fix bug - don't mess up count number and CSR0 register value 2008-08-25 9:13 [PATCH 0/2] usb: musb bug fixing patches Bryan Wu @ 2008-08-25 9:13 ` Bryan Wu 2008-08-25 11:49 ` Felipe Balbi 2008-08-25 9:13 ` [PATCH 2/2] usb: musb: fix bug - should call usb_hcd_unlink_urb_from_ep before usb_hcd_giveback_urb Bryan Wu 1 sibling, 1 reply; 7+ messages in thread From: Bryan Wu @ 2008-08-25 9:13 UTC (permalink / raw) To: felipe.balbi, linux-usb; +Cc: linux-kernel, Bryan Wu Signed-off-by: Bryan Wu <cooloney@kernel.org> --- drivers/usb/musb/musb_gadget_ep0.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c index a57652f..3f5e30d 100644 --- a/drivers/usb/musb/musb_gadget_ep0.c +++ b/drivers/usb/musb/musb_gadget_ep0.c @@ -437,7 +437,7 @@ static void ep0_rxstate(struct musb *musb) { void __iomem *regs = musb->control_ep->regs; struct usb_request *req; - u16 tmp; + u16 count, csr; req = next_ep0_request(musb); @@ -449,35 +449,35 @@ static void ep0_rxstate(struct musb *musb) unsigned len = req->length - req->actual; /* read the buffer */ - tmp = musb_readb(regs, MUSB_COUNT0); - if (tmp > len) { + count = musb_readb(regs, MUSB_COUNT0); + if (count > len) { req->status = -EOVERFLOW; - tmp = len; + count = len; } - musb_read_fifo(&musb->endpoints[0], tmp, buf); - req->actual += tmp; - tmp = MUSB_CSR0_P_SVDRXPKTRDY; - if (tmp < 64 || req->actual == req->length) { + musb_read_fifo(&musb->endpoints[0], count, buf); + req->actual += count; + csr = MUSB_CSR0_P_SVDRXPKTRDY; + if (count < 64 || req->actual == req->length) { musb->ep0_state = MUSB_EP0_STAGE_STATUSIN; - tmp |= MUSB_CSR0_P_DATAEND; + csr |= MUSB_CSR0_P_DATAEND; } else req = NULL; } else - tmp = MUSB_CSR0_P_SVDRXPKTRDY | MUSB_CSR0_P_SENDSTALL; + csr = MUSB_CSR0_P_SVDRXPKTRDY | MUSB_CSR0_P_SENDSTALL; /* Completion handler may choose to stall, e.g. because the * message just received holds invalid data. */ if (req) { - musb->ackpend = tmp; + musb->ackpend = csr; musb_g_ep0_giveback(musb, req); if (!musb->ackpend) return; musb->ackpend = 0; } musb_ep_select(musb->mregs, 0); - musb_writew(regs, MUSB_CSR0, tmp); + musb_writew(regs, MUSB_CSR0, csr); } /* -- 1.5.6 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] usb: musb: fix bug - don't mess up count number and CSR0 register value 2008-08-25 9:13 ` [PATCH 1/2] usb: musb: fix bug - don't mess up count number and CSR0 register value Bryan Wu @ 2008-08-25 11:49 ` Felipe Balbi 2008-08-25 14:28 ` Felipe Balbi 0 siblings, 1 reply; 7+ messages in thread From: Felipe Balbi @ 2008-08-25 11:49 UTC (permalink / raw) To: ext Bryan Wu; +Cc: felipe.balbi, linux-usb, linux-kernel, Greg Kroah-Hartman [-- Attachment #1: Type: text/plain, Size: 252 bytes --] On Mon, Aug 25, 2008 at 05:13:40PM +0800, ext Bryan Wu wrote: > Signed-off-by: Bryan Wu <cooloney@kernel.org> This patch looks ok, but doesn't apply on top of the previous series. Greg, please apply the refreshed version attached. thanks, -- balbi [-- Attachment #2: 0001-usb-musb-do-not-mess-up-count-number-and-CSR0-regis.diff --] [-- Type: text/plain, Size: 2104 bytes --] >From efae64b5f222e1b57b354e8207c62975c5a7618b Mon Sep 17 00:00:00 2001 From: Bryan Wu <cooloney@kernel.org> Date: Mon, 25 Aug 2008 14:39:39 +0300 Subject: [PATCH] usb: musb: do not mess up count number and CSR0 register value Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com> --- drivers/usb/musb/musb_gadget_ep0.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c index b50a30d..9b59824 100644 --- a/drivers/usb/musb/musb_gadget_ep0.c +++ b/drivers/usb/musb/musb_gadget_ep0.c @@ -437,7 +437,7 @@ static void ep0_rxstate(struct musb *musb) { void __iomem *regs = musb->control_ep->regs; struct usb_request *req; - u16 tmp; + u16 count, csr; req = next_ep0_request(musb); @@ -449,34 +449,34 @@ static void ep0_rxstate(struct musb *musb) unsigned len = req->length - req->actual; /* read the buffer */ - tmp = musb_readb(regs, MUSB_COUNT0); - if (tmp > len) { + count = musb_readb(regs, MUSB_COUNT0); + if (count > len) { req->status = -EOVERFLOW; - tmp = len; + count = len; } - musb_read_fifo(&musb->endpoints[0], tmp, buf); - req->actual += tmp; - tmp = MUSB_CSR0_P_SVDRXPKTRDY; - if (tmp < 64 || req->actual == req->length) { + musb_read_fifo(&musb->endpoints[0], count, buf); + req->actual += count; + csr = MUSB_CSR0_P_SVDRXPKTRDY; + if (count < 64 || req->actual == req->length) { musb->ep0_state = MUSB_EP0_STAGE_STATUSIN; - tmp |= MUSB_CSR0_P_DATAEND; + csr |= MUSB_CSR0_P_DATAEND; } else req = NULL; } else - tmp = MUSB_CSR0_P_SVDRXPKTRDY | MUSB_CSR0_P_SENDSTALL; + csr = MUSB_CSR0_P_SVDRXPKTRDY | MUSB_CSR0_P_SENDSTALL; /* Completion handler may choose to stall, e.g. because the * message just received holds invalid data. */ if (req) { - musb->ackpend = tmp; + musb->ackpend = csr; musb_g_ep0_giveback(musb, req); if (!musb->ackpend) return; musb->ackpend = 0; } - musb_writew(regs, MUSB_CSR0, tmp); + musb_writew(regs, MUSB_CSR0, csr); } /* -- 1.6.0.rc1.71.gfba5 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] usb: musb: fix bug - don't mess up count number and CSR0 register value 2008-08-25 11:49 ` Felipe Balbi @ 2008-08-25 14:28 ` Felipe Balbi 2008-08-25 14:33 ` Gadiyar, Anand 0 siblings, 1 reply; 7+ messages in thread From: Felipe Balbi @ 2008-08-25 14:28 UTC (permalink / raw) To: Felipe Balbi; +Cc: ext Bryan Wu, linux-usb, linux-kernel, Greg Kroah-Hartman On Mon, Aug 25, 2008 at 02:49:28PM +0300, Felipe Balbi wrote: > On Mon, Aug 25, 2008 at 05:13:40PM +0800, ext Bryan Wu wrote: > > Signed-off-by: Bryan Wu <cooloney@kernel.org> > > This patch looks ok, but doesn't apply on top of the previous series. > > Greg, please apply the refreshed version attached. > > thanks, > -- > balbi > >From efae64b5f222e1b57b354e8207c62975c5a7618b Mon Sep 17 00:00:00 2001 > From: Bryan Wu <cooloney@kernel.org> > Date: Mon, 25 Aug 2008 14:39:39 +0300 > Subject: [PATCH] usb: musb: do not mess up count number and CSR0 register value > Signed-off-by: Bryan Wy <cooloney@kernel.org> > Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com> > --- > drivers/usb/musb/musb_gadget_ep0.c | 24 ++++++++++++------------ > 1 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c > index b50a30d..9b59824 100644 > --- a/drivers/usb/musb/musb_gadget_ep0.c > +++ b/drivers/usb/musb/musb_gadget_ep0.c > @@ -437,7 +437,7 @@ static void ep0_rxstate(struct musb *musb) > { > void __iomem *regs = musb->control_ep->regs; > struct usb_request *req; > - u16 tmp; > + u16 count, csr; > > req = next_ep0_request(musb); > > @@ -449,34 +449,34 @@ static void ep0_rxstate(struct musb *musb) > unsigned len = req->length - req->actual; > > /* read the buffer */ > - tmp = musb_readb(regs, MUSB_COUNT0); > - if (tmp > len) { > + count = musb_readb(regs, MUSB_COUNT0); > + if (count > len) { > req->status = -EOVERFLOW; > - tmp = len; > + count = len; > } > - musb_read_fifo(&musb->endpoints[0], tmp, buf); > - req->actual += tmp; > - tmp = MUSB_CSR0_P_SVDRXPKTRDY; > - if (tmp < 64 || req->actual == req->length) { > + musb_read_fifo(&musb->endpoints[0], count, buf); > + req->actual += count; > + csr = MUSB_CSR0_P_SVDRXPKTRDY; > + if (count < 64 || req->actual == req->length) { > musb->ep0_state = MUSB_EP0_STAGE_STATUSIN; > - tmp |= MUSB_CSR0_P_DATAEND; > + csr |= MUSB_CSR0_P_DATAEND; > } else > req = NULL; > } else > - tmp = MUSB_CSR0_P_SVDRXPKTRDY | MUSB_CSR0_P_SENDSTALL; > + csr = MUSB_CSR0_P_SVDRXPKTRDY | MUSB_CSR0_P_SENDSTALL; > > > /* Completion handler may choose to stall, e.g. because the > * message just received holds invalid data. > */ > if (req) { > - musb->ackpend = tmp; > + musb->ackpend = csr; > musb_g_ep0_giveback(musb, req); > if (!musb->ackpend) > return; > musb->ackpend = 0; > } > - musb_writew(regs, MUSB_CSR0, tmp); > + musb_writew(regs, MUSB_CSR0, csr); > } > > /* > -- > 1.6.0.rc1.71.gfba5 > -- balbi ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/2] usb: musb: fix bug - don't mess up count number and CSR0 register value 2008-08-25 14:28 ` Felipe Balbi @ 2008-08-25 14:33 ` Gadiyar, Anand 0 siblings, 0 replies; 7+ messages in thread From: Gadiyar, Anand @ 2008-08-25 14:33 UTC (permalink / raw) To: felipe.balbi; +Cc: ext Bryan Wu, linux-usb, linux-kernel, Greg Kroah-Hartman > On Mon, Aug 25, 2008 at 02:49:28PM +0300, Felipe Balbi wrote: > > On Mon, Aug 25, 2008 at 05:13:40PM +0800, ext Bryan Wu wrote: > > > Signed-off-by: Bryan Wu <cooloney@kernel.org> > > > > This patch looks ok, but doesn't apply on top of the > previous series. > > > > Greg, please apply the refreshed version attached. > > > > thanks, > > -- > > balbi > > > >From efae64b5f222e1b57b354e8207c62975c5a7618b Mon Sep 17 00:00:00 2001 > > From: Bryan Wu <cooloney@kernel.org> > > Date: Mon, 25 Aug 2008 14:39:39 +0300 > > Subject: [PATCH] usb: musb: do not mess up count number and CSR0 register value > > > Signed-off-by: Bryan Wy <cooloney@kernel.org> Signed-off-by: Bryan Wu <cooloney@kernel.org> > > Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com> > > --- > > drivers/usb/musb/musb_gadget_ep0.c | 24 ++++++++++++------------ > > 1 files changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c > > index b50a30d..9b59824 100644 > > --- a/drivers/usb/musb/musb_gadget_ep0.c > > +++ b/drivers/usb/musb/musb_gadget_ep0.c > > @@ -437,7 +437,7 @@ static void ep0_rxstate(struct musb *musb) > > { > > void __iomem *regs = musb->control_ep->regs; > > struct usb_request *req; > > - u16 tmp; > > + u16 count, csr; > > > > req = next_ep0_request(musb); > > > > @@ -449,34 +449,34 @@ static void ep0_rxstate(struct musb *musb) > > unsigned len = req->length - req->actual; > > > > /* read the buffer */ > > - tmp = musb_readb(regs, MUSB_COUNT0); > > - if (tmp > len) { > > + count = musb_readb(regs, MUSB_COUNT0); > > + if (count > len) { > > req->status = -EOVERFLOW; > > - tmp = len; > > + count = len; > > } > > - musb_read_fifo(&musb->endpoints[0], tmp, buf); > > - req->actual += tmp; > > - tmp = MUSB_CSR0_P_SVDRXPKTRDY; > > - if (tmp < 64 || req->actual == req->length) { > > + musb_read_fifo(&musb->endpoints[0], count, buf); > > + req->actual += count; > > + csr = MUSB_CSR0_P_SVDRXPKTRDY; > > + if (count < 64 || req->actual == req->length) { > > musb->ep0_state = MUSB_EP0_STAGE_STATUSIN; > > - tmp |= MUSB_CSR0_P_DATAEND; > > + csr |= MUSB_CSR0_P_DATAEND; > > } else > > req = NULL; > > } else > > - tmp = MUSB_CSR0_P_SVDRXPKTRDY | MUSB_CSR0_P_SENDSTALL; > > + csr = MUSB_CSR0_P_SVDRXPKTRDY | MUSB_CSR0_P_SENDSTALL; > > > > > > /* Completion handler may choose to stall, e.g. because the > > * message just received holds invalid data. > > */ > > if (req) { > > - musb->ackpend = tmp; > > + musb->ackpend = csr; > > musb_g_ep0_giveback(musb, req); > > if (!musb->ackpend) > > return; > > musb->ackpend = 0; > > } > > - musb_writew(regs, MUSB_CSR0, tmp); > > + musb_writew(regs, MUSB_CSR0, csr); > > } > > > > /* > > -- > > 1.6.0.rc1.71.gfba5 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] usb: musb: fix bug - should call usb_hcd_unlink_urb_from_ep before usb_hcd_giveback_urb 2008-08-25 9:13 [PATCH 0/2] usb: musb bug fixing patches Bryan Wu 2008-08-25 9:13 ` [PATCH 1/2] usb: musb: fix bug - don't mess up count number and CSR0 register value Bryan Wu @ 2008-08-25 9:13 ` Bryan Wu 2008-08-25 9:18 ` Bryan Wu 1 sibling, 1 reply; 7+ messages in thread From: Bryan Wu @ 2008-08-25 9:13 UTC (permalink / raw) To: felipe.balbi, linux-usb; +Cc: linux-kernel, Bryan Wu This bug was found on Blackfin BF54x and BF52x board since kernel 2.6.24 USB API change http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=4265 __musb_giveback is called by not only musb_giveback but also musb_urb_dequeue. musb_urb_dequeue also need call usb_hcd_unlink_urb_from_ep before usb_hcd_giveback_urb. So move usb_hcd_unlink_urb_from_ep from musb_giveback to __musb_giveback to fix this bug. Signed-off-by: Bryan Wu <cooloney@kernel.org> --- drivers/usb/musb/musb_host.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index 8b4be01..ba98ac5 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -291,6 +291,8 @@ __acquires(musb->lock) urb->actual_length, urb->transfer_buffer_length ); + usb_hcd_unlink_urb_from_ep(musb_to_hcd(musb), urb); + spin_unlock(&musb->lock); usb_hcd_giveback_urb(musb_to_hcd(musb), urb, status); spin_lock(&musb->lock); @@ -353,8 +355,6 @@ musb_giveback(struct musb_qh *qh, struct urb *urb, int status) break; } - usb_hcd_unlink_urb_from_ep(musb_to_hcd(musb), urb); - qh->is_ready = 0; __musb_giveback(musb, urb, status); qh->is_ready = ready; -- 1.5.6 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] usb: musb: fix bug - should call usb_hcd_unlink_urb_from_ep before usb_hcd_giveback_urb 2008-08-25 9:13 ` [PATCH 2/2] usb: musb: fix bug - should call usb_hcd_unlink_urb_from_ep before usb_hcd_giveback_urb Bryan Wu @ 2008-08-25 9:18 ` Bryan Wu 0 siblings, 0 replies; 7+ messages in thread From: Bryan Wu @ 2008-08-25 9:18 UTC (permalink / raw) To: felipe.balbi, linux-usb, ajay.gupta; +Cc: linux-kernel This patch was posted by Ajay. Please ignore it. -Bryan On Mon, Aug 25, 2008 at 5:13 PM, Bryan Wu <cooloney@kernel.org> wrote: > This bug was found on Blackfin BF54x and BF52x board since kernel 2.6.24 USB API change > http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=4265 > > __musb_giveback is called by not only musb_giveback but also musb_urb_dequeue. > musb_urb_dequeue also need call usb_hcd_unlink_urb_from_ep before usb_hcd_giveback_urb. > > So move usb_hcd_unlink_urb_from_ep from musb_giveback to __musb_giveback to fix this bug. > > Signed-off-by: Bryan Wu <cooloney@kernel.org> > --- > drivers/usb/musb/musb_host.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c > index 8b4be01..ba98ac5 100644 > --- a/drivers/usb/musb/musb_host.c > +++ b/drivers/usb/musb/musb_host.c > @@ -291,6 +291,8 @@ __acquires(musb->lock) > urb->actual_length, urb->transfer_buffer_length > ); > > + usb_hcd_unlink_urb_from_ep(musb_to_hcd(musb), urb); > + > spin_unlock(&musb->lock); > usb_hcd_giveback_urb(musb_to_hcd(musb), urb, status); > spin_lock(&musb->lock); > @@ -353,8 +355,6 @@ musb_giveback(struct musb_qh *qh, struct urb *urb, int status) > break; > } > > - usb_hcd_unlink_urb_from_ep(musb_to_hcd(musb), urb); > - > qh->is_ready = 0; > __musb_giveback(musb, urb, status); > qh->is_ready = ready; > -- > 1.5.6 > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-08-25 14:33 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-08-25 9:13 [PATCH 0/2] usb: musb bug fixing patches Bryan Wu 2008-08-25 9:13 ` [PATCH 1/2] usb: musb: fix bug - don't mess up count number and CSR0 register value Bryan Wu 2008-08-25 11:49 ` Felipe Balbi 2008-08-25 14:28 ` Felipe Balbi 2008-08-25 14:33 ` Gadiyar, Anand 2008-08-25 9:13 ` [PATCH 2/2] usb: musb: fix bug - should call usb_hcd_unlink_urb_from_ep before usb_hcd_giveback_urb Bryan Wu 2008-08-25 9:18 ` Bryan Wu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®