[U-Boot] CONFIG_USB_EHCI_HCD breaks xHCI drivers even on DM

[Marek Vasut]

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.

Even better would be to figure out why the xhci needs this 20 blocks
limit and fix it though.

-- 
Best regards,
Marek Vasut