[PATCH 2/4] serial: do not overwrite not-consumed characters in rx buffer

[Simon Glass]

On Thu, 3 Oct 2024 at 08:10, Rasmus Villemoes <ravi at prevas.dk> wrote:
>
> Before the previous patch, pasting a string of length x >
> CONFIG_SERIAL_RX_BUFFER_SIZE results in getting the
> last (x%CONFIG_SERIAL_RX_BUFFER_SIZE) characters from that string.
>
> With the previous patch, one instead gets the last
> CONFIG_SERIAL_RX_BUFFER_SIZE characters repeatedly until the ->rd_ptr
> catches up.
>
> Both behaviours are counter-intuitive, and happen because the code
> that checks for a character available from the hardware does not
> account for whether there is actually room in the software buffer to
> receive it. Fix that by adding such accounting. This also brings the
> software buffering more in line with how most hardware FIFOs
> behave (first received characters are kept, overflowing characters are
> dropped).
>
> Signed-off-by: Rasmus Villemoes <ravi at prevas.dk>
> ---
>  drivers/serial/serial-uclass.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>


>
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 05fe9645bee..28d7a202afc 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -328,10 +328,11 @@ static int __serial_tstc(struct udevice *dev)
>  static int _serial_tstc(struct udevice *dev)
>  {
>         struct serial_dev_priv *upriv = dev_get_uclass_priv(dev);
> -       uint wr;
> +       uint wr, avail;
>
> -       /* Read all available chars into the RX buffer */
> -       while (__serial_tstc(dev)) {
> +       /* Read all available chars into the RX buffer while there's room */
> +       avail = CONFIG_SERIAL_RX_BUFFER_SIZE - (upriv->wr_ptr - upriv->rd_ptr);
> +       while (avail-- && __serial_tstc(dev)) {
>                 wr = upriv->wr_ptr++ % CONFIG_SERIAL_RX_BUFFER_SIZE;
>                 upriv->buf[wr] = __serial_getc(dev);
>         }
> --
> 2.46.2
>