[Qemu-devel] [PATCH v2] nrf51_gpio: reflect pull-up/pull-down to IRQs

Paolo Bonzini posted 1 patch 5 years, 1 month ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190317141001.3346-1-pbonzini@redhat.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Joel Stanley <joel@jms.id.au>
hw/gpio/nrf51_gpio.c | 64 +++++++++++++++++++++++++++-----------------
1 file changed, 39 insertions(+), 25 deletions(-)
[Qemu-devel] [PATCH v2] nrf51_gpio: reflect pull-up/pull-down to IRQs
Posted by Paolo Bonzini 5 years, 1 month ago
Some drivers do I2C bitbanging by keeping the output to 0 and flipping
the GPIO direction between input and output (see for example in Linux
gpio_set_open_drain_value_commit, in drivers/gpio/gpiolib.c).
When the GPIO is set to input, the pull-up resistor brings the output
to 1, while when the GPIO is set to output, the output driver brings
the output to 0.

Implement this for the nRF51 GPIO device model.  First, if both input and
output are floating, and there is a pull-up or pull-down resistor
configured, do not just set s->in, but also make any devices listening
on the output qemu_irq receive that value.  Second, if the pin is
driven both internally (output pin) and externally you don't get a
short circuit if both sides drive the pin to the same value.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        v1->v2: fixed short circuit conditions, reordering the code according
        to the schematic in the datasheet
---
 hw/gpio/nrf51_gpio.c | 64 +++++++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 25 deletions(-)

diff --git a/hw/gpio/nrf51_gpio.c b/hw/gpio/nrf51_gpio.c
index 86e047d649..e061c58dd2 100644
--- a/hw/gpio/nrf51_gpio.c
+++ b/hw/gpio/nrf51_gpio.c
@@ -43,6 +43,17 @@ static bool is_connected(uint32_t config, uint32_t level)
     return state;
 }
 
+static int pull_value(uint32_t config)
+{
+    int pull = extract32(config, 2, 2);
+    if (pull == NRF51_GPIO_PULLDOWN) {
+        return 0;
+    } else if (pull == NRF51_GPIO_PULLUP) {
+        return 1;
+    }
+    return -1;
+}
+
 static void update_output_irq(NRF51GPIOState *s, size_t i,
                               bool connected, bool level)
 {
@@ -61,43 +72,46 @@ static void update_output_irq(NRF51GPIOState *s, size_t i,
 
 static void update_state(NRF51GPIOState *s)
 {
-    uint32_t pull;
+    int pull;
     size_t i;
-    bool connected_out, dir, connected_in, out, input;
+    bool connected_out, dir, connected_in, out, in, input;
 
     for (i = 0; i < NRF51_GPIO_PINS; i++) {
-        pull = extract32(s->cnf[i], 2, 2);
+        pull = pull_value(s->cnf[i]);
         dir = extract32(s->cnf[i], 0, 1);
         connected_in = extract32(s->in_mask, i, 1);
         out = extract32(s->out, i, 1);
+        in = extract32(s->in, i, 1);
         input = !extract32(s->cnf[i], 1, 1);
         connected_out = is_connected(s->cnf[i], out) && dir;
 
-        update_output_irq(s, i, connected_out, out);
-
-        /* Pin both driven externally and internally */
-        if (connected_out && connected_in) {
-            qemu_log_mask(LOG_GUEST_ERROR, "GPIO pin %zu short circuited\n", i);
-        }
-
-        /*
-         * Input buffer disconnected from internal/external drives, so
-         * pull-up/pull-down becomes relevant
-         */
-        if (!input || (input && !connected_in && !connected_out)) {
-            if (pull == NRF51_GPIO_PULLDOWN) {
-                s->in = deposit32(s->in, i, 1, 0);
-            } else if (pull == NRF51_GPIO_PULLUP) {
-                s->in = deposit32(s->in, i, 1, 1);
+        if (!input) {
+            if (pull >= 0) {
+                /* Input buffer disconnected from external drives */
+                s->in = deposit32(s->in, i, 1, pull);
+            }
+        } else {
+            if (connected_out && connected_in && out != in) {
+                /* Pin both driven externally and internally */
+                qemu_log_mask(LOG_GUEST_ERROR, "GPIO pin %zu short circuited\n", i);
+            }
+            if (!connected_in) {
+                /*
+                 * Floating input: the output stimulates IN if connected,
+                 * otherwise pull-up/pull-down resistors put a value on both
+                 * IN and OUT.
+                 */
+                if (pull >= 0 && !connected_out) {
+                    connected_out = true;
+                    out = pull;
+                }
+                if (connected_out) {
+                    s->in = deposit32(s->in, i, 1, out);
+                }
             }
         }
-
-        /* Self stimulation through internal output driver */
-        if (connected_out && !connected_in && input) {
-            s->in = deposit32(s->in, i, 1, out);
-        }
+        update_output_irq(s, i, connected_out, out);
     }
-
 }
 
 /*
-- 
2.20.1


Re: [Qemu-devel] [PATCH v2] nrf51_gpio: reflect pull-up/pull-down to IRQs
Posted by Stefan Hajnoczi 5 years ago
On Sun, Mar 17, 2019 at 03:10:01PM +0100, Paolo Bonzini wrote:
> Some drivers do I2C bitbanging by keeping the output to 0 and flipping
> the GPIO direction between input and output (see for example in Linux
> gpio_set_open_drain_value_commit, in drivers/gpio/gpiolib.c).
> When the GPIO is set to input, the pull-up resistor brings the output
> to 1, while when the GPIO is set to output, the output driver brings
> the output to 0.
> 
> Implement this for the nRF51 GPIO device model.  First, if both input and
> output are floating, and there is a pull-up or pull-down resistor
> configured, do not just set s->in, but also make any devices listening
> on the output qemu_irq receive that value.  Second, if the pin is
> driven both internally (output pin) and externally you don't get a
> short circuit if both sides drive the pin to the same value.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         v1->v2: fixed short circuit conditions, reordering the code according
>         to the schematic in the datasheet
> ---
>  hw/gpio/nrf51_gpio.c | 64 +++++++++++++++++++++++++++-----------------
>  1 file changed, 39 insertions(+), 25 deletions(-)

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [Qemu-devel] [Qemu-arm] [PATCH v2] nrf51_gpio: reflect pull-up/pull-down to IRQs
Posted by Peter Maydell 5 years ago
On Sun, 17 Mar 2019 at 14:13, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Some drivers do I2C bitbanging by keeping the output to 0 and flipping
> the GPIO direction between input and output (see for example in Linux
> gpio_set_open_drain_value_commit, in drivers/gpio/gpiolib.c).
> When the GPIO is set to input, the pull-up resistor brings the output
> to 1, while when the GPIO is set to output, the output driver brings
> the output to 0.
>
> Implement this for the nRF51 GPIO device model.  First, if both input and
> output are floating, and there is a pull-up or pull-down resistor
> configured, do not just set s->in, but also make any devices listening
> on the output qemu_irq receive that value.  Second, if the pin is
> driven both internally (output pin) and externally you don't get a
> short circuit if both sides drive the pin to the same value.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         v1->v2: fixed short circuit conditions, reordering the code according
>         to the schematic in the datasheet



Applied to target-arm.next, thanks.

-- PMM