RE: [PATCH 01/68] xhci: fix usb3 streams
On 11/15/2013 04:46 PM, David Laight wrote: From: Hans de Goede Sent: 15 November 2013 15:06 To: Sarah Sharp Cc: linux-usb@vger.kernel.org; Gerd Hoffmann; Alan Stern; Hans de Goede Subject: [PATCH 01/68] xhci: fix usb3 streams From: Gerd Hoffmann kra...@redhat.com xhci maintains a radix tree for each stream endpoint because it must be able to map a trb address to the stream ring. Each ring segment must be added to the ring for this to work. Currently xhci sticks only the first segment of each stream ring into the radix tree. ... Seems to me that this code fails badly on the KISS principle. If an 'event data' TRB were added to all transfers, then in the normal case the 64bit value in it could be used to identify which transfer completed. In the unusual case of an error event just search through the endpoints/rings (etc) until the correct one is found. Adding and dealing with event TRB-s is not exactly simple either, actually I believe doing so would be much more complicated then the radix tree we've now. Not to mention that this code has been in development for months (and was first posted months ago) and has been tested and debugged quite a lot already, iow this code has proven to work well. Sorry - I didn't mean to imply that this patch itself should not be applied. David -- 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
RE: [PATCH 01/68] xhci: fix usb3 streams
From: Hans de Goede Sent: 15 November 2013 15:06 To: Sarah Sharp Cc: linux-usb@vger.kernel.org; Gerd Hoffmann; Alan Stern; Hans de Goede Subject: [PATCH 01/68] xhci: fix usb3 streams From: Gerd Hoffmann kra...@redhat.com xhci maintains a radix tree for each stream endpoint because it must be able to map a trb address to the stream ring. Each ring segment must be added to the ring for this to work. Currently xhci sticks only the first segment of each stream ring into the radix tree. ... Seems to me that this code fails badly on the KISS principle. If an 'event data' TRB were added to all transfers, then in the normal case the 64bit value in it could be used to identify which transfer completed. In the unusual case of an error event just search through the endpoints/rings (etc) until the correct one is found. David -- 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
Re: [PATCH 01/68] xhci: fix usb3 streams
Hi, On 11/15/2013 04:46 PM, David Laight wrote: From: Hans de Goede Sent: 15 November 2013 15:06 To: Sarah Sharp Cc: linux-usb@vger.kernel.org; Gerd Hoffmann; Alan Stern; Hans de Goede Subject: [PATCH 01/68] xhci: fix usb3 streams From: Gerd Hoffmann kra...@redhat.com xhci maintains a radix tree for each stream endpoint because it must be able to map a trb address to the stream ring. Each ring segment must be added to the ring for this to work. Currently xhci sticks only the first segment of each stream ring into the radix tree. ... Seems to me that this code fails badly on the KISS principle. If an 'event data' TRB were added to all transfers, then in the normal case the 64bit value in it could be used to identify which transfer completed. In the unusual case of an error event just search through the endpoints/rings (etc) until the correct one is found. Adding and dealing with event TRB-s is not exactly simple either, actually I believe doing so would be much more complicated then the radix tree we've now. Not to mention that this code has been in development for months (and was first posted months ago) and has been tested and debugged quite a lot already, iow this code has proven to work well. Regards, Hans -- 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
Re: [PATCH 01/68] xhci: fix usb3 streams
On Fri, Nov 15, 2013 at 04:56:59PM +0100, Hans de Goede wrote: Hi, On 11/15/2013 04:46 PM, David Laight wrote: From: Hans de Goede Sent: 15 November 2013 15:06 To: Sarah Sharp Cc: linux-usb@vger.kernel.org; Gerd Hoffmann; Alan Stern; Hans de Goede Subject: [PATCH 01/68] xhci: fix usb3 streams From: Gerd Hoffmann kra...@redhat.com xhci maintains a radix tree for each stream endpoint because it must be able to map a trb address to the stream ring. Each ring segment must be added to the ring for this to work. Currently xhci sticks only the first segment of each stream ring into the radix tree. ... Seems to me that this code fails badly on the KISS principle. If an 'event data' TRB were added to all transfers, then in the normal case the 64bit value in it could be used to identify which transfer completed. In the unusual case of an error event just search through the endpoints/rings (etc) until the correct one is found. Adding and dealing with event TRB-s is not exactly simple either, actually I believe doing so would be much more complicated then the radix tree we've now. The driver was designed around the assumption that we wouldn't use event TRBs, and the ring cancellation code would be really complex to change and verify in order to facilitate that. I don't want to change the driver to use event data TRBs. Not to mention that this code has been in development for months (and was first posted months ago) and has been tested and debugged quite a lot already, iow this code has proven to work well. Agreed. Sarah Sharp -- 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
