[U-Boot] [PATCH 0/2] usb_kbd with DM enabled

Hi,
I am playing with usb keyboard support with USB_DM on and I see some issues which I want to share with you. First of all there is missing support for multiple usb keyboards because all are registered as usbkbd. This can be changed by - strcpy(usb_kbd_dev.name, DEVNAME); + snprintf(usb_kbd_dev.name, sizeof(usb_kbd_dev.name), + "%s%x", DEVNAME, dev->devnum);
That devnum ID will be added there and then from usb tree you can see which usb device should be used for example "setenv stdin usbkbd5" Unfortunatelly remove part should be also handled to point to proper stdio_dev.
Why there is usb-keyboard compatible string? Code is discovering usb when usb start is called. Or is there also support via DT?
Next thing what I found a problematic is when probe_usb_keyboard() is called from usb_kbd_probe(). There is code which reads stdin variable if it is not setup to usbkbd then probe fails which seems to me wrong. I for example want to start with serial and then start usb and use input from keyboard or both. That's why I think this code should be out of DM probe function and probing shouldn't be mixed with user setting.
Last but not least I see with kermit double echo when usbkbd is used which is quite weird. Similar problem was reported here. https://lists.denx.de/pipermail/u-boot/2014-November/196713.html Do you know what's the reason for that echo? (usb_kbd_getc() is really returning only one char).
Thanks, Michal
Michal Simek (2): usb_kbd: Add support for watchdog usb_kdb: Get stdio_dev directly from sdev pointer
common/usb_kbd.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)

There is need to service watchdog in while loop or system will be restarted when idlying.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
common/usb_kbd.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 4c394d5613d1..20255dcf951f 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -16,6 +16,7 @@ #include <malloc.h> #include <memalign.h> #include <stdio_dev.h> +#include <watchdog.h> #include <asm/byteorder.h>
#include <usb.h> @@ -398,8 +399,10 @@ static int usb_kbd_getc(struct stdio_dev *sdev) usb_kbd_dev = (struct usb_device *)dev->priv; data = usb_kbd_dev->privptr;
- while (data->usb_in_pointer == data->usb_out_pointer) + while (data->usb_in_pointer == data->usb_out_pointer) { + WATCHDOG_RESET(); usb_kbd_poll_for_event(usb_kbd_dev); + }
if (data->usb_out_pointer == USB_KBD_BUFFER_LEN - 1) data->usb_out_pointer = 0;

Driver supports only one instance of usb keyboard. Remove the first dependency on generic usbkbd DEVNAME.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
common/usb_kbd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 20255dcf951f..ce8cdc20fac5 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -372,7 +372,7 @@ static int usb_kbd_testc(struct stdio_dev *sdev) return 0; kbd_testc_tms = get_timer(0); #endif - dev = stdio_get_by_name(DEVNAME); + dev = stdio_get_by_name(sdev->name); usb_kbd_dev = (struct usb_device *)dev->priv; data = usb_kbd_dev->privptr;
@@ -395,7 +395,7 @@ static int usb_kbd_getc(struct stdio_dev *sdev) struct usb_device *usb_kbd_dev; struct usb_kbd_pdata *data;
- dev = stdio_get_by_name(DEVNAME); + dev = stdio_get_by_name(sdev->name); usb_kbd_dev = (struct usb_device *)dev->priv; data = usb_kbd_dev->privptr;

Hi,
Last but not least I see with kermit double echo when usbkbd is used which is quite weird. Similar problem was reported here. https://lists.denx.de/pipermail/u-boot/2014-November/196713.html Do you know what's the reason for that echo? (usb_kbd_getc() is really returning only one char).
FYI: this was caused by enabling DEBUG in that driver. When this is disabled echo is gone.
Thanks, Michal
participants (2)
-
Michal Simek
-
Michal Simek