From: David Laight <David.Laight@ACULAB.COM>
To: 'Ronald Wahl' <ronald.wahl@raritan.com>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Richard Weinberger <richard@nod.at>
Cc: Mark Brown <broonie@kernel.org>,
linux-spi <linux-spi@vger.kernel.org>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
Ryan Wanner <ryan.wanner@microchip.com>,
stable <stable@vger.kernel.org>,
"Richard Weinberger" <richard.weinberger@gmail.com>,
David Woodhouse <dwmw2@infradead.org>
Subject: RE: [PATCH 1/2] spi: atmel: Do not cancel a transfer upon any signal
Date: Fri, 1 Dec 2023 11:13:06 +0000 [thread overview]
Message-ID: <4642281ef2e749a3b69bbea5474ecdf1@AcuMS.aculab.com> (raw)
In-Reply-To: <d4ffca97-bb5d-4c42-a025-69b308c24f82@raritan.com>
...
> Anyway, the whole issue started with commit e0205d6203c2 "spi: atmel:
> Prevent
> false timeouts on long transfers". Citing the commit message here:
> "spi: atmel: Prevent false timeouts on long transfers
>
> A slow SPI bus clocks at ~20MHz, which means it would transfer about
> 2500 bytes per second with a single data line. Big transfers, like when
> dealing with flashes can easily reach a few MiB. The current DMA
> timeout
> is set to 1 second, which means any working transfer of about 4MiB will
> always be cancelled.
>
> With the above derivations, on a slow bus, we can assume every byte
> will
> take at most 0.4ms. Said otherwise, we could add 4ms to the 1-second
> timeout delay every 10kiB. On a 4MiB transfer, it would bring the
> timeout delay up to 2.6s which still seems rather acceptable for a
> timeout.
>
> The consequence of this is that long transfers might be allowed, which
> hence requires the need to interrupt the transfer if wanted by the
> user. We can hence switch to the _interruptible variant of
> wait_for_completion. This leads to a little bit more handling to also
> handle the interrupted case but looks really acceptable overall.
>
> While at it, we drop the useless, noisy and redundant WARN_ON() call."
>
> This calculation is actually wrong by factor 1000. A 20MHz SPI bus is not
> really slow and it will transfer ~2.5MB/s over a single lane.
> The calculation would be right for 20kHz but I think this is a more
> esoteric case, isn't it?
Some of the sums are wrong, but the conclusion might be right.
A 4MB transfer at 20MHz only has 5 clocks/byte - seems low if it
is only using 1 data bit.
The spi devices usually support 'nibble mode' for read/write that
will speed things up - provided the data lines are connected.
OTOH a better fix is (probably) to do the transfer in sane sized chunks.
The extra interrupt and code won't make much difference to something
that takes that long.
As an aside, has anyone managed to get an spi memory device to calculate
a CRC? The ones we have claim to support the request but I've failed to
get it to work.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
next prev parent reply other threads:[~2023-12-01 11:14 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-27 9:58 [PATCH 1/2] spi: atmel: Do not cancel a transfer upon any signal Miquel Raynal
2023-11-27 9:58 ` [PATCH 2/2] spi: atmel: Drop unused defines Miquel Raynal
2023-11-27 15:10 ` [PATCH 1/2] spi: atmel: Do not cancel a transfer upon any signal Ronald Wahl
2023-11-27 17:54 ` Ronald Wahl
2023-11-29 8:49 ` Miquel Raynal
2023-11-29 11:05 ` Ronald Wahl
2023-11-30 12:46 ` Richard Weinberger
2023-11-30 18:26 ` Richard Weinberger
2023-11-30 18:36 ` Ronald Wahl
2023-11-30 20:15 ` Miquel Raynal
2023-11-30 20:43 ` Ronald Wahl
2023-11-30 20:58 ` Richard Weinberger
2023-12-01 11:13 ` David Laight [this message]
2023-12-01 13:38 ` Ronald Wahl
2023-12-04 11:54 ` Ronald Wahl
2023-12-04 12:26 ` Mark Brown
2023-12-04 12:39 ` Ronald Wahl
2023-12-04 12:19 ` David Laight
2023-12-05 7:49 ` Miquel Raynal
2023-11-27 16:48 ` Mark Brown
2023-12-01 14:16 ` Mark Brown
2023-12-04 15:47 ` Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4642281ef2e749a3b69bbea5474ecdf1@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=broonie@kernel.org \
--cc=dwmw2@infradead.org \
--cc=linux-spi@vger.kernel.org \
--cc=miquel.raynal@bootlin.com \
--cc=richard.weinberger@gmail.com \
--cc=richard@nod.at \
--cc=ronald.wahl@raritan.com \
--cc=ryan.wanner@microchip.com \
--cc=stable@vger.kernel.org \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).