[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 3/3] tests/microbit-test: Check nRF51 UART fu
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH v4 3/3] tests/microbit-test: Check nRF51 UART functionality |
Date: |
Mon, 21 Jan 2019 15:35:16 +0000 |
User-agent: |
mu4e 1.1.0; emacs 26.1.91 |
Thomas Huth <address@hidden> writes:
> On 2019-01-17 17:16, Julia Suvorova via Qemu-devel wrote:
>> Some functional tests for:
>> Basic reception/transmittion
>> Suspending
>> INTEN* registers
>>
>> Signed-off-by: Julia Suvorova <address@hidden>
>> ---
>> tests/microbit-test.c | 84 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 84 insertions(+)
>>
>> diff --git a/tests/microbit-test.c b/tests/microbit-test.c
>> index afeb6b082a..3da6d9529f 100644
>> --- a/tests/microbit-test.c
>> +++ b/tests/microbit-test.c
>> @@ -19,10 +19,93 @@
>> #include "libqtest.h"
>>
>> #include "hw/arm/nrf51.h"
>> +#include "hw/char/nrf51_uart.h"
>> #include "hw/gpio/nrf51_gpio.h"
>> #include "hw/timer/nrf51_timer.h"
>> #include "hw/i2c/microbit_i2c.h"
>>
>> +#include <sys/socket.h>
>> +#include <sys/un.h>
>> +
>> +static bool uart_wait_for_event(QTestState *qts, uint32_t event_addr)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < 1000; i++) {
>> + if (qtest_readl(qts, event_addr) == 1) {
>> + qtest_writel(qts, event_addr, 0x00);
>> + return true;
>> + }
>> + g_usleep(10000);
>> + }
>
> So this times out after 10 seconds? ... this is likely plenty on a
> normal host, but we run the tests on overloaded CI systems sometimes,
> where 10 seconds are not that much...
>
> I'd suggest to replace the condition in the for-loop with "i < 30000" or
> even "i < 60000", just to be sure.
Or use a g_timer and look at elapsed time rather than having open coded loops?
>
>> + return false;
>> +}
>> +
>> +static void uart_rw_to_rxd(QTestState *qts, int sock_fd, const char *in,
>> + char *out)
>> +{
>> + int i, in_len = strlen(in);
>> +
>> + g_assert(write(sock_fd, in, in_len) == in_len);
>
> Sorry for being pedantic, but I'd really prefer to write it without
> side-effects in g_assert() please. (same for the other occurrences in
> this patch)
+1
--
Alex Bennée