On Wed, 26 Jul 2023 at 04:32, Chris Laplante <[email protected]> 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 <[email protected]>
> ---
> 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
