On 9/18/19 4:13 AM, Bin Meng wrote: > On Tue, Sep 17, 2019 at 10:43 PM Marek Vasut <marek.va...@gmail.com> wrote: >> >> Due to constant influx of more and more weird and broken USB sticks, >> do as Linux does in commit 779b457f66e10de3471479373463b27fd308dc85 >> >> usb: storage: scsiglue: further describe our 240 sector limit >> >> Just so we have some sort of documentation as to why >> we limit our Mass Storage transfers to 240 sectors, >> let's update the comment to make clearer that >> devices were found that would choke with larger >> transfers. >> >> While at that, also make sure to clarify that other >> operating systems have similar, albeit different, >> limits on mass storage transfers. >> >> And reduce the maximum transfer length of USB storage to 120 kiB. >> >> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> >> Cc: Bin Meng <bmeng...@gmail.com> >> Cc: Simon Glass <s...@chromium.org> >> --- >> V2: Reshuffle the code a bit, always clamp the transfer size to 240 blocks >> --- >> common/usb_storage.c | 43 ++++++++++++++++++++++--------------------- >> 1 file changed, 22 insertions(+), 21 deletions(-) >> >> diff --git a/common/usb_storage.c b/common/usb_storage.c >> index 8c889bb1a6..e1b539a082 100644 >> --- a/common/usb_storage.c >> +++ b/common/usb_storage.c >> @@ -938,31 +938,32 @@ do_retry: >> static void usb_stor_set_max_xfer_blk(struct usb_device *udev, >> struct us_data *us) >> { >> - unsigned short blk; >> - size_t __maybe_unused size; >> - int __maybe_unused ret; >> - >> -#if !CONFIG_IS_ENABLED(DM_USB) >> -#ifdef CONFIG_USB_EHCI_HCD >> /* >> - * The U-Boot EHCI driver can handle any transfer length as long as >> - * there is enough free heap space left, but the SCSI READ(10) and >> - * WRITE(10) commands are limited to 65535 blocks. >> + * Limit the total size of a transfer to 120 KB. >> + * >> + * Some devices are known to choke with anything larger. It seems >> like >> + * the problem stems from the fact that original IDE controllers had >> + * only an 8-bit register to hold the number of sectors in one >> transfer >> + * and even those couldn't handle a full 256 sectors. >> + * >> + * Because we want to make sure we interoperate with as many devices >> as >> + * possible, we will maintain a 240 sector transfer size limit for >> USB >> + * Mass Storage devices. >> + * >> + * Tests show that other operating have similar limits with Microsoft >> + * Windows 7 limiting transfers to 128 sectors for both USB2 and USB3 >> + * and Apple Mac OS X 10.11 limiting transfers to 256 sectors for >> USB2 >> + * and 2048 for USB3 devices. >> */ >> - blk = USHRT_MAX; >> -#else >> - blk = 20; >> -#endif >> -#else >> + unsigned short blk = 240; >> + >> +#if CONFIG_IS_ENABLED(DM_USB) >> + size_t size; >> + int ret; >> + >> ret = usb_get_max_xfer_size(udev, (size_t *)&size); >> - if (ret < 0) { >> - /* unimplemented, let's use default 20 */ >> - blk = 20; >> - } else { >> - if (size > USHRT_MAX * 512) >> - size = USHRT_MAX * 512; >> + if ((ret >= 0) && (size < 240 * 512)) > > size < blk * 512 > >> blk = size / 512; >> - } >> #endif >> >> us->max_xfer_blk = blk; >> -- > > Looks good otherwise > > Reviewed-by: Bin Meng <bmeng...@gmail.com>
Fixed and added to next . This really needs a LOT of testing. I am worried about performance here. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot