[U-Boot] [PATCH v2 11/17] usb: Add support for data alignment

[Simon Glass]

Hi Remy,

On Sun, Dec 4, 2011 at 3:13 AM, Remy Bohmer <linux at bohmer.net> wrote:
> Hi,
>
> 2011/12/3 Simon Glass <sjg at chromium.org>:
>> CONFIG_USB_EHCI_DATA_ALIGN sets the required alignment of data for
>> USB packets (e.g. 4 means word-aligned). This is required for Tegra
>> to operate.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>> Changes in v2:
>> - Tidy code style in USB buffer alignment code
>> - Display prominent warning when alignment code is triggered
>>
>>  README                      |    6 ++++++
>>  drivers/usb/host/ehci-hcd.c |   29 +++++++++++++++++++++++++++++
>>  2 files changed, 35 insertions(+), 0 deletions(-)
>>
>> diff --git a/README b/README
>> index fda0190..3194846 100644
>> --- a/README
>> +++ b/README
>> @@ -1096,6 +1096,12 @@ The following options need to be configured:
>>                                May be defined to allow interrupt polling
>>                                instead of using asynchronous interrupts
>>
>> +               CONFIG_USB_EHCI_DATA_ALIGN sets the required alignment of
>> +               data for USB packets (e.g. 4 means word-aligned). This is
>> +               required for Tegra to operate. Since we want all callers to
>> +               align data for us, we display a warning when the alignment
>> +               code is triggered.
>> +
>>  - USB Device:
>>                Define the below if you wish to use the USB console.
>>                Once firmware is rebuilt from a serial console issue the
>> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
>> index 2197119..23458b6 100644
>> --- a/drivers/usb/host/ehci-hcd.c
>> +++ b/drivers/usb/host/ehci-hcd.c
>> @@ -322,6 +322,24 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
>>        int timeout;
>>        int ret = 0;
>>
>> +#ifdef CONFIG_USB_EHCI_DATA_ALIGN
>> +       /* In case ehci host requires alignment for buffers */
>> +       void *align_buf = NULL;
>> +       void *orig_buf = buffer;
>> +       int unaligned = (int)buffer & (CONFIG_USB_EHCI_DATA_ALIGN - 1);
>> +
>> +       if (unaligned) {
>> +               printf("EHCI: Unaligned buffer: performance will suffer\n");
>> +               align_buf = malloc(length + CONFIG_USB_EHCI_DATA_ALIGN);
>> +               if (!align_buf)
>> +                       return -1;
>> +               buffer = (void *)(((ulong)align_buf +
>> +                       CONFIG_USB_EHCI_DATA_ALIGN - 1) &
>> +                               ~(CONFIG_USB_EHCI_DATA_ALIGN - 1));
>> +               if (usb_pipeout(pipe))
>> +                       memcpy(buffer, orig_buf, length);
>> +       }
>
> I do not really like this solution. Maybe we can introduce something
> like an usbbuf_alloc() function that takes care of the alignment.
> This saves the memcpy() for every transmit. I know it has more impact
> if we do it like that, but I think it is cleaner.

This is called from usb_bulk_msg() which is called from usb_storage
and USB ethernet drivers. A similar problem happened in the MMC stack
and was fixed by changing all the callers to cache-align their
buffers.

Do you mean that you want a patch which changes the callers so that
they align their data to a cache boundary as with MMC? Given the small
number of callers at this stage it shouldn't be too hard. Then we
could drop this patch.

Please let me know.

Regards,
Simon

>
> Kind regards,
>
> Remy