[U-Boot] [PATCH 3/6] sunxi: video: Add cfb console driver for sunxi

[Ian Campbell]

On Sun, 2014-11-16 at 14:15 +0100, Hans de Goede wrote:
> Hi,
> 
> Thanks for the reviews!
> 
> 
> On 11/16/2014 12:35 PM, Ian Campbell wrote:
> > On Fri, 2014-11-14 at 17:54 +0100, Hans de Goede wrote:
> >> +/* this is needed so the above will actually do something */
> >> +#define CONFIG_SYS_CONSOLE_IS_IN_ENV
> >> [...] 
> >> +#ifdef CONFIG_VIDEO
> >> +#define CONSOLE_ENV_SETTINGS \
> >> +	"stdin=serial\0" \
> >> +	"stdout=serial,vga\0" \
> >> +	"stderr=serial,vga\0"
> >> +#else
> >> +#define CONSOLE_ENV_SETTINGS
> > 
> > Now that CONFIG_SYS_CONSOLE_IS_IN_ENV is set do we need to either say
> > something here and/or provide the overwrite_console() hook?
> 
> No, if we have a need for a overwrite_console() hook, we need to also
> define CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE, see common/console.c:

Ah, README didn't mention this under CONSOLE_IS_IN_ENV, only under
OVERWRITE_ROUTINE which I didn't know to look for.

> But even then I prefer this style:
> 
> #ifdef CONFIG_VIDEO
> #define CONSOLE_OUT_SETTINGS \
>         "stdout=serial,vga\0" \
>         "stderr=serial,vga\0"
> #else
> #define CONSOLE_OUT_SETTINGS \
>         "stdout=serial\0" \
>         "stderr=serial\0"
> #endif
> 
> Over the pre-processor magic you're suggesting, as it is more
> readable IMHO, also it matches what we're do for
> #ifdef CONFIG_MMC, #ifdef CONFIG_AHCI, etc.

OK.

> 
> > Has this been tested on a serial-only board?
> 
> Erm, good question.
> 
> So the A13 olinuxino boards do not have hdmi, which means I should
> add CONFIG_VIDEO=n to their defconfigs, fixed in my local tree.
> 
> After that I've just ran some tests on an A13 board, which work fine
> (as expected, but you're right this ought to be tested).

Great, thanks,

Acked-by: Ian Campbell <ijc at hellion.org.uk>