[U-Boot] [PATCH] efi_loader: GOP fix for no display

[U-Boot] [PATCH] efi_loader: GOP fix for no display

[Rob Clark]

uclass_first_device() returns 0 if there is no device, but error if
there is a device that failed to probe.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 lib/efi_loader/efi_gop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
index e063e0c79b..411a8c9226 100644
--- a/lib/efi_loader/efi_gop.c
+++ b/lib/efi_loader/efi_gop.c
@@ -137,7 +137,7 @@ int efi_gop_register(void)
 	struct udevice *vdev;
 
 	/* We only support a single video output device for now */
-	if (uclass_first_device(UCLASS_VIDEO, &vdev))
+	if (uclass_first_device(UCLASS_VIDEO, &vdev) || !vdev)
 		return -1;
 
 	struct video_priv *priv = dev_get_uclass_priv(vdev);
-- 
2.13.0

[U-Boot] [PATCH] usb: kbd: don't fail with iomux

[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.

This fixes an issue with usbkbd on dragonboard410c with distro-
bootcmd, where stdin is not set (so stdinname is null).

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
v2: address Bin's review comments
v3: fix fail with free()ing if usbkbd is already in stdin env variable
    pointed out by Simon

 common/usb_kbd.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index d2d29cc98f..703dd748f5 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -517,7 +517,22 @@ 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 && !strstr(stdinname, DEVNAME)) {
+		char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
+		sprintf(newstdin, "%s,"DEVNAME, stdinname);
+		stdinname = newstdin;
+	} else if (!stdinname) {
+		stdinname = devname;
+	}
 	error = iomux_doenv(stdin, stdinname);
+	if (stdinname != devname)
+		free(stdinname);
 	if (error)
 		return error;
 #else
-- 
2.13.0

[U-Boot] [PATCH] usb: kbd: don't fail with iomux

[Bin Meng]

Hi Rob,

On Fri, Aug 4, 2017 at 7:52 PM, Rob Clark <robdclark@gmail.com> wrote:
> 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.
>
> This fixes an issue with usbkbd on dragonboard410c with distro-
> bootcmd, where stdin is not set (so stdinname is null).
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> v2: address Bin's review comments
> v3: fix fail with free()ing if usbkbd is already in stdin env variable
>     pointed out by Simon
>

Can you please indicate the version number in the email title next
time? It's really hard to track which version the patch is.

>  common/usb_kbd.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> index d2d29cc98f..703dd748f5 100644
> --- a/common/usb_kbd.c
> +++ b/common/usb_kbd.c
> @@ -517,7 +517,22 @@ 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 && !strstr(stdinname, DEVNAME)) {
> +               char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
> +               sprintf(newstdin, "%s,"DEVNAME, stdinname);
> +               stdinname = newstdin;
> +       } else if (!stdinname) {
> +               stdinname = devname;
> +       }
>         error = iomux_doenv(stdin, stdinname);
> +       if (stdinname != devname)
> +               free(stdinname);
>         if (error)
>                 return error;
>  #else

And what's the difference between this "v3" and previous "v2"?

Previous v2 is here: http://patchwork.ozlabs.org/patch/797418/

Regards,
Bin

[U-Boot] [PATCH] usb: kbd: don't fail with iomux

[Rob Clark]

On Fri, Aug 4, 2017 at 8:35 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Rob,
>
> On Fri, Aug 4, 2017 at 7:52 PM, Rob Clark <robdclark@gmail.com> wrote:
>> 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.
>>
>> This fixes an issue with usbkbd on dragonboard410c with distro-
>> bootcmd, where stdin is not set (so stdinname is null).
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>> v2: address Bin's review comments
>> v3: fix fail with free()ing if usbkbd is already in stdin env variable
>>     pointed out by Simon
>>
>
> Can you please indicate the version number in the email title next
> time? It's really hard to track which version the patch is.

