[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 3/6] qtest: implement named interception of out-GPIO
From: |
Peter Maydell |
Subject: |
Re: [PATCH v2 3/6] qtest: implement named interception of out-GPIO |
Date: |
Thu, 27 Jul 2023 17:23:32 +0100 |
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
- [PATCH v2 0/6] Add nRF51 DETECT signal with test, Chris Laplante, 2023/07/25
- [PATCH v2 5/6] qtest: irq_intercept_[out/in]: return FAIL if no intercepts are installed, Chris Laplante, 2023/07/25
- [PATCH v2 4/6] qtest: bail from irq_intercept_in if name is specified, Chris Laplante, 2023/07/25
- [PATCH v2 6/6] qtest: microbit-test: add tests for nRF51 DETECT, Chris Laplante, 2023/07/25
- Re: [PATCH v2 0/6] Add nRF51 DETECT signal with test, Peter Maydell, 2023/07/27