On Sun, Aug 6, 2017 at 1:16 AM, Simon Glass <s...@chromium.org> wrote: > On 4 August 2017 at 07:16, Bin Meng <bmeng...@gmail.com> wrote: >> Hi Rob, >> >> On Fri, Aug 4, 2017 at 8:51 PM, Rob Clark <robdcl...@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 <robdcl...@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 >>> >>> (the real v3 this time) >>> >> >> As I mentioned, it's the email title, not the commit title with >> version number. If you prefer format-patch, then use >> --subject-prefix="PATCH v3" >> >>> common/usb_kbd.c | 15 +++++++++++++++ >>> 1 file changed, 15 insertions(+) > > Reviewed-by: Simon Glass <s...@chromium.org> > > Question below > >>> >>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c >>> index d2d29cc98f..d71eae61ec 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) > > Would this work: > > if (CONFIG_IS_ENABLED(CONSOLE_MUX) { > .. > } > > The #ifdef adds a code path that is not tested, so if possible we > should try to use the compiler.
I gave this a quick try.. it would require either adding an unconditional #include <iomux.h> in usb_kbd.c or dropping the #ifdef CONFIG_CONSOLE_MUX guarding the #include <iomux.h> in console.h.. not sure which is preferred? I suspect it would cause problems with -O0 but when I tried KCFLAGS=-O0 there were enough other unrelated compile errors, that I guess this isn't a legit use-case. If you want I can send a v4 that uses "if (CONFIG_IS_ENABLED(...))" (or a separate patch on top.. in which case, do you mind removing the "(v3)" in $subject when you apply, or do you prefer I spam list with yet another resend?) And in either case let me know what you prefer about iomux.h BR, -R >>> + 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 >>> -- > > Regards, > Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot