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

Hans de Goede
2014-11-16T13:28:17Z

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