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

Ian Campbell
2014-11-16T13:34:49Z

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@hellion.org.uk>