Skip to content

Commit

Permalink
tests/periph/selftest_shield: improve SPI test
Browse files Browse the repository at this point in the history
- fix a copy-paste error (`TIMER_FREQ_UART_TEST` was used in the SPI
  test, but that should be `TIMER_FREQ_SPI_TEST`)
- use 400 kHz as slow SPI frequency, as faster STM32 MCUs just cannot
  divide the APB clock down to 100 kHz
- when detailed output is enabled, print the SPI clock in addition to
  the SPI mode to ease figuring out what went wrong
- only have one `FAILURE` message for a too fast byte transfer per
  check, rather than per transmitted byte, to reduce the noise
- work around a bug of `periph_timer` on STM32 by reducing the clock
  speed of the timer for the SPI test
  • Loading branch information
maribu committed Nov 15, 2023
1 parent b4dc8e3 commit 134582d
Showing 1 changed file with 27 additions and 11 deletions.
38 changes: 27 additions & 11 deletions tests/periph/selftest_shield/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* @}
*/

#include <inttypes.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdatomic.h>
Expand Down Expand Up @@ -137,7 +138,7 @@
* directly, so the CPU clock is the highest clock frequency available. But
* some can't, so we handle them here explicitly. */
#ifndef TIMER_FREQ_SPI_TEST
# if defined(CPU_SAM3)
# if defined(CPU_SAM3) || defined(CPU_STM32)
# define TIMER_FREQ_SPI_TEST CLOCK_CORECLOCK / 4
# elif defined(CPU_NRF52) || defined(CPU_NRF51)
# define TIMER_FREQ_SPI_TEST MHZ(16)
Expand Down Expand Up @@ -784,13 +785,14 @@ static bool periph_spi_rxtx_test(spi_t bus, spi_mode_t mode, spi_clk_t clk,
const char *test_in_detail)
{
(void)test_in_detail;
bool failed = 0;
bool failed = false;
bool transfer_too_fast = false;
print_start("SPI", test_in_detail);
uint16_t byte_transfer_ticks = 0;
memset(&serial_buf, 0, sizeof(serial_buf));

if (IS_USED(MODULE_PERIPH_TIMER)) {
byte_transfer_ticks = 8ULL * TIMER_FREQ_UART_TEST / clk_hz;
byte_transfer_ticks = 8ULL * TIMER_FREQ_SPI_TEST / clk_hz;
}

/* D10 is C̅S̅, D7 is connected to C̅S̅ */
Expand All @@ -808,6 +810,7 @@ static bool periph_spi_rxtx_test(spi_t bus, spi_mode_t mode, spi_clk_t clk,
/* C̅S̅ should still be HIGH while no chip is selected */
failed |= TEST(gpio_read(cs_check) != 0);

uint16_t byte_time;
for (uint8_t i = 0; i < UINT8_MAX; i++) {
uint16_t start = 0;
if (IS_USED(MODULE_PERIPH_TIMER)) {
Expand All @@ -819,15 +822,25 @@ static bool periph_spi_rxtx_test(spi_t bus, spi_mode_t mode, spi_clk_t clk,
stop = timer_read(TIMER);
}
failed |= TEST(received == i);
uint16_t byte_time = (uint16_t)(stop - start);
/* We allow the actual SPI clock to be slower than requested, but not
* faster. So the transfer needs to take *at least* the theoretical
* time. Given the overhead of, this already has some room for error */
failed |= TEST(byte_time >= byte_transfer_ticks);
/* C̅S̅ should be still LOW while chip is selected */
if (IS_USED(MODULE_PERIPH_TIMER)) {
byte_time = (uint16_t)(stop - start);
/* We allow the actual SPI clock to be slower than requested, but
* not faster. So the transfer needs to take *at least* the
* theoretical time. Given the overhead of, this already has some
* room for error */
transfer_too_fast |= (byte_time < byte_transfer_ticks);
/* C̅S̅ should be still LOW while chip is selected */
}
failed |= TEST(gpio_read(cs_check) == 0);
}

if (DETAILED_OUTPUT && transfer_too_fast) {
printf("Ticks expected to transfer byte: >= %" PRIu16 ", but got: %"
PRIu16 "\n",
byte_transfer_ticks, byte_time);
}

failed |= TEST(!transfer_too_fast);
failed |= TEST(spi_transfer_byte(bus, cs, false, UINT8_MAX) == UINT8_MAX);
/* C̅S̅ should be again HIGH while now that no chip is selected */
failed |= TEST(gpio_read(cs_check) != 0);
Expand Down Expand Up @@ -871,8 +884,8 @@ static bool periph_spi_test(void)
}

bool failed = false;
static const spi_clk_t clocks[] = { SPI_CLK_100KHZ, SPI_CLK_1MHZ, SPI_CLK_10MHZ };
static const uint32_t clk_hzs[] = { KHZ(100), MHZ(1), MHZ(10) };
static const spi_clk_t clocks[] = { SPI_CLK_400KHZ, SPI_CLK_1MHZ, SPI_CLK_10MHZ };
static const uint32_t clk_hzs[] = { KHZ(400), MHZ(1), MHZ(10) };

if (IS_USED(MODULE_PCF857X)) {
for (int i = 0; i < (int)ARRAY_SIZE(spi_clk_check_pins); i++) {
Expand All @@ -888,6 +901,9 @@ static bool periph_spi_test(void)
for (unsigned j = 0; j < ARRAY_SIZE(clocks); j++) {
spi_clk_t clk = clocks[j];
uint32_t clk_hz = clk_hzs[j];
if (DETAILED_OUTPUT) {
printf("SPI CLK %" PRIu32 " Hz\n", clk_hz);
}
failed |= periph_spi_rxtx_test(bus, SPI_MODE_0, clk, clk_hz, clk_check, false, "mode 0");

Check warning on line 907 in tests/periph/selftest_shield/main.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
failed |= periph_spi_rxtx_test(bus, SPI_MODE_1, clk, clk_hz, clk_check, false, "mode 1");

Check warning on line 908 in tests/periph/selftest_shield/main.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
failed |= periph_spi_rxtx_test(bus, SPI_MODE_2, clk, clk_hz, clk_check, true, "mode 2");
Expand Down

0 comments on commit 134582d

Please sign in to comment.