[U-Boot] [PATCH 6/6] sunxi: Add usb keyboard Kconfig option

[Ian Campbell]

On Sun, 2014-11-16 at 14:28 +0100, Hans de Goede wrote:
> Hi,
> 
> On 11/16/2014 12:55 PM, Ian Campbell wrote:
> > On Fri, 2014-11-14 at 17:54 +0100, Hans de Goede wrote:
> >> For use together with the hdmi console.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> >> ---
> >>  board/sunxi/Kconfig            |  7 +++++++
> >>  configs/Ippo_q8h_v5_defconfig  |  1 +
> >>  include/configs/sunxi-common.h | 25 +++++++++++++++++++++----
> >>  3 files changed, 29 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> >> index 422033a..246cd9a 100644
> >> --- a/board/sunxi/Kconfig
> >> +++ b/board/sunxi/Kconfig
> >> @@ -223,4 +223,11 @@ config VIDEO
> >>  	Say Y here to add support for using a cfb console on the HDMI output
> >>  	found on most sunxi devices.
> >>  
> >> +config USB_KEYBOARD
> > 
> > This seems like it ought to be under drivers/ somewhere, either
> > drivers/usb/Kconfig or drivers/input/Kconfig perhaps.
> 
> You're right the problem with that is, that what we really need is
> to Kconfig-ify the entire USB stuff (eg also CONFIG_USB_EHCI), but that
> is a bit more then I was planning on working on atm, esp. since that
> touches many many boards. So this seems best for now.

OK, is the USB custodian OK with this plan?

> >> +	boolean "Enable USB keyboard support"
> >> +	default y
> >> +	---help---
> >> +	Say Y here to add support for using a USB keyboard (typically used
> >> +	in combination with a graphical console on HDMI).
> >> +
> >>  endif
> >> diff --git a/configs/Ippo_q8h_v5_defconfig b/configs/Ippo_q8h_v5_defconfig
> >> index 53df213..50c2f93 100644
> >> --- a/configs/Ippo_q8h_v5_defconfig
> >> +++ b/configs/Ippo_q8h_v5_defconfig
> >> @@ -5,3 +5,4 @@ CONFIG_MACH_SUN8I=y
> >>  CONFIG_TARGET_IPPO_Q8H_V5=y
> >>  CONFIG_DEFAULT_DEVICE_TREE="sun8i-a23-ippo-q8h-v5.dtb"
> >>  CONFIG_VIDEO=n
> >> +CONFIG_USB_KEYBOARD=n
> > 
> > Is this the only platform with video+usb which you have? What about e.g.
> > cubie*, bananapi etc?
> 
> You mean without video + usb, since the default is y, and it is being
> forced to n here.

Sorry, I read this backwards somehow!

> No this is not the only board we support without HDMI,
> I has forgotten about the A13 boards (fixed locally now), this is the
> only board without a host usb connector (it is a tablet), it does support
> otg, but we do not support that yet.
> 
> I've also made a local change to disable CONFIG_USB_KEYBOARD by default
> in the A13 boards since although it does work there it makes little
> sense to have it when their is no video out support.

Sounds good.

> >> @@ -298,17 +304,28 @@
> >>  
> >>  #include <config_distro_bootcmd.h>
> >>  
> >> +#ifdef CONFIG_USB_KEYBOARD
> >> +#define CONSOLE_IN_SETTINGS \
> >> +	"preboot=usb start\0" \
> >> +	"stdin=serial,usbkbd\0"
> >> +#else
> >> +#define CONSOLE_IN_SETTINGS \
> >> +	"stdin=serial\0"
> >> +#endif
> >> +
> >>  #ifdef CONFIG_VIDEO
> >> -#define CONSOLE_ENV_SETTINGS \
> >> -	"stdin=serial\0" \
> >> +#define CONSOLE_OUT_SETTINGS \
> >>  	"stdout=serial,vga\0" \
> >>  	"stderr=serial,vga\0"
> >>  #else
> >> -#define CONSOLE_ENV_SETTINGS
> >> +#define CONSOLE_OUT_SETTINGS \
> >> +	"stdout=serial\0" \
> >> +	"stderr=serial\0"
> > 
> > Ah, here are the settings I asked about earlier. Are these in the wrong
> > patch or did something change in this patch which makes them needed only
> > now?
> 
> These are only necessary when CONFIG_SYS_CONSOLE_IS_IN_ENV is set, which
> sofar was only happening when CONFIG_VIDEO is set. In this patch I'm setting
> them always to also support just CONFIG_USB_KEYBOARD without CONFIG_VIDEO,
> which is a bit weird, but I do not want to disallow it.
> 
> This does mean that we should also unconditionally enable
> CONFIG_SYS_CONSOLE_IS_IN_ENV with this patch (moving it out of the
> #ifdef CONFIG_VIDEO) block, which I've not done, I'll fix this locally.

OK. Not sure what you were planning but it may makes sense to fold some
portion of that change into the earlier patch.

> > I'm not sure but I wonder if the cpp string pasting thing I suggested
> > earlier would reduce the amount of #else and duplication around here?
> 
> See my previous mail (I would like to keep this as is).

Sure.

The changes you proposed here all sound fine, I'll defer a formal ack
till I've seen them though.

Ian.