ChangeSet 1.1504.2.27, 2003/12/09 11:48:52-08:00, [EMAIL PROTECTED]
[PATCH] USB storage: Add comments explaining new s-g usage
On Sun, 30 Nov 2003, Matthew Dharm wrote:
> I'm going to pass this one along to Greg, but I think some places in this
> could really use some better comments. Especially the way you use a single
> buffer inside the loop -- it took me a few minutes to figure out how your
> logic to refresh the buffer with new data worked.
>
> I'm also wondering if the access_xfer_buf() function could use some more
> header comments, stating why this is needed (i.e. spelling out the
> kmap()-isms).
Okay, here it is. This patch basically just adds comments. Each routine
that uses the new scatter-gather function gets a brief explanation of
what's going on, and access_xfer_buf() itself gets detailed comments
saying what it's doing and why it's necessary. You may even want to cut
some of it back; I was pretty verbose.
drivers/usb/storage/datafab.c | 15 +++++++++++++--
drivers/usb/storage/jumpshot.c | 15 +++++++++++++--
drivers/usb/storage/protocol.c | 34 +++++++++++++++++++++++++++++++---
drivers/usb/storage/sddr09.c | 17 +++++++++++++++--
drivers/usb/storage/sddr55.c | 17 +++++++++++++++--
drivers/usb/storage/shuttle_usbat.c | 10 ++++++++--
6 files changed, 95 insertions(+), 13 deletions(-)
diff -Nru a/drivers/usb/storage/datafab.c b/drivers/usb/storage/datafab.c
--- a/drivers/usb/storage/datafab.c Mon Dec 29 14:23:59 2003
+++ b/drivers/usb/storage/datafab.c Mon Dec 29 14:23:59 2003
@@ -116,7 +116,11 @@
totallen = sectors * info->ssize;
// Since we don't read more than 64 KB at a time, we have to create
- // a bounce buffer if the transfer uses scatter-gather.
+ // a bounce buffer if the transfer uses scatter-gather. We will
+ // move the data a piece at a time between the bounce buffer and
+ // the actual transfer buffer. If we're not using scatter-gather,
+ // we can simply update the transfer buffer pointer to get the
+ // same effect.
alloclen = min(totallen, 65536u);
if (use_sg) {
@@ -153,6 +157,7 @@
if (result != USB_STOR_XFER_GOOD)
goto leave;
+ // Store the data (s-g) or update the pointer (!s-g)
if (use_sg)
usb_stor_access_xfer_buf(buffer, len, us->srb,
&sg_idx, &sg_offset, TO_XFER_BUF);
@@ -205,7 +210,11 @@
totallen = sectors * info->ssize;
// Since we don't write more than 64 KB at a time, we have to create
- // a bounce buffer if the transfer uses scatter-gather.
+ // a bounce buffer if the transfer uses scatter-gather. We will
+ // move the data a piece at a time between the bounce buffer and
+ // the actual transfer buffer. If we're not using scatter-gather,
+ // we can simply update the transfer buffer pointer to get the
+ // same effect.
alloclen = min(totallen, 65536u);
if (use_sg) {
@@ -221,6 +230,7 @@
len = min(totallen, alloclen);
thistime = (len / info->ssize) & 0xff;
+ // Get the data from the transfer buffer (s-g)
if (use_sg)
usb_stor_access_xfer_buf(buffer, len, us->srb,
&sg_idx, &sg_offset, FROM_XFER_BUF);
@@ -259,6 +269,7 @@
goto leave;
}
+ // Update the transfer buffer pointer (!s-g)
if (!use_sg)
buffer += len;
diff -Nru a/drivers/usb/storage/jumpshot.c b/drivers/usb/storage/jumpshot.c
--- a/drivers/usb/storage/jumpshot.c Mon Dec 29 14:23:59 2003
+++ b/drivers/usb/storage/jumpshot.c Mon Dec 29 14:23:59 2003
@@ -130,7 +130,11 @@
totallen = sectors * info->ssize;
// Since we don't read more than 64 KB at a time, we have to create
- // a bounce buffer if the transfer uses scatter-gather.
+ // a bounce buffer if the transfer uses scatter-gather. We will
+ // move the data a piece at a time between the bounce buffer and
+ // the actual transfer buffer. If we're not using scatter-gather,
+ // we can simply update the transfer buffer pointer to get the
+ // same effect.
alloclen = min(totallen, 65536u);
if (use_sg) {
@@ -167,6 +171,7 @@
US_DEBUGP("jumpshot_read_data: %d bytes\n", len);
+ // Store the data (s-g) or update the pointer (!s-g)
if (use_sg)
usb_stor_access_xfer_buf(buffer, len, us->srb,
&sg_idx, &sg_offset, TO_XFER_BUF);
@@ -212,7 +217,11 @@
totallen = sectors * info->ssize;
// Since we don't write more than 64 KB at a time, we have to create
- // a bounce buffer if the transfer uses scatter-gather.
+ // a bounce buffer if the transfer uses scatter-gather. We will
+ // move the data a piece at a time between the bounce buffer and
+ // the actual transfer buffer. If we're not using scatter-gather,
+ // we can simply update the transfer buffer pointer to get the
+ // same effect.
alloclen = min(totallen, 65536u);
if (use_sg) {
@@ -228,6 +237,7 @@
len = min(totallen, alloclen);
thistime = (len / info->ssize) & 0xff;
+ // Get the data from the transfer buffer (s-g)
if (use_sg)
usb_stor_access_xfer_buf(buffer, len, us->srb,
&sg_idx, &sg_offset, FROM_XFER_BUF);
@@ -268,6 +278,7 @@
if (result != USB_STOR_TRANSPORT_GOOD)
US_DEBUGP("jumpshot_write_data: Gah! Waitcount = 10. Bad
write!?\n");
+ // Update the transfer buffer pointer (!s-g)
if (!use_sg)
buffer += len;
diff -Nru a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c
--- a/drivers/usb/storage/protocol.c Mon Dec 29 14:23:59 2003
+++ b/drivers/usb/storage/protocol.c Mon Dec 29 14:23:59 2003
@@ -233,7 +233,11 @@
***********************************************************************/
/* Copy a buffer of length buflen to/from the srb's transfer buffer.
- * Update the index and offset variables so that the next copy will
+ * (Note: for scatter-gather transfers (srb->use_sg > 0), srb->request_buffer
+ * points to a list of s-g entries and we ignore srb->request_bufflen.
+ * For non-scatter-gather transfers, srb->request_buffer points to the
+ * transfer buffer itself and srb->request_bufflen is the buffer's length.)
+ * Update the *index and *offset variables so that the next copy will
* pick up from where this one left off. */
unsigned int usb_stor_access_xfer_buf(unsigned char *buffer,
@@ -242,6 +246,8 @@
{
unsigned int cnt;
+ /* If not using scatter-gather, just transfer the data directly.
+ * Make certain it will fit in the available buffer space. */
if (srb->use_sg == 0) {
if (*offset >= srb->request_bufflen)
return 0;
@@ -254,11 +260,22 @@
*offset, cnt);
*offset += cnt;
+ /* Using scatter-gather. We have to go through the list one entry
+ * at a time. Each s-g entry contains some number of pages, and
+ * each page has to be kmap()'ed separately. If the page is already
+ * in kernel-addressable memory then kmap() will return its address.
+ * If the page is not directly accessible -- such as a user buffer
+ * located in high memory -- then kmap() will map it to a temporary
+ * position in the kernel's virtual address space. */
} else {
struct scatterlist *sg =
(struct scatterlist *) srb->request_buffer
+ *index;
+ /* This loop handles a single s-g list entry, which may
+ * include multiple pages. Find the initial page structure
+ * and the starting offset within the page, and update
+ * the *offset and *index values for the next loop. */
cnt = 0;
while (cnt < buflen && *index < srb->use_sg) {
struct page *page = sg->page +
@@ -268,14 +285,21 @@
unsigned int sglen = sg->length - *offset;
if (sglen > buflen - cnt) {
+
+ /* Transfer ends within this s-g entry */
sglen = buflen - cnt;
*offset += sglen;
} else {
+
+ /* Transfer continues to next s-g entry */
*offset = 0;
- ++sg;
++*index;
+ ++sg;
}
+ /* Transfer the data for all the pages in this
+ * s-g entry. For each page: call kmap(), do the
+ * transfer, and call kunmap() immediately after. */
while (sglen > 0) {
unsigned int plen = min(sglen, (unsigned int)
PAGE_SIZE - poff);
@@ -286,6 +310,8 @@
else
memcpy(buffer + cnt, ptr + poff, plen);
kunmap(page);
+
+ /* Start at the beginning of the next page */
poff = 0;
++page;
cnt += plen;
@@ -293,11 +319,13 @@
}
}
}
+
+ /* Return the amount actually transferred */
return cnt;
}
/* Store the contents of buffer into srb's transfer buffer and set the
- * residue. */
+ * SCSI residue. */
void usb_stor_set_xfer_buf(unsigned char *buffer,
unsigned int buflen, Scsi_Cmnd *srb)
{
diff -Nru a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c
--- a/drivers/usb/storage/sddr09.c Mon Dec 29 14:23:59 2003
+++ b/drivers/usb/storage/sddr09.c Mon Dec 29 14:23:59 2003
@@ -673,7 +673,11 @@
int result;
// Since we only read in one block at a time, we have to create
- // a bounce buffer if the transfer uses scatter-gather.
+ // a bounce buffer if the transfer uses scatter-gather. We will
+ // move the data a piece at a time between the bounce buffer and
+ // the actual transfer buffer. If we're not using scatter-gather,
+ // we can simply update the transfer buffer pointer to get the
+ // same effect.
if (use_sg) {
len = min(sectors, (unsigned int) info->blocksize) *
@@ -738,6 +742,8 @@
if (result != USB_STOR_TRANSPORT_GOOD)
break;
}
+
+ // Store the data (s-g) or update the pointer (!s-g)
if (use_sg)
usb_stor_access_xfer_buf(buffer, len, us->srb,
&index, &offset, TO_XFER_BUF);
@@ -918,7 +924,10 @@
// Since we don't write the user data directly to the device,
// we have to create a bounce buffer if the transfer uses
- // scatter-gather.
+ // scatter-gather. We will move the data a piece at a time
+ // between the bounce buffer and the actual transfer buffer.
+ // If we're not using scatter-gather, we can simply update
+ // the transfer buffer pointer to get the same effect.
if (use_sg) {
len = min(sectors, (unsigned int) info->blocksize) *
@@ -944,6 +953,8 @@
pages = min(sectors, info->blocksize - page);
len = (pages << info->pageshift);
+
+ // Get the data from the transfer buffer (s-g)
if (use_sg)
usb_stor_access_xfer_buf(buffer, len, us->srb,
&index, &offset, FROM_XFER_BUF);
@@ -952,6 +963,8 @@
buffer, blockbuffer);
if (result != USB_STOR_TRANSPORT_GOOD)
break;
+
+ // Update the transfer buffer pointer (!s-g)
if (!use_sg)
buffer += len;
diff -Nru a/drivers/usb/storage/sddr55.c b/drivers/usb/storage/sddr55.c
--- a/drivers/usb/storage/sddr55.c Mon Dec 29 14:23:59 2003
+++ b/drivers/usb/storage/sddr55.c Mon Dec 29 14:23:59 2003
@@ -169,7 +169,11 @@
unsigned int len, index, offset;
// Since we only read in one block at a time, we have to create
- // a bounce buffer if the transfer uses scatter-gather.
+ // a bounce buffer if the transfer uses scatter-gather. We will
+ // move the data a piece at a time between the bounce buffer and
+ // the actual transfer buffer. If we're not using scatter-gather,
+ // we can simply update the transfer buffer pointer to get the
+ // same effect.
if (use_sg) {
len = min((unsigned int) sectors,
@@ -253,6 +257,8 @@
goto leave;
}
}
+
+ // Store the data (s-g) or update the pointer (!s-g)
if (use_sg)
usb_stor_access_xfer_buf(buffer, len, us->srb,
&index, &offset, TO_XFER_BUF);
@@ -300,7 +306,11 @@
}
// Since we only write one block at a time, we have to create
- // a bounce buffer if the transfer uses scatter-gather.
+ // a bounce buffer if the transfer uses scatter-gather. We will
+ // move the data a piece at a time between the bounce buffer and
+ // the actual transfer buffer. If we're not using scatter-gather,
+ // we can simply update the transfer buffer pointer to get the
+ // same effect.
if (use_sg) {
len = min((unsigned int) sectors,
@@ -325,6 +335,8 @@
pages = min((unsigned int) sectors << info->smallpageshift,
info->blocksize - page);
len = pages << info->pageshift;
+
+ // Get the data from the transfer buffer (s-g)
if (use_sg)
usb_stor_access_xfer_buf(buffer, len, us->srb,
&index, &offset, FROM_XFER_BUF);
@@ -468,6 +480,7 @@
/* update the pba<->lba maps for new_pba */
info->pba_to_lba[new_pba] = lba % 1000;
+ // Update the transfer buffer pointer (!s-g)
if (!use_sg)
buffer += len;
page = 0;
diff -Nru a/drivers/usb/storage/shuttle_usbat.c b/drivers/usb/storage/shuttle_usbat.c
--- a/drivers/usb/storage/shuttle_usbat.c Mon Dec 29 14:23:59 2003
+++ b/drivers/usb/storage/shuttle_usbat.c Mon Dec 29 14:23:59 2003
@@ -568,6 +568,13 @@
srb->transfersize);
}
+ // Since we only read in one block at a time, we have to create
+ // a bounce buffer if the transfer uses scatter-gather. We will
+ // move the data a piece at a time between the bounce buffer and
+ // the actual transfer buffer. If we're not using scatter-gather,
+ // we can simply update the transfer buffer pointer to get the
+ // same effect.
+
len = (65535/srb->transfersize) * srb->transfersize;
US_DEBUGP("Max read is %d bytes\n", len);
len = min(len, srb->request_bufflen);
@@ -614,8 +621,7 @@
if (result != USB_STOR_TRANSPORT_GOOD)
break;
- // Transfer the received data into the srb buffer
-
+ // Store the data (s-g) or update the pointer (!s-g)
if (srb->use_sg)
usb_stor_access_xfer_buf(buffer, len, srb,
&sg_segment, &sg_offset, TO_XFER_BUF);
-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills. Sign up for IBM's
Free Linux Tutorials. Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id78&alloc_id371&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
