Re: [U-Boot] [PATCH v5 2/2] common: usb_storage : Implement logic to calculate optimal usb maximum trasfer blocks

On 06/13/2016 06:03 AM, Rajesh Bhagat wrote:

From: Rajesh Bhagat rajesh.bhagat@freescale.com

Implements the logic to calculate the optimal usb maximum trasfer blocks instead of sending USB_MAX_XFER_BLK blocks which is 65535 and 20 in case of EHCI and other USB protocols respectively.

It defines USB_MIN_XFER_BLK/USB_MAX_XFER_BLK trasfer blocks that should be checked for success starting from minimum to maximum, and rest of the read/write are performed with that optimal value. It tries to increase/ decrease the blocks in follwing scenarios:

1.decrease blocks: when read/write for a particular number of blocks fails. 2. increase blocks: when read/write for a particular number of blocks pass and amount left to trasfer is greater than current number of blocks.

Currently changes are done for EHCI where min = 4096 and max = 65535 is taken. And for other cases code is left unchanged by keeping min = max = 20.

Signed-off-by: Sriram Dash sriram.dash@nxp.com Signed-off-by: Rajesh Bhagat rajesh.bhagat@nxp.com

---

Changes in v5:

• None

Changes in v4:

• Adds udev paramater in dec/inc_cur_xfer_blks function and adds sanity check on it.
• Changes type of pos varible to unsigned int in dec/inc_cur_xfer_blks
• Removes usage of pos varible from usb_stor_read/write

Changes in v3:

• Adds cur_xfer_blks in struct usb_device to retain values
• Adds functions dec/inc_cur_xfer_blks to remove code duplication
• Moves check from macro to calling functions

Changes in v2:

• Removes table to store blocks and use formula (1 << (12 + n)) - 1
• Adds logic to start from minimum, go to maximum in each read/write

common/usb_storage.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++--- include/usb.h | 1 + 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/common/usb_storage.c b/common/usb_storage.c index f060637..9b09412 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -106,11 +106,16 @@ struct us_data {

• enough free heap space left, but the SCSI READ(10) and WRITE(10) commands are
• limited to 65535 blocks.

*/ +#define USB_MIN_XFER_BLK 4095 #define USB_MAX_XFER_BLK 65535 #else +#define USB_MIN_XFER_BLK 20 #define USB_MAX_XFER_BLK 20 #endif

+#define GET_CUR_XFER_BLKS(blks) (LOG2((blks + 1) / (USB_MIN_XFER_BLK + 1))) +#define CALC_CUR_XFER_BLKS(pos) ((1 << (12 + pos)) - 1)

Parenthesis around variables are missing.

#ifndef CONFIG_BLK static struct us_data usb_stor[USB_MAX_STOR_DEV]; #endif @@ -141,6 +146,44 @@ static void usb_show_progress(void) debug("."); }

+static int dec_cur_xfer_blks(struct usb_device *udev) +{

• /* decrease the cur_xfer_blks */
• unsigned int pos;
• unsigned short size;
• if (!udev)

• return -EINVAL;
  

• pos = GET_CUR_XFER_BLKS(udev->cur_xfer_blks);
• size = pos ? CALC_CUR_XFER_BLKS(pos - 1) : 0;

If pos == 0 , the condition below will be true (size will be 2047), so this extra ternary is unnecessary.

• if (size < USB_MIN_XFER_BLK)

• return -EINVAL;
  

• udev->cur_xfer_blks = size;
• return 0;

+}

+static int inc_cur_xfer_blks(struct usb_device *udev, lbaint_t blks) +{

• /* try to increase the cur_xfer_blks */
• unsigned int pos;
• unsigned short size;
• if (!udev)

• return -EINVAL;
  

• pos = GET_CUR_XFER_BLKS(udev->cur_xfer_blks);
• size = CALC_CUR_XFER_BLKS(pos + 1);
• if (size > blks || size > USB_MAX_XFER_BLK)

• return -EINVAL;
  

Why don't you clamp the size to min(blks, USB_MAX_XFER_BLK) instead ?

• udev->cur_xfer_blks = size;
• return 0;

+}

/*******************************************************************************

• show info on storage devices; 'usb start/init' must be invoked earlier
• as we only retrieve structures populated during devices initialization

@@ -1128,6 +1171,7 @@ static unsigned long usb_stor_read_write(struct blk_desc *block_dev, struct usb_device *udev; struct us_data *ss; int retry;

• bool retry_flag = false; ccb *srb = &usb_ccb;

#ifdef CONFIG_BLK struct blk_desc *block_dev; @@ -1167,26 +1211,36 @@ static unsigned long usb_stor_read_write(struct blk_desc *block_dev, */ retry = 2; srb->pdata = (unsigned char *)buf_addr;

• if (blks > USB_MAX_XFER_BLK)
  

• 	smallblks = USB_MAX_XFER_BLK;
  

• if (blks > udev->cur_xfer_blks)
  

• 	smallblks = udev->cur_xfer_blks;
  

  else smallblks = (unsigned short) blks;

retry_it:

• if (smallblks == USB_MAX_XFER_BLK)
  

• debug("usb_%s: retry #%d, cur_xfer_blks %hu, smallblks %hu\n",
  

•       USB_STOR_OP_TYPE, retry, udev->cur_xfer_blks,
  

•       smallblks);
  

• if (smallblks == udev->cur_xfer_blks)
  usb_show_progress();
  

  srb->datalen = block_dev->blksz * smallblks; srb->pdata = (unsigned char *)buf_addr; if (usb_stor_operation(srb, ss, start, smallblks, is_write)) { debug("%s ERROR\n", USB_STOR_OP_TYPE); usb_request_sense(srb, ss);

• 	if (retry--)
  

• 	if (retry--) {
  

• 		if (!dec_cur_xfer_blks(udev))
  

• 			smallblks = udev->cur_xfer_blks;
  

• 		retry_flag = true;
  	goto retry_it;
  

• 	}
  blkcnt -= blks;
  break;
  

  } start += smallblks; blks -= smallblks; buf_addr += srb->datalen;

• if (!retry_flag && !inc_cur_xfer_blks(udev, blks))
  

• 	smallblks = udev->cur_xfer_blks;
  

  } while (blks != 0); ss->flags &= ~USB_READY;

@@ -1195,7 +1249,7 @@ retry_it: USB_STOR_OP_TYPE, start, smallblks, buf_addr);

usb_disable_asynch(0); /* asynch transfer allowed */

• if (blkcnt >= USB_MAX_XFER_BLK)

• if (blkcnt >= udev->cur_xfer_blks) debug("\n"); return blkcnt;

} @@ -1282,6 +1336,9 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, break; }

• /* Initialize the current transfer blocks to minimum value */
• dev->cur_xfer_blks = USB_MIN_XFER_BLK;
• /*
  • We are expecting a minimum of 2 endpoints - in and out (bulk).
  • An optional interrupt is OK (necessary for CBI protocol).

diff --git a/include/usb.h b/include/usb.h index 02a0ccd..b815816 100644 --- a/include/usb.h +++ b/include/usb.h @@ -153,6 +153,7 @@ struct usb_device { struct udevice *dev; /* Pointer to associated device */ struct udevice *controller_dev; /* Pointer to associated controller */ #endif

• unsigned short cur_xfer_blks; /* Current maximum transfer blocks */

};

struct int_queue;