diff mbox series

[v3,26/29] serial: dm: Add support for puts

Message ID 20220322205938.1721846-27-sean.anderson@seco.com
State Accepted
Commit 7a763471894feb58d5a1bdf78ea7014c7a952264
Delegated to: Tom Rini
Headers show
Series arm: semihosting: Cleanups and new features | expand

Commit Message

Sean Anderson March 22, 2022, 8:59 p.m. UTC
Some serial drivers can be vastly more efficient when printing multiple
characters at once. Non-DM serial has had a puts option for these sorts
of drivers; implement it for DM serial as well.

Because we have to add carriage returns, we can't just pass the whole
string directly to the serial driver. Instead, we print up to the
newline, then print a carriage return, and then continue on. This is
less efficient, but it is better than printing each character
individually. It also avoids having to allocate memory just to add a few
characters.

Drivers may perform short writes (such as filling a FIFO) and return the
number of characters written in len. We loop over them in the same way
that _serial_putc loops over putc.

This results in around sizeof(void *) growth for all boards with
DM_SERIAL. The full implementation takes around 140 bytes.

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

Changes in v3:
- Add a config for puts to reduce the impact on the (vast majority) of
  boards which don't need it.
- Fix null pointer dereference in _serial_puts caused by a missing
  return.
- Make puts return the number of characters written on success, instead
  of reusing the len parameter.

Changes in v2:
- New

 drivers/serial/Kconfig         | 13 +++++++++++++
 drivers/serial/serial-uclass.c | 26 ++++++++++++++++++++++++--
 include/serial.h               | 18 ++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

Comments

Simon Glass March 28, 2022, 6:35 a.m. UTC | #1
On Tue, 22 Mar 2022 at 15:00, Sean Anderson <sean.anderson@seco.com> wrote:
>
> Some serial drivers can be vastly more efficient when printing multiple
> characters at once. Non-DM serial has had a puts option for these sorts
> of drivers; implement it for DM serial as well.
>
> Because we have to add carriage returns, we can't just pass the whole
> string directly to the serial driver. Instead, we print up to the
> newline, then print a carriage return, and then continue on. This is
> less efficient, but it is better than printing each character
> individually. It also avoids having to allocate memory just to add a few
> characters.
>
> Drivers may perform short writes (such as filling a FIFO) and return the
> number of characters written in len. We loop over them in the same way
> that _serial_putc loops over putc.
>
> This results in around sizeof(void *) growth for all boards with
> DM_SERIAL. The full implementation takes around 140 bytes.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
> Changes in v3:
> - Add a config for puts to reduce the impact on the (vast majority) of
>   boards which don't need it.
> - Fix null pointer dereference in _serial_puts caused by a missing
>   return.
> - Make puts return the number of characters written on success, instead
>   of reusing the len parameter.
>
> Changes in v2:
> - New
>
>  drivers/serial/Kconfig         | 13 +++++++++++++
>  drivers/serial/serial-uclass.c | 26 ++++++++++++++++++++++++--
>  include/serial.h               | 18 ++++++++++++++++++
>  3 files changed, 55 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tom Rini April 3, 2022, 12:16 a.m. UTC | #2
On Tue, Mar 22, 2022 at 04:59:34PM -0400, Sean Anderson wrote:

> Some serial drivers can be vastly more efficient when printing multiple
> characters at once. Non-DM serial has had a puts option for these sorts
> of drivers; implement it for DM serial as well.
> 
> Because we have to add carriage returns, we can't just pass the whole
> string directly to the serial driver. Instead, we print up to the
> newline, then print a carriage return, and then continue on. This is
> less efficient, but it is better than printing each character
> individually. It also avoids having to allocate memory just to add a few
> characters.
> 
> Drivers may perform short writes (such as filling a FIFO) and return the
> number of characters written in len. We loop over them in the same way
> that _serial_putc loops over putc.
> 
> This results in around sizeof(void *) growth for all boards with
> DM_SERIAL. The full implementation takes around 140 bytes.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!
diff mbox series

Patch

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index cc20759505..1a109b2411 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -133,6 +133,19 @@  config SERIAL_RX_BUFFER_SIZE
 	help
 	  The size of the RX buffer (needs to be power of 2)
 
+config SERIAL_PUTS
+	bool "Enable printing strings all at once"
+	depends on DM_SERIAL
+	help
+	  Some serial drivers are much more efficient when printing multiple
+	  characters at once rather than printing characters individually. This
+	  can be because they can load a fifo, or because individual print
+	  calls have a constant overhead. With this option set, the serial
+	  subsystem will try to provide serial drivers with as many characters
+	  at once as possible, instead of printing characters one by one. Most
+	  serial drivers do not need this config to print efficiently. If
+	  unsure, say N.
+
 config SERIAL_SEARCH_ALL
 	bool "Search for serial devices after default one failed"
 	depends on DM_SERIAL
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index f30f352bd7..10d6b800e2 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -200,8 +200,30 @@  static void _serial_putc(struct udevice *dev, char ch)
 
 static void _serial_puts(struct udevice *dev, const char *str)
 {
-	while (*str)
-		_serial_putc(dev, *str++);
+	struct dm_serial_ops *ops = serial_get_ops(dev);
+
+	if (!CONFIG_IS_ENABLED(SERIAL_PUTS) || !ops->puts) {
+		while (*str)
+			_serial_putc(dev, *str++);
+		return;
+	}
+
+	do {
+		const char *newline = strchrnul(str, '\n');
+		size_t len = newline - str + !!*newline;
+
+		do {
+			ssize_t written = ops->puts(dev, str, len);
+
+			if (written < 0)
+				return;
+			str += written;
+			len -= written;
+		} while (len);
+
+		if (*newline)
+			_serial_putc(dev, '\r');
+	} while (*str);
 }
 
 static int __serial_getc(struct udevice *dev)
diff --git a/include/serial.h b/include/serial.h
index 2681d26c82..8c2e7adbc3 100644
--- a/include/serial.h
+++ b/include/serial.h
@@ -195,6 +195,24 @@  struct dm_serial_ops {
 	 * @return 0 if OK, -ve on error
 	 */
 	int (*putc)(struct udevice *dev, const char ch);
+	/**
+	 * puts() - Write a string
+	 *
+	 * This writes a string. This function should be implemented only if
+	 * writing multiple characters at once is more performant than just
+	 * calling putc() in a loop.
+	 *
+	 * If the whole string cannot be written at once, then this function
+	 * should return the number of characters written. Returning a negative
+	 * error code implies that no characters were written. If this function
+	 * returns 0, then it will be called again with the same arguments.
+	 *
+	 * @dev: Device pointer
+	 * @s: The string to write
+	 * @len: The length of the string to write.
+	 * @return The number of characters written on success, or -ve on error
+	 */
+	ssize_t (*puts)(struct udevice *dev, const char *s, size_t len);
 	/**
 	 * pending() - Check if input/output characters are waiting
 	 *