On Mon, Sep 16, 2019 at 4:10 PM Marek Vasut <marek.va...@gmail.com> wrote: > > On 9/16/19 8:53 AM, Bin Meng wrote: > > On Mon, Sep 16, 2019 at 6:24 AM Marek Vasut 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. > >> > >> --- > >> common/usb_storage.c | 27 +++++++++++++++++---------- > >> 1 file changed, 17 insertions(+), 10 deletions(-) > >> > >> diff --git a/common/usb_storage.c b/common/usb_storage.c > >> index 8c889bb1a6..130eab7832 100644 > >> --- a/common/usb_storage.c > >> +++ b/common/usb_storage.c > >> @@ -943,21 +943,28 @@ static void usb_stor_set_max_xfer_blk(struct > >> usb_device *udev, > >> 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 > >> + blk = 240; > >> #else > >> ret = usb_get_max_xfer_size(udev, (size_t *)&size); > > > > If we blindly set blk to 240 for both non-dm usb and dm usb here, > > Blindly ?
So based on the comments you added (also in kernel commit 779b457f66e1), it looks to me we have to set 240 for whatever usb storage devices. That suggests if usb_get_max_xfer_size() is implemented by DM USB drivers (so far only EHCI and xHCI implemented it) and return a value larger than 120KiB, some usb storage devices is subject to break. > > > which makes usb_get_max_xfer_size() useless. Should we completely drop > > the usb_get_max_xfer_size() call? > > No, I think we should keep it if we ever decided to start implementing > quirks like US_FL_MAX_SECTORS_64 (cfr. Linux scsiglue.c). > > >> if (ret < 0) { > >> - /* unimplemented, let's use default 20 */ > >> - blk = 20; > >> + blk = 240; > >> } else { > >> if (size > USHRT_MAX * 512) > >> size = USHRT_MAX * 512; > >> -- Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot