* [U-Boot] [PATCH 1/2] usb: kbd: don't fail with iomux
@ 2017-08-03 13:17 Rob Clark
2017-08-03 18:25 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: Rob Clark @ 2017-08-03 13:17 UTC (permalink / raw)
To: u-boot
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>
---
v2: address Bin Meng's comments
common/usb_kbd.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index d2d29cc98f..72cf78abd4 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.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [U-Boot] [PATCH 1/2] usb: kbd: don't fail with iomux
2017-08-03 13:17 [U-Boot] [PATCH 1/2] usb: kbd: don't fail with iomux Rob Clark
@ 2017-08-03 18:25 ` Simon Glass
2017-08-03 18:57 ` Rob Clark
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2017-08-03 18:25 UTC (permalink / raw)
To: u-boot
Hi Rob,
On 3 August 2017 at 07:17, 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.
>
Is this intended to fix a bug? What board do you see this on, and
under what circumstances?
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> v2: address Bin Meng's comments
> common/usb_kbd.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> index d2d29cc98f..72cf78abd4 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
/*
* stdin might not be
> + * 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.13.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread* [U-Boot] [PATCH 1/2] usb: kbd: don't fail with iomux
2017-08-03 18:25 ` Simon Glass
@ 2017-08-03 18:57 ` Rob Clark
2017-08-03 19:07 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: Rob Clark @ 2017-08-03 18:57 UTC (permalink / raw)
To: u-boot
On Thu, Aug 3, 2017 at 2:25 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Rob,
>
> On 3 August 2017 at 07:17, 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.
>>
>
> 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.
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>> v2: address Bin Meng's comments
>> common/usb_kbd.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>> index d2d29cc98f..72cf78abd4 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
>
> /*
> * 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
>> + * 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.13.0
>>
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 6+ messages in thread* [U-Boot] [PATCH 1/2] usb: kbd: don't fail with iomux
2017-08-03 18:57 ` Rob Clark
@ 2017-08-03 19:07 ` Simon Glass
0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2017-08-03 19:07 UTC (permalink / raw)
To: u-boot
Hi Rob,
On 3 August 2017 at 12:57, Rob Clark <robdclark@gmail.com> wrote:
> On Thu, Aug 3, 2017 at 2:25 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Rob,
>>
>> On 3 August 2017 at 07:17, 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.
>>>
>>
>> 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.
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>> v2: address Bin Meng's comments
>>> common/usb_kbd.c | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>>> index d2d29cc98f..72cf78abd4 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
>>
>> /*
>> * 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
>
>>> + * 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.13.0
>>>
>>
>> Regards,
>> Simon
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 1/2] usb: kbd: don't fail with iomux
@ 2017-07-21 19:13 Rob Clark
2017-07-23 2:53 ` Bin Meng
0 siblings, 1 reply; 6+ messages in thread
From: Rob Clark @ 2017-07-21 19:13 UTC (permalink / raw)
To: u-boot
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 d2d29cc98f..72cf78abd4 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.13.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [U-Boot] [PATCH 1/2] usb: kbd: don't fail with iomux
2017-07-21 19:13 Rob Clark
@ 2017-07-23 2:53 ` Bin Meng
0 siblings, 0 replies; 6+ messages in thread
From: Bin Meng @ 2017-07-23 2:53 UTC (permalink / raw)
To: u-boot
Hi Rob,
On Sat, Jul 22, 2017 at 3:13 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.
>
> 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 d2d29cc98f..72cf78abd4 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
nits: /* should be on a single line
> + * 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);
We should check whether are are already in the stdin, if not, then add
by ourselves.
> + stdinname = newstdin;
> + } else {
> + stdinname = devname;
> + }
> error = iomux_doenv(stdin, stdinname);
> + if (stdinname != devname)
> + free(stdinname);
> if (error)
> return error;
> #else
> --
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-03 19:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-03 13:17 [U-Boot] [PATCH 1/2] usb: kbd: don't fail with iomux Rob Clark
2017-08-03 18:25 ` Simon Glass
2017-08-03 18:57 ` Rob Clark
2017-08-03 19:07 ` Simon Glass
-- strict thread matches above, loose matches on Subject: below --
2017-07-21 19:13 Rob Clark
2017-07-23 2:53 ` Bin Meng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox