On 01.12.23 14:38, Ronald Wahl wrote:
On 01.12.23 12:13, David Laight wrote:
[You don't often get email from david.laight@xxxxxxxxxx. Learn why
this is important at https://aka.ms/LearnAboutSenderIdentification ]
...
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.
Can't really follow. Each data bit requires one clock on single wire
SPI. Adressing and the like may require a bit of overhead but that
should not be that much (12.5%?).
The spi devices usually support 'nibble mode' for read/write that
will speed things up - provided the data lines are connected.
We use SPI (single wire) in our devices with ~40MHz (older hat even just
12MHz) and we never had issues with transfers lasting to long. This is
because file systems transfer much smaller blocks in the multi kB range.
The speed is ~4-5 MB/s with ~40MHz - with 20MHz it would be half.
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.
Exactly. This is what file systems usually do.
Anyway, at the moment the code breaks at least two file systems
(JFFS2, UBIFS) and probably more. So I request the following:
Revert e0205d6203c2 "spi: atmel: Prevent false timeouts on long transfers"
and the two new patches that are already added to one or more devel trees
[1/2] spi: atmel: Do not cancel a transfer upon any signal
commit: 595d2639451d3490c545c644ece726a0410ad39b
[2/2] spi: atmel: Drop unused defines
commit: 28d8051efae17b6d83544f3c1cf06f6a71677e91
Any potential improvements can then be done without hurry.
- ron
________________________________
Ce message, ainsi que tous les fichiers joints à ce message, peuvent contenir des informations sensibles et/ ou confidentielles ne devant pas être divulguées. Si vous n'êtes pas le destinataire de ce message (ou que vous recevez ce message par erreur), nous vous remercions de le notifier immédiatement à son expéditeur, et de détruire ce message. Toute copie, divulgation, modification, utilisation ou diffusion, non autorisée, directe ou indirecte, de tout ou partie de ce message, est strictement interdite.
This e-mail, and any document attached hereby, may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorized, direct or indirect, copying, disclosure, distribution or other use of the material or parts thereof is strictly forbidden.