ok, I had assumed u-boot followed linux kernel conventions of history
below the "---" so it doesn't end up in git history.

>>  common/usb_kbd.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>> index d2d29cc98f..703dd748f5 100644
>> --- a/common/usb_kbd.c
>> +++ b/common/usb_kbd.c
>> @@ -517,7 +517,22 @@ 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 && !strstr(stdinname, DEVNAME)) {
>> +               char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
>> +               sprintf(newstdin, "%s,"DEVNAME, stdinname);
>> +               stdinname = newstdin;
>> +       } else if (!stdinname) {
>> +               stdinname = devname;
>> +       }
>>         error = iomux_doenv(stdin, stdinname);
>> +       if (stdinname != devname)
>> +               free(stdinname);
>>         if (error)
>>                 return error;
>>  #else
>
> And what's the difference between this "v3" and previous "v2"?
>
> Previous v2 is here: http://patchwork.ozlabs.org/patch/797418/
>

Nothing because I apparently passed the old commit sha to
git-format-patch again.  Sorry about that.  (I guess I'm doing too
many things at once with trying to finalize a big efi_loader patchset
in parallel with bisecting a couple remaining nonsensical travis
failures as well as updating and resending these various other
unrelated patches.)

I'll resend the real v3 in a moment.

BR,
-R

[U-Boot] [PATCH] usb: kbd: don't fail with iomux

[Bin Meng]

Hi Rob,

On Fri, Aug 4, 2017 at 8:51 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Fri, Aug 4, 2017 at 8:35 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Rob,
>>
>> On Fri, Aug 4, 2017 at 7:52 PM, Rob Clark <robdclark@gmail.com> wrote:
>>> 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.
>>>
>>> This fixes an issue with usbkbd on dragonboard410c with distro-
>>> bootcmd, where stdin is not set (so stdinname is null).
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>> v2: address Bin's review comments
>>> v3: fix fail with free()ing if usbkbd is already in stdin env variable
>>>     pointed out by Simon
>>>
>>
>> Can you please indicate the version number in the email title next
>> time? It's really hard to track which version the patch is.
>
> ok, I had assumed u-boot followed linux kernel conventions of history
> below the "---" so it doesn't end up in git history.

Yes, U-Boot follows the same Linux kernel conventions with regard to
patch submission. What I was saying is not the commit title. It's the
email title, something like "[PATCH v3] usb: kbd: don't fail with
iomux"

>
>>>  common/usb_kbd.c | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>>> index d2d29cc98f..703dd748f5 100644
>>> --- a/common/usb_kbd.c
>>> +++ b/common/usb_kbd.c
>>> @@ -517,7 +517,22 @@ 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 && !strstr(stdinname, DEVNAME)) {
>>> +               char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
>>> +               sprintf(newstdin, "%s,"DEVNAME, stdinname);
>>> +               stdinname = newstdin;
>>> +       } else if (!stdinname) {
>>> +               stdinname = devname;
>>> +       }
>>>         error = iomux_doenv(stdin, stdinname);
>>> +       if (stdinname != devname)
>>> +               free(stdinname);
>>>         if (error)
>>>                 return error;
>>>  #else
>>
>> And what's the difference between this "v3" and previous "v2"?
>>
>> Previous v2 is here: http://patchwork.ozlabs.org/patch/797418/
>>
>
> Nothing because I apparently passed the old commit sha to
> git-format-patch again.  Sorry about that.  (I guess I'm doing too
> many things at once with trying to finalize a big efi_loader patchset
> in parallel with bisecting a couple remaining nonsensical travis
> failures as well as updating and resending these various other
> unrelated patches.)
>

You really should start trying patman instead of git-format-patch :)

> I'll resend the real v3 in a moment.
>

Regards,
Bin

[U-Boot] efi_loader: GOP fix for no display

[Alexander Graf]

> uclass_first_device() returns 0 if there is no device, but error if
> there is a device that failed to probe.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Thanks, applied to efi-next

