Dear Allen Martin,
> On Tue, Oct 23, 2012 at 12:26:31AM -0700, Marek Vasut wrote:
> > Dear Allen Martin,
> >
> > > Change usb_kbd driver to obey alignment requirements for USB DMA on
> > > the buffer used for data transfer. This is necessary for
> > > architectures that enable dcache and enable USB DMA.
> > >
> > > Signed-off-by: Allen Martin <amar...@nvidia.com>
> > > ---
> > >
> > > common/usb_kbd.c | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> > > index 19f01db..57928d9 100644
> > > --- a/common/usb_kbd.c
> > > +++ b/common/usb_kbd.c
> > > @@ -106,15 +106,15 @@ static const unsigned char usb_kbd_num_keypad[] =
> > > {
> > >
> > > (USB_KBD_NUMLOCK | USB_KBD_CAPSLOCK | USB_KBD_SCROLLLOCK)
> > >
> > > struct usb_kbd_pdata {
> > >
> > > + uint8_t new[8];
> > > + uint8_t old[8];
> > > +
> >
> > Some comment about the alignment won't hurt.
>
> Good idea.
>
> > > uint32_t repeat_delay;
> > >
> > > uint32_t usb_in_pointer;
> > > uint32_t usb_out_pointer;
> > > uint8_t usb_kbd_buffer[USB_KBD_BUFFER_LEN];
> > >
> > > - uint8_t new[8];
> > > - uint8_t old[8];
> > > -
> > >
> > > uint8_t flags;
> > >
> > > };
> > >
> > > @@ -426,7 +426,7 @@ static int usb_kbd_probe(struct usb_device *dev,
> > > unsigned int ifnum)
> > >
> > > USB_KBD_PRINTF("USB KBD: found set protocol...\n");
> > >
> > > - data = malloc(sizeof(struct usb_kbd_pdata));
> > > + data = memalign(USB_DMA_MINALIGN, sizeof(struct usb_kbd_pdata));
> >
> > Don't we have ALLOC_CACHE_ALIGN_BUFFER and such stuff in include/common.h
> > for this purpose ?
>
> There seems to be some discrepency here, because ehci-hcd.c uses
> USB_DMA_MINALIGN for all cache operations, which may or may not be the
> same as ARCH_DMA_MINALIGN, see usb.h:
>
> /*
> * The EHCI spec says that we must align to at least 32 bytes.
> However,
> * some platforms require larger alignment.
> */
> #if ARCH_DMA_MINALIGN > 32
> #define USB_DMA_MINALIGN ARCH_DMA_MINALIGN
> #else
> #define USB_DMA_MINALIGN 32
> #endif
>
> For tegra this is fine, because ARCH_DMA_MINALIGN > 32, but grepping
> through header files, I see a few:
>
> #define ARCH_DMA_MINALIGN 16
>
> which will definately break. It looks like all other usb class
> drivers use ALLOC_CACHE_ALIGN_BUFFER() though, so for consistency the
> usb keyboard driver probably should too, but it seems like a potential
> problem.
ALLOC_CACHE_ALIGN_BUFFER is what I had in mind, I think the EHCI driver already
uses this.
> > > if (!data) {
> > >
> > > printf("USB KBD: Error allocating private data\n");
> > > return 0;
> >
> > Best regards,
> > Marek Vasut
>
> -Allen
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
