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 <[email protected]> _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
