On Tue, Nov 12, 2013 at 01:58:05PM -0000, David Laight wrote: > > There needs to be a wmb() barrier between the write to the cycle bit > in the first TRB and the write to the doorbell register. > > Since it isn't needed in the other places the doobell is rung > (because the ring contents haven't been changed) add it to > giveback_first_trb() rather than somewhere later. > > Signed-off-by: David Laight <david.lai...@aculab.com> > --- > This patch will only apply cleanly if my earlier patch that affects > the previous line has also been applied. > > drivers/usb/host/xhci-ring.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index 35dfed0..8bce4c3 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -3136,6 +3136,7 @@ static void giveback_first_trb(struct xhci_hcd *xhci, > int slot_id, > */ > wmb(); > start_trb->field[3] ^= cpu_to_le32(TRB_CYCLE); > + wmb();
I don't understand, why is this needed? field[] isn't being used elsewhere at this point in time, is it? When adding barriers, you need to also add a big comment as to exactly why this is needed, in the code itself, which this patch does not do, so we can't take it (well, I will not take it, Sarah might, but then I'll complain to her...) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
