[PATCH v2 26/28] serial: dm: Add support for puts

[Simon Glass]

Hi Sean,

On Thu, 10 Mar 2022 at 13:51, Sean Anderson <sean.anderson at 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 148 bytes of bloat for all boards with DM_SERIAL
> enabled:

So let's put it behind a Kconfig, particularly for SPL.

>
> vexpress_aemv8a_juno: all +148 rodata +8 text +140
>    u-boot: add: 2/0, grow: 0/-2 bytes: 232/-92 (140)
>          function                                   old     new   delta
>          _serial_puts                                 -     200    +200
>          strchrnul                                    -      32     +32
>          serial_puts                                 68      24     -44
>          serial_stub_puts                            56       8     -48
>
> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
> ---
>
> Changes in v2:
> - New
>
>  drivers/serial/serial-uclass.c | 27 +++++++++++++++++++++++++--
>  include/serial.h               | 18 ++++++++++++++++++
>  2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 362cedd955..352ad986f7 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -199,8 +199,31 @@ 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 (!ops->puts) {
> +               while (*str)
> +                       _serial_putc(dev, *str++);
> +       }
> +
> +       do {
> +               const char *newline = strchrnul(str, '\n');
> +               size_t len = newline - str + !!*newline;
> +
> +               do {
> +                       int err;
> +                       size_t written = len;
> +
> +                       err = ops->puts(dev, str, &written);
> +                       if (err && err != -EAGAIN)
> +                               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..ea96d904d8 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 @len should be
> +        * set to the number of characters written, and this function should
> +        * return -EAGAIN.
> +        *
> +        * @dev: Device pointer
> +        * @s: The string to write
> +        * @len: The length of the string to write. This should be set to the
> +        *       number of characters actually written on return.

How about returning the number of characters written? That is more
like the posix write() function and saves an arg.

> +        * @return 0 if OK, -ve on error
> +        */
> +       int (*puts)(struct udevice *dev, const char *s, size_t *len);
>         /**
>          * pending() - Check if input/output characters are waiting
>          *
> --
> 2.25.1
>

Is it possible to add a test to test/dm/serial.c ?

Regards,
Simon