[U-Boot] [PATCH] 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().
Signed-off-by: Rob Clark robdclark@gmail.com --- 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(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index e081d5a0ee..7998b4cf5f 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -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; diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index dfa39b0d1b..dcaceed42c 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -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 */ diff --git a/include/video.h b/include/video.h index 5b4e78b182..61ff653121 100644 --- a/include/video.h +++ b/include/video.h @@ -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 diff --git a/include/video_console.h b/include/video_console.h index 26047934da..9dce234bd9 100644 --- a/include/video_console.h +++ b/include/video_console.h @@ -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]; };
/**
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.
Signed-off-by: Rob Clark robdclark@gmail.com --- drivers/video/vidconsole-uclass.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index b5afd72227..e081d5a0ee 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -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; }
On 7 September 2017 at 14:28, Rob Clark robdclark@gmail.com wrote:
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().
Signed-off-by: Rob Clark robdclark@gmail.com
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(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index e081d5a0ee..7998b4cf5f 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -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)
Can you use simple_strtoul() or similar?
+{
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
+{
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;}
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.
switch (ch) {case '\x1b':priv->escape_len = 0;priv->escape = 1;break; case '\a': /* beep */ break;diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index dfa39b0d1b..dcaceed42c 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -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 */ diff --git a/include/video.h b/include/video.h index 5b4e78b182..61ff653121 100644 --- a/include/video.h +++ b/include/video.h @@ -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
diff --git a/include/video_console.h b/include/video_console.h index 26047934da..9dce234bd9 100644 --- a/include/video_console.h +++ b/include/video_console.h @@ -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
On Mon, Sep 11, 2017 at 2:18 AM, Simon Glass sjg@chromium.org wrote:
On 7 September 2017 at 14:28, Rob Clark robdclark@gmail.com wrote:
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().
Signed-off-by: Rob Clark robdclark@gmail.com
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(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index e081d5a0ee..7998b4cf5f 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -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)
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
+{
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
+{
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;}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.
switch (ch) {case '\x1b':priv->escape_len = 0;priv->escape = 1;break; case '\a': /* beep */ break;diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index dfa39b0d1b..dcaceed42c 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -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 */ diff --git a/include/video.h b/include/video.h index 5b4e78b182..61ff653121 100644 --- a/include/video.h +++ b/include/video.h @@ -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
diff --git a/include/video_console.h b/include/video_console.h index 26047934da..9dce234bd9 100644 --- a/include/video_console.h +++ b/include/video_console.h @@ -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
Hi,
On Mon, 11 Sep 2017 05:42:01 -0400 Rob Clark wrote:
On Mon, Sep 11, 2017 at 2:18 AM, Simon Glass sjg@chromium.org wrote:
On 7 September 2017 at 14:28, Rob Clark robdclark@gmail.com wrote:
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().
Signed-off-by: Rob Clark robdclark@gmail.com
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(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index e081d5a0ee..7998b4cf5f 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -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)
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..
stroul(s, NULL, 10) will only parse decimal numbers and stop at non-decimal digits.
Lothar Waßmann
On Mon, Sep 11, 2017 at 7:50 AM, Lothar Waßmann LW@karo-electronics.de wrote:
Hi,
On Mon, 11 Sep 2017 05:42:01 -0400 Rob Clark wrote:
On Mon, Sep 11, 2017 at 2:18 AM, Simon Glass sjg@chromium.org wrote:
On 7 September 2017 at 14:28, Rob Clark robdclark@gmail.com wrote:
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().
Signed-off-by: Rob Clark robdclark@gmail.com
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(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index e081d5a0ee..7998b4cf5f 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -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)
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..
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
On Mon, Sep 11, 2017 at 8:31 AM, Rob Clark robdclark@gmail.com wrote:
On Mon, Sep 11, 2017 at 7:50 AM, Lothar Waßmann LW@karo-electronics.de wrote:
Hi,
On Mon, 11 Sep 2017 05:42:01 -0400 Rob Clark wrote:
On Mon, Sep 11, 2017 at 2:18 AM, Simon Glass sjg@chromium.org wrote:
On 7 September 2017 at 14:28, Rob Clark robdclark@gmail.com wrote:
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().
Signed-off-by: Rob Clark robdclark@gmail.com
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(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index e081d5a0ee..7998b4cf5f 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -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)
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..
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
Hi Rob,
On 11 September 2017 at 06:31, Rob Clark robdclark@gmail.com wrote:
On Mon, Sep 11, 2017 at 7:50 AM, Lothar Waßmann LW@karo-electronics.de wrote:
Hi,
On Mon, 11 Sep 2017 05:42:01 -0400 Rob Clark wrote:
On Mon, Sep 11, 2017 at 2:18 AM, Simon Glass sjg@chromium.org wrote:
On 7 September 2017 at 14:28, Rob Clark robdclark@gmail.com wrote:
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().
Signed-off-by: Rob Clark robdclark@gmail.com
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(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index e081d5a0ee..7998b4cf5f 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -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)
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..
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.
BR, -R
Regards, Simon
participants (3)
-
Lothar Waßmann -
Rob Clark -
Simon Glass