Re: [PATCHv2] usb: musb: gadget: clear PKTRDY flags when set FLUSHFIFO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

Yauheni Kaliuta wrote:

 > > From: Yauheni Kaliuta<yauheni.kaliuta@xxxxxxxxx>

 > > The specification says about FLUSHFIFO, that it "May be set simultaneously
 > > with TxPktRdy to abort the packet that is currently being loaded into the
 > > FIFO". This is a situation, where TxPktRdy hasn't been set yet, but some
 > > data already loaded into the fifo. It looks, that if TxPktRdy has been
 > > set before and there is no loading in progress but we set FLUSHFIFO with
 > > the TxPktRdy, controller tries to use the same logic to abort loading
 > > and as the result just does nothing (because there is no loading in progress)

 > > Signed-off-by: Yauheni Kaliuta<yauheni.kaliuta@xxxxxxxxx>
 > > ---
 > >   drivers/usb/musb/musb_gadget.c |    2 ++
 > >   1 files changed, 2 insertions(+), 0 deletions(-)

 > > diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
 > > index 0a50a35..830768f 100644
 > > --- a/drivers/usb/musb/musb_gadget.c
 > > +++ b/drivers/usb/musb/musb_gadget.c
 > > @@ -1524,6 +1524,7 @@ static void musb_gadget_fifo_flush(struct usb_ep *ep)
 > >                   csr = musb_readw(epio, MUSB_TXCSR);
 > >                   if (csr&  MUSB_TXCSR_FIFONOTEMPTY) {
 > >                           csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_P_WZC_BITS;
 > > +                        csr &= ~MUSB_TXCSR_TXPKTRDY;

 >    A comment wouldn't hurt here either...

Is /* * Setting both TXPKTRDY and FLUSHFIFO makes controller to interrupt

   "To" not needed.

 * current FIFO loading, but not flushing the already loaded ones.

   Only "flush".

 */

ok?

Yes, excluding the grammar. I see you have already posted the patch though... too fast for me. :-)

 > >                           musb_writew(epio, MUSB_TXCSR, csr);
 > >                           /* REVISIT may be inappropriate w/o FIFONOTEMPTY ... */
 > >                           musb_writew(epio, MUSB_TXCSR, csr);
 > > @@ -1531,6 +1532,7 @@ static void musb_gadget_fifo_flush(struct usb_ep *ep)
 > >           } else {
 > >                   csr = musb_readw(epio, MUSB_RXCSR);
 > >                   csr |= MUSB_RXCSR_FLUSHFIFO | MUSB_RXCSR_P_WZC_BITS;
 > > +                csr &= ~MUSB_RXCSR_RXPKTRDY;

 >    This still contradicts the manual. Did you really see the issue here?

No issue (yet?). I wouldn't say it contradicts, FLUSHFIFO must reset it and
CPU can clear it when it unloaded (cleared) the fifo.

   Reminding you again, read the description of the RXCSR.FlushFIFO bit:

"Note: FlushFIFO should only be used when RxPktRdy is set. At other times, it may cause data to be corrupted."

But well, since I do not have an issue and I do not see a problem in the
documentation with it, agree, it's worth not to change, removing the chunk.

  Thanks.

WBR, Sergei
--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux