A string like "test\n" would be broken up into the following sequence of prints by _serial_puts:

puts("test\n") putc('\r')

Although functionally this is the same as \r\n, it is not the standard sequence and caused tests to fail. Fix this by excluding the '\n' from the initial print. The above string will now be broken up like

puts("test") puts("\r\n")

Since we may now need to call ops->puts twice (with the associated retry logic), break that part of the function off into a helper.

Fixes: 7a76347189 ("serial: dm: Add support for puts") Signed-off-by: Sean Anderson sean.anderson@seco.com ---

drivers/serial/serial-uclass.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 10d6b800e2..30650e37b0 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -198,6 +198,22 @@ static void _serial_putc(struct udevice *dev, char ch) } while (err == -EAGAIN); }

+static int __serial_puts(struct udevice *dev, const char *str, size_t len) +{ + struct dm_serial_ops *ops = serial_get_ops(dev); + + do { + ssize_t written = ops->puts(dev, str, len); + + if (written < 0) + return written; + str += written; + len -= written; + } while (len); + + return 0; +} + static void _serial_puts(struct udevice *dev, const char *str) { struct dm_serial_ops *ops = serial_get_ops(dev); @@ -210,19 +226,15 @@ static void _serial_puts(struct udevice *dev, const char *str)

do { const char *newline = strchrnul(str, '\n'); - size_t len = newline - str + !!*newline; + size_t len = newline - str;

- do { - ssize_t written = ops->puts(dev, str, len); + if (__serial_puts(dev, str, len)) + return;

- if (written < 0) - return; - str += written; - len -= written; - } while (len); + if (*newline && __serial_puts(dev, "\r\n", 2)) + return;

- if (*newline) - _serial_putc(dev, '\r'); + str += len + !!*newline; } while (*str); }

This implements puts for sandbox. It is fairly straightforward, except that we break out the shared color printing functionality into its own function.

Signed-off-by: Sean Anderson sean.anderson@seco.com Reviewed-by: Simon Glass sjg@chromium.org ---

drivers/serial/Kconfig | 1 + drivers/serial/sandbox.c | 21 ++++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 76171e7146..fae82598cb 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -779,6 +779,7 @@ config S5P_SERIAL config SANDBOX_SERIAL bool "Sandbox UART support" depends on SANDBOX + imply SERIAL_PUTS help Select this to enable a seral UART for sandbox. This is required to operate correctly, otherwise you will see no serial output from diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c index 0b1756f5c0..50cf2c74a7 100644 --- a/drivers/serial/sandbox.c +++ b/drivers/serial/sandbox.c @@ -67,7 +67,7 @@ static int sandbox_serial_remove(struct udevice *dev) return 0; }

-static int sandbox_serial_putc(struct udevice *dev, const char ch) +static void sandbox_print_color(struct udevice *dev) { struct sandbox_serial_priv *priv = dev_get_priv(dev); struct sandbox_serial_plat *plat = dev_get_plat(dev); @@ -78,7 +78,13 @@ static int sandbox_serial_putc(struct udevice *dev, const char ch) priv->start_of_line = false; output_ansi_colour(plat->colour); } +}

+static int sandbox_serial_putc(struct udevice *dev, const char ch) +{ + struct sandbox_serial_priv *priv = dev_get_priv(dev); + + sandbox_print_color(dev); os_write(1, &ch, 1); if (ch == '\n') priv->start_of_line = true; @@ -86,6 +92,18 @@ static int sandbox_serial_putc(struct udevice *dev, const char ch) return 0; }

