Skip to contentSkip to conversation

usb: kbd: don't fail with iomux

PATCH New Rob Clark 2 participants 4 messages Aug 3, 2017

lore
Share & tools
lore

Rob Clark

author
#0 lore raw

PATCH 1/2 usb: kbd: don't fail with iomux

stdin might not be set, which would cause iomux_doenv() to fail therefore causing probe_usb_keyboard() to fail. Furthermore if we do have iomux enabled, the sensible thing (in terms of user experience) would be to simply add ourselves to the list of stdin devices.

---
v2: address Bin Meng's comments

 common/usb_kbd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
common/usb_kbd.c +14 -0
index d2d29cc98f..72cf78abd4 100644
@@ -517,7 +517,21 @@ static int probe_usb_keyboard(struct usb_device *dev)
 
 	stdinname = getenv("stdin");
 #if CONFIG_IS_ENABLED(CONSOLE_MUX)
+	char *devname = DEVNAME;
+	/* stdin might not be set yet.. either way, with console-mux the
+	 * sensible thing to do is add ourselves to the list of stdio
+	 * devices:
+	 */
+	if (stdinname) {
+		char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
+		sprintf(newstdin, "%s,"DEVNAME, stdinname);
+		stdinname = newstdin;
+	} else {
+		stdinname = devname;
+	}
 	error = iomux_doenv(stdin, stdinname);
+	if (stdinname != devname)
+		free(stdinname);
 	if (error)
 		return error;
 #else
Signature · 1 line
2.13.0
#1 lore raw

PATCH 1/2 usb: kbd: don't fail with iomux

Hi Rob,

Quoted 5 lines · Rob Clark

stdin might not be set, which would cause iomux_doenv() to fail therefore causing probe_usb_keyboard() to fail. Furthermore if we do have iomux enabled, the sensible thing (in terms of user experience) would be to simply add ourselves to the list of stdin devices.

Is this intended to fix a bug? What board do you see this on, and under what circumstances?

Quoted 11 lines

---
v2: address Bin Meng's comments

 common/usb_kbd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
common/usb_kbd.c +0 -0
index d2d29cc98f..72cf78abd4 100644

common/usb_kbd.c static int probe_usb_keyboard(struct usb_device *dev) · line 521

Quoted 5 lines
common/usb_kbd.c +2 -0
@@ -517,7 +517,21 @@ static int probe_usb_keyboard(struct usb_device *dev)
 
        stdinname = getenv("stdin");
 #if CONFIG_IS_ENABLED(CONSOLE_MUX)
+       char *devname = DEVNAME;
+       /* stdin might not be set yet.. either way, with console-mux the

/*
* stdin might not be

Quoted 17 lines
common/usb_kbd.c +12 -1
+        * sensible thing to do is add ourselves to the list of stdio
+        * devices:
+        */
+       if (stdinname) {
+               char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
+               sprintf(newstdin, "%s,"DEVNAME, stdinname);
+               stdinname = newstdin;
+       } else {
+               stdinname = devname;
+       }
        error = iomux_doenv(stdin, stdinname);
+       if (stdinname != devname)
+               free(stdinname);
        if (error)
                return error;
 #else
--
Quoted 2 lines

2.13.0

Regards,
Simon

author
#2 lore raw

PATCH 1/2 usb: kbd: don't fail with iomux

Quoted 11 lines · Simon Glass

Hi Rob,

Quoted 5 lines · Rob Clark

stdin might not be set, which would cause iomux_doenv() to fail therefore causing probe_usb_keyboard() to fail. Furthermore if we do have iomux enabled, the sensible thing (in terms of user experience) would be to simply add ourselves to the list of stdin devices.

Is this intended to fix a bug? What board do you see this on, and under what circumstances?

yes, db410c w/ distro-bootcmd + usb keyboard.. stdin is not set.

Quoted 12 lines
Quoted 12 lines

---
v2: address Bin Meng's comments

 common/usb_kbd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
common/usb_kbd.c +0 -0
index d2d29cc98f..72cf78abd4 100644
@@ -517,7 +517,21 @@ static int probe_usb_keyboard(struct usb_device *dev)
 
Quoted 7 lines

common/usb_kbd.c static int probe_usb_keyboard(struct usb_device *dev) · line 521

Quoted 4 lines
common/usb_kbd.c +2 -0
        stdinname = getenv("stdin");
 #if CONFIG_IS_ENABLED(CONSOLE_MUX)
+       char *devname = DEVNAME;
+       /* stdin might not be set yet.. either way, with console-mux the

/*
* stdin might not be

hmm, somehow I screwed up and re-sent the wrong patch, rather than the one that *actually* addressed Bin's comments (this was one of them)

BR,
-R

Quoted 22 lines
Quoted 17 lines
common/usb_kbd.c +12 -1
+        * sensible thing to do is add ourselves to the list of stdio
+        * devices:
+        */
+       if (stdinname) {
+               char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
+               sprintf(newstdin, "%s,"DEVNAME, stdinname);
+               stdinname = newstdin;
+       } else {
+               stdinname = devname;
+       }
        error = iomux_doenv(stdin, stdinname);
+       if (stdinname != devname)
+               free(stdinname);
        if (error)
                return error;
 #else
--
Quoted 2 lines

2.13.0

Regards,
Simon

#3 lore raw

PATCH 1/2 usb: kbd: don't fail with iomux

Hi Rob,

Quoted 10 lines · Rob Clark
Quoted 9 lines · Simon Glass

Hi Rob,

Quoted 5 lines · Rob Clark

stdin might not be set, which would cause iomux_doenv() to fail therefore causing probe_usb_keyboard() to fail. Furthermore if we do have iomux enabled, the sensible thing (in terms of user experience) would be to simply add ourselves to the list of stdin devices.

Quoted 5 lines
Quoted 2 lines

Is this intended to fix a bug? What board do you see this on, and under what circumstances?

yes, db410c w/ distro-bootcmd + usb keyboard.. stdin is not set.

OK I see. Can you please add that to the commit message also? It provides motivation for anyone looking at it later.

Quoted 20 lines
Quoted 12 lines
Quoted 12 lines

---
v2: address Bin Meng's comments

 common/usb_kbd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
common/usb_kbd.c +0 -0
index d2d29cc98f..72cf78abd4 100644
@@ -517,7 +517,21 @@ static int probe_usb_keyboard(struct usb_device *dev)
 
Quoted 7 lines

common/usb_kbd.c static int probe_usb_keyboard(struct usb_device *dev) · line 521

Quoted 4 lines
common/usb_kbd.c +2 -0
        stdinname = getenv("stdin");
 #if CONFIG_IS_ENABLED(CONSOLE_MUX)
+       char *devname = DEVNAME;
+       /* stdin might not be set yet.. either way, with console-mux the

/*
* stdin might not be

Quoted 28 lines

hmm, somehow I screwed up and re-sent the wrong patch, rather than the one that *actually* addressed Bin's comments (this was one of them)

BR,
-R

Quoted 22 lines
Quoted 17 lines
common/usb_kbd.c +12 -1
+        * sensible thing to do is add ourselves to the list of stdio
+        * devices:
+        */
+       if (stdinname) {
+               char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
+               sprintf(newstdin, "%s,"DEVNAME, stdinname);
+               stdinname = newstdin;
+       } else {
+               stdinname = devname;
+       }
        error = iomux_doenv(stdin, stdinname);
+       if (stdinname != devname)
+               free(stdinname);
        if (error)
                return error;
 #else
--
Quoted 2 lines

2.13.0

Regards,
Simon

Regards,
Simon