> > ENDPTFLUSH and ENDPTPRIME are registers that are set by software and > clear by hardware. > > The current code manipulate them with > "hw_write(ci, register, BIT(n), BIT(n));" > > This do not look sane to me, because hw_write will read the register, > apply the mask and write the value. > > What happen if the hardware clear the value of another endpoint between > the read and the write ? >
It may let hardware begin a new transfer which does not expect. > I think we don't see any problem ATM because the endpoint operation > are serialized. > > Do you thing a patch like this is ok ? > > Good fix, a patch is welcome. It is a similar problem I find for access otgsc http://marc.info/?l=linux-usb&m=138933555505826&w=2 Peter > Matthieu > > > > 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(); > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
