On Fri, Nov 01, 2013 at 05:10:17PM -0000, David Laight wrote: > Sarah, > > The patch below is a real bug that will hit BE systems, so probably > ought to be a candidate for 3.12 even though it is late in the cycle. Thanks for catching this bug. However, it's too late to make it into 3.12. Greg has already sent off his final pull request, and my tree was closed several weeks ago. > There is a 2nd copy of the bug later in the file - the second time > TRB_CYCLE is used. In fact, looking back through my mail, someone already sent me a patch to fix this issue, and caught the other bug as well: http://marc.info/?l=linux-usb&m=137874982629699&w=2 I missed that patchset while sending off my final pull request for 3.13, so it will have to wait three weeks until Greg's usb-linus tree opens up after 3.13-rc1. I will mark it for stable, so it should eventually get into the 3.12 kernel. > > -----Original Message----- > > From: David Laight > > Sent: 31 October 2013 15:10 > > To: linux-usb@xxxxxxxxxxxxxxx > > Subject: [PATCH] Fix ring expansion on BE hosts. > > > > Convert TRB_CYCLE to LE when initialising the ring entries. > > --- > > drivers/usb/host/xhci-mem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > > index 83bcd13..79cdcc2 100644 > > --- a/drivers/usb/host/xhci-mem.c > > +++ b/drivers/usb/host/xhci-mem.c > > @@ -57,7 +57,7 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci, > > /* If the cycle state is 0, set the cycle bit to 1 for all the TRBs */ > > if (cycle_state == 0) { > > for (i = 0; i < TRBS_PER_SEGMENT; i++) > > - seg->trbs[i].link.control |= TRB_CYCLE; > > + seg->trbs[i].link.control = cpu_to_le32(TRB_CYCLE); This next bit is moot, since I'll be taking Xenia's patch instead of yours, but for future reference... Your patch went a little farther than just fixing the BE issue. You also changed the code from ORing the cycle bit in, to simply setting the control dword of the TRB to the cycle bit. After looking at the code, this is fine, since the segment is memset to zero just before this for loop runs. However, you should note that you made this change and why it's safe to do so in your commit message. Otherwise people (like me) will wonder if the change was intentional or a bug. Sarah Sharp -- 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
