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

[Hans de Goede]

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.

> 
>> +	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. 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.

>> @@ -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.

> 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).


> 
>>  #endif
>>  
>>  #define CONFIG_EXTRA_ENV_SETTINGS \
>> -	CONSOLE_ENV_SETTINGS \
>> +	CONSOLE_IN_SETTINGS \
>> +	CONSOLE_OUT_SETTINGS \
> 
> I suggest to #define CONSOLE_ENV_SETTINGS as the other two, and add STD
> to their names.

Fixed in my local tree.

Regards,

Hans