On Fri, Jan 17, 2014 at 01:21:45PM +0100, Michael Grzeschik wrote:
> On Tue, Jan 14, 2014 at 03:39:25PM +0100, Matthieu CASTET wrote:
> > ENDPTFLUSH and ENDPTPRIME registers are set by software and
> > clear by hardware.
> > There is a bit for each endpoint.
> > When we are setting a bit for an endpoint we should make sure we not touch
> > other endpoint bit. There is a race condition if the hardware clear the
> > bit between the read and the write in hw_write.
> >
> > Signed-off-by: Matthieu CASTET <matthieu.castet@xxxxxxxxxx>
> > ---
> > drivers/usb/chipidea/udc.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > index 69d20fb..9e2e39b 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -105,7 +105,7 @@ static int hw_ep_flush(struct ci_hdrc *ci, int num, int dir)
> >
> > do {
> > /* flush any pending transfer */
> > - hw_write(ci, OP_ENDPTFLUSH, BIT(n), BIT(n));
> > + hw_write(ci, OP_ENDPTFLUSH, ~0, BIT(n));
> > while (hw_read(ci, OP_ENDPTFLUSH, BIT(n)))
> > cpu_relax();
> > } while (hw_read(ci, OP_ENDPTSTAT, BIT(n)));
> > @@ -205,7 +205,7 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
> > if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
> > return -EAGAIN;
> >
> > - hw_write(ci, OP_ENDPTPRIME, BIT(n), BIT(n));
> > + hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
> > while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
> > cpu_relax();
> > --
> > 1.8.5.2
>
> This patch is not working here. The whole udc stalls after this patch
> is applied.
>
> I asume it is not safe changing the bits for running endpoints back to
> zero, the hardware is probably stoping the transfer in that case.
>
> As we currently never saw real issues regarding this case, it is probably
> safe to stick with the current behaviour.
>
> It's probably also not a problem to prime endpoints which got ready in
> the meantime between the ioread and the iowrite, as the software did not
> add new payload data for that particular endpoint. So that the hardware
> will disable the endpoint in the case the transfer descriptor is empty
> for that endpoint.
Forget my previous statements. It seems that my stack was dirty when I
compiled with that patch applied. Now that I tested again with a clean
setup:
Tested-by: Michael Grzeschik <mgrzeschik@xxxxxxxxxxxxxx>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
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
