Patchwork [U-Boot,1/2] USB: make usb_kbd obey USB DMA alignment requirements

login
register
mail settings
Submitter Allen Martin
Date Oct. 22, 2012, 9:39 p.m.
Message ID <1350941971-10282-1-git-send-email-amartin@nvidia.com>
Download mbox | patch
Permalink /patch/193292/
State Superseded
Delegated to: Marek Vasut
Headers show

Comments

Allen Martin - Oct. 22, 2012, 9:39 p.m.
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@nvidia.com>
---
 common/usb_kbd.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
Stephen Warren - Oct. 22, 2012, 9:59 p.m.
On 10/22/2012 03:39 PM, Allen Martin wrote:
> 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.

> diff --git a/common/usb_kbd.c b/common/usb_kbd.c

>  struct usb_kbd_pdata {
> +	uint8_t		new[8];
> +	uint8_t		old[8];
> +
>  	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;
> -};
> +} __aligned(USB_DMA_MINALIGN);

Surely you need to edit the malloc() call in usb_kbd_probe() instead of
adding __aligned to the type; does the alignment on the type really get
propagated into malloc(), or as custom code at the call-site somehow?
Allen Martin - Oct. 22, 2012, 10:35 p.m.
On Mon, Oct 22, 2012 at 02:59:43PM -0700, Stephen Warren wrote:
> On 10/22/2012 03:39 PM, Allen Martin wrote:
> > 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.
> 
> > diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> 
> >  struct usb_kbd_pdata {
> > +	uint8_t		new[8];
> > +	uint8_t		old[8];
> > +
> >  	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;
> > -};
> > +} __aligned(USB_DMA_MINALIGN);
> 
> Surely you need to edit the malloc() call in usb_kbd_probe() instead of
> adding __aligned to the type; does the alignment on the type really get
> propagated into malloc(), or as custom code at the call-site somehow?

Yes, you're right.  I misread the code and thought it came from a
static allocation.  I got reassured when I added the change and the
cache flush alignment warnings went away, but I guess these alignment
things are always a crapshoot anyway.

I'll fix, thanks for finding that.

-Allen

Patch

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 19f01db..cfc1281 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -28,6 +28,7 @@ 
 #include <malloc.h>
 #include <stdio_dev.h>
 #include <asm/byteorder.h>
+#include <linux/compiler.h>
 
 #include <usb.h>
 
@@ -106,17 +107,17 @@  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];
+
 	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;
-};
+} __aligned(USB_DMA_MINALIGN);
 
 /* Generic keyboard event polling. */
 void usb_kbd_generic_poll(void)