[U-Boot] [PATCH v2 1/3] USB: make usb_kbd obey USB DMA alignment requirements

[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 <amartin at 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.


> 
> >  	if (!data) {
> >  		printf("USB KBD: Error allocating private data\n");
> >  		return 0;
> 
> Best regards,
> Marek Vasut

-Allen
-- 
nvpublic