diff mbox series

[06/11] hw/gpio/pl061: Honour Luminary PL061 PUR and PDR registers

Message ID 20210702104018.19881-7-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 Luminary variant of the PL061 has registers GPIOPUR and GPIOPDR
which lets the guest configure whether the GPIO lines are pull-up,
pull-down, or truly floating. Instead of assuming all lines are pulled
high, honour the PUR and PDR registers.

For the plain PL061, continue to assume that lines have an external
pull-up resistor, as we did before.

The stellaris board actually relies on this behaviour -- the CD line
of the ssd0323 display device is connected to GPIO output C7, and it
is only because of a different bug which we're about to fix that we
weren't incorrectly driving this line high on reset and putting the
ssd0323 into data mode.

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

---
 hw/gpio/pl061.c      | 58 +++++++++++++++++++++++++++++++++++++++++---
 hw/gpio/trace-events |  2 +-
 2 files changed, 55 insertions(+), 5 deletions(-)

-- 
2.20.1

Comments

Richard Henderson July 7, 2021, 1:29 a.m. UTC | #1
On 7/2/21 3:40 AM, Peter Maydell wrote:
> The Luminary variant of the PL061 has registers GPIOPUR and GPIOPDR

> which lets the guest configure whether the GPIO lines are pull-up,

> pull-down, or truly floating. Instead of assuming all lines are pulled

> high, honour the PUR and PDR registers.

> 

> For the plain PL061, continue to assume that lines have an external

> pull-up resistor, as we did before.

> 

> The stellaris board actually relies on this behaviour -- the CD line

> of the ssd0323 display device is connected to GPIO output C7, and it

> is only because of a different bug which we're about to fix that we

> weren't incorrectly driving this line high on reset and putting the

> ssd0323 into data mode.

> 

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

> ---

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

>   hw/gpio/trace-events |  2 +-

>   2 files changed, 55 insertions(+), 5 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 06a1b82a503..44bed56fef0 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -94,18 +94,68 @@  static const VMStateDescription vmstate_pl061 = {
     }
 };
 
+static uint8_t pl061_floating(PL061State *s)
+{
+    /*
+     * Return mask of bits which correspond to pins configured as inputs
+     * and which are floating (neither pulled up to 1 nor down to 0).
+     */
+    uint8_t floating;
+
+    if (s->id == pl061_id_luminary) {
+        /*
+         * If both PUR and PDR bits are clear, there is neither a pullup
+         * nor a pulldown in place, and the output truly floats.
+         */
+        floating = ~(s->pur | s->pdr);
+    } else {
+        /* Assume outputs are pulled high. FIXME: this is board dependent. */
+        floating = 0;
+    }
+    return floating & ~s->dir;
+}
+
+static uint8_t pl061_pullups(PL061State *s)
+{
+    /*
+     * Return mask of bits which correspond to pins configured as inputs
+     * and which are pulled up to 1.
+     */
+    uint8_t pullups;
+
+    if (s->id == pl061_id_luminary) {
+        /*
+         * The Luminary variant of the PL061 has an extra registers which
+         * the guest can use to configure whether lines should be pullup
+         * or pulldown.
+         */
+        pullups = s->pur;
+    } else {
+        /* Assume outputs are pulled high. FIXME: this is board dependent. */
+        pullups = 0xff;
+    }
+    return pullups & ~s->dir;
+}
+
 static void pl061_update(PL061State *s)
 {
     uint8_t changed;
     uint8_t mask;
     uint8_t out;
     int i;
+    uint8_t pullups = pl061_pullups(s);
+    uint8_t floating = pl061_floating(s);
 
-    trace_pl061_update(DEVICE(s)->canonical_path, s->dir, s->data);
+    trace_pl061_update(DEVICE(s)->canonical_path, s->dir, s->data,
+                       pullups, floating);
 
-    /* Outputs float high.  */
-    /* FIXME: This is board dependent.  */
-    out = (s->data & s->dir) | ~s->dir;
+    /*
+     * Pins configured as output are driven from the data register;
+     * otherwise if they're pulled up they're 1, and if they're floating
+     * then we give them the same value they had previously, so we don't
+     * report any change to the other end.
+     */
+    out = (s->data & s->dir) | pullups | (s->old_out_data & floating);
     changed = s->old_out_data ^ out;
     if (changed) {
         s->old_out_data = out;
diff --git a/hw/gpio/trace-events b/hw/gpio/trace-events
index 442be9406f5..eb5fb4701c6 100644
--- a/hw/gpio/trace-events
+++ b/hw/gpio/trace-events
@@ -14,7 +14,7 @@  nrf51_gpio_set(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64
 nrf51_gpio_update_output_irq(int64_t line, int64_t value) "line %" PRIi64 " value %" PRIi64
 
 # pl061.c
-pl061_update(const char *id, uint32_t dir, uint32_t data) "%s GPIODIR 0x%x GPIODATA 0x%x"
+pl061_update(const char *id, uint32_t dir, uint32_t data, uint32_t pullups, uint32_t floating) "%s GPIODIR 0x%x GPIODATA 0x%x pullups 0x%x floating 0x%x"
 pl061_set_output(const char *id, int gpio, int level) "%s setting output %d to %d"
 pl061_input_change(const char *id, int gpio, int level) "%s input %d changed to %d"
 pl061_update_istate(const char *id, uint32_t istate, uint32_t im, int level) "%s GPIORIS 0x%x GPIOIE 0x%x interrupt level %d"