[Qemu-devel] LEON3 timer patch

[Qemu-devel] LEON3 timer patch

[Gabriele Galeotti]

Hi all.
According to "GR712RC Dual-Core LEON3FT SPARC V8 Processor User’s Manual",
"11.3 Registers", pg 87-88, Table 55 Timer control register, the IP 
"interrupt pending"
bit:

Interrupt Pending (IP): The core sets this bit to ‘1’ when an interrupt 
is signalled. This bit remains ‘1’
until cleared by writing ‘0’ to this bit.

Thus  the code handling should changed so that the pending bit is 
mantained when "value" has a 1
in that position.


Signed-off-by: Gabriele Galeotti <gabriele.galeotti.xyz@gmail.com>
---
  hw/timer/grlib_gptimer.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c
index 4ed96e9..c555ae8 100644
--- a/hw/timer/grlib_gptimer.c
+++ b/hw/timer/grlib_gptimer.c
@@ -276,9 +276,6 @@ static void grlib_gptimer_write(void *opaque, hwaddr 
addr,
              trace_grlib_gptimer_writel(id, addr, value);
.
              if (value & GPTIMER_INT_PENDING) {
-                /* clear pending bit */
-                value &= ~GPTIMER_INT_PENDING;
-            } else {
                  /* keep pending bit */
                  value |= unit->timers[id].config & GPTIMER_INT_PENDING;
              }
--.
2.9.0

Re: [Qemu-devel] LEON3 timer patch

[Fabien Chouteau]

On 30/03/2017 21:30, Gabriele Galeotti wrote:
> 
> Hi all.
> According to "GR712RC Dual-Core LEON3FT SPARC V8 Processor User’s Manual",
> "11.3 Registers", pg 87-88, Table 55 Timer control register, the IP "interrupt pending"
> bit:
> 
> Interrupt Pending (IP): The core sets this bit to ‘1’ when an interrupt is signalled. This bit remains ‘1’
> until cleared by writing ‘0’ to this bit.
> 
> Thus  the code handling should changed so that the pending bit is mantained when "value" has a 1
> in that position.

Thanks Gabriele,

I think my implementation is based on the "GRLIB IP Core User' Manual"
which says "Interrupt Pending (IP): The core sets this bit to "1" when
an interrupt is signalled. This bit remains "1" until cleared by writing
"1" to this bit, writes of "0" have no effect."

Do you know why the two docs are different on that aspect?


> 
> Signed-off-by: Gabriele Galeotti <gabriele.galeotti.xyz@gmail.com>
> ---
>  hw/timer/grlib_gptimer.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c
> index 4ed96e9..c555ae8 100644
> --- a/hw/timer/grlib_gptimer.c
> +++ b/hw/timer/grlib_gptimer.c
> @@ -276,9 +276,6 @@ static void grlib_gptimer_write(void *opaque, hwaddr addr,
>              trace_grlib_gptimer_writel(id, addr, value);
> .
>              if (value & GPTIMER_INT_PENDING) {
> -                /* clear pending bit */
> -                value &= ~GPTIMER_INT_PENDING;
> -            } else {
>                  /* keep pending bit */
>                  value |= unit->timers[id].config & GPTIMER_INT_PENDING;
>              }

My understanding is that with your modifications, users can set the
pending bit by writing one to it.


I suggest something like this:

diff --git a/hw/timer/grlib_gptimer.c b/hw/timer/grlib_gptimer.c
index a05304d..4344787 100644
--- a/hw/timer/grlib_gptimer.c
+++ b/hw/timer/grlib_gptimer.c
@@ -275,11 +275,14 @@ static void grlib_gptimer_write(void *opaque, hwaddr addr,
         case CONFIG_OFFSET:
             trace_grlib_gptimer_writel(id, addr, value);

+            /* Interrupt pending. Remains 1 until cleared by writing 0 to this
+             * bit.
+             */
             if (value & GPTIMER_INT_PENDING) {
-                /* clear pending bit */
+                /* Clear pending bit in value */
                 value &= ~GPTIMER_INT_PENDING;
-            } else {
-                /* keep pending bit */
+
+                /* Read current pending bit from register */
                 value |= unit->timers[id].config & GPTIMER_INT_PENDING;
             }


Regards,