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
