diff mbox series

[09/11] hw/gpio/pl061: Convert to 3-phase reset and assert GPIO lines correctly on reset

Message ID 20210702104018.19881-10-peter.maydell@linaro.org
State Superseded
Headers show
Series hw/arm: Make virt board secure powerdown/reset work | expand

Commit Message

Peter Maydell July 2, 2021, 10:40 a.m. UTC 
The PL061 comes out of reset with all its lines configured as input,
which means they might need to be pulled to 0 or 1 depending on the
'pullups' and 'pulldowns' properties.  Currently we do not assert
these lines on reset; they will only be set whenever the guest first
touches a register that triggers a call to pl061_update().

Convert the device to three-phase reset so we have a place where we
can safely call qemu_set_irq() to set the floating lines to their
correct values.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 hw/gpio/pl061.c      | 29 +++++++++++++++++++++++++----
 hw/gpio/trace-events |  1 +
 2 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.20.1

Comments

Philippe Mathieu-Daudé July 2, 2021, 10:56 a.m. UTC | #1 
On 7/2/21 12:40 PM, Peter Maydell wrote:
> The PL061 comes out of reset with all its lines configured as input,

> which means they might need to be pulled to 0 or 1 depending on the

> 'pullups' and 'pulldowns' properties.  Currently we do not assert

> these lines on reset; they will only be set whenever the guest first

> touches a register that triggers a call to pl061_update().

> 

> Convert the device to three-phase reset so we have a place where we

> can safely call qemu_set_irq() to set the floating lines to their

> correct values.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

>  hw/gpio/pl061.c      | 29 +++++++++++++++++++++++++----

>  hw/gpio/trace-events |  1 +

>  2 files changed, 26 insertions(+), 4 deletions(-)


Nice.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Richard Henderson July 7, 2021, 1:33 a.m. UTC | #2 
On 7/2/21 3:40 AM, Peter Maydell wrote:
> The PL061 comes out of reset with all its lines configured as input,

> which means they might need to be pulled to 0 or 1 depending on the

> 'pullups' and 'pulldowns' properties.  Currently we do not assert

> these lines on reset; they will only be set whenever the guest first

> touches a register that triggers a call to pl061_update().

> 

> Convert the device to three-phase reset so we have a place where we

> can safely call qemu_set_irq() to set the floating lines to their

> correct values.

> 

> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>

> ---

>   hw/gpio/pl061.c      | 29 +++++++++++++++++++++++++----

>   hw/gpio/trace-events |  1 +

>   2 files changed, 26 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
diff mbox series

Patch

diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index bb496a19ade..8d12b2d6b97 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -448,13 +448,14 @@  static void pl061_write(void *opaque, hwaddr offset,
     return;
 }
 
-static void pl061_reset(DeviceState *dev)
+static void pl061_enter_reset(Object *obj, ResetType type)
 {
-    PL061State *s = PL061(dev);
+    PL061State *s = PL061(obj);
+
+    trace_pl061_reset(DEVICE(s)->canonical_path);
 
     /* reset values from PL061 TRM, Stellaris LM3S5P31 & LM3S8962 Data Sheet */
     s->data = 0;
-    s->old_out_data = 0;
     s->old_in_data = 0;
     s->dir = 0;
     s->isense = 0;
@@ -476,6 +477,24 @@  static void pl061_reset(DeviceState *dev)
     s->amsel = 0;
 }
 
+static void pl061_hold_reset(Object *obj)
+{
+    PL061State *s = PL061(obj);
+    int i, level;
+    uint8_t floating = pl061_floating(s);
+    uint8_t pullups = pl061_pullups(s);
+
+    for (i = 0; i < N_GPIOS; i++) {
+        if (extract32(floating, i, 1)) {
+            continue;
+        }
+        level = extract32(pullups, i, 1);
+        trace_pl061_set_output(DEVICE(s)->canonical_path, i, level);
+        qemu_set_irq(s->out[i], level);
+    }
+    s->old_out_data = pullups;
+}
+
 static void pl061_set_irq(void * opaque, int irq, int level)
 {
     PL061State *s = (PL061State *)opaque;
@@ -545,11 +564,13 @@  static Property pl061_props[] = {
 static void pl061_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
 
     dc->vmsd = &vmstate_pl061;
-    dc->reset = &pl061_reset;
     dc->realize = pl061_realize;
     device_class_set_props(dc, pl061_props);
+    rc->phases.enter = pl061_enter_reset;
+    rc->phases.hold = pl061_hold_reset;
 }
 
 static const TypeInfo pl061_info = {
diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
index eb5fb4701c6..1dab99c5604 100644
--- a/hw/gpio/trace-events
+++ b/hw/gpio/trace-events
@@ -20,6 +20,7 @@  pl061_input_change(const char *id, int gpio, int level) "%s input %d changed to
 pl061_update_istate(const char *id, uint32_t istate, uint32_t im, int level) "%s GPIORIS 0x%x GPIOIE 0x%x interrupt level %d"
 pl061_read(const char *id, uint64_t offset, uint64_t r) "%s offset 0x%" PRIx64 " value 0x%" PRIx64
 pl061_write(const char *id, uint64_t offset, uint64_t value) "%s offset 0x%" PRIx64 " value 0x%" PRIx64
+pl061_reset(const char *id) "%s reset"
 
 # sifive_gpio.c
 sifive_gpio_read(uint64_t offset, uint64_t r) "offset 0x%" PRIx64 " value 0x%" PRIx64