[U-Boot] [PATCH] usb: kbd: don't fail with iomux (v3)
Rob Clark
robdclark at gmail.com
Sun Aug 6 10:41:53 UTC 2017
On Sun, Aug 6, 2017 at 1:16 AM, Simon Glass <sjg at chromium.org> wrote:
> On 4 August 2017 at 07:16, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Rob,
>>
>> On Fri, Aug 4, 2017 at 8:51 PM, 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
>>>
>>> (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 <sjg at 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 think it would, except maybe if someone compiled w/ -O0 (unresolved
symbols at link time).. not sure if that is something we care about?
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
More information about the U-Boot
mailing list