Re: [PATCH] serial: qcom-geni: Show '@' characters if we have a FIFO underrun

[Greg Kroah-Hartman]

On Tue, Jul 09, 2024 at 04:28:45PM -0700, Douglas Anderson wrote:
> As of commit 2ac33975abda ("serial: qcom-geni: do not kill the machine
> on fifo underrun") a FIFO underrun will no longer hard lockup the
> machine. Instead, a FIFO underrun will cause the UART to output a
> bunch of '\0' characters. The '\0' characters don't seem to show up on
> most terminal programs and this hides the fact that we had an
> underrun. An underrun is aq sign of problems in the driver and
> should be obvious / debugged.
> 
> Change the driver to put '@' characters in the case of an underrun
> which should make it much more obvious.
> 
> Adding this extra initialization doesn't add any real overhead. In
> fact, this patch reduces code size because the code was calling
> memset() to init 4 bytes of data. Disassembling the new code shows
> that early in the function w22 is setup to hold the '@@@@' constant:
>   mov     w22, #0x40404040
> 
> Each time through the loop w22 is simply stored:
>   str     w22, [sp, #4]
> 
> Cc: Johan Hovold <johan@xxxxxxxxxx>
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> ---
> 
>  drivers/tty/serial/qcom_geni_serial.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 69a632fefc41..332eaa2faa2b 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -872,10 +872,10 @@ static void qcom_geni_serial_send_chunk_fifo(struct uart_port *uport,
>  {
>  	struct qcom_geni_serial_port *port = to_dev_port(uport);
>  	unsigned int tx_bytes, remaining = chunk;
> -	u8 buf[BYTES_PER_FIFO_WORD];
>  
>  	while (remaining) {
> -		memset(buf, 0, sizeof(buf));
> +		u8 buf[BYTES_PER_FIFO_WORD] = { '@', '@', '@', '@' };

Why is '@' a valid character for an underrun?  Why would any characters
be ok?  Where is this now documented?

And shouldn't you use a memset to get the BYTES_PER_FIFO_WORD amount of
'@' here?

thanks,

greg k-h