Hi Rob,
On 6 October 2017 at 10:56, Rob Clark robdclark@gmail.com wrote:
On Fri, Oct 6, 2017 at 12:15 PM, Anatolij Gustschin agust@denx.de wrote:
On Sat, 30 Sep 2017 10:19:17 +0200 Anatolij Gustschin agust@denx.de wrote:
As mentioned in review comments for ANSI escape sequence support patches, this should be optional to reduce code size. Disable escape sequence support when CONFIG_VIDEO_ANSI is not enabled.
Signed-off-by: Anatolij Gustschin agust@denx.de
This patch is based on basic ANSI escape seq. support series: https://www.mail-archive.com/u-boot@lists.denx.de/msg263777.html
drivers/video/vidconsole-uclass.c | 6 ++++++ include/video_console.h | 4 ++++ 2 files changed, 10 insertions(+)
Applied to u-boot-video/master.
Like I mentioned before, we shouldn't really need this patch.. It only makes a trivial difference in size (which you could probably get back by just adding #ifdef VIDEO_ANSI / #endif around the single case statement in vidconsole_put_char())
text data bss dec hex 1937 224 0 2161 871 VIDEO_ANSI disabled with this patch 2002 224 0 2226 8b2 VIDEO_ANSI disabled without this patch 2698 224 0 2922 b6a VIDEO_ANSI enabled
And it makes the code a lot uglier and removes at least compile-time testing when VIDEO_ANSI is disabled.
So I think you should drop/revert this patch
Seems reasonable to me.
Regards, Simon