hw/char/cmsdk-apb-uart.c: Correctly clear INTSTATUS bits on writes

[Qemu-devel] [PATCH for-2.12] hw/char/cmsdk-apb-uart.c: Correctly clear INTSTATUS bits on writes

Posted by Peter Maydell 6 years ago

The CMSDK APB UART INTSTATUS register bits are all write-one-to-clear.
We were getting this correct for the TXO and RXO bits (which need
special casing because their state lives in the STATE register),
but had forgotten to handle the normal bits for RX and TX which
we do store in our s->intstatus field.

Perform the W1C operation on the bits in s->intstatus too.

Fixes: https://bugs.launchpad.net/qemu/+bug/1760262
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Not a disaster if this doesn't get into 2.12, I guess. I think
it's missed the rc3 boat, so if we need an rc4 for some other
reason we can put it in.

 hw/char/cmsdk-apb-uart.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c
index 1ad1e14295..9c0929d8a2 100644
--- a/hw/char/cmsdk-apb-uart.c
+++ b/hw/char/cmsdk-apb-uart.c
@@ -274,6 +274,7 @@ static void uart_write(void *opaque, hwaddr offset, uint64_t value,
          * is then reflected into the intstatus value by the update function).
          */
         s->state &= ~(value & (R_INTSTATUS_TXO_MASK | R_INTSTATUS_RXO_MASK));
+        s->intstatus &= ~value;
         cmsdk_apb_uart_update(s);
         break;
     case A_BAUDDIV:
-- 
2.16.2

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.12] hw/char/cmsdk-apb-uart.c: Correctly clear INTSTATUS bits on writes

Posted by Philippe Mathieu-Daudé 6 years ago

On 04/10/2018 10:42 AM, Peter Maydell wrote:
> The CMSDK APB UART INTSTATUS register bits are all write-one-to-clear.
> We were getting this correct for the TXO and RXO bits (which need
> special casing because their state lives in the STATE register),
> but had forgotten to handle the normal bits for RX and TX which
> we do store in our s->intstatus field.
> 
> Perform the W1C operation on the bits in s->intstatus too.
> 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1760262
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Not a disaster if this doesn't get into 2.12, I guess. I think
> it's missed the rc3 boat, so if we need an rc4 for some other
> reason we can put it in.
> 
>  hw/char/cmsdk-apb-uart.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c
> index 1ad1e14295..9c0929d8a2 100644
> --- a/hw/char/cmsdk-apb-uart.c
> +++ b/hw/char/cmsdk-apb-uart.c
> @@ -274,6 +274,7 @@ static void uart_write(void *opaque, hwaddr offset, uint64_t value,
>           * is then reflected into the intstatus value by the update function).

The comment was correct :)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>           */
>          s->state &= ~(value & (R_INTSTATUS_TXO_MASK | R_INTSTATUS_RXO_MASK));
> +        s->intstatus &= ~value;
>          cmsdk_apb_uart_update(s);
>          break;
>      case A_BAUDDIV:
>