[v2,2/2] tests/qtest: Check that EXTI fan-in irqs are correctly connected

This commit adds a QTest that verifies each input line of a specific
EXTI OR gate can influence the output line.

Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---

Hello,

I expected this test to fail after switching the two patch commits,
but it didn't.
I'm mentionning it in case it reveals a problem with the test I didn't notice.


 tests/qtest/stm32l4x5_exti-test.c | 37 +++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

On Tue, 20 Feb 2024 at 18:41, Inès Varhol <ines.varhol@telecom-paris.fr> wrote:
>
> This commit adds a QTest that verifies each input line of a specific
> EXTI OR gate can influence the output line.
>
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>
> Hello,
>
> I expected this test to fail after switching the two patch commits,
> but it didn't.
> I'm mentionning it in case it reveals a problem with the test I didn't notice.

The specific thing that goes wrong if you don't have the OR
gate handling is that the NVIC input will see every up
and down transition from each input separately. (This happens
because a GPIO/irq 'input' in QEMU is basically a function,
and wiring up an 'output' to an 'input' is setting "this
is the function pointer you should call when the output
changes". Nothing syntactically stops you passing the
same function pointer to multiple outputs.)

So if you have for instance
 raise A; raise B; drop B; drop A
where A and B are ORed together into an NVIC input,
the NVIC input is supposed to see the line go high
at "raise A" and only drop at the last "drop B". Without
the OR gate, it will see it go high at "raise A", and then
drop at "drop B". (Well, it sees "level is 1", "level is 1",
"level is 0", "level is 0", but inputs expect to sometimes see
calls for "level happens to be the same thing it was
previously", so it doesn't cause the NVIC to change state.)

Your test case doesn't as far as I can see check this situation,
because it's (a) testing every input in order, not checking
what happens when multiple inputs are raised and lowered
in overlapping ways and (b) using rising-edge interrupts.
So that's why it doesn't fail even without the bug fix in
the previous patch.

>  #define EXTI0_IRQ 6
>  #define EXTI1_IRQ 7
> +#define EXTI5_9_IRQ 23
>  #define EXTI35_IRQ 1
>
>  static void enable_nvic_irq(unsigned int n)
> @@ -499,6 +500,40 @@ static void test_interrupt(void)
>      g_assert_false(check_nvic_pending(EXTI1_IRQ));
>  }
>
> +static void test_orred_interrupts(void)
> +{
> +    /*
> +     * For lines EXTI5..9 (fanned-in to NVIC irq 23),
> +     * test that raising the line pends interrupt
> +     * 23 in NVIC.
> +     */
> +    enable_nvic_irq(EXTI5_9_IRQ);
> +    /* Check that there are no interrupts already pending in PR */
> +    g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
> +    /* Check that this specific interrupt isn't pending in NVIC */
> +    g_assert_false(check_nvic_pending(EXTI5_9_IRQ));
> +
> +    /* Enable interrupt lines EXTI[5..9] */
> +    exti_writel(EXTI_IMR1, (0x1F << 5));
> +
> +    /* Configure interrupt on rising edge */
> +    exti_writel(EXTI_RTSR1, (0x1F << 5));
> +
> +    /* Raise GPIO line i, check that the interrupt is pending */
> +    for (unsigned i = 5; i < 10; i++) {
> +        exti_set_irq(i, 1);
> +        g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 1 << i);
> +        g_assert_true(check_nvic_pending(EXTI5_9_IRQ));
> +
> +        exti_writel(EXTI_PR1, 1 << i);
> +        g_assert_cmpuint(exti_readl(EXTI_PR1), ==, 0x00000000);
> +        g_assert_true(check_nvic_pending(EXTI5_9_IRQ));
> +
> +        unpend_nvic_irq(EXTI5_9_IRQ);
> +        g_assert_false(check_nvic_pending(EXTI5_9_IRQ));
> +    }
> +}
> +
>  int main(int argc, char **argv)
>  {
>      int ret;
> @@ -515,6 +550,8 @@ int main(int argc, char **argv)
>      qtest_add_func("stm32l4x5/exti/masked_interrupt", test_masked_interrupt);
>      qtest_add_func("stm32l4x5/exti/interrupt", test_interrupt);
>      qtest_add_func("stm32l4x5/exti/test_edge_selector", test_edge_selector);
> +    qtest_add_func("stm32l4x5/exti/test_orred_interrupts",
> +                   test_orred_interrupts);
>
>      qtest_start("-machine b-l475e-iot01a");
>      ret = g_test_run();
> --
> 2.43.2

thanks
-- PMM

On Thu, 22 Feb 2024 at 15:09, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 20 Feb 2024 at 18:41, Inès Varhol <ines.varhol@telecom-paris.fr> wrote:
> >
> > This commit adds a QTest that verifies each input line of a specific
> > EXTI OR gate can influence the output line.
> >
> > Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >
> > Hello,
> >
> > I expected this test to fail after switching the two patch commits,
> > but it didn't.
> > I'm mentionning it in case it reveals a problem with the test I didn't notice.
>
> The specific thing that goes wrong if you don't have the OR
> gate handling is that the NVIC input will see every up
> and down transition from each input separately. (This happens
> because a GPIO/irq 'input' in QEMU is basically a function,
> and wiring up an 'output' to an 'input' is setting "this
> is the function pointer you should call when the output
> changes". Nothing syntactically stops you passing the
> same function pointer to multiple outputs.)
>
> So if you have for instance
>  raise A; raise B; drop B; drop A
> where A and B are ORed together into an NVIC input,
> the NVIC input is supposed to see the line go high
> at "raise A" and only drop at the last "drop B". Without

...at the last "drop *A*", I mean.

> the OR gate, it will see it go high at "raise A", and then
> drop at "drop B". (Well, it sees "level is 1", "level is 1",
> "level is 0", "level is 0", but inputs expect to sometimes see
> calls for "level happens to be the same thing it was
> previously", so it doesn't cause the NVIC to change state.)

-- PMM