On Thu, Nov 25, 2010 at 7:06 PM, Felipe Balbi <balbi@xxxxxx> wrote: > On Thu, Nov 25, 2010 at 06:24:46PM +0800, Ming Lei wrote: >> >> 2010/11/25 Anand Gadiyar <gadiyar@xxxxxx>: >>> >>> Felipe Balbi wrote: >>>> >>>> Hi, >>>> >>>> On Thu, Nov 25, 2010 at 04:54:05PM +0800, Bob Liu wrote: >>>> >>> The only work around is to config the receive FIFO into single >>> >>> buffer >>>> >>>> >>> mode. >>>> >> >>>> >> And how to do that ? >>>> >> >>>> > >>>> >It's used to by "USB: musb: disable double buffering for older RTL >>>> >versions" >>>> >>>> >>>> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdif >>> >>> f;h=9f445cb29918dc488b7a9a92ef018599cce33df7 >>>> >>>> yeah, but that breaks other stuff high bandwidth eps, so we need a >>>> better way to "disable double buffering". >>>> >>> >>> Here's how I understand the problem. >>> >>> Blackfin has a statically configured fifo allocation - with no >>> possibility to change it. The IP uses double buffering by default >>> if a FIFO is allocated space enough to hold 2*maxp (which is the >>> case for at least some of the endpoints). >>> >>> The only way to make the IP use single buffering is to tell the >>> IP that the maxp is different. >>> >>> This is an anomaly only in the blackfin - I don't see any way to >>> work around this (for lack of information), than what has been >>> implemented in the patch that already got merged. >> >> Yes, I agree. >> >>> >>> However, that code should be made to execute only on blackfin >>> and not on other platforms - so that the others are not broken >>> by that change. >> >> Even in blackfin platform, the code may break some function, >> especially for full speed device, which was explained a lot in >> the previous discussion: > > how about something like: > > From 0cbcba73308ca808d898c2b5d8220e3ff9f52e58 Mon Sep 17 00:00:00 2001 > From: Felipe Balbi <balbi@xxxxxx> > Date: Thu, 18 Nov 2010 13:44:29 +0200 > Subject: [PATCH] usb: musb: disable double buffer when it's broken > Organization: Texas Instruments\n > > We know that blackfin doesn't support double > buffering feature as of today. So we add a > flag set by musb_platform_init() to forcefully > disable that feature. > > Such flag is created and marked as deprecated > to force us to find a solution for the missing > double buffering support on blackfin. > > NYET-Signed-off-by: Felipe Balbi <balbi@xxxxxx> > --- > Âdrivers/usb/musb/blackfin.c  Â|  Â1 + > Âdrivers/usb/musb/musb_core.h  |  13 +++++++++++++ > Âdrivers/usb/musb/musb_gadget.c |  28 ++++++++++++++++++++++------ > Âdrivers/usb/musb/musb_host.c  |  17 +++++++++-------- > Â4 files changed, 45 insertions(+), 14 deletions(-) > > diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c > index fcb5206..dba1f63 100644 > --- a/drivers/usb/musb/blackfin.c > +++ b/drivers/usb/musb/blackfin.c > @@ -397,6 +397,7 @@ int __init musb_platform_init(struct musb *musb, void > *board_data) >        Âmusb->xceiv->set_power = bfin_set_power; >    Âmusb->isr = blackfin_interrupt; > +    musb->double_buffer_not_ok = true; >    Âreturn 0; > Â} > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > index febaabc..05cf5ef 100644 > --- a/drivers/usb/musb/musb_core.h > +++ b/drivers/usb/musb/musb_core.h > @@ -443,6 +443,19 @@ struct musb { >    Âunsigned        Âtest_mode:1; >    Âunsigned        Âsoftconnect:1; > Â+   Â/* > +    Â* FIXME: Remove this flag. > +    Â* > +    Â* This is only added to allow Blackfin to work > +    Â* with current driver. For some unknown reason > +    Â* Blackfin doesn't work with double buffering > +    Â* and that's enabled by default. > +    Â* > +    Â* We added this flag to forcefully disable double > +    Â* buffering until we get it working. > +    Â*/ > +    unsigned        Âdouble_buffer_not_ok:1 __deprecated; > + >    Âu8           Âaddress; >    Âu8           Âtest_mode_nr; >    Âu16           ackpend;        Â/* ep0 */ > diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c > index 36cfd06..d6bfad8 100644 > --- a/drivers/usb/musb/musb_gadget.c > +++ b/drivers/usb/musb/musb_gadget.c > @@ -925,10 +925,18 @@ static int musb_gadget_enable(struct usb_ep *ep, >        Â/* REVISIT if can_bulk_split(), use by updating "tmp"; >         * likewise high bandwidth periodic tx >         */ > -        /* Set TXMAXP with the FIFO size of the endpoint > -        Â* to disable double buffering mode. > + > +        /* > +        Â* At least blackfin doesn't like Double Buffering > +        Â* which is enabled by default. So check if we can > +        Â* support double buffering and set MaxPacket register > +        Â* accordingly. >         */ > -        musb_writew(regs, MUSB_TXMAXP, musb_ep->packet_sz | > (musb_ep->hb_mult << 11)); > +        if (musb->double_buffer_not_ok) > +            musb_writew(regs, MUSB_TXMAXP, 1024); And I think musb_writew(regs, MUSB_TXMAXP, hw_ep->max_packet_sz_tx); is better. > +        else > +            musb_writew(regs, MUSB_TXMAXP, musb_ep->packet_sz > +                    | (musb_ep->hb_mult << 11)); >        Âcsr = MUSB_TXCSR_MODE | MUSB_TXCSR_CLRDATATOG; >        Âif (musb_readw(regs, MUSB_TXCSR) > @@ -961,10 +969,18 @@ static int musb_gadget_enable(struct usb_ep *ep, >        Â/* REVISIT if can_bulk_combine() use by updating "tmp" >         * likewise high bandwidth periodic rx >         */ > -        /* Set RXMAXP with the FIFO size of the endpoint > -        Â* to disable double buffering mode. > + > +        /* > +        Â* At least blackfin doesn't like Double Buffering > +        Â* which is enabled by default. So check if we can > +        Â* support double buffering and MaxPacket register > +        Â* accordingly. >         */ > -        musb_writew(regs, MUSB_RXMAXP, musb_ep->packet_sz | > (musb_ep->hb_mult << 11)); > +        if (musb->double_buffer_not_ok) > +            musb_writew(regs, MUSB_TXMAXP, 1024); So as here, musb_writew(regs, MUSB_RXMAXP, hw_ep->max_packet_sz_rx); And here should write MUSB_RXMAXP instead of MUSB_TXMAXP. > +        else > +            musb_writew(regs, MUSB_TXMAXP, musb_ep->packet_sz > +                    | (musb_ep->hb_mult << 11)); >        Â/* force shared fifo to OUT-only mode */ >        Âif (hw_ep->is_shared_fifo) { > diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c > index 4d5bcb4..0484bad 100644 > --- a/drivers/usb/musb/musb_host.c > +++ b/drivers/usb/musb/musb_host.c > @@ -606,11 +606,12 @@ musb_rx_reinit(struct musb *musb, struct musb_qh *qh, > struct musb_hw_ep *ep) >    Âmusb_writeb(ep->regs, MUSB_RXTYPE, qh->type_reg); >    Âmusb_writeb(ep->regs, MUSB_RXINTERVAL, qh->intv_reg); >    Â/* NOTE: bulk combining rewrites high bits of maxpacket */ > -    /* Set RXMAXP with the FIFO size of the endpoint > +    /* > +    Â* Set RXMAXP with the FIFO size of the endpoint >     * to disable double buffer mode. >     */ > -    if (musb->hwvers < MUSB_HWVERS_2000) > -        musb_writew(ep->regs, MUSB_RXMAXP, ep->max_packet_sz_rx); > +    if (musb->double_buffer_not_ok) > +        musb_writew(ep->regs, MUSB_RXMAXP, 1024); musb_writew(ep->regs, MUSB_RXMAXP, ep->max_packet_sz_rx); is better; And tx route should also be changed. >    Âelse >        Âmusb_writew(ep->regs, MUSB_RXMAXP, >                Âqh->maxpacket | ((qh->hb_mult - 1) << 11)); > @@ -784,14 +785,14 @@ static void musb_ep_program(struct musb *musb, u8 > epnum, >        Â/* protocol/endpoint/interval/NAKlimit */ >        Âif (epnum) { >            Âmusb_writeb(epio, MUSB_TXTYPE, qh->type_reg); > -            if (can_bulk_split(musb, qh->type)) > +            if (musb->double_buffer_not_ok) >                Âmusb_writew(epio, MUSB_TXMAXP, > -                    packet_sz > -                    | ((hw_ep->max_packet_sz_tx / > -                        packet_sz) - 1) << 11); > +                        1024); >            Âelse >                Âmusb_writew(epio, MUSB_TXMAXP, > -                    packet_sz); > +                        qh->maxpacket | > +                        ((qh->hb_mult - 1) << 11)); > + >            Âmusb_writeb(epio, MUSB_TXINTERVAL, qh->intv_reg); >        Â} else { >            Âmusb_writeb(epio, MUSB_NAKLIMIT0, qh->intv_reg); > -- > 1.7.3.2.245.g03276 > > > -- > balbi > -- Thanks, --Bob -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
