On 07/22/2016 01:59 AM, Matthew Bright wrote: > On 07/22/2016 01:24 AM, Rajesh Bhagat wrote:
Hi, >>> -----Original Message----- >>> From: Marek Vasut [mailto:marex at denx.de] >>> Sent: Thursday, July 21, 2016 5:13 PM >>> To: Rajesh Bhagat <rajesh.bhagat at nxp.com>; Matthew Bright >>> <Matthew.Bright at alliedtelesis.co.nz> >>> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham at >>> alliedtelesis.co.nz>; Mark >>> Tomlinson <Mark.Tomlinson at alliedtelesis.co.nz> >>> Subject: Re: testing: [PATCH v7 0/3] common: usb_storage: Implement logic to >>> calculate optimal usb maximum trasfer blocks >>> >>> On 07/21/2016 10:08 AM, Rajesh Bhagat wrote: >>>> Hello Marek, >>>> >>>> Any comments? >> >> Hello Marek, >> >>> >>> If I recall correctly, this broke things for Matthew. >>> Is this resolved ? >>> >> >> Let me try to explain the exact scenario, Matthew Please correct me if this >> is not the case: >> >> 1. Matthew performed some testing on some pen drives. >> 2. Matthew shared some review comments on the patch. >> >> Regarding item#1, He is facing issue when some of the pen drives are __not__ >> recovering once >> Y number blocks are sent, which is increased when X number of blocks passed >> (where Y > X). > > This is correct. > > It appears that some usb storage devices can only be recovered by resetting > the > ehci interface. Unfortunately resetting the ehci interface part way through a > usb transfer is not a practical solution. So what is the solution here ? > http://lists.denx.de/pipermail/u-boot/2016-June/258838.html > >> This is true for some of the pen drives, though a large number of pen drives >> (which I also tested) Last time this large number was 3 or so, did that change ? >> are passing which were not passing when MAX_XFER_BLKS was hardcoded. To add >> to it, the pen drives failing >> i.e. not recovering were not passing previously (prior to this patch) also. > > This is correct. > > However I suggest that we can extent our test coverage to obtain a more > representative sample. There are several other people who have reported > being able to reproduce the same echi timeout issue. > > http://lists.denx.de/pipermail/u-boot/2016-June/258905.html > >> Hence, the new logic is making many pen drives better, and some pen drives >> which were not working >> with previous logic are failing due to no recovery of HW with new logic >> also. > > This is correct. > > This patch does not *NOT* introduce any regressions and provides a recovery > mechanism that appears to work with the majority of usb storage devices. > >> Regarding item#2, He provided some review comments which are valid, and >> suggested it to be applied on >> XHCI also which I tested after enabling CONFIG_DM_USB. Please refer below >> (my last comments). >> >>>>> When both EHCI and XHCI are defined using CONFIG_DM_USB, the value of >>>>> USB_MAX_XFER_BLK becomes 65535 according to current implementation. >>>>> And is causing read/write issues in XHCI, though EHCI is working fine. > > I was expecting to see a v8 patch based on the review comments. > Not all comments were specific to the xhci interface. > > http://lists.denx.de/pipermail/u-boot/2016-June/258839.html OK, then V8 please. >> IMHO, we should work on it to close it. Please share your opinion. > > Agreed. > >> Best Regards, >> Rajesh Bhagat >>>>> -----Original Message----- >>>>> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of >>>>> Rajesh Bhagat >>>>> Sent: Tuesday, June 28, 2016 12:14 PM >>>>> To: Matthew Bright <Matthew.Bright at alliedtelesis.co.nz>; Marek Vasut >>>>> <marex at denx.de> >>>>> Cc: u-boot at lists.denx.de; Chris Packham >>>>> <Chris.Packham at alliedtelesis.co.nz>; Mark Tomlinson >>>>> <Mark.Tomlinson at alliedtelesis.co.nz> >>>>> Subject: Re: [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: >>>>> Implement logic to calculate optimal usb maximum trasfer blocks >>>>> >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Matthew Bright [mailto:Matthew.Bright at alliedtelesis.co.nz] >>>>>> Sent: Thursday, June 23, 2016 8:23 AM >>>>>> To: Marek Vasut <marex at denx.de>; Rajesh Bhagat >>>>>> <rajesh.bhagat at nxp.com> >>>>>> Cc: u-boot at lists.denx.de; Chris Packham >>>>>> <Chris.Packham at alliedtelesis.co.nz>; Mark Tomlinson >>>>>> <Mark.Tomlinson at alliedtelesis.co.nz> >>>>>> Subject: Re: testing: [PATCH v7 0/3] common: usb_storage: Implement >>>>>> logic to calculate optimal usb maximum trasfer blocks >>>>>> >>>>>> On 06/23/2016 01:02 AM, Marek Vasut wrote: >>>>>>> >>>>>>> On 06/22/2016 08:36 AM, Rajesh Bhagat wrote: >>>>>>>> >>>>>>>> From: Matthew Bright [mailto:Matthew.Bright at alliedtelesis.co.nz] >>>>>>>> Sent: Wednesday, June 22, 2016 11:42 AM >>>>>>>> To: Rajesh Bhagat <rajesh.bhagat at nxp.com>; marex at denx.de >>>>>>>> Cc: u-boot at lists.denx.de; Chris Packham >>>>>>>> <Chris.Packham at alliedtelesis.co.nz>; Mark Tomlinson >>>>>>>> <Mark.Tomlinson at alliedtelesis.co.nz> >>>>>>>> Subject: testing: [PATCH v7 0/3] common: usb_storage: Implement >>>>>>>> logic to calculate optimal usb maximum trasfer blocks >>>>>>>> >>>>>>>> On 06/16/2016 12:35 PM, Rajesh Bhagat wrote: >>>>>>>>> Performs code cleanup by making common function for >>>>>>>>> usb_stor_read/write and 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. >>>>>>>>> >>>>>>>>> Rajesh Bhagat (3): >>>>>>>>> common: usb_storage: Make common function for >>>>> usb_read_10/usb_write_10 >>>>>>>>> common: usb_storage: Make common function for >>>>>>>>> usb_stor_read/usb_stor_write >>>>>>>>> common: usb_storage : Implement logic to calculate optimal usb >>> maximum >>>>>>>>> trasfer blocks >>>>>>>>> >>>>>>>>> common/usb_storage.c | 213 +++++++++++++++++++++++-------- >>> --- >>>>> --- >>>>>> -------------- >>>>>>>>> include/usb.h | 1 + >>>>>>>>> 2 files changed, 98 insertions(+), 116 deletions(-) >>>>>>>>> >>>>>>>>> >>>>>>>>> Hi Rajesh & Marek >>>>>>>>> >>>>>>>>> I have spend the last couple of days testing these patches on the >>>>>>>>> v2016.05 release, with an usb mass storage device that is able to >>>>>>>>> consistently reproduce the USB_MAX_XFER_BLK issue as described in >>>>>>>>> the "Issue with USB mass storage (thumb drives)" u-boot thread. >>>>>>>>> >>>>>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/244464.html​ >>>>>>>>> >>>>>>>> >>>>>>>> Hello Matt, >>>>>>>> >>>>>>>>> I can confirm the patch correctly increases the max transfer >>>>>>>>> bocks after a successful read, and decreases the max transfer >>>>>>>>> bocks after a read failure. However, I have noticed that once the >>>>>>>>> ehci time out error occurs, the usb device appears to lock up. >>>>>>>>> When in this state the usb device will stop responding to any >>>>>>>>> further transfers. This behavior is independent of the number of >>>>>>>>> blocks, and will continue until the ehci has been reset. >>>>>>>>> >>>>>>>> >>>>>>>> I believe the lockup behavior mentioned by you to be device specific >>>>>>>> quirk. >>>>>>>> I tested 3 pen drives, which recovered from EHCI timeout behavior >>>>>>>> by reducing the number of blocks (check below output): >>>>>>>> >>>>>>> >>>>>>> 3 devices is not a representative sample. >>>>>>> >>>>>>> -- >>>>>>> Best regards, >>>>>>> Marek Vasut >>>>>> >>>>>> I agree. >>>>>> >>>>>> Several others on the u-boot threads have also reported the same >>>>>> ehci time out issue related to the max transfer blocks. Perhaps we >>>>>> could kindly ask if they could also test the patch ... >>>>>> >>>>>> Schrempf Frieder >>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/244464.html >>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/245893.html >>>>>> >>>>>> Hannes Schmelzer (hannes at schmelzer.or.at) >>>>>> http://lists.denx.de/pipermail/u- boot/2016-February/244621.html >>>>>> >>>>>> Maxime Jayat >>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/246267.html >>>>>> >>>>>> Diego >>>>>> http://lists.denx.de/pipermail/u-boot/2016-April/251799.html >>>>>> >>>>>> Nicolas Chauvet >>>>>> http://lists.denx.de/pipermail/u-boot/2016-May/256117.html >>>>>> >>>>>> As a side note, there appears to be a subtle a difference in the >>>>>> output when the usb device locks up: >>>>>> >>>>>> CASE 1: EHCI Time Out - Device Remains Responsive: >>>>>> >>>>>> => fatload usb 0 0x18000000 test.file EHCI timed out on TD - >>>>>> token=0x2c008d80 EHCI timed out on TD - >>>>>> token=0xac008d80 EHCI timed out on TD - token=0xac008d80 Error >>>>>> reading cluster >>>>>> ** Unable to read file test.file ** >>>>>> => >>>>>> >>>>>> The three time out errors are caused by three attempts to send the >>>>>> over-sized transfer before giving up. >>>>>> >>>>>> CASE 2: EHCI Time Out - Device Locks Up: >>>>>> >>>>>> => fatload usb 0:auto 0x1000000 test.rel reading test.rel EHCI timed >>>>>> out on TD - >>>>>> token=0x26008d80 EHCI timed out on TD - token=0x80008d80 EHCI timed >>>>>> out on TD >>>>>> - token=0x80008d80 EHCI timed out on TD - token=0x80008d80 EHCI >>>>>> timed out on TD - token=0x80008d80 EHCI timed out on TD - >>>>>> token=0x80008d80 EHCI timed out on TD - token=0x80008d80 EHCI timed >>>>>> out on TD - >>>>>> token=0x80008d80 EHCI timed out on TD - token=0x80008d80 EHCI timed >>>>>> out on TD - token=0x801f8c80 EHCI timed out on TD - token=0x80008d80 >>>>>> EHCI timed out on TD - token=0x80008d80 EHCI timed out on TD - >>>>>> token=0x80008d80 EHCI timed out on TD - token=0x801f8c80 EHCI timed >>>>>> out on TD - token=0x80008d80 EHCI timed out on TD - token=0x80008d80 >>>>>> EHCI timed out on TD - token=0x80008d80 EHCI timed out on TD - >>>>>> token=0x801f8c80 EHCI timed out on TD - token=0x80008d80 EHCI timed >>>>>> out on TD - >>>>>> token=0x80008d80 EHCI timed out on TD - token=0x80008d80 EHCI timed >>>>>> out on TD >>>>>> - token=0x801f8c80 EHCI timed out on TD - token=0x80008d80 EHCI >>>>>> timed out on TD >>>>>> - token=0x80008d80 EHCI timed out on TD - token=0x80008d80 Error >>>>>> reading cluster >>>>>> ** Unable to read file test.rel ** >>>>>> => >>>>>> >>>>>> The additional time out errors are caused because the usb device >>>>>> also fails to respond to several reset messages after the initial >>>>>> time out; therefore providing a clear indication that the device has >>>>>> locked up. >>>>>> >>>>>> The usb device in the initial thread appears to exhibit such behavior: >>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/244464.html >>>>>> >>>>> >>>>> Hello Matt/Marek, >>>>> >>>>> I'm working on enabling CONFIG_DM_USB for EHCI and XHCI controllers >>>>> and found one more issue which this patch is solving. >>>>> >>>>> When both EHCI and XHCI are defined using CONFIG_DM_USB, the value of >>>>> USB_MAX_XFER_BLK becomes 65535 according to current implementation. >>>>> And is causing read/write issues in XHCI, though EHCI is working fine. >>>>> (Please check below logs). >>>>> >>>>> Hence, extending this patch from EHCI to XHCI is very much required >>>>> as pointed out by Matt. When I apply this patch after extending it >>>>> for XHCI also, below problem is solved (check observations below). >>>>> >>>>> >>>>> Observations: >>>>> 1. "usb start" correctly detected two devices one in high speed and >>>>> one in super speed on EHCI and XHCI controller respectively . >>>>> >>>>> => usb start >>>>> starting USB... >>>>> USB0: USB EHCI 1.00 >>>>> USB1: Register 200017f NbrPorts 2 >>>>> Starting the controller >>>>> USB XHCI 1.00 >>>>> scanning bus 0 for devices... 2 USB Device(s) found scanning bus 1 >>>>> for devices... 2 USB Device(s) found >>>>> >>>>> 2. Read/write on high speed device on EHCI port worked fine. >>>>> >>>>> => usb dev 0 >>>>> >>>>> USB device 0: >>>>> Device 0: Vendor: SanDisk Rev: 1.26 Prod: Cruzer Blade >>>>> Type: Hard Disk >>>>> Capacity: 7633.5 MB = 7.4 GB (15633408 x 512) ... is now >>>>> current device => mw 81000000 55555555 800000; mw a0000000 aaaaaaaa >>>>> 800000; usb write >>>>> a0000000 0 10000; usb read 81000000 0 10000; cmp.b a0000000 81000000 >>>>> 2000000; >>>>> >>>>> USB write: device 0 block # 0, count 65536 ... 65536 blocks write: OK >>>>> >>>>> USB read: device 0 block # 0, count 65536 ... 65536 blocks read: OK >>>>> Total of >>>>> 33554432 byte(s) were the same >>>>> >>>>> 3. Read/write on super speed device on XHCI port, I'm getting below >>>>> error >>>>> >>>>> => usb dev 1 >>>>> USB device 1: >>>>> Device 1: Vendor: JetFlash Rev: 1100 Prod: Transcend 16GB >>>>> Type: Removable Hard Disk >>>>> Capacity: 15360.0 MB = 15.0 GB (31457280 x 512) ... is >>>>> now current device => mw 81000000 55555555 800000; mw a0000000 >>>>> aaaaaaaa 800000; usb write >>>>> a0000000 0 10000; usb read 81000000 0 10000; cmp.b a0000000 81000000 >>>>> 2000000; >>>>> >>>>> USB write: device 1 block # 0, count 65536 ... WARN halted endpoint, >>>>> queueing URB anyway. >>>>> Unexpected XHCI event TRB, skipping... (fef27250 00000000 13000000 >>>>> 01008401) >>>>> BUG: failure at drivers/usb/host/xhci-ring.c:489/abort_td()! >>>>> BUG! >>>>> >>>>> >>>>> Best Regards, >>>>> Rajesh Bhagat >>>>> >>>>>> Cheers. >>>>>> - Matt Bright >>>>> _______________________________________________ >>>>> U-Boot mailing list >>>>> U-Boot at lists.denx.de >>>>> http://lists.denx.de/mailman/listinfo/u-boot >>>> >>>> Best Regards, >>>> Rajesh Bhagat >>>> >>> >>> >>> -- >>> Best regards, >>> Marek Vasut -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
