| Message ID | 20240220184145.106107-3-ines.varhol@telecom-paris.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm: Fix STM32L4x5 EXTI to CPU irq fan-in connections | expand |
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
diff --git a/tests/qtest/stm32l4x5_exti-test.c b/tests/qtest/stm32l4x5_exti-test.c index c390077713..81830be8ae 100644 --- a/tests/qtest/stm32l4x5_exti-test.c +++ b/tests/qtest/stm32l4x5_exti-test.c @@ -31,6 +31,7 @@ #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();