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

[Bin Meng]

Hi Rob,

On Tue, Aug 8, 2017 at 3:51 AM, Rob Clark <robdclark at 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 at 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
> v4: use if (CONFIG_IS_ENABLED(CONSOLE_MUX)) { ... }
>
>  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 d2d29cc98f..8edeb6c4f5 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 = getenv("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);

Sorry I should have asked this before: isn't free(devname) OK? In
previous version, it has a test logic to see whether free is needed.

> +               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
> --

Regards,
Bin