Dear Benoît Thébaudeau,

> Relax the qTD transfer alignment constraints in order to need less qTDs for
> buffers that are aligned to 512 bytes but not to pages.
> 
> Signed-off-by: Benoît Thébaudeau <benoit.thebaud...@advansee.com>
> Cc: Marek Vasut <ma...@denx.de>
> Cc: Ilya Yanok <ilya.ya...@cogentembedded.com>
> Cc: Stefan Herbrechtsmeier <ste...@herbrechtsmeier.net>
> ---
> Changes for v2: N/A.
> Changes for v3:
>  - New patch.
> 
>  .../drivers/usb/host/ehci-hcd.c                    |   68
> +++++++++++--------- 1 file changed, 38 insertions(+), 30 deletions(-)
> 
> diff --git u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c
> u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c index 84c7d08..37517cb
> 100644
> --- u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c
> +++ u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c
> @@ -215,7 +215,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long
> pipe, void *buffer, volatile struct qTD *vtd;
>       unsigned long ts;
>       uint32_t *tdp;
> -     uint32_t endpt, token, usbsts;
> +     uint32_t endpt, maxpacket, token, usbsts;
>       uint32_t c, toggle;
>       uint32_t cmd;
>       int timeout;
> @@ -230,6 +230,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long
> pipe, void *buffer, le16_to_cpu(req->value), le16_to_cpu(req->value),
>                     le16_to_cpu(req->index));
> 
> +#define PKT_ALIGN    512

Make this const int maybe ?

>       /*
>        * The USB transfer is split into qTD transfers. Eeach qTD transfer is
>        * described by a transfer descriptor (the qTD). The qTDs form a linked
> @@ -251,43 +252,41 @@ ehci_submit_async(struct usb_device *dev, unsigned
> long pipe, void *buffer, if (length > 0 || req == NULL) {
>               /*
>                * Determine the qTD transfer size that will be used for the
> -              * data payload (not considering the final qTD transfer, which
> -              * may be shorter).
> +              * data payload (not considering the first qTD transfer, which
> +              * may be longer or shorter, and the final one, which may be
> +              * shorter).
>                *
>                * In order to keep each packet within a qTD transfer, the qTD
> -              * transfer size is aligned to EHCI_PAGE_SIZE, which is a
> -              * multiple of wMaxPacketSize (except in some cases for
> -              * interrupt transfers, see comment in submit_int_msg()).
> +              * transfer size is aligned to PKT_ALIGN, which is a multiple of
> +              * wMaxPacketSize (except in some cases for interrupt transfers,
> +              * see comment in submit_int_msg()).
>                *
> -              * By default, i.e. if the input buffer is page-aligned,
> +              * By default, i.e. if the input buffer is aligned to PKT_ALIGN,
>                * QT_BUFFER_CNT full pages will be used.
>                */
>               int xfr_sz = QT_BUFFER_CNT;
>               /*
> -              * However, if the input buffer is not page-aligned, the qTD
> -              * transfer size will be one page shorter, and the first qTD
> +              * However, if the input buffer is not aligned to PKT_ALIGN, the
> +              * qTD transfer size will be one page shorter, and the first qTD
>                * data buffer of each transfer will be page-unaligned.
>                */
> -             if ((uint32_t)buffer & (EHCI_PAGE_SIZE - 1))
> +             if ((uint32_t)buffer & (PKT_ALIGN - 1))
>                       xfr_sz--;
>               /* Convert the qTD transfer size to bytes. */
>               xfr_sz *= EHCI_PAGE_SIZE;
>               /*
> -              * Determine the number of qTDs that will be required for the
> -              * data payload. This value has to be rounded up since the final
> -              * qTD transfer may be shorter than the regular qTD transfer
> -              * size that has just been computed.
> +              * Approximate by excess the number of qTDs that will be
> +              * required for the data payload. The exact formula is way more
> +              * complicated and saves at most 2 qTDs, i.e. a total of 128
> +              * bytes.
>                */
> -             qtd_count += DIV_ROUND_UP(length, xfr_sz);
> -             /* ZLPs also need a qTD. */
> -             if (!qtd_count)
> -                     qtd_count++;
> +             qtd_count += 2 + length / xfr_sz;
>       }
>  /*
> - * Threshold value based on the worst-case total size of the qTDs to
> allocate - * for a mass-storage transfer of 65535 blocks of 512 bytes.
> + * Threshold value based on the worst-case total size of the allocated
> qTDs for + * a mass-storage transfer of 65535 blocks of 512 bytes.
>   */
> -#if CONFIG_SYS_MALLOC_LEN <= 128 * 1024
> +#if CONFIG_SYS_MALLOC_LEN <= 64 + 128 * 1024
>  #warning CONFIG_SYS_MALLOC_LEN may be too small for EHCI
>  #endif
>       qtd = memalign(USB_DMA_MINALIGN, qtd_count * sizeof(struct qTD));
> @@ -314,9 +313,10 @@ ehci_submit_async(struct usb_device *dev, unsigned
> long pipe, void *buffer, qh->qh_link = cpu_to_hc32((uint32_t)qh_list |
> QH_LINK_TYPE_QH);
>       c = (usb_pipespeed(pipe) != USB_SPEED_HIGH &&
>            usb_pipeendpoint(pipe) == 0);
> +     maxpacket = usb_maxpacket(dev, pipe);
>       endpt = (8 << QH_ENDPT1_RL) |
>           (c << QH_ENDPT1_C) |
> -         (usb_maxpacket(dev, pipe) << QH_ENDPT1_MAXPKTLEN) |
> +         (maxpacket << QH_ENDPT1_MAXPKTLEN) |

Is this change really needed? (not that I care much).

[...]

Took me a bit to make it through, but I think I get it ... just real nits above.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to