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

[Sean Anderson]

On 3/12/22 12:02 AM, Simon Glass wrote:
> 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.

I've added a config for this for v3.

>>
>> 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.

OK, how about positive return is bytes written and negative error.

>> +        * @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 ?

I can have a look, but note that there is no test for putc/getc/etc. If
putc/puts is broken, then the console output will be missing/garbled. This
also happens after console recording IIRC, so I think we would need a
second buffer in sandbox_serial...

--Sean