+static ssize_t sandbox_serial_puts(struct udevice *dev, const char *s, + size_t len) +{ + struct sandbox_serial_priv *priv = dev_get_priv(dev); + + sandbox_print_color(dev); + if (s[len - 1] == '\n') + priv->start_of_line = true; + + return os_write(1, s, len); +} + static int sandbox_serial_pending(struct udevice *dev, bool input) { struct sandbox_serial_priv *priv = dev_get_priv(dev); @@ -212,6 +230,7 @@ static int sandbox_serial_of_to_plat(struct udevice *dev)

static const struct dm_serial_ops sandbox_serial_ops = { .putc = sandbox_serial_putc, + .puts = sandbox_serial_puts, .pending = sandbox_serial_pending, .getc = sandbox_serial_getc, .getconfig = sandbox_serial_getconfig,

This adds a test to ensure that puts is equivalent to putc called in a loop. We don't verify the contents of the message to avoid having to record console output a second time (though that could be added in the future). The globals are initialized to non-zero values to avoid a warning; in particular, the character count is off-by-one (but we always make relative measurements).

Signed-off-by: Sean Anderson sean.anderson@seco.com Reviewed-by: Simon Glass sjg@chromium.org --- Changes since v3 of the original series: - Use helper functions to en/disable the serial device and access the number of characters written.

arch/sandbox/include/asm/serial.h | 22 +++++++++++++++++++++ drivers/serial/sandbox.c | 33 +++++++++++++++++++++++++++---- test/dm/serial.c | 19 ++++++++++++++++++ 3 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/include/asm/serial.h b/arch/sandbox/include/asm/serial.h index bc82aebd0e..16589a1b21 100644 --- a/arch/sandbox/include/asm/serial.h +++ b/arch/sandbox/include/asm/serial.h @@ -16,6 +16,28 @@ struct sandbox_serial_plat { int colour; /* Text colour to use for output, -1 for none */ };

+/** + * sandbox_serial_written() - Get the total number of characters written + * + * This returns the number of characters written by the sandbox serial + * device. It is intended for performing tests of the serial subsystem + * where a console buffer cannot be used. The absolute number should not be + * relied upon; call this function twice and compare the difference. + * + * Return: The number of characters written + */ +size_t sandbox_serial_written(void); + +/** + * sandbox_serial_endisable() - Enable or disable serial output + * @enabled: Whether serial output should be enabled or not + * + * This allows tests to enable or disable the sandbox serial output. All + * processes relating to writing output (except the actual writing) will be + * performed. + */ +void sandbox_serial_endisable(bool enabled); + /** * struct sandbox_serial_priv - Private data for this driver * diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c index 50cf2c74a7..e726e19c46 100644 --- a/drivers/serial/sandbox.c +++ b/drivers/serial/sandbox.c @@ -23,6 +23,19 @@

DECLARE_GLOBAL_DATA_PTR;

+static size_t _sandbox_serial_written = 1; +static bool sandbox_serial_enabled = true; + +size_t sandbox_serial_written(void) +{ + return _sandbox_serial_written; +} + +void sandbox_serial_endisable(bool enabled) +{ + sandbox_serial_enabled = enabled; +} + /** * output_ansi_colour() - Output an ANSI colour code * @@ -84,11 +97,14 @@ static int sandbox_serial_putc(struct udevice *dev, const char ch) { struct sandbox_serial_priv *priv = dev_get_priv(dev);

- sandbox_print_color(dev); - os_write(1, &ch, 1); if (ch == '\n') priv->start_of_line = true;

+ if (sandbox_serial_enabled) { + sandbox_print_color(dev); + os_write(1, &ch, 1); + } + _sandbox_serial_written += 1; return 0; }

@@ -96,12 +112,21 @@ static ssize_t sandbox_serial_puts(struct udevice *dev, const char *s, size_t len) { struct sandbox_serial_priv *priv = dev_get_priv(dev); + ssize_t ret;

- sandbox_print_color(dev); if (s[len - 1] == '\n') priv->start_of_line = true;

- return os_write(1, s, len); + if (sandbox_serial_enabled) { + sandbox_print_color(dev); + ret = os_write(1, s, len); + if (ret < 0) + return ret; + } else { + ret = len; + } + _sandbox_serial_written += ret; + return ret; }

static int sandbox_serial_pending(struct udevice *dev, bool input) diff --git a/test/dm/serial.c b/test/dm/serial.c index 0662b5f09b..37d17a65f1 100644 --- a/test/dm/serial.c +++ b/test/dm/serial.c @@ -7,14 +7,22 @@ #include <log.h> #include <serial.h> #include <dm.h> +#include <asm/serial.h> #include <dm/test.h> #include <test/test.h> #include <test/ut.h>

+static const char test_message[] = + "This is a test message\n" + "consisting of multiple lines\n"; + static int dm_test_serial(struct unit_test_state *uts) { + int i; struct serial_device_info info_serial = {0}; struct udevice *dev_serial; + size_t start, putc_written; + uint value_serial;

ut_assertok(uclass_get_device_by_name(UCLASS_SERIAL, "serial", @@ -66,6 +74,17 @@ static int dm_test_serial(struct unit_test_state *uts) SERIAL_8_BITS, SERIAL_TWO_STOP)));

+ /* Verify that putc and puts print the same number of characters */ + sandbox_serial_endisable(false); + start = sandbox_serial_written(); + for (i = 0; i < sizeof(test_message) - 1; i++) + serial_putc(test_message[i]); + putc_written = sandbox_serial_written(); + serial_puts(test_message); + sandbox_serial_endisable(true); + ut_asserteq(putc_written - start, + sandbox_serial_written() - putc_written); + return 0; }

This adds an implementation of puts for DM. The implementation is not as clean as for the non-DM puts because we have to handle non-nul-terminated string. We also handle short writes (though these are probably very unusual).

Signed-off-by: Sean Anderson sean.anderson@seco.com ---

drivers/serial/Kconfig | 1 + drivers/serial/serial_semihosting.c | 31 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index fae82598cb..4ced6c0a5d 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -808,6 +808,7 @@ config SCIF_CONSOLE config SEMIHOSTING_SERIAL bool "Semihosting UART support" depends on SEMIHOSTING && !SERIAL_RX_BUFFER + imply SERIAL_PUTS help Select this to enable a serial UART using semihosting. Special halt instructions will be issued which an external debugger (such as a diff --git a/drivers/serial/serial_semihosting.c b/drivers/serial/serial_semihosting.c index 62b1b2241b..4328b3dac5 100644 --- a/drivers/serial/serial_semihosting.c +++ b/drivers/serial/serial_semihosting.c @@ -5,12 +5,14 @@

#include <common.h> #include <dm.h> +#include <malloc.h> #include <serial.h> #include <semihosting.h>

/** * struct smh_serial_priv - Semihosting serial private data * @infd: stdin file descriptor (or error) + * @outfd: stdout file descriptor (or error) */ struct smh_serial_priv { int infd; @@ -36,8 +38,36 @@ static int smh_serial_putc(struct udevice *dev, const char ch) return 0; }

+static ssize_t smh_serial_puts(struct udevice *dev, const char *s, size_t len) +{ + int ret; + struct smh_serial_priv *priv = dev_get_priv(dev); + unsigned long written; + + if (priv->outfd < 0) { + char *buf; + + /* Try and avoid a copy if we can */ + if (!s[len + 1]) { + smh_puts(s); + return len; + } + + buf = strndup(s, len); + smh_puts(buf); + free(buf); + return len; + } + + ret = smh_write(priv->outfd, s, len, &written); + if (written) + return written; + return ret; +} + static const struct dm_serial_ops smh_serial_ops = { .putc = smh_serial_putc, + .puts = smh_serial_puts, .getc = smh_serial_getc, };

@@ -53,6 +83,7 @@ static int smh_serial_probe(struct udevice *dev) struct smh_serial_priv *priv = dev_get_priv(dev);

priv->infd = smh_open(":tt", MODE_READ); + priv->outfd = smh_open(":tt", MODE_WRITE); return 0; }