USB: make usb_kbd obey USB DMA alignment requirements

PATCH Superseded Allen Martin 2 participants 5 messages Oct 23, 2012 Tegra architecture support

Share & tools

Allen Martin

author

2012-10-23 21:37 UTC #0 lore raw

PATCH v3 1/3 USB: make usb_kbd obey USB DMA alignment requirements

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 <[email protected]>

Tested-by Stephen Warren <[email protected]>

---
v3: add comment about alignemnt requirement
v2: use memalign instead of __align()

---
 common/usb_kbd.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

common/usb_kbd.c +8 -4

index 19f01db..be5e8bc 100644
@@ -105,16 +105,20 @@ static const unsigned char usb_kbd_num_keypad[] = {
 #define USB_KBD_LEDMASK		\
 	(USB_KBD_NUMLOCK | USB_KBD_CAPSLOCK | USB_KBD_SCROLLLOCK)
 
+/*
+ * This structure must be aligned to USB_DMA_MINALIGN to allow DMA to
+ * buffer "new" below.
+ */
 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;
 };

@@ -426,7 +430,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));
 	if (!data) {
 		printf("USB KBD: Error allocating private data\n");
 		return 0;

Signature · 1 line

1.7.10.4

Allen Martin

author

2012-10-23 21:37 UTC #1 lore raw

PATCH v3 2/3 tegra: move TEGRA_DEVICE_SETTINGS to tegra-common-post.h

Move environment settings for stdin/stdout/stderr to tegra-common-post.h and generate them automaticaly based on input device selection.

Signed-off-by Allen Martin <[email protected]>

Acked-by Stephen Warren <[email protected]>

Tested-by Stephen Warren <[email protected]>

---
v3: no changes
v2: new patch

---
 include/configs/seaboard.h          |    5 -----
 include/configs/tegra-common-post.h |   19 +++++++++++++++++++
 include/configs/tegra20-common.h    |    4 ----
 3 files changed, 19 insertions(+), 9 deletions(-)

include/configs/seaboard.h +0 -5

index 0727a4c..2cb3ac9 100644
@@ -98,11 +98,6 @@
 #define CONFIG_TEGRA_KEYBOARD
 #define CONFIG_KEYBOARD
 
-#undef TEGRA_DEVICE_SETTINGS
-#define TEGRA_DEVICE_SETTINGS	"stdin=serial,tegra-kbc\0" \
-				"stdout=serial\0" \
-				"stderr=serial\0"
-
 #include "tegra-common-post.h"
 
 /* NAND support */

include/configs/tegra-common-post.h +19 -0

index 9698c23..8d21d9c 100644
@@ -140,6 +140,25 @@
 
 #endif
 
+#ifdef CONFIG_TEGRA_KEYBOARD
+#define STDIN_KBD_KBC ",tegra-kbc"
+#else
+#define STDIN_KBD_KBC ""
+#endif
+
+#ifdef CONFIG_USB_KEYBOARD
+#define STDIN_KBD_USB ",usbkbd"
+#define CONFIG_SYS_USB_EVENT_POLL
+#define CONFIG_PREBOOT			"usb start"
+#else
+#define STDIN_KBD_USB ""
+#endif
+
+#define TEGRA_DEVICE_SETTINGS \
+	"stdin=serial" STDIN_KBD_KBC STDIN_KBD_USB "\0" \
+	"stdout=serial\0" \
+	"stderr=serial\0" \
+
 #define CONFIG_EXTRA_ENV_SETTINGS \
 	TEGRA_DEVICE_SETTINGS \
 	"fdt_load=0x01000000\0" \

include/configs/tegra20-common.h +0 -4

index dc7444d..ea89d76 100644
@@ -125,12 +125,8 @@
 
 #define CONFIG_SYS_NO_FLASH
 
-/* Environment information, boards can override if required */
 #define CONFIG_CONSOLE_MUX
 #define CONFIG_SYS_CONSOLE_IS_IN_ENV
-#define TEGRA_DEVICE_SETTINGS	"stdin=serial\0" \
-				"stdout=serial\0" \
-				"stderr=serial\0"
 
 #define CONFIG_LOADADDR		0x408000	/* def. location for kernel */
 #define CONFIG_BOOTDELAY	2		/* -1 to disable auto boot */

Signature · 1 line

1.7.10.4

Allen Martin

author

2012-10-23 21:37 UTC #2 lore raw

PATCH v3 3/3 tegra: Enable USB keyboard

In reply to Allen Martin

Enable USB keyboard for seaboard and ventana

Signed-off-by Allen Martin <[email protected]>

Acked-by Stephen Warren <[email protected]>

Tested-by Stephen Warren <[email protected]>

---
v3: no changes
v2: added ventana

---
 include/configs/seaboard.h |    3 +++
 include/configs/ventana.h  |    3 +++
 2 files changed, 6 insertions(+)

include/configs/seaboard.h +3 -0

index 2cb3ac9..332970c 100644
@@ -98,6 +98,9 @@
 #define CONFIG_TEGRA_KEYBOARD
 #define CONFIG_KEYBOARD
 
+/* USB keyboard */
+#define CONFIG_USB_KEYBOARD
+
 #include "tegra-common-post.h"
 
 /* NAND support */

include/configs/ventana.h +3 -0

index b751d58..4c9b31c 100644
@@ -75,6 +75,9 @@
 #define CONFIG_CMD_NET
 #define CONFIG_CMD_DHCP
 
+/* USB keyboard */
+#define CONFIG_USB_KEYBOARD
+
 #include "tegra-common-post.h"
 
 #endif /* __CONFIG_H */

Signature · 1 line

1.7.10.4

Stephen Warren

2012-10-23 21:51 UTC #3 lore raw

PATCH v3 1/3 USB: make usb_kbd obey USB DMA alignment requirements

In reply to Allen Martin

common/usb_kbd.c static const unsigned char usb_kbd_num_keypad[] = { · line 114

Quoted 13 lines · 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.
common/usb_kbd.c +6 -0
 
+/*
+ * This structure must be aligned to USB_DMA_MINALIGN to allow DMA to
+ * buffer "new" below.
+ */
 struct usb_kbd_pdata {
+	uint8_t		new[8];
+	uint8_t		old[8];

Oh, one more thought on this: Those fields should both be aligned, and their size be aligned too, at least to cache size. In particular, if we have HW write to new[], then invalidate the cache that contains new[] because it just did, we want to make sure that fields after new[] (i.e. old[], ...) don't get invalidate too, in case the cache contained stale data for those fields. That's one of the things that the (stack-based) ALLOC_CACHE_ALIGN_BUFFER handles. Perhaps make those two fields pointers, and point those at a memalign()-allocated blob, where the size of those blobs are something like ROUND_UP(USB_DMA_MINALIGN, 8)?

Sorry for forgetting about this before!

Allen Martin

author

2012-10-23 21:58 UTC #4 lore raw

PATCH v3 1/3 USB: make usb_kbd obey USB DMA alignment requirements

Quoted 25 lines · Stephen Warren

common/usb_kbd.c static const unsigned char usb_kbd_num_keypad[] = { · line 114
Quoted 13 lines · 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.
common/usb_kbd.c +6 -0
 
+/*
+ * This structure must be aligned to USB_DMA_MINALIGN to allow DMA to
+ * buffer "new" below.
+ */
 struct usb_kbd_pdata {
+	uint8_t		new[8];
+	uint8_t		old[8];
Oh, one more thought on this: Those fields should both be aligned, and their size be aligned too, at least to cache size. In particular, if we have HW write to new[], then invalidate the cache that contains new[] because it just did, we want to make sure that fields after new[] (i.e. old[], ...) don't get invalidate too, in case the cache contained stale data for those fields. That's one of the things that the (stack-based) ALLOC_CACHE_ALIGN_BUFFER handles. Perhaps make those two fields pointers, and point those at a memalign()-allocated blob, where the size of those blobs are something like ROUND_UP(USB_DMA_MINALIGN, 8)?

Quoted 1 line

Sorry for forgetting about this before!

That's a good point. We also don't want to touch old[] from the CPU right after we just flushed new[] and dirty the cacheline again. It does seem like these should be dynamically allocated, then I can avoid the memalign() on the whole structure and don't have to worry if a new member gets added in front of new[] later.

-Allen

Signature · 1 line

nvpublic