Re: [PATCH 5/6] spi: loopback-test: add elapsed time check

From: Mark Brown <broonie@kernel.org>
To: Tudor Ambarus <tudor.ambarus@linaro.org>
Cc: akinobu.mita@gmail.com,
	"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
	kernel@martin.sperl.org
Subject: Re: [PATCH 5/6] spi: loopback-test: add elapsed time check
Date: Fri, 12 Jan 2024 18:37:20 +0000	[thread overview]
Message-ID: <e3db65ab-be7e-45dc-9104-fe2a3b046b7d@sirena.org.uk> (raw)
In-Reply-To: <4f097d14-dc7f-453e-94f6-96ef568e7770@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]

On Fri, Jan 12, 2024 at 04:19:10PM +0000, Tudor Ambarus wrote:

> I'm playing with the spi-s3c64xx.c driver and the spi-loopback-test and
> sometimes I see that I get the following error:

> ```elapsed time x ns is shorter than minimum estimated time y ns```

> What's strange to me is that if I ignore the return value of
> spi_test_check_elapsed_time(), the test passes, there's no transfer
> mismatch and what we expect we actually get at RX.

> What kind of errors does the spi_test_check_elapsed_time() check want to
> discover? Under what conditions the test->elapsed_time is inaccurate?

The test is calculating the expected time based on the intended clock
rate for the bus and any delays in the message.  This should identify if
a driver is either ignoring specified delays or overclocking the bus.

> [ 6748.910773] spi-loopback-test spi13.0:     spi_transfer@00000000774992f1
> [ 6748.911035] spi-loopback-test spi13.0:       len:    1
> [ 6748.911225] spi-loopback-test spi13.0:       tx_buf: 00000000401e03ed
> [ 6748.911472]           TX: 00000000: 01
> [ 6748.911614] spi-loopback-test spi13.0:       rx_buf: 0000000010f7e3e4
> [ 6748.911860]           RX: 00000000: aa
> [ 6748.912004] spi-loopback-test spi13.0:       rx_buf filled with aa
> starts at offset: 0

> [ 6748.913400] spi-loopback-test spi13.0: elapsed time 532837 ns is
> shorter than minimum estimated time 82240000 ns

That's a *very* substantial error, it makes me suspect that the hardware
might be doing loopback at a stage before it actually clocks the bus and
doing something like just connecting the TX and RX FIFOs.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]