RE: [PATCH 2/5] USB: Add stream ID field to struct urb.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 07, 2010 at 08:29:27PM +0500, Hrant Dalalyan wrote:
> > -----Original Message-----
> > From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx]
> > Sent: Saturday, April 03, 2010 3:34 AM
> > To: Greg KH
> > Cc: linux-usb@xxxxxxxxxxxxxxx; usb-storage@xxxxxxxxxxxxxxxxxxxxxxxx;
> > Hrant Dalalyan; Alan Stern; Paul Zimmerman; Ashot Madatyan
> > Subject: [PATCH 2/5] USB: Add stream ID field to struct urb.
> > 
> > Bulk endpoint streams were added in the USB 3.0 specification.
> > Streams
> > allow a device driver to overload a bulk endpoint so that multiple
> > transfers can be queued at once.
> > 
> > Add a new field, stream_id, to struct urb so that USB 3.0 drivers
> > can
> > specify which stream they want the URB to be queued to.
> Is this kind of patch planned to be done for scatter-gather
> requests? Now usb_sg_request structure has no stream_id
> field like urb structure. Also, there is no way to pass the stream_id
> field for the selected scatter-gather request using the usb_sg_init
> function.
> The only way to pass the stream_id field for each urb located in
> scatter-gather request is loop over the urb list of the scatter-gather
> request and set that field manually, which I think is not the best
> way.

usb_sg_init() should set up only one URB when it's passing an
usb_sg_request to an xHCI driver.  But yes, there's nothing in
usb_sg_request to indicate which stream this usb_sg_request is intended
for.

> So, please provide your feedback.

What about the following patch?  Matthew Wilcox has said that using
usb_sg_request is a bit of a pain, and it would be better if urb->sg was
a pointer to the actual scatterlist.  If you change the urb's sg pointer
to an actual scatterlist, then you don't have to use usb_sg_init(), you
can just enqueue the sglist.  You also don't have to use usb_sg_wait(),
and you can just handle the completion asynchronously (and thus make it
easier to handle multiple queued commands).

Note that this patch still needs work on the usbmon side.  We didn't
know how to fix it, so we just disabled output for scattergather lists.
Alan, can you give us some guidance on how to fix usbmon if this change
were to go through?

Sarah Sharp

----------------------------------------------------------------

Subject: [PATCH] USB: Change scatterlist type in URB
From: Matthew Wilcox <willy@xxxxxxxxxxxxxxx>

Change the URB's 'sg' pointer from a usb_sg_request to a scatterlist.
This will allow drivers to submit scatter-gather URBs without using the
usb_sg_wait() interface.

NOTE: uhci / ohci need to be updated to handle this

NOTE: usbmon needs to be fixed

Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxxxx>
Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
---
 drivers/usb/core/message.c   |    2 +-
 drivers/usb/host/ehci-q.c    |    2 +-
 drivers/usb/host/whci/qset.c |    4 ++--
 drivers/usb/host/xhci-ring.c |    4 ++--
 drivers/usb/mon/mon_bin.c    |    4 ++--
 drivers/usb/mon/mon_text.c   |    4 ++--
 include/linux/usb.h          |    2 +-
 7 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index cd22027..06f237e 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -431,7 +431,7 @@ int usb_sg_init(struct usb_sg_request *io, struct usb_device *dev,
 					sg_dma_len(sg);
 			}
 		}
