USB: make usb_kbd obey USB DMA alignment requirements
PATCH v2 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.
--- common/usb_kbd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
common/usb_kbd.c +4 -4
@@ -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 ]; +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 ));if (!data) {printf ("USB KBD: Error allocating private data\n" );return 0 ;
Signature · 1 line
1.7.10.4
PATCH v2 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.
--- 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
@@ -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
@@ -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
@@ -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
PATCH v2 2/3 tegra: move TEGRA_DEVICE_SETTINGS to tegra-common-post.h
Dear Allen Martin,
Quoted 5 lines
Move environment settings for stdin/stdout/stderr to
tegra-common-post.hand generate them automaticaly based on input device selection.Signed-off-by Allen Martin <[email protected]>
2/3 and 3/3 should go through tegra, thanks
Quoted 58 lines
--- 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@@ -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 */ (quoted diff) +0
-0
b/include/configs/tegra-common-post.hindex 9698c23..8d21d9c 100644include/configs/tegra-common-post.h +19
-0@@ -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" \(quoted diff) +0
-0
b/include/configs/tegra20-common.hindex dc7444d..ea89d76 100644include/configs/tegra20-common.h +0
-0@@ -125,12 +125,8 @@
include/configs/tegra20-common.h line 131
Quoted 10 lines
include/configs/tegra20-common.h +0
-4#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 */
include/configs/tegra20-common.h line 132
Quoted 1 line
include/configs/tegra20-common.h +0
-0#define CONFIG_BOOTDELAY 2/* -1 to disable auto boot */
Best regards,
Marek Vasut
PATCH v2 2/3 tegra: move TEGRA_DEVICE_SETTINGS to tegra-common-post.h
Quoted 9 lines · Marek Vasut
Dear Allen Martin,
Quoted 5 lines
Move environment settings for stdin/stdout/stderr to
tegra-common-post.hand generate them automaticaly based on input device selection.Signed-off-by Allen Martin <[email protected]>2/3 and 3/3 should go through tegra, thanks
Although of course patches 2 & 3 depend on patch 1. So, the process would have to be:
1) Apply patch 1 to USB tree.
2) Send pull request for USB to main U-Boot repo.
3) Pull request actioned.
4) Wait until latest u-boot/master is picked in u-boot/arm and hence u-boot/tegra.
5) Apply patches 2 & 3 to the Tegra tree.
That's quite a long round-trip. What's the typical delay between (1) and
(2) or (3) and (4)?
In the Linux kernel, this would be handled by putting patch (1) into a topic branch on its own, which can then be merged into both the USB branch and immediately into Tegra's branch. Of course, that all relies on never rebasing during pull requests (otherwise, the branch with patch 1 might end up being applied as two different commits when merging into u-boot/master); something which apparently isn't acceptable for U-Boot:-(
PATCH v2 2/3 tegra: move TEGRA_DEVICE_SETTINGS to tegra-common-post.h
Dear Stephen Warren,
Quoted 24 lines · Marek Vasut
Quoted 9 lines
Dear Allen Martin,
Quoted 5 lines
Move environment settings for stdin/stdout/stderr to
tegra-common-post.hand generate them automaticaly based on input device selection.Signed-off-by Allen Martin <[email protected]>2/3 and 3/3 should go through tegra, thanks
Although of course patches 2 & 3 depend on patch 1. So, the process would have to be:
1) Apply patch 1 to USB tree.
2) Send pull request for USB to main U-Boot repo.
3) Pull request actioned.
4) Wait until latest u-boot/master is picked in u-boot/arm and hence u-boot/tegra.
5) Apply patches 2 & 3 to the Tegra tree.That's quite a long round-trip. What's the typical delay between (1) and
(2) or (3) and (4)?
Quoted 6 lines
In the Linux kernel, this would be handled by putting patch (1) into a topic branch on its own, which can then be merged into both the USB branch and immediately into Tegra's branch. Of course, that all relies on never rebasing during pull requests (otherwise, the branch with patch 1 might end up being applied as two different commits when merging into u-boot/master); something which apparently isn't acceptable for U-Boot:-(
Fine, then if you agree, I'll pull them all through usb tree ?
Best regards,
Marek Vasut
PATCH v2 2/3 tegra: move TEGRA_DEVICE_SETTINGS to tegra-common-post.h
Quoted 26 lines · Marek Vasut
Dear Stephen Warren,
Quoted 24 lines · Marek Vasut
Quoted 9 lines
Dear Allen Martin,
Quoted 5 lines
Move environment settings for stdin/stdout/stderr to
tegra-common-post.hand generate them automaticaly based on input device selection.Signed-off-by Allen Martin <[email protected]>2/3 and 3/3 should go through tegra, thanks
Although of course patches 2 & 3 depend on patch 1. So, the process would have to be:
1) Apply patch 1 to USB tree.
2) Send pull request for USB to main U-Boot repo.
3) Pull request actioned.
4) Wait until latest u-boot/master is picked in u-boot/arm and hence u-boot/tegra.
5) Apply patches 2 & 3 to the Tegra tree.That's quite a long round-trip. What's the typical delay between (1) and
(2) or (3) and (4)?
Quoted 8 lines
Quoted 6 lines
In the Linux kernel, this would be handled by putting patch (1) into a topic branch on its own, which can then be merged into both the USB branch and immediately into Tegra's branch. Of course, that all relies on never rebasing during pull requests (otherwise, the branch with patch 1 might end up being applied as two different commits when merging into u-boot/master); something which apparently isn't acceptable for U-Boot:-(
Fine, then if you agree, I'll pull them all through usb tree ?
That's up to Tom Warren; he gets back from vacation in a couple days. Taking it all through the USB tree might not be best either, since there are merge conflicts with some Tegra patches that are already queued up, but I'll let Tom comment on what he wants to do.
PATCH v2 2/3 tegra: move TEGRA_DEVICE_SETTINGS to tegra-common-post.h
In reply to Marek VasutQuoted 26 lines · Marek Vasut
Dear Stephen Warren,
Quoted 24 lines · Marek Vasut
Quoted 9 lines
Dear Allen Martin,
Quoted 5 lines
Move environment settings for stdin/stdout/stderr to
tegra-common-post.hand generate them automaticaly based on input device selection.Signed-off-by Allen Martin <[email protected]>2/3 and 3/3 should go through tegra, thanks
Although of course patches 2 & 3 depend on patch 1. So, the process would have to be:
1) Apply patch 1 to USB tree.
2) Send pull request for USB to main U-Boot repo.
3) Pull request actioned.
4) Wait until latest u-boot/master is picked in u-boot/arm and hence u-boot/tegra.
5) Apply patches 2 & 3 to the Tegra tree.That's quite a long round-trip. What's the typical delay between (1) and
(2) or (3) and (4)?
Quoted 9 lines
Quoted 6 lines
In the Linux kernel, this would be handled by putting patch (1) into a topic branch on its own, which can then be merged into both the USB branch and immediately into Tegra's branch. Of course, that all relies on never rebasing during pull requests (otherwise, the branch with patch 1 might end up being applied as two different commits when merging into u-boot/master); something which apparently isn't acceptable for U-Boot:-(
Fine, then if you agree, I'll pull them all through usb tree ?
That's fine by me. Tom is out on vacation right now anyway, so he wouldn't be able to pull them into the tegra tree until next week at the earliest anyway.
Quoted 2 lines
Best regards,
Marek Vasut
-Allen
Signature · 1 line
nvpublic
PATCH v2 2/3 tegra: move TEGRA_DEVICE_SETTINGS to tegra-common-post.h
In reply to Marek VasutQuoted 26 lines · Marek Vasut
Dear Stephen Warren,
Quoted 24 lines · Marek Vasut
Quoted 9 lines
Dear Allen Martin,
Quoted 5 lines
Move environment settings for stdin/stdout/stderr to
tegra-common-post.hand generate them automaticaly based on input device selection.Signed-off-by Allen Martin <[email protected]>2/3 and 3/3 should go through tegra, thanks
Although of course patches 2 & 3 depend on patch 1. So, the process would have to be:
1) Apply patch 1 to USB tree.
2) Send pull request for USB to main U-Boot repo.
3) Pull request actioned.
4) Wait until latest u-boot/master is picked in u-boot/arm and hence u-boot/tegra.
5) Apply patches 2 & 3 to the Tegra tree.That's quite a long round-trip. What's the typical delay between (1) and
(2) or (3) and (4)?
Quoted 8 lines
Quoted 6 lines
In the Linux kernel, this would be handled by putting patch (1) into a topic branch on its own, which can then be merged into both the USB branch and immediately into Tegra's branch. Of course, that all relies on never rebasing during pull requests (otherwise, the branch with patch 1 might end up being applied as two different commits when merging into u-boot/master); something which apparently isn't acceptable for U-Boot:-(
Fine, then if you agree, I'll pull them all through usb tree ?
Hi Marek, I just wanted to check on that status of this series. Were you going to apply them to u-boot-usb ?
Thanks,
-Allen
Signature · 1 line
nvpublic
PATCH v2 2/3 tegra: move TEGRA_DEVICE_SETTINGS to tegra-common-post.h
Dear Allen Martin,
Quoted 35 lines · Marek Vasut
Quoted 34 lines
Dear Stephen Warren,
Quoted 31 lines · Marek Vasut
Quoted 9 lines
Dear Allen Martin,
Quoted 5 lines
Move environment settings for stdin/stdout/stderr to
tegra-common-post.hand generate them automaticaly based on input device selection.Signed-off-by Allen Martin <[email protected]>2/3 and 3/3 should go through tegra, thanks
Although of course patches 2 & 3 depend on patch 1. So, the process would have to be:
1) Apply patch 1 to USB tree.
2) Send pull request for USB to main U-Boot repo.
3) Pull request actioned.
4) Wait until latest u-boot/master is picked in u-boot/arm and hence u-boot/tegra.
5) Apply patches 2 & 3 to the Tegra tree.That's quite a long round-trip. What's the typical delay between (1) and (2) or (3) and (4)?
In the Linux kernel, this would be handled by putting patch (1) into a topic branch on its own, which can then be merged into both the USB branch and immediately into Tegra's branch. Of course, that all relies on never rebasing during pull requests (otherwise, the branch with patch 1 might end up being applied as two different commits when merging into u-boot/master); something which apparently isn't acceptable for U-Boot:-(
Quoted 4 lines
Quoted 1 line
Fine, then if you agree, I'll pull them all through usb tree ?
Hi Marek, I just wanted to check on that status of this series. Were you going to apply them to u-boot-usb ?
Damn, I'll apply them ;-)
Best regards,
Marek Vasut
PATCH v2 2/3 tegra: move TEGRA_DEVICE_SETTINGS to tegra-common-post.h
In reply to Allen MartinQuoted 3 lines · Allen Martin
Move environment settings for stdin/stdout/stderr to
tegra-common-post.hand generate them automaticaly based on input device selection.
Patches 2 and 3,
PATCH v2 3/3 tegra: Enable USB keyboard
In reply to Allen MartinEnable USB keyboard for seaboard and ventana
--- include/configs/seaboard.h | 3 +++ include/configs/ventana.h | 3 +++ 2 files changed, 6 insertions(+)
include/configs/seaboard.h +3 -0
@@ -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
@@ -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
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
In reply to Allen MartinDear Allen Martin,
Quoted 14 lines
Change
usb_kbddriver 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]>--- common/usb_kbd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)common/usb_kbd.c +0
-0
common/usb_kbd.c static const unsigned char usb_kbd_num_keypad[] = { · line 111
Quoted 6 lines
common/usb_kbd.c +3
-0@@ -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.
Quoted 13 lines
common/usb_kbd.c +0
-3uint32_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; };
common/usb_kbd.c static int usb_kbd_probe(struct usb_device *dev, · line 430
Quoted 6 lines
common/usb_kbd.c +1
-1@@ -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 ?
Quoted 3 lines
common/usb_kbd.c +0
-0if (!data) {printf ("USB KBD: Error allocating private data\n" );return 0;
Best regards,
Marek Vasut
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
Quoted 9 lines · Marek Vasut
Dear Allen Martin,
Quoted 7 lines
Change
usb_kbddriver 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.(quoted diff) +0
-0@@ -426,7 +426,7 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum)
Quoted 7 lines
common/usb_kbd.c static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum) · line 429
Quoted 4 lines
common/usb_kbd.c +1
-1USB_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_BUFFERand such stuff ininclude/common.hfor this purpose ?
That's for stack-based data structures, whereas this data structure sticks around for more than the duration of this function.
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
In reply to Marek VasutQuoted 18 lines · Marek Vasut
Dear Allen Martin,
Quoted 16 lines
Change
usb_kbddriver 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]>--- common/usb_kbd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)common/usb_kbd.c +0
-0@@ -106,15 +106,15 @@ static const unsigned char usb_kbd_num_keypad[] = { (USB_KBD_NUMLOCK |USB_KBD_CAPSLOCK |USB_KBD_SCROLLLOCK )
Quoted 6 lines
common/usb_kbd.c static const unsigned char usb_kbd_num_keypad[] = { · line 111
Quoted 4 lines
common/usb_kbd.c +3
-0struct usb_kbd_pdata { +uint8_t new[8 ]; +uint8_t old[8 ]; +Some comment about the alignment won't hurt.
Good idea.
Quoted 16 lines
Quoted 16 lines
common/usb_kbd.c +0
-3uint32_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)
Quoted 7 lines
common/usb_kbd.c static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum) · line 429
Quoted 4 lines
common/usb_kbd.c +1
-1USB_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_BUFFERand such stuff ininclude/common.hfor 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.
Quoted 7 lines
Quoted 4 lines
common/usb_kbd.c +0
-0if (!data) {printf ("USB KBD: Error allocating private data\n" );return 0;
Best regards,
Marek Vasut
-Allen
Signature · 1 line
nvpublic
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
Dear Allen Martin,
Quoted 75 lines · Marek Vasut
Quoted 21 lines
Dear Allen Martin,
Quoted 19 lines
Change
usb_kbddriver 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]>---
common/usb_kbd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)common/usb_kbd.c +0
-0@@ -106,15 +106,15 @@ static const unsigned char usb_kbd_num_keypad[] = { (USB_KBD_NUMLOCK |USB_KBD_CAPSLOCK |USB_KBD_SCROLLLOCK )Quoted 7 lines
common/usb_kbd.c static const unsigned char usb_kbd_num_keypad[] = { · line 111
Quoted 5 lines
common/usb_kbd.c +3
-0struct usb_kbd_pdata { +uint8_t new[8 ]; +uint8_t old[8 ]; +Some comment about the alignment won't hurt.
Good idea.
Quoted 17 lines
Quoted 17 lines
common/usb_kbd.c +0
-3uint32_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 ]; -common/usb_kbd.c +0
-0uint8_t flags; };@@ -426,7 +426,7 @@ static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum)Quoted 7 lines
common/usb_kbd.c static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum) · line 429
Quoted 4 lines
common/usb_kbd.c +1
-1USB_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_BUFFERand such stuff ininclude/common.hfor this purpose ?There seems to be some discrepency here, because
ehci-hcd.cusesUSB_DMA_MINALIGNfor all cache operations, which may or may not be the same asARCH_DMA_MINALIGN, seeusb.h:/*
* The EHCI spec says that we must align to at least 32 bytes. However,
* some platforms require larger alignment.
*/
#ifARCH_DMA_MINALIGN> 32
#defineUSB_DMA_MINALIGNARCH_DMA_MINALIGN
#else
#defineUSB_DMA_MINALIGN32
#endifFor tegra this is fine, because
ARCH_DMA_MINALIGN> 32, but grepping through header files, I see a few:
Quoted 6 lines
#define
ARCH_DMA_MINALIGN16which 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.
Quoted 9 lines
Quoted 7 lines
common/usb_kbd.c static int usb_kbd_probe(struct usb_device *dev, unsigned int ifnum) · line 432
Quoted 4 lines
common/usb_kbd.c +0
-0if (!data) {common/usb_kbd.c +0
-0printf ("USB KBD: Error allocating private data\n" );return 0 ;Best regards,
Marek Vasut-Allen
Best regards,
Marek Vasut
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
In reply to Allen MartinQuoted 3 lines · Allen Martin
Change
usb_kbddriver 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.
The series,
BTW, I tested tegra-kbc too, and that does indeed currently work (at least in my local dev branch based on u-boot/master).
Note that patch 2 has a merge conflict with the following patch in u-boot-tegra/next, since I assume your series is based on u-boot/master not u-boot-tegra/next:
799f182 ARM: tegra: use standard variables to define load addresses
It's pretty simple to resolve though.
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
Quoted 10 lines · Stephen Warren
Quoted 3 lines · Allen Martin
Change
usb_kbddriver 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.The series,
Tested-by Stephen Warren <[email protected]>BTW, I tested tegra-kbc too, and that does indeed currently work (at least in my local dev branch based on u-boot/master).
Yes, I also tried on a seaboard with internal keyboard and it works, although once the USB keyboard driver loads the internal keyboard stops working. I haven't tracked down why, but it seems like a bug I can live with for now as seaboards with internal keyboards are pretty rare these days, and how many keyboards do you need in u-boot anyway? :^)
Quoted 8 lines
Note that patch 2 has a merge conflict with the following patch in u-boot-tegra/next, since I assume your series is based on u-boot/master not u-boot-tegra/next:
799f182 ARM: tegra: use standard variables to define load addresses
It's pretty simple to resolve though.
Yes, I based it on u-boot/master, once we figure out what trees each patch is destined for I'll rebase appropriately.
-Allen
Signature · 1 line
nvpublic
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
Dear Allen Martin,
Quoted 9 lines · Stephen Warren
Quoted 8 lines · Allen Martin
Quoted 3 lines
Change
usb_kbddriver 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.The series,
Tested-by Stephen Warren <[email protected]>
Quoted 8 lines
Quoted 2 lines
BTW, I tested tegra-kbc too, and that does indeed currently work (at least in my local dev branch based on u-boot/master).
Yes, I also tried on a seaboard with internal keyboard and it works, although once the USB keyboard driver loads the internal keyboard stops working. I haven't tracked down why, but it seems like a bug I can live with for now as seaboards with internal keyboards are pretty rare these days, and how many keyboards do you need in u-boot anyway?
Good thing you pointed it out. Please let's not ignore a bug. How come it happens? What happens if you have two usb keyboards connected?
Quoted 6 lines
:^)
:Quoted 4 lines
Note that patch 2 has a merge conflict with the following patch in u-boot-tegra/next, since I assume your series is based on u-boot/master not u-boot-tegra/next:
Quoted 6 lines
Quoted 3 lines
799f182 ARM: tegra: use standard variables to define load addresses
It's pretty simple to resolve though.
Yes, I based it on u-boot/master, once we figure out what trees each patch is destined for I'll rebase appropriately.
I pushed u-boot-usb/master today, can you check if it still applies ?
Best regards,
Marek Vasut
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
Quoted 14 lines · Marek Vasut
Dear Allen Martin,
Quoted 12 lines · Stephen Warren
Quoted 10 lines · Allen Martin
Quoted 3 lines
Change
usb_kbddriver 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.The series,
Tested-by Stephen Warren <[email protected]>BTW, I tested tegra-kbc too, and that does indeed currently work (at least in my local dev branch based on u-boot/master).
Quoted 9 lines
Quoted 5 lines
Yes, I also tried on a seaboard with internal keyboard and it works, although once the USB keyboard driver loads the internal keyboard stops working. I haven't tracked down why, but it seems like a bug I can live with for now as seaboards with internal keyboards are pretty rare these days, and how many keyboards do you need in u-boot anyway?
Good thing you pointed it out. Please let's not ignore a bug. How come it happens? What happens if you have two usb keyboards connected?
I'm pretty sure the USB keyboard driver doesn't support multiple devices, I see this in drv_usb_kbd_init():
/* We found a keyboard, check if it is already
registered. */
USB_KBD_PRINTF("USB KBD: found set up device.\n");
old_dev = stdio_get_by_name(DEVNAME);
if (old_dev) {
/* Already registered, just return ok. */
USB_KBD_PRINTF("USB KBD: is already
registered.\n");
return 1;
}The bug is almost certainly inside the tegra kbd driver, which is why I'm not terribly concerned about it. The only boards that use that driver are inside NVIDIA, and even those are rare.
Quoted 6 lines
Quoted 6 lines
:^)
:Quoted 4 lines
Note that patch 2 has a merge conflict with the following patch in u-boot-tegra/next, since I assume your series is based on u-boot/master not u-boot-tegra/next:
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
799f182 ARM: tegra: use standard variables to define load addresses
It's pretty simple to resolve though.
Yes, I based it on u-boot/master, once we figure out what trees each patch is destined for I'll rebase appropriately.
I pushed u-boot-usb/master today, can you check if it still applies ?
Yes, it still applies cleanly to u-boot-usb/master
Quoted 3 lines
Best regards,
Marek Vasut
-Allen
Signature · 1 line
nvpublic
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
Quoted 39 lines · Allen Martin
Quoted 14 lines · Marek Vasut
Dear Allen Martin,
Quoted 12 lines · Stephen Warren
Quoted 10 lines · Allen Martin
Quoted 3 lines
Change
usb_kbddriver 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.The series,
Tested-by Stephen Warren <[email protected]>BTW, I tested tegra-kbc too, and that does indeed currently work (at least in my local dev branch based on u-boot/master).
Quoted 9 lines
Quoted 5 lines
Yes, I also tried on a seaboard with internal keyboard and it works, although once the USB keyboard driver loads the internal keyboard stops working. I haven't tracked down why, but it seems like a bug I can live with for now as seaboards with internal keyboards are pretty rare these days, and how many keyboards do you need in u-boot anyway?
Good thing you pointed it out. Please let's not ignore a bug. How come it happens? What happens if you have two usb keyboards connected?
I'm pretty sure the USB keyboard driver doesn't support multiple devices, I see this in
drv_usb_kbd_init():/* We found a keyboard, check if it is already registered. */ USB_KBD_PRINTF("USB KBD: found set up device.\n"); old_dev = stdio_get_by_name(DEVNAME); if (old_dev) { /* Already registered, just return ok. */ USB_KBD_PRINTF("USB KBD: is already registered.\n"); return 1; }
Quoted 4 lines
The bug is almost certainly inside the tegra kbd driver, which is why I'm not terribly concerned about it. The only boards that use that driver are inside NVIDIA, and even those are rare.
Ok, I dug into the driver code a little. It's because the tegra keyboard driver doesn't support CONSOLE_MUX, so when the USB keyboard calls into iomux it takes away stdin from the tegra kbd driver, and it has no way of ever getting it back. Definately a bug, but unrelated to this change.
-Allen
Signature · 1 line
nvpublic
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
In reply to Allen MartinDear Allen Martin,
Quoted 38 lines · Marek Vasut
Quoted 14 lines
Dear Allen Martin,
Quoted 12 lines · Stephen Warren
Quoted 10 lines · Allen Martin
Quoted 3 lines
Change
usb_kbddriver 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.The series,
Tested-by Stephen Warren <[email protected]>BTW, I tested tegra-kbc too, and that does indeed currently work (at least in my local dev branch based on u-boot/master).
Quoted 8 lines
Quoted 5 lines
Yes, I also tried on a seaboard with internal keyboard and it works, although once the USB keyboard driver loads the internal keyboard stops working. I haven't tracked down why, but it seems like a bug I can live with for now as seaboards with internal keyboards are pretty rare these days, and how many keyboards do you need in u-boot anyway?
Good thing you pointed it out. Please let's not ignore a bug. How come it happens? What happens if you have two usb keyboards connected?
I'm pretty sure the USB keyboard driver doesn't support multiple devices, I see this in
drv_usb_kbd_init():/* We found a keyboard, check if it is already registered. */ USB_KBD_PRINTF("USB KBD: found set up device.\n"); old_dev = stdio_get_by_name(DEVNAME); if (old_dev) { /* Already registered, just return ok. */ USB_KBD_PRINTF("USB KBD: is already registered.\n"); return 1; }
Quoted 3 lines
The bug is almost certainly inside the tegra kbd driver, which is why I'm not terribly concerned about it. The only boards that use that driver are inside NVIDIA, and even those are rare.
[...]
Good, now please fix the bug. I'm terribly unhappy seeing there is a bug that is about to go unfixed.
Best regards,
Marek Vasut
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
Quoted 40 lines · Marek Vasut
Dear Allen Martin,
Quoted 38 lines · Marek Vasut
Quoted 14 lines
Dear Allen Martin,
Quoted 12 lines · Stephen Warren
Quoted 10 lines · Allen Martin
Quoted 3 lines
Change
usb_kbddriver 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.The series,
Tested-by Stephen Warren <[email protected]>BTW, I tested tegra-kbc too, and that does indeed currently work (at least in my local dev branch based on u-boot/master).
Quoted 8 lines
Quoted 5 lines
Yes, I also tried on a seaboard with internal keyboard and it works, although once the USB keyboard driver loads the internal keyboard stops working. I haven't tracked down why, but it seems like a bug I can live with for now as seaboards with internal keyboards are pretty rare these days, and how many keyboards do you need in u-boot anyway?
Good thing you pointed it out. Please let's not ignore a bug. How come it happens? What happens if you have two usb keyboards connected?
I'm pretty sure the USB keyboard driver doesn't support multiple devices, I see this in
drv_usb_kbd_init():/* We found a keyboard, check if it is already registered. */ USB_KBD_PRINTF("USB KBD: found set up device.\n"); old_dev = stdio_get_by_name(DEVNAME); if (old_dev) { /* Already registered, just return ok. */ USB_KBD_PRINTF("USB KBD: is already registered.\n"); return 1; }
Quoted 7 lines
Quoted 3 lines
The bug is almost certainly inside the tegra kbd driver, which is why I'm not terribly concerned about it. The only boards that use that driver are inside NVIDIA, and even those are rare.
[...]
Good, now please fix the bug. I'm terribly unhappy seeing there is a bug that is about to go unfixed.
I didn't say the bug will go unfixed, I've opened an issue in our internal bug tracker so it doesn't go forgotten. It's just a matter of prioritization. It's just not important to fix a corner case bug in a driver that noone outside of NVIDIA can actually use when there are major functionality holes and regressions (like your change to serial_assign() that broke serial output on tegra). I only work on u-boot on the side, so I have to pick my battles carefully.
-Allen
Signature · 1 line
nvpublic
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
Dear Allen Martin,
[...]
Quoted 8 lines
Quoted 4 lines
[...]
Good, now please fix the bug. I'm terribly unhappy seeing there is a bug that is about to go unfixed.
I didn't say the bug will go unfixed, I've opened an issue in our internal bug tracker so it doesn't go forgotten. It's just a matter of prioritization.
Yes, the bug is really simple to fix, so instead of arguing, please get to work. It could have been fixed already!
Quoted 1 line
It's just not important to fix a corner case bug
I'm sorry, I really need to be careful with wording here. I'm always baffled how an engineer can ignore a bug.
Quoted 3 lines
in a driver that noone outside of NVIDIA can actually use when there are major functionality holes and regressions (like your change to
serial_assign()that broke serial output on tegra).
Stop being personal, this hurts and doesn't help. Care to send a patch for this? I don't have a working tegra setup (yet), but you can test this. I already outlined the fix, so it's just a matter to implement it.
Quoted 2 lines
I only work on
u-boot on the side, so I have to pick my battles carefully.
I'm glad for any contribution from your end, don't be mistaken. I really appreciate it, sorry if my working is sometimes way too blunt or heartless.
Quoted 1 line
-Allen
Best regards,
Marek Vasut
PATCH v2 1/3 USB: make usb_kbd obey USB DMA alignment requirements
Quoted 9 lines · Marek Vasut
Dear Allen Martin,
[...]
Quoted 5 lines
Quoted 4 lines
[...]
Good, now please fix the bug. I'm terribly unhappy seeing there is a bug that is about to go unfixed.
Quoted 6 lines
Quoted 3 lines
I didn't say the bug will go unfixed, I've opened an issue in our internal bug tracker so it doesn't go forgotten. It's just a matter of prioritization.
Yes, the bug is really simple to fix, so instead of arguing, please get to work. It could have been fixed already!
I'm sorry, but that's not a constructive response. If you want it fixed so badly, I'm sure we'd gratefully receive a patch from you for it.
As Allen pointed out, there are currently more important issues to concentrate on, such as Tegra not booting at all. I'm sure that once we've made the system work at all, then we can concentrate on the minor use-cases that simply aren't causing anyone any problems right now.