[U-Boot] [PATCH v2] usb: fix usb_stor_read/write on DM
Marek Vasut
marex at denx.de
Fri Jul 14 10:07:06 UTC 2017
On 07/14/2017 04:31 AM, Masahiro Yamada wrote:
> Prior to DM, we could not enable different types of USB controllers
> at the same time. DM was supposed to loosen the limitation. It is
> true that we can compile drivers, but they do not work.
>
> For example, if EHCI is enabled, xHCI fails as follows:
>
> => usb read 82000000 0 2000
>
> USB read: device 0 block # 0, count 8192 ... WARN halted endpoint, queueing URB anyway.
> Unexpected XHCI event TRB, skipping... (3fb54010 00000001 13000000 01008401)
> BUG: failure at drivers/usb/host/xhci-ring.c:489/abort_td()!
> BUG!
> ### ERROR ### Please RESET the board ###
>
> The cause of the error seems the following code:
>
> #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
>
> To fix the problem, choose the chunk size at run-time for CONFIG_BLK.
What happens if CONFIG_BLK is not set ?
Why is it 20 for XHCI anyway ?
> Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com>
> ---
>
> Changes in v2:
> - Run-time detection of host if
>
> common/usb_storage.c | 30 ++++++++++++++++++++----------
> drivers/usb/host/ehci-hcd.c | 1 +
> drivers/usb/host/ohci-hcd.c | 1 +
> drivers/usb/host/xhci.c | 1 +
> include/usb.h | 9 +++++++++
> 5 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index df0b05730879..d17b12639cad 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -1109,7 +1109,7 @@ static unsigned long usb_stor_read(struct blk_desc *block_dev, lbaint_t blknr,
> lbaint_t blkcnt, void *buffer)
> #endif
> {
> - lbaint_t start, blks;
> + lbaint_t start, blks, max_xfer_blk;
> uintptr_t buf_addr;
> unsigned short smallblks;
> struct usb_device *udev;
> @@ -1118,6 +1118,7 @@ static unsigned long usb_stor_read(struct blk_desc *block_dev, lbaint_t blknr,
> struct scsi_cmd *srb = &usb_ccb;
> #ifdef CONFIG_BLK
> struct blk_desc *block_dev;
> + struct usb_bus_priv *bus_priv;
> #endif
>
> if (blkcnt == 0)
> @@ -1127,6 +1128,9 @@ static unsigned long usb_stor_read(struct blk_desc *block_dev, lbaint_t blknr,
> block_dev = dev_get_uclass_platdata(dev);
> udev = dev_get_parent_priv(dev_get_parent(dev));
> debug("\nusb_read: udev %d\n", block_dev->devnum);
> +
> + bus_priv = dev_get_uclass_priv(dev_get_parent(dev_get_parent(dev_get_parent(dev))));
> + max_xfer_blk = bus_priv->host_if == USB_HOST_EHCI ? 65535 : 20;
> #else
> debug("\nusb_read: udev %d\n", block_dev->devnum);
> udev = usb_dev_desc[block_dev->devnum].priv;
> @@ -1134,6 +1138,7 @@ static unsigned long usb_stor_read(struct blk_desc *block_dev, lbaint_t blknr,
> debug("%s: No device\n", __func__);
> return 0;
> }
> + max_xfer_blk = USB_MAX_XFER_BLK;
> #endif
> ss = (struct us_data *)udev->privptr;
>
> @@ -1150,12 +1155,12 @@ static unsigned long usb_stor_read(struct blk_desc *block_dev, lbaint_t blknr,
> /* XXX need some comment here */
> retry = 2;
> srb->pdata = (unsigned char *)buf_addr;
> - if (blks > USB_MAX_XFER_BLK)
> - smallblks = USB_MAX_XFER_BLK;
> + if (blks > max_xfer_blk)
> + smallblks = max_xfer_blk;
> else
> smallblks = (unsigned short) blks;
> retry_it:
> - if (smallblks == USB_MAX_XFER_BLK)
> + if (smallblks == max_xfer_blk)
> usb_show_progress();
> srb->datalen = block_dev->blksz * smallblks;
> srb->pdata = (unsigned char *)buf_addr;
> @@ -1178,7 +1183,7 @@ retry_it:
> start, smallblks, buf_addr);
>
> usb_disable_asynch(0); /* asynch transfer allowed */
> - if (blkcnt >= USB_MAX_XFER_BLK)
> + if (blkcnt >= max_xfer_blk)
> debug("\n");
> return blkcnt;
> }
> @@ -1191,7 +1196,7 @@ static unsigned long usb_stor_write(struct blk_desc *block_dev, lbaint_t blknr,
> lbaint_t blkcnt, const void *buffer)
> #endif
> {
> - lbaint_t start, blks;
> + lbaint_t start, blks, max_xfer_blk;
> uintptr_t buf_addr;
> unsigned short smallblks;
> struct usb_device *udev;
> @@ -1200,6 +1205,7 @@ static unsigned long usb_stor_write(struct blk_desc *block_dev, lbaint_t blknr,
> struct scsi_cmd *srb = &usb_ccb;
> #ifdef CONFIG_BLK
> struct blk_desc *block_dev;
> + struct usb_bus_priv *bus_priv;
> #endif
>
> if (blkcnt == 0)
> @@ -1210,6 +1216,9 @@ static unsigned long usb_stor_write(struct blk_desc *block_dev, lbaint_t blknr,
> block_dev = dev_get_uclass_platdata(dev);
> udev = dev_get_parent_priv(dev_get_parent(dev));
> debug("\nusb_read: udev %d\n", block_dev->devnum);
> +
> + bus_priv = dev_get_uclass_priv(dev_get_parent(dev_get_parent(dev_get_parent(dev))));
> + max_xfer_blk = bus_priv->host_if == USB_HOST_EHCI ? 65535 : 20;
> #else
> debug("\nusb_read: udev %d\n", block_dev->devnum);
> udev = usb_dev_desc[block_dev->devnum].priv;
> @@ -1217,6 +1226,7 @@ static unsigned long usb_stor_write(struct blk_desc *block_dev, lbaint_t blknr,
> debug("%s: No device\n", __func__);
> return 0;
> }
> + max_xfer_blk = USB_MAX_XFER_BLK;
> #endif
> ss = (struct us_data *)udev->privptr;
>
> @@ -1236,12 +1246,12 @@ static unsigned long usb_stor_write(struct blk_desc *block_dev, lbaint_t blknr,
> */
> retry = 2;
> srb->pdata = (unsigned char *)buf_addr;
> - if (blks > USB_MAX_XFER_BLK)
> - smallblks = USB_MAX_XFER_BLK;
> + if (blks > max_xfer_blk)
> + smallblks = max_xfer_blk;
> else
> smallblks = (unsigned short) blks;
> retry_it:
> - if (smallblks == USB_MAX_XFER_BLK)
> + if (smallblks == max_xfer_blk)
> usb_show_progress();
> srb->datalen = block_dev->blksz * smallblks;
> srb->pdata = (unsigned char *)buf_addr;
> @@ -1263,7 +1273,7 @@ retry_it:
> PRIxPTR "\n", start, smallblks, buf_addr);
>
> usb_disable_asynch(0); /* asynch transfer allowed */
> - if (blkcnt >= USB_MAX_XFER_BLK)
> + if (blkcnt >= max_xfer_blk)
> debug("\n");
> return blkcnt;
>
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index f08709d0212d..b55961954a33 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -1604,6 +1604,7 @@ int ehci_register(struct udevice *dev, struct ehci_hccr *hccr,
> dev->name, ctrl, hccr, hcor, init);
>
> priv->desc_before_addr = true;
> + priv->host_if = USB_HOST_EHCI;
>
> ehci_setup_ops(ctrl, ops);
> ctrl->hccr = hccr;
> diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
> index fdfc870efa3e..d877f2bb0bbf 100644
> --- a/drivers/usb/host/ohci-hcd.c
> +++ b/drivers/usb/host/ohci-hcd.c
> @@ -2193,6 +2193,7 @@ int ohci_register(struct udevice *dev, struct ohci_regs *regs)
> u32 reg;
>
> priv->desc_before_addr = true;
> + priv->host_if = USB_HOST_OHCI;
>
> ohci->regs = regs;
> ohci->hcca = memalign(256, sizeof(struct ohci_hcca));
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index bd8f4c43b4b2..2df78f29cd78 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1207,6 +1207,7 @@ int xhci_register(struct udevice *dev, struct xhci_hccr *hccr,
> * of that is done for XHCI unlike EHCI.
> */
> priv->desc_before_addr = false;
> + priv->host_if = USB_HOST_XHCI;
>
> ret = xhci_reset(hcor);
> if (ret)
> diff --git a/include/usb.h b/include/usb.h
> index 62f051fe535c..675761edfc08 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -604,6 +604,14 @@ struct usb_dev_platdata {
> int configno;
> };
>
> +enum usb_host_if {
> + USB_HOST_UNKNOWN,
> + USB_HOST_OHCI,
> + USB_HOST_UHCI,
> + USB_HOST_EHCI,
> + USB_HOST_XHCI,
> +};
> +
> /**
> * struct usb_bus_priv - information about the USB controller
> *
> @@ -623,6 +631,7 @@ struct usb_bus_priv {
> int next_addr;
> bool desc_before_addr;
> bool companion;
> + enum usb_host_if host_if;
> };
>
> /**
>
--
Best regards,
Marek Vasut
More information about the U-Boot
mailing list