[Qemu-devel] [PATCH 1/1] pl061: fix wrong calculation of GPIOMIS register

[Qemu-devel] [PATCH 1/1] pl061: fix wrong calculation of GPIOMIS register

[Victor CLEMENT]

The masked interrupt status register should be the state of the interrupt
after masking.
There should be a logical AND instead of a logical OR between the
interrupt status and the interrupt mask.

Signed-off-by: Victor CLEMENT <victor.clement@openwide.fr>

---

See ARM PrimeCell GPIO manual below
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0190b/I36092.html
---
 hw/gpio/pl061.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index bd03e99..4ba730b 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -173,7 +173,7 @@ static uint64_t pl061_read(void *opaque, hwaddr offset,
     case 0x414: /* Raw interrupt status */
         return s->istate;
     case 0x418: /* Masked interrupt status */
-        return s->istate | s->im;
+        return s->istate & s->im;
     case 0x420: /* Alternate function select */
         return s->afsel;
     case 0x500: /* 2mA drive */
-- 
2.3.7

Re: [Qemu-devel] [PATCH 1/1] pl061: fix wrong calculation of GPIOMIS register

[Peter Crosthwaite]

gpio: prefix on subject line (in addition to pl061:)

On Mon, Jun 1, 2015 at 3:33 AM, Victor CLEMENT
<victor.clement@openwide.fr> wrote:
> The masked interrupt status register should be the state of the interrupt
> after masking.
> There should be a logical AND instead of a logical OR between the
> interrupt status and the interrupt mask.
>
> Signed-off-by: Victor CLEMENT <victor.clement@openwide.fr>
>

So this makes sense to me, given that the code uses ->im with &
operation for the actual interrupt pin calculation. That | result in
original code makes little sense.

With just the subject line change:

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>
> See ARM PrimeCell GPIO manual below
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0190b/I36092.html
> ---
>  hw/gpio/pl061.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
> index bd03e99..4ba730b 100644
> --- a/hw/gpio/pl061.c
> +++ b/hw/gpio/pl061.c
> @@ -173,7 +173,7 @@ static uint64_t pl061_read(void *opaque, hwaddr offset,
>      case 0x414: /* Raw interrupt status */
>          return s->istate;
>      case 0x418: /* Masked interrupt status */
> -        return s->istate | s->im;
> +        return s->istate & s->im;
>      case 0x420: /* Alternate function select */
>          return s->afsel;
>      case 0x500: /* 2mA drive */
> --
> 2.3.7
>
>

Re: [Qemu-devel] [PATCH 1/1] pl061: fix wrong calculation of GPIOMIS register

[Peter Maydell]

On 1 June 2015 at 11:33, Victor CLEMENT <victor.clement@openwide.fr> wrote:
> The masked interrupt status register should be the state of the interrupt
> after masking.
> There should be a logical AND instead of a logical OR between the
> interrupt status and the interrupt mask.
>
> Signed-off-by: Victor CLEMENT <victor.clement@openwide.fr>

Applied, thanks.

-- PMM