From: Ian Campbell Date: Sun, 16 Nov 2014 13:34:49 +0000 Subject: [U-Boot] [PATCH 3/6] sunxi: video: Add cfb console driver for sunxi In-Reply-To: <5468A380.7010200@redhat.com> References: <1415984088-6800-1-git-send-email-hdegoede@redhat.com> <1415984088-6800-4-git-send-email-hdegoede@redhat.com> <1416137751.25454.8.camel@hellion.org.uk> <5468A380.7010200@redhat.com> Message-ID: <1416144889.25454.18.camel@hellion.org.uk> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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