[PATCH v2 0/6] Add nRF51 DETECT signal with test

[PATCH v2 0/6] Add nRF51 DETECT signal with test

[Chris Laplante]

This patch series implements the nRF51 DETECT signal
in the GPIO peripheral. A qtest is added exercising the signal.

To implement the test, named out-GPIO IRQ interception had to be added
to the qtest framework. I also took the opportunity to improve IRQ
interception a bit by adding 'FAIL' responses when interception fails.
Otherwise, it is frustrating to troubleshoot why calls to
qtest_irq_intercept_out and friends appears to do nothing.

v1: https://patchwork.kernel.org/project/qemu-devel/list/?series=766078

Testing
=======
Passes 'make check'

Changelog
=========
v2: factor out qtest_install_gpio_out_intercept before usage (Peter)
    renamed qtest_install_gpio_out_intercepts => qtest_install_gpio_out_intercept
    don't pass DETECT to soc level (Peter)
    change qtest to use DETECT at GPIO level (Peter)


Chris Laplante (6):
  hw/gpio/nrf51: implement DETECT signal
  qtest: factor out qtest_install_gpio_out_intercept
  qtest: implement named interception of out-GPIO
  qtest: bail from irq_intercept_in if name is specified
  qtest: irq_intercept_[out/in]: return FAIL if no intercepts are
    installed
  qtest: microbit-test: add tests for nRF51 DETECT

 hw/gpio/nrf51_gpio.c         | 14 ++++++++-
 include/hw/gpio/nrf51_gpio.h |  1 +
 softmmu/qtest.c              | 56 ++++++++++++++++++++++++++----------
 tests/qtest/libqtest.c       |  6 ++++
 tests/qtest/libqtest.h       | 11 +++++++
 tests/qtest/microbit-test.c  | 42 +++++++++++++++++++++++++++
 6 files changed, 114 insertions(+), 16 deletions(-)

--
2.41.0

[PATCH v2 1/6] hw/gpio/nrf51: implement DETECT signal

[Chris Laplante]

Implement nRF51 DETECT signal in the GPIO peripheral.

The reference manual makes mention of a per-pin DETECT signal, but these
are not exposed to the user. See https://devzone.nordicsemi.com/f/nordic-q-a/39858/gpio-per-pin-detect-signal-available
for more information. Currently, I don't see a reason to model these.

Signed-off-by: Chris Laplante <chris@laplante.io>
---
 hw/gpio/nrf51_gpio.c         | 14 +++++++++++++-
 include/hw/gpio/nrf51_gpio.h |  1 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/gpio/nrf51_gpio.c b/hw/gpio/nrf51_gpio.c
index b47fddf4ed..08396c69a4 100644
--- a/hw/gpio/nrf51_gpio.c
+++ b/hw/gpio/nrf51_gpio.c
@@ -78,6 +78,7 @@ static void update_state(NRF51GPIOState *s)
     int pull;
     size_t i;
     bool connected_out, dir, connected_in, out, in, input;
+    bool assert_detect = false;
 
     for (i = 0; i < NRF51_GPIO_PINS; i++) {
         pull = pull_value(s->cnf[i]);
@@ -99,7 +100,15 @@ static void update_state(NRF51GPIOState *s)
                 qemu_log_mask(LOG_GUEST_ERROR,
                               "GPIO pin %zu short circuited\n", i);
             }