Alex

[U-Boot] [PATCH] usb: kbd: Don't fail with iomux

[Simon Glass]

+Tom

On 26 September 2017 at 19:19, Rob Clark <robdclark@gmail.com> wrote:
> 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.
>
> This fixes an issue with usbkbd on dragonboard410c with distro-
> bootcmd, where stdin is not set (so stdinname is null).
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> Somehow this patch was dropped on the floor.  I don't remember
> which version # this is up to, search the list if you care.  But
> this is the latest.  I only noticed it was missing because u-boot
> crashes when you boot with usb-keyboard plugged in (at least on
> db410c) without it.  So someone please apply this patch before it
> gets lost again.
>
>  common/usb_kbd.c  | 46 +++++++++++++++++++++++++++++++---------------
>  include/console.h |  2 --
>  2 files changed, 31 insertions(+), 17 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH] usb: kbd: Don't fail with iomux

[Bin Meng]

On Wed, Sep 27, 2017 at 9:19 AM, Rob Clark <robdclark@gmail.com> wrote:
> 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.
>
> This fixes an issue with usbkbd on dragonboard410c with distro-
> bootcmd, where stdin is not set (so stdinname is null).
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> Somehow this patch was dropped on the floor.  I don't remember
> which version # this is up to, search the list if you care.  But
> this is the latest.  I only noticed it was missing because u-boot
> crashes when you boot with usb-keyboard plugged in (at least on
> db410c) without it.  So someone please apply this patch before it
> gets lost again.
>
>  common/usb_kbd.c  | 46 +++++++++++++++++++++++++++++++---------------
>  include/console.h |  2 --
>  2 files changed, 31 insertions(+), 17 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH] usb: kbd: Don't fail with iomux

[Peter Robinson]

On Wed, Sep 27, 2017 at 2:19 AM, Rob Clark <robdclark@gmail.com> wrote:
> 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.
>
> This fixes an issue with usbkbd on dragonboard410c with distro-
> bootcmd, where stdin is not set (so stdinname is null).
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
Tested-by: Peter Robinson <pbrobinson@gmail.com>

Tested on RPi3 and a couple of other devices.

> ---
> Somehow this patch was dropped on the floor.  I don't remember
> which version # this is up to, search the list if you care.  But
> this is the latest.  I only noticed it was missing because u-boot
> crashes when you boot with usb-keyboard plugged in (at least on
> db410c) without it.  So someone please apply this patch before it
> gets lost again.
>
>  common/usb_kbd.c  | 46 +++++++++++++++++++++++++++++++---------------
>  include/console.h |  2 --
>  2 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> index a323d72a36..4c3ad95fca 100644
> --- a/common/usb_kbd.c
> +++ b/common/usb_kbd.c
> @@ -516,23 +516,39 @@ static int probe_usb_keyboard(struct usb_device *dev)
>                 return error;
>
>         stdinname = env_get("stdin");
> -#if CONFIG_IS_ENABLED(CONSOLE_MUX)
> -       error = iomux_doenv(stdin, stdinname);
> -       if (error)
> -               return error;
> -#else
> -       /* Check if this is the standard input device. */
> -       if (strcmp(stdinname, DEVNAME))
> -               return 1;
> +       if (CONFIG_IS_ENABLED(CONSOLE_MUX)) {
> +               char *devname = DEVNAME;
> +               char *newstdin = NULL;
> +               /*
> +                * 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 && !strstr(stdinname, DEVNAME)) {
> +                       newstdin = malloc(strlen(stdinname) +
> +                                       strlen(","DEVNAME) + 1);
> +                       sprintf(newstdin, "%s,"DEVNAME, stdinname);
> +                       stdinname = newstdin;
> +               } else if (!stdinname) {
> +                       stdinname = devname;
> +               }
> +               error = iomux_doenv(stdin, stdinname);
> +               free(newstdin);
> +               if (error)
> +                       return error;
> +       } else {
> +               /* Check if this is the standard input device. */
> +               if (strcmp(stdinname, DEVNAME))
> +                       return 1;
>
> -       /* Reassign the console */
> -       if (overwrite_console())
> -               return 1;
> +               /* Reassign the console */
> +               if (overwrite_console())
> +                       return 1;
>
> -       error = console_assign(stdin, DEVNAME);
> -       if (error)
> -               return error;
> -#endif
> +               error = console_assign(stdin, DEVNAME);
> +               if (error)
> +                       return error;
> +       }
>
>         return 0;
>  }
> diff --git a/include/console.h b/include/console.h
> index cea29ed6dc..7dfd36d7d1 100644
> --- a/include/console.h
> +++ b/include/console.h
> @@ -57,8 +57,6 @@ int console_announce_r(void);
>  /*
>   * CONSOLE multiplexing.
>   */
> -#ifdef CONFIG_CONSOLE_MUX
>  #include <iomux.h>
> -#endif
>
>  #endif
> --
> 2.13.5
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH] usb: kbd: Don't fail with iomux

[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.

This fixes an issue with usbkbd on dragonboard410c with distro-
bootcmd, where stdin is not set (so stdinname is null).

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
Somehow this patch was dropped on the floor.  I don't remember
which version # this is up to, search the list if you care.  But
this is the latest.  I only noticed it was missing because u-boot
crashes when you boot with usb-keyboard plugged in (at least on
db410c) without it.  So someone please apply this patch before it
gets lost again.

 common/usb_kbd.c  | 46 +++++++++++++++++++++++++++++++---------------
 include/console.h |  2 --
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index a323d72a36..4c3ad95fca 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -516,23 +516,39 @@ static int probe_usb_keyboard(struct usb_device *dev)
 		return error;
 
 	stdinname = env_get("stdin");
-#if CONFIG_IS_ENABLED(CONSOLE_MUX)
-	error = iomux_doenv(stdin, stdinname);
-	if (error)
-		return error;
-#else
-	/* Check if this is the standard input device. */
-	if (strcmp(stdinname, DEVNAME))
-		return 1;
+	if (CONFIG_IS_ENABLED(CONSOLE_MUX)) {
+		char *devname = DEVNAME;
+		char *newstdin = NULL;
+		/*
+		 * 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 && !strstr(stdinname, DEVNAME)) {
+			newstdin = malloc(strlen(stdinname) +
+					strlen(","DEVNAME) + 1);
+			sprintf(newstdin, "%s,"DEVNAME, stdinname);
+			stdinname = newstdin;
+		} else if (!stdinname) {
+			stdinname = devname;
+		}
+		error = iomux_doenv(stdin, stdinname);
+		free(newstdin);
+		if (error)
+			return error;
+	} else {
+		/* Check if this is the standard input device. */
+		if (strcmp(stdinname, DEVNAME))
+			return 1;
 
-	/* Reassign the console */
-	if (overwrite_console())
-		return 1;
+		/* Reassign the console */
+		if (overwrite_console())
+			return 1;
 
-	error = console_assign(stdin, DEVNAME);
-	if (error)
-		return error;
-#endif
+		error = console_assign(stdin, DEVNAME);
+		if (error)
+			return error;
+	}
 
 	return 0;
 }
diff --git a/include/console.h b/include/console.h
index cea29ed6dc..7dfd36d7d1 100644
--- a/include/console.h
+++ b/include/console.h
@@ -57,8 +57,6 @@ int console_announce_r(void);
 /*
  * CONSOLE multiplexing.
  */
-#ifdef CONFIG_CONSOLE_MUX
 #include <iomux.h>
-#endif
 
 #endif
-- 
2.13.5

[U-Boot] [PATCH] usb: kbd: don't fail with iomux

[Bin Meng]

