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

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

[Michal Simek]

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(-)

-- 
1.9.1

[U-Boot] [PATCH 1/2] usb_kbd: Add support for watchdog

[Michal Simek]

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;
-- 
1.9.1

[U-Boot] [PATCH 2/2] usb_kdb: Get stdio_dev directly from sdev pointer

[Michal Simek]

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;
 
-- 
1.9.1

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

[Michal Simek]

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

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180627/ec9d2498/attachment.sig>