Hi, On 4 July 2017 at 03:15, Masahiro Yamada <yamada.masah...@socionext.com> wrote: > Hi Marek, > > > 2017-07-03 17:13 GMT+09:00 Marek Vasut <ma...@denx.de>: >> On 07/03/2017 08:12 AM, Masahiro Yamada wrote: >>> Hi Marek, Simon. >>> >>> >>> Basically, Driver Mode allows us to enable multiple drivers without >>> affecting each other >>> because drivers are configured in probe functions >>> instead of compile-time configuration by #ifdef:s >>> >>> With DM, I think it is legitimate to enable EHCI and xHCI at the same time, >>> but it is not actually true. >>> >>> >>> If CONFIG_USB_EHCI_HCD is disabled, xHCI drivers work fine, >>> but with CONFIG_USB_EHCI_HCD enabled, xHCI drivers go wrong as follows: >>> >>> => fatload usb 0 82000000 Image >>> reading Image >>> WARN halted endpoint, queueing URB anyway. >>> Unexpected XHCI event TRB, skipping... (3fb54090 00000001 13000000 01008401) >>> BUG: failure at >>> /home/masahiro/workspace/u-boot-yamada/drivers/usb/host/xhci-ring.c:489/abort_td()! >>> BUG! >>> ### ERROR ### Please RESET the board ### >>> >>> >>> Here, "Image" is larger than 10MB. >>> >>> >>> >>> I narrowed down the cause of the problem >>> in the following code in common/usb_storage.c >>> >>> >>> #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. >>> */ >>> #define USB_MAX_XFER_BLK 65535 >>> #else >>> #define USB_MAX_XFER_BLK 20 >>> #endif >>> >>> >>> >>> Of course, this ifdef is not acceptable in Driver Model, >>> so we need to fix it somehow. >>> >>> >>> I am not familiar with that part at all, >>> but I just aligned the value to the lowest common denominator (20) >>> as follows: >>> >>> >>> >>> diff --git a/common/usb_storage.c b/common/usb_storage.c >>> index 03171f74cb02..b6d16e3dead3 100644 >>> --- a/common/usb_storage.c >>> +++ b/common/usb_storage.c >>> @@ -100,16 +100,7 @@ struct us_data { >>> trans_cmnd transport; /* transport routine */ >>> }; >>> >>> -#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. >>> - */ >>> -#define USB_MAX_XFER_BLK 65535 >>> -#else >>> #define USB_MAX_XFER_BLK 20 >>> -#endif >>> >>> #ifndef CONFIG_BLK >>> static struct us_data usb_stor[USB_MAX_STOR_DEV]; >>> >>> >>> >>> With with, both EHCI and xHCI seem to work >>> but I am not sure if this is the right way to fix the problem. >>> >>> Thought? >> >> You need to set the maximum blocksize based on whether the controller >> communicating with the storage device is USB 2.0 or 3.0 , which should >> be possible to extract from the info provided by DM. > > > Thanks for your advise. > Could you tell me how to do it? > > Also, your comment sounds like the current implementation is already > crappy, but I can keep it as follows if you prefer. > > /* REVISIT: why chunk size is different? */ > max_block = usb_is_ehci(dev) ? 65535 : 20; > > I do not know how to make usb_is_ehci(), though.
You can perhaps add a new field to the private USB structure and set it to different values for each stack. > > > >> Even better would be to figure out why the xhci needs this 20 blocks >> limit and fix it though. > > If I had plenty of time, I could take a look into it. > However, it does not sound fair to require it > to fix the current problem. > > -- > Best Regards > Masahiro Yamada Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot