dm: video: Add basic ANSI escape sequence support
PATCH 1/1 dm: video: Add basic ANSI escape sequence support
Really just the subset that is needed by efi_console. Perhaps more will be added later, for example color support would be useful to implement efi_cout_set_attribute().
--- drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++ drivers/video/video-uclass.c | 4 +- include/video.h | 7 +++ include/video_console.h | 11 ++++ 4 files changed, 131 insertions(+), 3 deletions(-)
drivers/video/vidconsole-uclass.c +112 -0
@@ -9,6 +9,7 @@*/ #include <common.h> +#include <linux/ctype.h> #include <dm.h> #include <video.h> #include <video_console.h>
@@ -107,12 +108,123 @@ static void vidconsole_newline(struct udevice *dev)video_sync (dev->parent); } +/* +* Parse a number from string that ends in a non-numeric character.. +* sscanf() would be nice. This is just enough for parsing ANSI escape +* sequences. +*/ +static char *parsenum (char *s,int *num) +{ +int n =0 ; + +while (isdigit (*s)) { + n = (10 * n) + (*s -'0' ); + s++; + } + + *num = n; +return s; +} + +static void vidconsole_escape_char (struct udevice *dev,char ch) +{ +struct vidconsole_priv *priv =dev_get_uclass_priv (dev); + +/* Sanity checking for bogus ESC sequences: */ +if (priv->escape_len >=sizeof (priv->escape_buf)) + goto error; +if (priv->escape_len ==0 && ch !='[' ) + goto error; + + priv->escape_buf[priv->escape_len++] = ch; + +/* +* Escape sequences are terminated by a letter, so keep +* accumulating until we get one: +*/ +if (!isalpha (ch)) +return ; + +/* +* clear escape mode first, otherwise things will get highly +* surprising if you hit any debug prints that come back to +* this console. +*/ + priv->escape =0 ; + +switch (ch) { +case 'H' : +case 'f' : { +int row, col; +char *s = priv->escape_buf; + +/* +* Set cursor position: [%d;%df or [%d;%dH +*/ + s++;/* [ */ + s =parsenum (s, &row); + s++;/* ; */ + s =parsenum (s, &col); + + priv->ycur = row * priv->y_charsize; + priv->xcur_frac = priv->xstart_frac + +VID_TO_POS (col * priv->x_charsize); + +break ; + } +case 'J' : { +int mode; + +/* +* Clear part/all screen: +* [J or [0J - clear screen from cursor down +* [1J - clear screen from cursor up +* [2J - clear entire screen +* +* TODO we really only handle entire-screen case, others +* probably require some additions to video-uclass (and +* are not really needed yet by efi_console) +*/ +parsenum (priv->escape_buf +1 , &mode); + +if (mode ==2 ) { +video_clear (dev->parent); +video_sync (dev->parent); + priv->ycur =0 ; + priv->xcur_frac = priv->xstart_frac; + }else { +debug ("unsupported clear mode: %d\n" , mode); + } +break ; + } +default : +debug ("unrecognized escape sequence: %*s\n" , + priv->escape_len, priv->escape_buf); + } + +return ; + +error: +/* something went wrong, just revert to normal mode: */ + priv->escape =0 ; +return ; +} +int vidconsole_put_char (struct udevice *dev,char ch) {struct vidconsole_priv *priv =dev_get_uclass_priv (dev);int ret; +if (priv->escape) { +vidconsole_escape_char (dev, ch); +return 0 ; + } +switch (ch) { +case '\x1b' : + priv->escape_len =0 ; + priv->escape =1 ; +break ;case '\a' :/* beep */ break ;
drivers/video/video-uclass.c +1 -3
@@ -87,7 +87,7 @@ int video_reserve(ulong *addrp)return 0 ; } -static int video_clear (struct udevice *dev) +void video_clear (struct udevice *dev) {struct video_priv *priv =dev_get_uclass_priv (dev);
@@ -100,8 +100,6 @@ static int video_clear(struct udevice *dev) }else {memset (priv->fb, priv->colour_bg, priv->fb_size); } - -return 0 ; }/* Flush video activity to the caches */
include/video.h +7 -0
@@ -115,6 +115,13 @@ struct video_ops {int video_reserve (ulong *addrp);/** +* video_clear() - Clear a device's frame buffer to background color. +* +* @dev: Device to clear +*/ +void video_clear (struct udevice *dev); + +/** * video_sync() - Sync a device's frame buffer with its hardware * * Some frame buffers are cached or have a secondary frame buffer. This
include/video_console.h +11 -0
@@ -29,6 +29,9 @@* @xsize_frac: Width of the display in fractional units * @xstart_frac: Left margin for the text console in fractional units * @last_ch: Last character written to the text console on this line +* @escape: TRUE if currently accumulating an ANSI escape sequence +* @escape_len: Length of accumulated escape sequence so far +* @escape_buf: Buffer to accumulate escape sequence */ struct vidconsole_priv {struct stdio_dev sdev;
@@ -42,6 +45,14 @@ struct vidconsole_priv {int xsize_frac;int xstart_frac;int last_ch; +/* +* ANSI escape sequences are accumulated character by character, +* starting after the ESC char (0x1b) until the entire sequence +* is consumed at which point it is acted upon. +*/ +int escape; +int escape_len; +char escape_buf[32 ]; };/**
Signature · 1 line
2.13.5
PATCH 1/1 dm: video: Fix cache flushes
Content can come to screen via putc() and we cannot always rely on updates ending with a puts(). This is the case with efi_console output to vidconsole. Fixes corruption with Shell.efi.
--- drivers/video/vidconsole-uclass.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/video/vidconsole-uclass.c +3 -0
@@ -163,6 +163,7 @@ static void vidconsole_putc(struct stdio_dev *sdev, const char ch)struct udevice *dev = sdev->priv;vidconsole_put_char (dev, ch); +video_sync (dev->parent); }static void vidconsole_puts (struct stdio_dev *sdev,const char *s)
@@ -260,6 +261,8 @@ static int do_video_puts(cmd_tbl_t *cmdtp, int flag, int argc,for (s = argv[1 ]; *s; s++)vidconsole_put_char (dev, *s); +video_sync (dev->parent); +return 0 ; }
Signature · 1 line
2.13.5
PATCH 1/1 dm: video: Add basic ANSI escape sequence support
In reply to Rob ClarkQuoted 25 lines · Rob Clark
Really just the subset that is needed by
efi_console. Perhaps more will be added later, for example color support would be useful to implementefi_cout_set_attribute().Signed-off-by Rob Clark <[email protected]>--- drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++ drivers/video/video-uclass.c | 4 +- include/video.h | 7 +++ include/video_console.h | 11 ++++ 4 files changed, 131 insertions(+), 3 deletions(-)drivers/video/vidconsole-uclass.c +1
-0@@ -9,6 +9,7 @@*/ #include <common.h> +#include <linux/ctype.h> #include <dm.h> #include <video.h> #include <video_console.h>
drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 116
Quoted 9 lines
drivers/video/vidconsole-uclass.c +6
-0@@ -107,12 +108,123 @@ static void vidconsole_newline(struct udevice *dev)video_sync (dev->parent); } +/* +* Parse a number from string that ends in a non-numeric character.. +* sscanf() would be nice. This is just enough for parsing ANSI escape +* sequences. +*/ +static char *parsenum (char *s,int *num)
Can you use simple_strtoul() or similar?
drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 129
Quoted 13 lines
drivers/video/vidconsole-uclass.c +13
-0+{ +int n =0 ; + +while (isdigit (*s)) { + n = (10 * n) + (*s -'0' ); + s++; + } + + *num = n; +return s; +} + +static void vidconsole_escape_char (struct udevice *dev,char ch)
Please add a function comment
Quoted 82 lines
drivers/video/vidconsole-uclass.c +82
-0+{ +struct vidconsole_priv *priv =dev_get_uclass_priv (dev); + +/* Sanity checking for bogus ESC sequences: */ +if (priv->escape_len >=sizeof (priv->escape_buf)) + goto error; +if (priv->escape_len ==0 && ch !='[' ) + goto error; + + priv->escape_buf[priv->escape_len++] = ch; + +/* +* Escape sequences are terminated by a letter, so keep +* accumulating until we get one: +*/ +if (!isalpha (ch)) +return ; + +/* +* clear escape mode first, otherwise things will get highly +* surprising if you hit any debug prints that come back to +* this console. +*/ + priv->escape =0 ; + +switch (ch) { +case 'H' : +case 'f' : { +int row, col; +char *s = priv->escape_buf; + +/* +* Set cursor position: [%d;%df or [%d;%dH +*/ + s++;/* [ */ + s =parsenum (s, &row); + s++;/* ; */ + s =parsenum (s, &col); + + priv->ycur = row * priv->y_charsize; + priv->xcur_frac = priv->xstart_frac + +VID_TO_POS (col * priv->x_charsize); + +break ; + } +case 'J' : { +int mode; + +/* +* Clear part/all screen: +* [J or [0J - clear screen from cursor down +* [1J - clear screen from cursor up +* [2J - clear entire screen +* +* TODO we really only handle entire-screen case, others +* probably require some additions to video-uclass (and +* are not really needed yet by efi_console) +*/ +parsenum (priv->escape_buf +1 , &mode); + +if (mode ==2 ) { +video_clear (dev->parent); +video_sync (dev->parent); + priv->ycur =0 ; + priv->xcur_frac = priv->xstart_frac; + }else { +debug ("unsupported clear mode: %d\n" , mode); + } +break ; + } +default : +debug ("unrecognized escape sequence: %*s\n" , + priv->escape_len, priv->escape_buf); + } + +return ; + +error: +/* something went wrong, just revert to normal mode: */ + priv->escape =0 ; +return ; +}
drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 221
Quoted 10 lines
drivers/video/vidconsole-uclass.c +5
-0+int vidconsole_put_char (struct udevice *dev,char ch) {struct vidconsole_priv *priv =dev_get_uclass_priv (dev);int ret; +if (priv->escape) { +vidconsole_escape_char (dev, ch); +return 0 ; + }
Is it possible to add a CONFIG_VIDEO_ANSI option to enable this? Perhaps it could be on by default. Then:
if (CONFIG_IS_ENABLED(VIDEO_ANSI) && priv-escape) {
...
This helps to reduce base code size.
Quoted 81 lines
drivers/video/vidconsole-uclass.c +5
-0+switch (ch) { +case '\x1b' : + priv->escape_len =0 ; + priv->escape =1 ; +break ;case '\a' :/* beep */ break ;drivers/video/video-uclass.c +1
-3@@ -87,7 +87,7 @@ int video_reserve(ulong *addrp)return 0 ; } -static int video_clear (struct udevice *dev) +void video_clear (struct udevice *dev) {struct video_priv *priv =dev_get_uclass_priv (dev);@@ -100,8 +100,6 @@ static int video_clear(struct udevice *dev) }else {memset (priv->fb, priv->colour_bg, priv->fb_size); } - -return 0 ; }/* Flush video activity to the caches */ include/video.h +7
-0@@ -115,6 +115,13 @@ struct video_ops {int video_reserve (ulong *addrp);/** +* video_clear() - Clear a device's frame buffer to background color. +* +* @dev: Device to clear +*/ +void video_clear (struct udevice *dev); + +/** * video_sync() - Sync a device's frame buffer with its hardware * * Some frame buffers are cached or have a secondary frame buffer. This include/video_console.h +11
-1@@ -29,6 +29,9 @@* @xsize_frac: Width of the display in fractional units * @xstart_frac: Left margin for the text console in fractional units * @last_ch: Last character written to the text console on this line +* @escape: TRUE if currently accumulating an ANSI escape sequence +* @escape_len: Length of accumulated escape sequence so far +* @escape_buf: Buffer to accumulate escape sequence */ struct vidconsole_priv {struct stdio_dev sdev;@@ -42,6 +45,14 @@ struct vidconsole_priv {int xsize_frac;int xstart_frac;int last_ch; +/* +* ANSI escape sequences are accumulated character by character, +* starting after the ESC char (0x1b) until the entire sequence +* is consumed at which point it is acted upon. +*/ +int escape; +int escape_len; +char escape_buf[32 ]; };/** -- 2.13.5
PATCH 1/1 dm: video: Add basic ANSI escape sequence support
Quoted 29 lines · Simon Glass
Quoted 28 lines · Rob Clark
Really just the subset that is needed by
efi_console. Perhaps more will be added later, for example color support would be useful to implementefi_cout_set_attribute().Signed-off-by Rob Clark <[email protected]>--- drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++ drivers/video/video-uclass.c | 4 +- include/video.h | 7 +++ include/video_console.h | 11 ++++ 4 files changed, 131 insertions(+), 3 deletions(-)drivers/video/vidconsole-uclass.c +1
-0@@ -9,6 +9,7 @@*/ #include <common.h> +#include <linux/ctype.h> #include <dm.h> #include <video.h> #include <video_console.h> @@ -107,12 +108,123 @@ static void vidconsole_newline(struct udevice *dev)video_sync (dev->parent); }
Quoted 8 lines
drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 116
Quoted 6 lines
drivers/video/vidconsole-uclass.c +6
-0+/* +* Parse a number from string that ends in a non-numeric character.. +* sscanf() would be nice. This is just enough for parsing ANSI escape +* sequences. +*/ +static char *parsenum (char *s,int *num)Can you use
simple_strtoul()or similar?
Possibly, but I'm not sure it is a good idea.. I don't think escape sequences are meant to be encoded with hex or octal number strings. From a quick look, I don't see any escape code terminated with 'x', so maybe it would end up working ok.. but something like ESC[0x1234m should be an escape sequence terminated with x followed by normal chars 1234m and strtoul would get that wrong..
BR,
-R
Quoted 198 lines
drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 129
Quoted 13 lines
drivers/video/vidconsole-uclass.c +13
-0+{ +int n =0 ; + +while (isdigit (*s)) { + n = (10 * n) + (*s -'0' ); + s++; + } + + *num = n; +return s; +} + +static void vidconsole_escape_char (struct udevice *dev,char ch)Please add a function comment
Quoted 82 lines
drivers/video/vidconsole-uclass.c +82
-0+{ +struct vidconsole_priv *priv =dev_get_uclass_priv (dev); + +/* Sanity checking for bogus ESC sequences: */ +if (priv->escape_len >=sizeof (priv->escape_buf)) + goto error; +if (priv->escape_len ==0 && ch !='[' ) + goto error; + + priv->escape_buf[priv->escape_len++] = ch; + +/* +* Escape sequences are terminated by a letter, so keep +* accumulating until we get one: +*/ +if (!isalpha (ch)) +return ; + +/* +* clear escape mode first, otherwise things will get highly +* surprising if you hit any debug prints that come back to +* this console. +*/ + priv->escape =0 ; + +switch (ch) { +case 'H' : +case 'f' : { +int row, col; +char *s = priv->escape_buf; + +/* +* Set cursor position: [%d;%df or [%d;%dH +*/ + s++;/* [ */ + s =parsenum (s, &row); + s++;/* ; */ + s =parsenum (s, &col); + + priv->ycur = row * priv->y_charsize; + priv->xcur_frac = priv->xstart_frac + +VID_TO_POS (col * priv->x_charsize); + +break ; + } +case 'J' : { +int mode; + +/* +* Clear part/all screen: +* [J or [0J - clear screen from cursor down +* [1J - clear screen from cursor up +* [2J - clear entire screen +* +* TODO we really only handle entire-screen case, others +* probably require some additions to video-uclass (and +* are not really needed yet by efi_console) +*/ +parsenum (priv->escape_buf +1 , &mode); + +if (mode ==2 ) { +video_clear (dev->parent); +video_sync (dev->parent); + priv->ycur =0 ; + priv->xcur_frac = priv->xstart_frac; + }else { +debug ("unsupported clear mode: %d\n" , mode); + } +break ; + } +default : +debug ("unrecognized escape sequence: %*s\n" , + priv->escape_len, priv->escape_buf); + } + +return ; + +error: +/* something went wrong, just revert to normal mode: */ + priv->escape =0 ; +return ; +}drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 221
Quoted 10 lines
drivers/video/vidconsole-uclass.c +5
-0+int vidconsole_put_char (struct udevice *dev,char ch) {struct vidconsole_priv *priv =dev_get_uclass_priv (dev);int ret; +if (priv->escape) { +vidconsole_escape_char (dev, ch); +return 0 ; + }Is it possible to add a
CONFIG_VIDEO_ANSIoption to enable this? Perhaps it could be on by default. Then:if (
CONFIG_IS_ENABLED(VIDEO_ANSI)&& priv-escape) {
...This helps to reduce base code size.
Quoted 81 lines
drivers/video/vidconsole-uclass.c +5
-0+switch (ch) { +case '\x1b' : + priv->escape_len =0 ; + priv->escape =1 ; +break ;case '\a' :/* beep */ break ;drivers/video/video-uclass.c +1
-3@@ -87,7 +87,7 @@ int video_reserve(ulong *addrp)return 0 ; } -static int video_clear (struct udevice *dev) +void video_clear (struct udevice *dev) {struct video_priv *priv =dev_get_uclass_priv (dev);@@ -100,8 +100,6 @@ static int video_clear(struct udevice *dev) }else {memset (priv->fb, priv->colour_bg, priv->fb_size); } - -return 0 ; }/* Flush video activity to the caches */ include/video.h +7
-0@@ -115,6 +115,13 @@ struct video_ops {int video_reserve (ulong *addrp);/** +* video_clear() - Clear a device's frame buffer to background color. +* +* @dev: Device to clear +*/ +void video_clear (struct udevice *dev); + +/** * video_sync() - Sync a device's frame buffer with its hardware * * Some frame buffers are cached or have a secondary frame buffer. This include/video_console.h +11
-1@@ -29,6 +29,9 @@* @xsize_frac: Width of the display in fractional units * @xstart_frac: Left margin for the text console in fractional units * @last_ch: Last character written to the text console on this line +* @escape: TRUE if currently accumulating an ANSI escape sequence +* @escape_len: Length of accumulated escape sequence so far +* @escape_buf: Buffer to accumulate escape sequence */ struct vidconsole_priv {struct stdio_dev sdev;@@ -42,6 +45,14 @@ struct vidconsole_priv {int xsize_frac;int xstart_frac;int last_ch; +/* +* ANSI escape sequences are accumulated character by character, +* starting after the ESC char (0x1b) until the entire sequence +* is consumed at which point it is acted upon. +*/ +int escape; +int escape_len; +char escape_buf[32 ]; };/** -- 2.13.5
PATCH 1/1 dm: video: Add basic ANSI escape sequence support
Hi,
Quoted 39 lines · On Mon, 11 Sep 2017 05:42:01 -0400 Rob Clark wrote:
Quoted 29 lines · Simon Glass
Quoted 28 lines · Rob Clark
Really just the subset that is needed by
efi_console. Perhaps more will be added later, for example color support would be useful to implementefi_cout_set_attribute().Signed-off-by Rob Clark <[email protected]>--- drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++ drivers/video/video-uclass.c | 4 +- include/video.h | 7 +++ include/video_console.h | 11 ++++ 4 files changed, 131 insertions(+), 3 deletions(-)drivers/video/vidconsole-uclass.c +1
-0@@ -9,6 +9,7 @@*/ #include <common.h> +#include <linux/ctype.h> #include <dm.h> #include <video.h> #include <video_console.h> @@ -107,12 +108,123 @@ static void vidconsole_newline(struct udevice *dev)video_sync (dev->parent); }Quoted 8 lines
drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 116
Quoted 6 lines
drivers/video/vidconsole-uclass.c +6
-0+/* +* Parse a number from string that ends in a non-numeric character.. +* sscanf() would be nice. This is just enough for parsing ANSI escape +* sequences. +*/ +static char *parsenum (char *s,int *num)Can you use
simple_strtoul()or similar?
Quoted 7 lines
Possibly, but I'm not sure it is a good idea.. I don't think escape sequences are meant to be encoded with hex or octal number strings. From a quick look, I don't see any escape code terminated with 'x', so maybe it would end up working ok.. but something like ESC[0x1234m should be an escape sequence terminated with x followed by normal chars 1234m and strtoul would get that wrong..
stroul(s, NULL, 10) will only parse decimal numbers and stop at non-decimal digits.
Lothar Waßmann
PATCH 1/1 dm: video: Add basic ANSI escape sequence support
Quoted 49 lines · Lothar Waßmann
Hi,
Quoted 39 lines · On Mon, 11 Sep 2017 05:42:01 -0400 Rob Clark wrote:
Quoted 29 lines · Simon Glass
Quoted 28 lines · Rob Clark
Really just the subset that is needed by
efi_console. Perhaps more will be added later, for example color support would be useful to implementefi_cout_set_attribute().Signed-off-by Rob Clark <[email protected]>--- drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++ drivers/video/video-uclass.c | 4 +- include/video.h | 7 +++ include/video_console.h | 11 ++++ 4 files changed, 131 insertions(+), 3 deletions(-)drivers/video/vidconsole-uclass.c +1
-0@@ -9,6 +9,7 @@*/ #include <common.h> +#include <linux/ctype.h> #include <dm.h> #include <video.h> #include <video_console.h> @@ -107,12 +108,123 @@ static void vidconsole_newline(struct udevice *dev)video_sync (dev->parent); }Quoted 8 lines
drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 116
Quoted 6 lines
drivers/video/vidconsole-uclass.c +6
-0+/* +* Parse a number from string that ends in a non-numeric character.. +* sscanf() would be nice. This is just enough for parsing ANSI escape +* sequences. +*/ +static char *parsenum (char *s,int *num)Can you use
simple_strtoul()or similar?Quoted 7 lines
Possibly, but I'm not sure it is a good idea.. I don't think escape sequences are meant to be encoded with hex or octal number strings. From a quick look, I don't see any escape code terminated with 'x', so maybe it would end up working ok.. but something like ESC[0x1234m should be an escape sequence terminated with x followed by normal chars 1234m and strtoul would get that wrong..
Quoted 3 lines
stroul(s, NULL, 10)will only parse decimal numbers and stop at non-decimal digits.
And you'd expect simple_strtoul() would too.. but that does not appear to be the case. Not sure if that is intentional.
BR,
-R
PATCH 1/1 dm: video: Add basic ANSI escape sequence support
Quoted 50 lines · Rob Clark
Quoted 49 lines · Lothar Waßmann
Hi,
Quoted 46 lines · On Mon, 11 Sep 2017 05:42:01 -0400 Rob Clark wrote:
Quoted 29 lines · Simon Glass
Quoted 28 lines · Rob Clark
Really just the subset that is needed by
efi_console. Perhaps more will be added later, for example color support would be useful to implementefi_cout_set_attribute().Signed-off-by Rob Clark <[email protected]>--- drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++ drivers/video/video-uclass.c | 4 +- include/video.h | 7 +++ include/video_console.h | 11 ++++ 4 files changed, 131 insertions(+), 3 deletions(-)drivers/video/vidconsole-uclass.c +1
-0@@ -9,6 +9,7 @@*/ #include <common.h> +#include <linux/ctype.h> #include <dm.h> #include <video.h> #include <video_console.h> @@ -107,12 +108,123 @@ static void vidconsole_newline(struct udevice *dev)video_sync (dev->parent); }Quoted 8 lines
drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 116
Quoted 6 lines
drivers/video/vidconsole-uclass.c +6
-0+/* +* Parse a number from string that ends in a non-numeric character.. +* sscanf() would be nice. This is just enough for parsing ANSI escape +* sequences. +*/ +static char *parsenum (char *s,int *num)Can you use
simple_strtoul()or similar?Possibly, but I'm not sure it is a good idea.. I don't think escape sequences are meant to be encoded with hex or octal number strings. From a quick look, I don't see any escape code terminated with 'x', so maybe it would end up working ok.. but something like ESC[0x1234m should be an escape sequence terminated with x followed by normal chars 1234m and strtoul would get that wrong..
Quoted 7 lines
Quoted 3 lines
stroul(s, NULL, 10)will only parse decimal numbers and stop at non-decimal digits.And you'd expect
simple_strtoul()would too.. but that does not appear to be the case. Not sure if that is intentional.
So I double checked simple_strtoul() in upstream kernel, and (apart from being re-written) it seems to have fixed this bug. So I'm guessing the u-boot copied a buggy version from the linux kernel src and never got a fix. I'll send a patch for that. It will be easy enough to drop parsenum() once the fix is merged.
BR,
-R
PATCH 1/1 dm: video: Add basic ANSI escape sequence support
In reply to Rob ClarkHi Rob,
Quoted 50 lines · Rob Clark
Quoted 49 lines · Lothar Waßmann
Hi,
Quoted 46 lines · On Mon, 11 Sep 2017 05:42:01 -0400 Rob Clark wrote:
Quoted 29 lines · Simon Glass
Quoted 28 lines · Rob Clark
Really just the subset that is needed by
efi_console. Perhaps more will be added later, for example color support would be useful to implementefi_cout_set_attribute().Signed-off-by Rob Clark <[email protected]>--- drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++ drivers/video/video-uclass.c | 4 +- include/video.h | 7 +++ include/video_console.h | 11 ++++ 4 files changed, 131 insertions(+), 3 deletions(-)drivers/video/vidconsole-uclass.c +1
-0@@ -9,6 +9,7 @@*/ #include <common.h> +#include <linux/ctype.h> #include <dm.h> #include <video.h> #include <video_console.h> @@ -107,12 +108,123 @@ static void vidconsole_newline(struct udevice *dev)video_sync (dev->parent); }Quoted 8 lines
drivers/video/vidconsole-uclass.c static void vidconsole_newline(struct udevice *dev) · line 116
Quoted 6 lines
drivers/video/vidconsole-uclass.c +6
-0+/* +* Parse a number from string that ends in a non-numeric character.. +* sscanf() would be nice. This is just enough for parsing ANSI escape +* sequences. +*/ +static char *parsenum (char *s,int *num)Can you use
simple_strtoul()or similar?Possibly, but I'm not sure it is a good idea.. I don't think escape sequences are meant to be encoded with hex or octal number strings. From a quick look, I don't see any escape code terminated with 'x', so maybe it would end up working ok.. but something like ESC[0x1234m should be an escape sequence terminated with x followed by normal chars 1234m and strtoul would get that wrong..
Quoted 6 lines
Quoted 3 lines
stroul(s, NULL, 10)will only parse decimal numbers and stop at non-decimal digits.And you'd expect
simple_strtoul()would too.. but that does not appear to be the case. Not sure if that is intentional.
That looks like a bug to me although there is no documentation or tests for that function. The while loop should stop at any character beyond base I think.
Quoted 3 lines
BR,
-R
Regards,
Simon