On Wed, 26 Jul 2023 at 04:32, Chris Laplante <ch...@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 <ch...@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