On Fri, Aug 4, 2017 at 3:22 AM, Rob Clark <robdclark@gmail.com> wrote:
> 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.
>
> This fixes an issue with usbkbd on dragonboard410c with distro-
> bootcmd, where stdin is not set (so stdinname is null).
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  common/usb_kbd.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH] usb: kbd: don't fail with iomux

[Rob Clark]

On Thu, Aug 3, 2017 at 3:37 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Rob,
>
> On 3 August 2017 at 13:22, Rob Clark <robdclark@gmail.com> wrote:
>> 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.
>>
>> This fixes an issue with usbkbd on dragonboard410c with distro-
>> bootcmd, where stdin is not set (so stdinname is null).
>
> Is this version 2?

yes, this is the actual v2 (note the fake v2 I sent earlier today)

>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  common/usb_kbd.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>> index d2d29cc98f..703dd748f5 100644
>> --- a/common/usb_kbd.c
>> +++ b/common/usb_kbd.c
>> @@ -517,7 +517,22 @@ 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 && !strstr(stdinname, DEVNAME)) {
>> +               char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
>> +               sprintf(newstdin, "%s,"DEVNAME, stdinname);
>> +               stdinname = newstdin;
>> +       } else if (!stdinname) {
>> +               stdinname = devname;
>> +       }
>>         error = iomux_doenv(stdin, stdinname);
>> +       if (stdinname != devname)
>> +               free(stdinname);
>
> Are you sure that free() will work? It looks like it is intended to
> free newstdin. I think it would be better to have a boolean for
> whether to free(), instead.

oh, that's true.. on v1 you would always go down one or the other leg
of the if/else, which is no longer the case

BR,
-R

>>         if (error)
>>                 return error;
>>  #else
>> --
>> 2.13.0
>>
>
> Regards,
> Simon

[U-Boot] [PATCH] usb: kbd: don't fail with iomux

[Simon Glass]

Hi Rob,

On 3 August 2017 at 13:22, Rob Clark <robdclark@gmail.com> wrote:
> 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.
>
> This fixes an issue with usbkbd on dragonboard410c with distro-
> bootcmd, where stdin is not set (so stdinname is null).

Is this version 2?

>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  common/usb_kbd.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> index d2d29cc98f..703dd748f5 100644
> --- a/common/usb_kbd.c
> +++ b/common/usb_kbd.c
> @@ -517,7 +517,22 @@ 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 && !strstr(stdinname, DEVNAME)) {
> +               char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
> +               sprintf(newstdin, "%s,"DEVNAME, stdinname);
> +               stdinname = newstdin;
> +       } else if (!stdinname) {
> +               stdinname = devname;
> +       }
>         error = iomux_doenv(stdin, stdinname);
> +       if (stdinname != devname)
> +               free(stdinname);

Are you sure that free() will work? It looks like it is intended to
free newstdin. I think it would be better to have a boolean for
whether to free(), instead.

>         if (error)
>                 return error;
>  #else
> --
> 2.13.0
>

Regards,
Simon

[U-Boot] [PATCH] usb: kbd: don't fail with iomux

[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.

This fixes an issue with usbkbd on dragonboard410c with distro-
bootcmd, where stdin is not set (so stdinname is null).

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 common/usb_kbd.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index d2d29cc98f..703dd748f5 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -517,7 +517,22 @@ 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 && !strstr(stdinname, DEVNAME)) {
+		char *newstdin = malloc(strlen(stdinname) + strlen(","DEVNAME) + 1);
+		sprintf(newstdin, "%s,"DEVNAME, stdinname);
+		stdinname = newstdin;
+	} else if (!stdinname) {
+		stdinname = devname;
+	}
 	error = iomux_doenv(stdin, stdinname);
+	if (stdinname != devname)
+		free(stdinname);
 	if (error)
 		return error;
 #else
-- 
2.13.0

[U-Boot] [PATCH] usb: kbd: don't fail with iomux

[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.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 common/usb_kbd.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index d2d29cc..72cf78a 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -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
-- 
2.9.4