-            if (!connected_in) {
+            if (connected_in) {
+                uint32_t detect_config = extract32(s->cnf[i], 16, 2);
+                if ((detect_config == 2) && (in == 1)) {
+                    assert_detect = true;
+                }
+                if ((detect_config == 3) && (in == 0)) {
+                    assert_detect = true;
+                }
+            } else {
                 /*
                  * Floating input: the output stimulates IN if connected,
                  * otherwise pull-up/pull-down resistors put a value on both
@@ -116,6 +125,8 @@ static void update_state(NRF51GPIOState *s)
         }
         update_output_irq(s, i, connected_out, out);
     }
+
+    qemu_set_irq(s->detect, assert_detect);
 }
 
 /*
@@ -291,6 +302,7 @@ static void nrf51_gpio_init(Object *obj)
 
     qdev_init_gpio_in(DEVICE(s), nrf51_gpio_set, NRF51_GPIO_PINS);
     qdev_init_gpio_out(DEVICE(s), s->output, NRF51_GPIO_PINS);
+    qdev_init_gpio_out_named(DEVICE(s), &s->detect, "detect", 1);
 }
 
 static void nrf51_gpio_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/gpio/nrf51_gpio.h b/include/hw/gpio/nrf51_gpio.h
index 8f9c2f86da..fcfa2bac17 100644
--- a/include/hw/gpio/nrf51_gpio.h
+++ b/include/hw/gpio/nrf51_gpio.h
@@ -64,6 +64,7 @@ struct NRF51GPIOState {
     uint32_t old_out_connected;
 
     qemu_irq output[NRF51_GPIO_PINS];
+    qemu_irq detect;
 };
 
 
-- 
2.41.0

[PATCH v2 2/6] qtest: factor out qtest_install_gpio_out_intercept

[Chris Laplante]

Signed-off-by: Chris Laplante <chris@laplante.io>
---
 softmmu/qtest.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index f8d764b719..1c92e5a6a3 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -365,6 +365,15 @@ void qtest_set_command_cb(bool (*pc_cb)(CharBackend *chr, gchar **words))
     process_command_cb = pc_cb;
 }
 
+static void qtest_install_gpio_out_intercept(DeviceState *dev, const char *name, int n)
+{
+    qemu_irq *disconnected = g_new0(qemu_irq, 1);
+    qemu_irq icpt = qemu_allocate_irq(qtest_irq_handler,
+                                      disconnected, n);
+
+    *disconnected = qdev_intercept_gpio_out(dev, icpt,name, n);
+}
+
 static void qtest_process_command(CharBackend *chr, gchar **words)
 {
     const gchar *command;
@@ -415,12 +424,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
             if (words[0][14] == 'o') {
                 int i;
                 for (i = 0; i < ngl->num_out; ++i) {
-                    qemu_irq *disconnected = g_new0(qemu_irq, 1);
-                    qemu_irq icpt = qemu_allocate_irq(qtest_irq_handler,
-                                                      disconnected, i);
-
-                    *disconnected = qdev_intercept_gpio_out(dev, icpt,
-                                                            ngl->name, i);
+                    qtest_install_gpio_out_intercept(dev, ngl->name, i);
                 }
             } else {
                 qemu_irq_intercept_in(ngl->in, qtest_irq_handler,
-- 
2.41.0

[PATCH v2 3/6] qtest: implement named interception of out-GPIO

[Chris Laplante]

Adds qtest_irq_intercept_out_named method, which utilizes a new optional
name parameter to the irq_intercept_out qtest command.

Signed-off-by: Chris Laplante <chris@laplante.io>
---
 softmmu/qtest.c        | 24 ++++++++++++++++--------
 tests/qtest/libqtest.c |  6 ++++++
 tests/qtest/libqtest.h | 11 +++++++++++
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 1c92e5a6a3..7fd8546ed2 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -397,8 +397,12 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         || strcmp(words[0], "irq_intercept_in") == 0) {
         DeviceState *dev;
         NamedGPIOList *ngl;
+        bool is_named;
+        bool is_outbound;
 
         g_assert(words[1]);
+        is_named = words[2] != NULL;
+        is_outbound = words[0][14] == 'o';
         dev = DEVICE(object_resolve_path(words[1], NULL));
         if (!dev) {
             qtest_send_prefix(chr);
@@ -417,14 +421,18 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         }
 
         QLIST_FOREACH(ngl, &dev->gpios, node) {
-            /* We don't support intercept of named GPIOs yet */
-            if (ngl->name) {
-                continue;
-            }
-            if (words[0][14] == 'o') {
-                int i;
-                for (i = 0; i < ngl->num_out; ++i) {
-                    qtest_install_gpio_out_intercept(dev, ngl->name, i);
+            /* We don't support inbound interception of named GPIOs yet */
+            if (is_outbound) {
+                if (is_named) {
+                    if (ngl->name && strcmp(ngl->name, words[2]) == 0) {
+                        qtest_install_gpio_out_intercept(dev, ngl->name, 0);
+                        break;
+                    }
+                } else if (!ngl->name) {
+                    int i;
+                    for (i = 0; i < ngl->num_out; ++i) {
+                        qtest_install_gpio_out_intercept(dev, ngl->name, i);
+                    }
                 }
             } else {
                 qemu_irq_intercept_in(ngl->in, qtest_irq_handler,
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index c22dfc30d3..471529e6cc 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -993,6 +993,12 @@ void qtest_irq_intercept_out(QTestState *s, const char *qom_path)
     qtest_rsp(s);
 }
 
+void qtest_irq_intercept_out_named(QTestState *s, const char *qom_path, const char *name)
+{
+    qtest_sendf(s, "irq_intercept_out %s %s\n", qom_path, name);
+    qtest_rsp(s);
+}
+
 void qtest_irq_intercept_in(QTestState *s, const char *qom_path)
 {
     qtest_sendf(s, "irq_intercept_in %s\n", qom_path);
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 3a71bc45fc..e53e350e3a 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -371,6 +371,17 @@ void qtest_irq_intercept_in(QTestState *s, const char *string);
  */
 void qtest_irq_intercept_out(QTestState *s, const char *string);
 
+/**
+ * qtest_irq_intercept_out_named:
+ * @s: #QTestState instance to operate on.
+ * @qom_path: QOM path of a device.
+ * @name: Name of the GPIO out pin
+ *
+ * Associate a qtest irq with the named GPIO-out pin of the device
+ * whose path is specified by @string and whose name is @name.
+ */
+void qtest_irq_intercept_out_named(QTestState *s, const char *qom_path, const char *name);
+
 /**
  * qtest_set_irq_in:
  * @s: QTestState instance to operate on.
-- 
2.41.0

[PATCH v2 4/6] qtest: bail from irq_intercept_in if name is specified

[Chris Laplante]

Named interception of in-GPIOs is not supported yet.

Signed-off-by: Chris Laplante <chris@laplante.io>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 softmmu/qtest.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 7fd8546ed2..1719bbddc3 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -410,6 +410,12 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
             return;
         }
 
+        if (is_named && !is_outbound) {
+            qtest_send_prefix(chr);
+            qtest_send(chr, "FAIL Interception of named in-GPIOs not yet supported\n");
+            return;
+        }
+
         if (irq_intercept_dev) {
             qtest_send_prefix(chr);
             if (irq_intercept_dev != dev) {
@@ -421,7 +427,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         }
 
         QLIST_FOREACH(ngl, &dev->gpios, node) {
-            /* We don't support inbound interception of named GPIOs yet */
             if (is_outbound) {
                 if (is_named) {
                     if (ngl->name && strcmp(ngl->name, words[2]) == 0) {
-- 
2.41.0

[PATCH v2 5/6] qtest: irq_intercept_[out/in]: return FAIL if no intercepts are installed

[Chris Laplante]

This is much better than just silently failing with OK.

Signed-off-by: Chris Laplante <chris@laplante.io>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 softmmu/qtest.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 1719bbddc3..c9751f527f 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -399,6 +399,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         NamedGPIOList *ngl;
         bool is_named;
         bool is_outbound;
+        bool interception_succeeded = false;
 
         g_assert(words[1]);
         is_named = words[2] != NULL;
@@ -431,6 +432,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
                 if (is_named) {
                     if (ngl->name && strcmp(ngl->name, words[2]) == 0) {
                         qtest_install_gpio_out_intercept(dev, ngl->name, 0);
+                        interception_succeeded = true;
                         break;
                     }
                 } else if (!ngl->name) {
@@ -438,15 +440,22 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
                     for (i = 0; i < ngl->num_out; ++i) {
                         qtest_install_gpio_out_intercept(dev, ngl->name, i);
                     }
+                    interception_succeeded = true;
                 }
             } else {
                 qemu_irq_intercept_in(ngl->in, qtest_irq_handler,
                                       ngl->num_in);
+                interception_succeeded = true;
             }
         }
-        irq_intercept_dev = dev;
+
         qtest_send_prefix(chr);
-        qtest_send(chr, "OK\n");
+        if (interception_succeeded) {
+            irq_intercept_dev = dev;
+            qtest_send(chr, "OK\n");
+        } else {
+            qtest_send(chr, "FAIL No intercepts installed\n");
+        }
     } else if (strcmp(words[0], "set_irq_in") == 0) {
         DeviceState *dev;
         qemu_irq irq;
-- 
2.41.0

[PATCH v2 6/6] qtest: microbit-test: add tests for nRF51 DETECT

[Chris Laplante]

Exercise the DETECT mechanism of the GPIO peripheral.

Signed-off-by: Chris Laplante <chris@laplante.io>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/microbit-test.c | 42 +++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tests/qtest/microbit-test.c b/tests/qtest/microbit-test.c
index 6022a92b6a..8f87810cd5 100644
--- a/tests/qtest/microbit-test.c
+++ b/tests/qtest/microbit-test.c
@@ -393,6 +393,47 @@ static void test_nrf51_gpio(void)
     qtest_quit(qts);
 }
 
+static void test_nrf51_gpio_detect(void) {
+    QTestState *qts = qtest_init("-M microbit");
+    int i;
+
+    // Connect input buffer on pins 1-7, configure SENSE for high level
+    for (i = 1; i <= 7; i++) {
+        qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START + i * 4, deposit32(0, 16, 2, 2));
+    }
+
+    qtest_irq_intercept_out_named(qts, "/machine/nrf51/gpio", "detect");
+
+    for (i = 1; i <= 7; i++) {
+        // Set pin high
+        qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", i, 1);
+        uint32_t actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN);
+        g_assert_cmpuint(actual, ==, 1 << i);
+
+        // Check that DETECT is high
+        g_assert_true(qtest_get_irq(qts, 0));
+
+        // Set pin low, check that DETECT goes low.
+        qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", i, 0);
+        actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN);
+        g_assert_cmpuint(actual, ==, 0x0);
+        g_assert_false(qtest_get_irq(qts, 0));
+    }
+
+    // Set pin 0 high, check that DETECT doesn't fire
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, 1);
+    g_assert_false(qtest_get_irq(qts, 0));
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, 0);
+
+    // Set pins 1, 2, and 3 high, then set 3 low. Check that DETECT is still high.
+    for (i = 1; i <= 3; i++) {
+        qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", i, 1);
+    }
+    g_assert_true(qtest_get_irq(qts, 0));
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 3, 0);
+    g_assert_true(qtest_get_irq(qts, 0));
+}
+
 static void timer_task(QTestState *qts, hwaddr task)
 {
     qtest_writel(qts, NRF51_TIMER_BASE + task, NRF51_TRIGGER_TASK);
@@ -499,6 +540,7 @@ int main(int argc, char **argv)
 
     qtest_add_func("/microbit/nrf51/uart", test_nrf51_uart);
     qtest_add_func("/microbit/nrf51/gpio", test_nrf51_gpio);
+    qtest_add_func("/microbit/nrf51/gpio_detect", test_nrf51_gpio_detect);
     qtest_add_func("/microbit/nrf51/nvmc", test_nrf51_nvmc);
     qtest_add_func("/microbit/nrf51/timer", test_nrf51_timer);
     qtest_add_func("/microbit/microbit/i2c", test_microbit_i2c);
-- 
2.41.0

Re: [PATCH v2 1/6] hw/gpio/nrf51: implement DETECT signal

[Peter Maydell]

On Wed, 26 Jul 2023 at 04:32, Chris Laplante <chris@laplante.io> wrote:
>
> Implement nRF51 DETECT signal in the GPIO peripheral.
>
> The reference manual makes mention of a per-pin DETECT signal, but these
> are not exposed to the user. See https://devzone.nordicsemi.com/f/nordic-q-a/39858/gpio-per-pin-detect-signal-available
> for more information. Currently, I don't see a reason to model these.
>
> Signed-off-by: Chris Laplante <chris@laplante.io>
> ---

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

thanks
-- PMM

Re: [PATCH v2 2/6] qtest: factor out qtest_install_gpio_out_intercept

[Peter Maydell]

On Wed, 26 Jul 2023 at 04:32, Chris Laplante <chris@laplante.io> wrote:
>
> Signed-off-by: Chris Laplante <chris@laplante.io>
> ---
>  softmmu/qtest.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/softmmu/qtest.c b/softmmu/qtest.c
> index f8d764b719..1c92e5a6a3 100644
> --- a/softmmu/qtest.c
> +++ b/softmmu/qtest.c
> @@ -365,6 +365,15 @@ void qtest_set_command_cb(bool (*pc_cb)(CharBackend *chr, gchar **words))
>      process_command_cb = pc_cb;
>  }
>
> +static void qtest_install_gpio_out_intercept(DeviceState *dev, const char *name, int n)
> +{
> +    qemu_irq *disconnected = g_new0(qemu_irq, 1);
> +    qemu_irq icpt = qemu_allocate_irq(qtest_irq_handler,
> +                                      disconnected, n);
> +
> +    *disconnected = qdev_intercept_gpio_out(dev, icpt,name, n);

Missing space after comma.
Otherwise

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

(If this is the only issue with the series I'll fix it
when I pick it up, no need for a respin.)

thanks
-- PMM

Re: [PATCH v2 3/6] qtest: implement named interception of out-GPIO

[Peter Maydell]

On Wed, 26 Jul 2023 at 04:32, Chris Laplante <chris@laplante.io> wrote:
>
> Adds qtest_irq_intercept_out_named method, which utilizes a new optional
> name parameter to the irq_intercept_out qtest command.
>
> Signed-off-by: Chris Laplante <chris@laplante.io>
> ---
>  softmmu/qtest.c        | 24 ++++++++++++++++--------
>  tests/qtest/libqtest.c |  6 ++++++
>  tests/qtest/libqtest.h | 11 +++++++++++
>  3 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/softmmu/qtest.c b/softmmu/qtest.c
> index 1c92e5a6a3..7fd8546ed2 100644
> --- a/softmmu/qtest.c
> +++ b/softmmu/qtest.c
> @@ -397,8 +397,12 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
>          || strcmp(words[0], "irq_intercept_in") == 0) {
>          DeviceState *dev;
>          NamedGPIOList *ngl;
> +        bool is_named;
> +        bool is_outbound;
>
>          g_assert(words[1]);
> +        is_named = words[2] != NULL;
> +        is_outbound = words[0][14] == 'o';
>          dev = DEVICE(object_resolve_path(words[1], NULL));
>          if (!dev) {
>              qtest_send_prefix(chr);
> @@ -417,14 +421,18 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
>          }
>
>          QLIST_FOREACH(ngl, &dev->gpios, node) {
> -            /* We don't support intercept of named GPIOs yet */
> -            if (ngl->name) {
> -                continue;
> -            }
> -            if (words[0][14] == 'o') {
> -                int i;
> -                for (i = 0; i < ngl->num_out; ++i) {
> -                    qtest_install_gpio_out_intercept(dev, ngl->name, i);
> +            /* We don't support inbound interception of named GPIOs yet */
> +            if (is_outbound) {
> +                if (is_named) {
> +                    if (ngl->name && strcmp(ngl->name, words[2]) == 0) {
> +                        qtest_install_gpio_out_intercept(dev, ngl->name, 0);
> +                        break;
> +                    }

Named gpio-outs can have more than one line, the same as
unnamed ones (you create them with
qdev_init_gpio_out_named(dev, pins, name, n) -- there's an
argument for how many to create), so I think this is_named
branch also needs to install an intercept for each one, not
just for 0.

> +                } else if (!ngl->name) {
> +                    int i;
> +                    for (i = 0; i < ngl->num_out; ++i) {
> +                        qtest_install_gpio_out_intercept(dev, ngl->name, i);
> +                    }

...at which point the code looks pretty similar in both branches
of the if(), so I think you end up with something like

         if (is_outbound) {
             /* NULL is valid and matchable, for "unnamed GPIO" */
             if (g_strcmp0(ngl->name, words[2]) == 0) {
                 int i;
;                for (i = 0; i < ngl->num_out; ++i) {
                     qtest_install_gpio_out_intercept(dev, ngl->name, i);
                 }
             }
         } ...

(g_strcmp0() can handle the NULL case without having
to special case it -- this is how qdev_get_named_gpio_list()
finds entries in the ngl list.)

Apologies for not noticing that on the first round of review.

thanks
-- PMM

Re: [PATCH v2 0/6] Add nRF51 DETECT signal with test

[Peter Maydell]

On Wed, 26 Jul 2023 at 04:32, Chris Laplante <chris@laplante.io> wrote:
>
> This patch series implements the nRF51 DETECT signal
> in the GPIO peripheral. A qtest is added exercising the signal.
>
> To implement the test, named out-GPIO IRQ interception had to be added
> to the qtest framework. I also took the opportunity to improve IRQ
> interception a bit by adding 'FAIL' responses when interception fails.
> Otherwise, it is frustrating to troubleshoot why calls to
> qtest_irq_intercept_out and friends appears to do nothing.
>
> v1: https://patchwork.kernel.org/project/qemu-devel/list/?series=766078
>
> Testing
> =======
> Passes 'make check'

There was a trivial typo in patch 2, and I think patch 3
needs some (not very big) changes to handle named GPIOs
where there's more than 1 GPIO in the array, but other
than that this is looking good.

thanks
-- PMM

Re: [PATCH v2 3/6] qtest: implement named interception of out-GPIO

[Chris Laplante]

> (g_strcmp0() can handle the NULL case without having
> to special case it -- this is how qdev_get_named_gpio_list()
> finds entries in the ngl list.)
> 
> Apologies for not noticing that on the first round of review.

No worries - it makes the code much simpler anyway. Should we bother factoring out qtest_install_gpio_out_intercept still? It is only used in one place now, as before.

Thanks,
Chris

Re: [PATCH v2 3/6] qtest: implement named interception of out-GPIO

[Peter Maydell]

On Thu, 27 Jul 2023 at 22:32, Chris Laplante <chris@laplante.io> wrote:
>
>
> > (g_strcmp0() can handle the NULL case without having
> > to special case it -- this is how qdev_get_named_gpio_list()
> > finds entries in the ngl list.)
> >
> > Apologies for not noticing that on the first round of review.
>
> No worries - it makes the code much simpler anyway. Should we bother factoring out qtest_install_gpio_out_intercept still? It is only used in one place now, as before.

I'd keep the function, I think, since you've already written
it. It's kind of usefully documenting of the intent.

-- PMM