Re: [Qemu-devel] [PATCH v2 2/4] pl011: fix UARTRSR accesses corrupting the UARTCR value

[Peter Maydell]

On 14 March 2014 18:22, Rob Herring <address@hidden> wrote:
> From: Rob Herring <address@hidden>
>
> Offset 4 is UARTRSR/UARTECR, not the UARTCR. The UARTCR would be
> corrupted if the UARTRSR is ever written. Fix by implementing a correct
> model of the UARTRSR/UARTECR register. Reads of this register simply
> reflect the error bits in data register. Only breaks can be triggered in
> QEMU. With the pl011_can_receive function, we effectively have flow
> control between the host and the model. Framing and parity errors simply
> don't make sense in the model and will never occur.
>
> Signed-off-by: Rob Herring <address@hidden>
> ---
>  hw/char/pl011.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> index f0c3fa3..920ba3f 100644
> --- a/hw/char/pl011.c
> +++ b/hw/char/pl011.c
> @@ -20,6 +20,7 @@ typedef struct PL011State {
>      uint32_t readbuff;
>      uint32_t flags;
>      uint32_t lcr;
> +    uint32_t rsr;

This needs a corresponding entry in the vmstate (and
the vmstate version_id/minimum_version_id/minimum_version_id_old
all need to be incremented).

>      uint32_t cr;
>      uint32_t dmacr;
>      uint32_t int_enabled;
> @@ -81,13 +82,14 @@ static uint64_t pl011_read(void *opaque, hwaddr offset,
>          }
>          if (s->read_count == s->read_trigger - 1)
>              s->int_level &= ~ PL011_INT_RX;
> +        s->rsr = c >> 8;
>          pl011_update(s);
>          if (s->chr) {
>              qemu_chr_accept_input(s->chr);
>          }
>          return c;
> -    case 1: /* UARTCR */
> -        return 0;
> +    case 1: /* UARTRSR */
> +        return s->rsr;
>      case 6: /* UARTFR */
>          return s->flags;
>      case 8: /* UARTILPR */
> @@ -146,8 +148,8 @@ static void pl011_write(void *opaque, hwaddr offset,
>          s->int_level |= PL011_INT_TX;
>          pl011_update(s);
>          break;
> -    case 1: /* UARTCR */
> -        s->cr = value;
> +    case 1: /* UARTRSR/UARTECR */
> +        s->rsr = 0;
>          break;
>      case 6: /* UARTFR */
>          /* Writes to Flag register are ignored.  */
> --
> 1.8.3.2
>

thanks
-- PMM