Re: [PATCH v3 5/5] hw/char/pl011: check if UART is enabled before RX or TX operation

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 20/1/23 16:54, Evgeny Iakovlev wrote:
> UART should be enabled in general and have RX enabled specifically to be
> able to receive data from peripheral device. Same goes for transmitting
> data to peripheral device and a TXE flag.
> 
> Check if UART CR register has EN and RXE or TXE bits enabled before
> trying to receive or transmit data.
> 
> Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/char/pl011.c | 22 +++++++++++++++++++---
>   1 file changed, 19 insertions(+), 3 deletions(-)

> +static inline bool pl011_can_transmit(PL011State *s)
> +{
> +    return s->cr & PL011_CR_UARTEN && s->cr & PL011_CR_TXE;
> +}
> +
>   static void pl011_write(void *opaque, hwaddr offset,
>                           uint64_t value, unsigned size)
>   {
> @@ -221,7 +231,9 @@ static void pl011_write(void *opaque, hwaddr offset,
>   
>       switch (offset >> 2) {
>       case 0: /* UARTDR */
> -        /* ??? Check if transmitter is enabled.  */
> +        if (!pl011_can_transmit(s)) {
> +            break;
> +        }
>           ch = value;
>           /* XXX this blocks entire thread. Rewrite to use
>            * qemu_chr_fe_write and background I/O callbacks */
> @@ -292,7 +304,11 @@ static int pl011_can_receive(void *opaque)
>       PL011State *s = (PL011State *)opaque;
>       int r;
>   
> -    r = s->read_count < pl011_get_fifo_depth(s);
> +    if (!(s->cr & PL011_CR_UARTEN) || !(s->cr & PL011_CR_RXE)) {

Maybe add pl011_can_receive() similarly to pl011_can_transmit().

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +        r = 0;
> +    } else {
> +        r = s->read_count < pl011_get_fifo_depth(s);
> +    }
>       trace_pl011_can_receive(s->lcr, s->read_count, r);
>       return r;
>   }