-		io->urbs[0]->sg = io;
+		io->urbs[0]->sg = io->sg;
 		io->urbs[0]->num_sgs = io->entries;
 		io->entries = 1;
 	} else {
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 8952177..11a79c4 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -663,7 +663,7 @@ qh_urb_transaction (
 	 */
 	i = urb->num_sgs;
 	if (len > 0 && i > 0) {
-		sg = urb->sg->sg;
+		sg = urb->sg;
 		buf = sg_dma_address(sg);
 
 		/* urb->transfer_buffer_length may be smaller than the
diff --git a/drivers/usb/host/whci/qset.c b/drivers/usb/host/whci/qset.c
index 141d049..1afe15c 100644
--- a/drivers/usb/host/whci/qset.c
+++ b/drivers/usb/host/whci/qset.c
@@ -443,7 +443,7 @@ static int qset_add_urb_sg(struct whc *whc, struct whc_qset *qset, struct urb *u
 
 	remaining = urb->transfer_buffer_length;
 
-	for_each_sg(urb->sg->sg, sg, urb->num_sgs, i) {
+	for_each_sg(urb->sg, sg, urb->num_sgs, i) {
 		dma_addr_t dma_addr;
 		size_t dma_remaining;
 		dma_addr_t sp, ep;
@@ -561,7 +561,7 @@ static int qset_add_urb_sg_linearize(struct whc *whc, struct whc_qset *qset,
 
 	remaining = urb->transfer_buffer_length;
 
-	for_each_sg(urb->sg->sg, sg, urb->sg->nents, i) {
+	for_each_sg(urb->sg, sg, urb->num_sgs, i) {
 		size_t len;
 		size_t sg_remaining;
 		void *orig;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 85d7e8f..e9158f8 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1672,7 +1672,7 @@ static unsigned int count_sg_trbs_needed(struct xhci_hcd *xhci, struct urb *urb)
 
 	xhci_dbg(xhci, "count sg list trbs: \n");
 	num_trbs = 0;
-	for_each_sg(urb->sg->sg, sg, num_sgs, i) {
+	for_each_sg(urb->sg, sg, num_sgs, i) {
 		unsigned int previous_total_trbs = num_trbs;
 		unsigned int len = sg_dma_len(sg);
 
@@ -1831,7 +1831,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 	 *    the amount of memory allocated for this scatter-gather list.
 	 * 3. TRBs buffers can't cross 64KB boundaries.
 	 */
-	sg = urb->sg->sg;
+	sg = urb->sg;
 	addr = (u64) sg_dma_address(sg);
 	this_sg_len = sg_dma_len(sg);
 	trb_buff_len = TRB_MAX_BUFF_SIZE -
diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index ddf7f9a..fb36fd9 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -416,13 +416,13 @@ static unsigned int mon_bin_get_data(const struct mon_reader_bin *rp,
 
 	} else {
 		/* If IOMMU coalescing occurred, we cannot trust sg_page */
-		if (urb->sg->nents != urb->num_sgs) {
+		if (/*urb->sg->nents != urb->num_sgs*/ 0) {
 			*flag = 'D';
 			return length;
 		}
 
 		/* Copy up to the first non-addressable segment */
-		for_each_sg(urb->sg->sg, sg, urb->num_sgs, i) {
+		for_each_sg(urb->sg, sg, urb->num_sgs, i) {
 			if (length == 0 || PageHighMem(sg_page(sg)))
 				break;
 			this_len = min_t(unsigned int, sg->length, length);
diff --git a/drivers/usb/mon/mon_text.c b/drivers/usb/mon/mon_text.c
index 4d0be13..09ab1b6 100644
--- a/drivers/usb/mon/mon_text.c
+++ b/drivers/usb/mon/mon_text.c
@@ -159,10 +159,10 @@ static inline char mon_text_get_data(struct mon_event_text *ep, struct urb *urb,
 		if (src == NULL)
 			return 'Z';	/* '0' would be not as pretty. */
 	} else {
-		struct scatterlist *sg = urb->sg->sg;
+		struct scatterlist *sg = urb->sg;
 
 		/* If IOMMU coalescing occurred, we cannot trust sg_page */
-		if (urb->sg->nents != urb->num_sgs ||
+		if (/*urb->sg->nents != urb->num_sgs || */
 				PageHighMem(sg_page(sg)))
 			return 'D';
 
diff --git a/include/linux/usb.h b/include/linux/usb.h
index ce1323c..edcf1e2 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1193,7 +1193,7 @@ struct urb {
 	unsigned int transfer_flags;	/* (in) URB_SHORT_NOT_OK | ...*/
 	void *transfer_buffer;		/* (in) associated data buffer */
 	dma_addr_t transfer_dma;	/* (in) dma addr for transfer_buffer */
-	struct usb_sg_request *sg;	/* (in) scatter gather buffer list */
+	struct scatterlist *sg;		/* (in) scatter gather buffer list */
 	int num_sgs;			/* (in) number of entries in the sg list */
 	u32 transfer_buffer_length;	/* (in) data buffer length */
 	u32 actual_length;		/* (return) actual transfer length */
-- 
1.7.0

--
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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux