Hi Rob, On 13 August 2017 at 12:07, Rob Clark <robdcl...@gmail.com> wrote: > On Sun, Aug 13, 2017 at 11:35 AM, Simon Glass <s...@chromium.org> wrote: >> Hi Rob, >> >> On 6 August 2017 at 05:58, Rob Clark <robdcl...@gmail.com> wrote: >>> >>> 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 don't think we should put #ifdefs around header files #includes so >> the first option seems best. There are still some header files in >> U-Boot which 'do stuff' like ensure that an option is set, etc. We >> should over time fix those up. >> >> The #include in console.h seems wrong as well. It would be good to >> move that to console.c >> . >> .> >>> 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. >> >> Yes we have had that for a while. I don't think people use it anymore. >> >>> >>> 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 >> >> I think a v4 patch is best. But you should not have the version in the >> subject there since it will end up in the commit. If you use patman it >> will produce the patch correctly. >> > > fwiw, I did already send a v4 which went with the first option..
OK. I can't seem to find it - can you please give me a patchwork link? Thanks, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot