From c4e121aeb754f1ac5f2bcc791aa4177dcfb3b401 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 18 Mar 2017 03:17:26 +0900 Subject: spi: loopback-test: correct mismatched test description and configuration The test "two tx-transfers - alter first" actually alters the second not the first transfer. Similarly the test "two tx-transfers - alter second" actually alters the first not the second transfer. The mismatches for the two symmetrical tests cancel each other's mistakes. But it's better to fix the mismatches to avoid confusion. Signed-off-by: Akinobu Mita Signed-off-by: Mark Brown --- drivers/spi/spi-loopback-test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/spi/spi-loopback-test.c') diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c index 50e620f4e8fe..99422f3a15e0 100644 --- a/drivers/spi/spi-loopback-test.c +++ b/drivers/spi/spi-loopback-test.c @@ -132,7 +132,7 @@ static struct spi_test spi_tests[] = { .fill_option = FILL_COUNT_8, .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, - .iterate_transfer_mask = BIT(1), + .iterate_transfer_mask = BIT(0), .transfers = { { .len = 1, @@ -149,7 +149,7 @@ static struct spi_test spi_tests[] = { .fill_option = FILL_COUNT_8, .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, - .iterate_transfer_mask = BIT(0), + .iterate_transfer_mask = BIT(1), .transfers = { { .len = 16, -- cgit v1.2.3 From 8494801db1fc21867f587dae465236100bf228cc Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 18 Mar 2017 03:17:27 +0900 Subject: spi: loopback-test: don't skip comparing the first byte of rx_buf When the loopback parameter is set, rx_buf are compared with tx_buf after the spi_message is executed. But the first byte of buffer is not checked. Signed-off-by: Akinobu Mita Signed-off-by: Mark Brown --- drivers/spi/spi-loopback-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/spi/spi-loopback-test.c') diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c index 99422f3a15e0..85c0c4e391d9 100644 --- a/drivers/spi/spi-loopback-test.c +++ b/drivers/spi/spi-loopback-test.c @@ -507,7 +507,7 @@ static int spi_test_check_loopback_result(struct spi_device *spi, continue; /* so depending on tx_buf we need to handle things */ if (xfer->tx_buf) { - for (i = 1; i < xfer->len; i++) { + for (i = 0; i < xfer->len; i++) { txb = ((u8 *)xfer->tx_buf)[i]; rxb = ((u8 *)xfer->rx_buf)[i]; if (txb != rxb) -- cgit v1.2.3 From 8916671e93a38c509e2ffb02c2ce3f37efe8af40 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 18 Mar 2017 03:17:28 +0900 Subject: spi: loopback-test: add ability to test zero-length transfer The spi-loopback-test module currently cannot test the spi_message including a zero-length transfer. Because the zero-length transfer is treated as a special value in several meanings. 1. The number of spi_transfer to execute in one test case is described by spi_test.transfer_count. It is normally computed by counting number of transfers with len > 0 in spi_test.transfers array. This change stops the detection for the number of spi_transfer. Each spi_test.transfer_count needs to be filled by hand now. 2. The spi_test.iterate_len is a list of transfer length to iterate on. This list is terminated by zero, so zero-length transfer cannot be included. This changes the terminal value from 0 to -1. 3. The length for the spi_transfer masked by spi_test.iterate_transfer_mask is iterated. Before starting the iteration, the default value which is statically initialized is applied. In order to specify the default value, zero-length is reserved. Currently, the default values are always '1'. So this removes this trick and add '1' to iterate_len list. By applying all these changes, the spi-loopback-test can execute spi messages with zero-length transfer. Signed-off-by: Akinobu Mita Signed-off-by: Mark Brown --- drivers/spi/spi-loopback-test.c | 55 +++++++++++++---------------------------- 1 file changed, 17 insertions(+), 38 deletions(-) (limited to 'drivers/spi/spi-loopback-test.c') diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c index 85c0c4e391d9..6df6dddc2890 100644 --- a/drivers/spi/spi-loopback-test.c +++ b/drivers/spi/spi-loopback-test.c @@ -63,9 +63,9 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_rx_align = ITERATE_ALIGN, + .transfer_count = 1, .transfers = { { - .len = 1, .tx_buf = TX(0), .rx_buf = RX(0), }, @@ -77,9 +77,9 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_rx_align = ITERATE_ALIGN, + .transfer_count = 1, .transfers = { { - .len = 1, .tx_buf = TX(PAGE_SIZE - 4), .rx_buf = RX(PAGE_SIZE - 4), }, @@ -90,9 +90,9 @@ static struct spi_test spi_tests[] = { .fill_option = FILL_COUNT_8, .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, + .transfer_count = 1, .transfers = { { - .len = 1, .tx_buf = TX(0), }, }, @@ -102,9 +102,9 @@ static struct spi_test spi_tests[] = { .fill_option = FILL_COUNT_8, .iterate_len = { ITERATE_MAX_LEN }, .iterate_rx_align = ITERATE_ALIGN, + .transfer_count = 1, .transfers = { { - .len = 1, .rx_buf = RX(0), }, }, @@ -115,13 +115,12 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_transfer_mask = BIT(0) | BIT(1), + .transfer_count = 2, .transfers = { { - .len = 1, .tx_buf = TX(0), }, { - .len = 1, /* this is why we cant use ITERATE_MAX_LEN */ .tx_buf = TX(SPI_TEST_MAX_SIZE_HALF), }, @@ -133,9 +132,9 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_transfer_mask = BIT(0), + .transfer_count = 2, .transfers = { { - .len = 1, .tx_buf = TX(64), }, { @@ -150,13 +149,13 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_transfer_mask = BIT(1), + .transfer_count = 2, .transfers = { { .len = 16, .tx_buf = TX(0), }, { - .len = 1, .tx_buf = TX(64), }, }, @@ -167,13 +166,12 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_transfer_mask = BIT(0) | BIT(1), + .transfer_count = 2, .transfers = { { - .len = 1, .tx_buf = TX(0), }, { - .len = 1, .rx_buf = RX(0), }, }, @@ -184,9 +182,9 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_transfer_mask = BIT(0), + .transfer_count = 2, .transfers = { { - .len = 1, .tx_buf = TX(0), }, { @@ -201,13 +199,13 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_transfer_mask = BIT(1), + .transfer_count = 2, .transfers = { { .len = 1, .tx_buf = TX(0), }, { - .len = 1, .rx_buf = RX(0), }, }, @@ -218,14 +216,13 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_transfer_mask = BIT(0) | BIT(1), + .transfer_count = 2, .transfers = { { - .len = 1, .tx_buf = TX(0), .rx_buf = RX(0), }, { - .len = 1, /* making sure we align without overwrite * the reason we can not use ITERATE_MAX_LEN */ @@ -240,9 +237,9 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_transfer_mask = BIT(0), + .transfer_count = 2, .transfers = { { - .len = 1, /* making sure we align without overwrite */ .tx_buf = TX(1024), .rx_buf = RX(1024), @@ -261,6 +258,7 @@ static struct spi_test spi_tests[] = { .iterate_len = { ITERATE_MAX_LEN }, .iterate_tx_align = ITERATE_ALIGN, .iterate_transfer_mask = BIT(1), + .transfer_count = 2, .transfers = { { .len = 1, @@ -268,7 +266,6 @@ static struct spi_test spi_tests[] = { .rx_buf = RX(0), }, { - .len = 1, /* making sure we align without overwrite */ .tx_buf = TX(1024), .rx_buf = RX(1024), @@ -503,7 +500,7 @@ static int spi_test_check_loopback_result(struct spi_device *spi, /* if applicable to transfer check that rx_buf is equal to tx_buf */ list_for_each_entry(xfer, &msg->transfers, transfer_list) { /* if there is no rx, then no check is needed */ - if (!xfer->rx_buf) + if (!xfer->len || !xfer->rx_buf) continue; /* so depending on tx_buf we need to handle things */ if (xfer->tx_buf) { @@ -745,15 +742,6 @@ static int spi_test_run_iter(struct spi_device *spi, /* copy the test template to test */ memcpy(&test, testtemplate, sizeof(test)); - /* set up test->transfers to the correct count */ - if (!test.transfer_count) { - for (i = 0; - (i < SPI_TEST_MAX_TRANSFERS) && test.transfers[i].len; - i++) { - test.transfer_count++; - } - } - /* if iterate_transfer_mask is not set, * then set it to first transfer only */ @@ -799,8 +787,7 @@ static int spi_test_run_iter(struct spi_device *spi, /* only when bit in transfer mask is set */ if (!(test.iterate_transfer_mask & BIT(i))) continue; - if (len) - test.transfers[i].len = len; + test.transfers[i].len = len; if (test.transfers[i].tx_buf) test.transfers[i].tx_buf += tx_off; if (test.transfers[i].tx_buf) @@ -910,15 +897,6 @@ int spi_test_run_test(struct spi_device *spi, const struct spi_test *test, /* iterate over all the iterable values using macros * (to make it a bit more readable... */ -#define FOR_EACH_ITERATE(var, defaultvalue) \ - for (idx_##var = -1, var = defaultvalue; \ - ((idx_##var < 0) || \ - ( \ - (idx_##var < SPI_TEST_MAX_ITERATE) && \ - (var = test->iterate_##var[idx_##var]) \ - ) \ - ); \ - idx_##var++) #define FOR_EACH_ALIGNMENT(var) \ for (var = 0; \ var < (test->iterate_##var ? \ @@ -928,7 +906,8 @@ int spi_test_run_test(struct spi_device *spi, const struct spi_test *test, 1); \ var++) - FOR_EACH_ITERATE(len, 0) { + for (idx_len = 0; idx_len < SPI_TEST_MAX_ITERATE && + (len = test->iterate_len[idx_len]) != -1; idx_len++) { FOR_EACH_ALIGNMENT(tx_align) { FOR_EACH_ALIGNMENT(rx_align) { /* and run the iteration */ -- cgit v1.2.3 From ea9936f32435699907807aaac87f993482208578 Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 18 Mar 2017 03:17:30 +0900 Subject: spi: loopback-test: add elapsed time check This adds checks whether the elapsed time is longer than the minimam estimated time. The estimated time is calculated with the total transfer length per clock rate and optional spi_transfer.delay_usecs. Signed-off-by: Akinobu Mita Signed-off-by: Mark Brown --- drivers/spi/spi-loopback-test.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) (limited to 'drivers/spi/spi-loopback-test.c') diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c index 6df6dddc2890..66e8cfd04395 100644 --- a/drivers/spi/spi-loopback-test.c +++ b/drivers/spi/spi-loopback-test.c @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -479,6 +480,35 @@ static int spi_check_rx_ranges(struct spi_device *spi, return ret; } +static int spi_test_check_elapsed_time(struct spi_device *spi, + struct spi_test *test) +{ + int i; + unsigned long long estimated_time = 0; + unsigned long long delay_usecs = 0; + + for (i = 0; i < test->transfer_count; i++) { + struct spi_transfer *xfer = test->transfers + i; + unsigned long long nbits = BITS_PER_BYTE * xfer->len; + + delay_usecs += xfer->delay_usecs; + if (!xfer->speed_hz) + continue; + estimated_time += div_u64(nbits * NSEC_PER_SEC, xfer->speed_hz); + } + + estimated_time += delay_usecs * NSEC_PER_USEC; + if (test->elapsed_time < estimated_time) { + dev_err(&spi->dev, + "elapsed time %lld ns is shorter than minimam estimated time %lld ns\n", + test->elapsed_time, estimated_time); + + return -EINVAL; + } + + return 0; +} + static int spi_test_check_loopback_result(struct spi_device *spi, struct spi_message *msg, void *tx, void *rx) @@ -817,12 +847,16 @@ int spi_test_execute_msg(struct spi_device *spi, struct spi_test *test, /* only if we do not simulate */ if (!simulate_only) { + ktime_t start; + /* dump the complete message before and after the transfer */ if (dump_messages == 3) spi_test_dump_message(spi, msg, true); + start = ktime_get(); /* run spi message */ ret = spi_sync(spi, msg); + test->elapsed_time = ktime_to_ns(ktime_sub(ktime_get(), start)); if (ret == -ETIMEDOUT) { dev_info(&spi->dev, "spi-message timed out - reruning...\n"); @@ -848,6 +882,10 @@ int spi_test_execute_msg(struct spi_device *spi, struct spi_test *test, /* run rx-buffer tests */ ret = spi_test_check_loopback_result(spi, msg, tx, rx); + if (ret) + goto exit; + + ret = spi_test_check_elapsed_time(spi, test); } /* if requested or on error dump message (including data) */ -- cgit v1.2.3 From 8687113e1515f4c9104a6eaedc384ec762c6550f Mon Sep 17 00:00:00 2001 From: Akinobu Mita Date: Sat, 18 Mar 2017 03:17:31 +0900 Subject: spi: loopback-test: add test spi_message with delay after transfers This adds a new test to check whether the spi_transfer.delay_usecs setting has properly taken effect. Signed-off-by: Akinobu Mita Signed-off-by: Mark Brown --- drivers/spi/spi-loopback-test.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'drivers/spi/spi-loopback-test.c') diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c index 66e8cfd04395..fcae3377ec26 100644 --- a/drivers/spi/spi-loopback-test.c +++ b/drivers/spi/spi-loopback-test.c @@ -273,6 +273,25 @@ static struct spi_test spi_tests[] = { }, }, }, + { + .description = "two tx+rx transfers - delay after transfer", + .fill_option = FILL_COUNT_8, + .iterate_len = { ITERATE_MAX_LEN }, + .iterate_transfer_mask = BIT(0) | BIT(1), + .transfer_count = 2, + .transfers = { + { + .tx_buf = TX(0), + .rx_buf = RX(0), + .delay_usecs = 1000, + }, + { + .tx_buf = TX(0), + .rx_buf = RX(0), + .delay_usecs = 1000, + }, + }, + }, { /* end of tests sequence */ } }; -- cgit v1.2.3 From d2c14c64d678713fced6f2261ce7d398b4351de5 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 20 Mar 2017 13:58:05 +0000 Subject: spi: loopback-test: fix potential integer overflow on multiple A multiplication of 8U * xfer-len with the type of a 32 bit unsigned int is evaluated using 32 bit arithmetic and then used in a context that expects an expression of type unsigned long long (64 bits). Avoid any potential overflow by casting BITS_PER_BYTE to unsigned long long. Detected by CoverityScan, CID#1419691 ("Unintentional integer overflow") Fixes: ea9936f324356 ("spi: loopback-test: add elapsed time check") Signed-off-by: Colin Ian King Signed-off-by: Mark Brown --- drivers/spi/spi-loopback-test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/spi/spi-loopback-test.c') diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c index fcae3377ec26..6888d5c34ac4 100644 --- a/drivers/spi/spi-loopback-test.c +++ b/drivers/spi/spi-loopback-test.c @@ -508,7 +508,8 @@ static int spi_test_check_elapsed_time(struct spi_device *spi, for (i = 0; i < test->transfer_count; i++) { struct spi_transfer *xfer = test->transfers + i; - unsigned long long nbits = BITS_PER_BYTE * xfer->len; + unsigned long long nbits = (unsigned long long)BITS_PER_BYTE * + xfer->len; delay_usecs += xfer->delay_usecs; if (!xfer->speed_hz) -- cgit v1.2.3 From 905e0b5ef9690c5433b642f46f1d756bb374da17 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Thu, 30 Mar 2017 11:01:25 +0100 Subject: spi: loopback-test: fix spelling mistake: "minimam" -> "minimum" trivial fix to spelling mistake in dev_err error message Signed-off-by: Colin Ian King Signed-off-by: Mark Brown --- drivers/spi/spi-loopback-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/spi/spi-loopback-test.c') diff --git a/drivers/spi/spi-loopback-test.c b/drivers/spi/spi-loopback-test.c index 6888d5c34ac4..e6041539afed 100644 --- a/drivers/spi/spi-loopback-test.c +++ b/drivers/spi/spi-loopback-test.c @@ -520,7 +520,7 @@ static int spi_test_check_elapsed_time(struct spi_device *spi, estimated_time += delay_usecs * NSEC_PER_USEC; if (test->elapsed_time < estimated_time) { dev_err(&spi->dev, - "elapsed time %lld ns is shorter than minimam estimated time %lld ns\n", + "elapsed time %lld ns is shorter than minimum estimated time %lld ns\n", test->elapsed_time, estimated_time); return -EINVAL; -- cgit v1.2.3