-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Somebody in the thread at some point said:
| Note that $SUBJECT concept is nonsense.
| Synchronous calls are by definition blocking ones...
What's meant here is that it won't sleep you and make you wait for an
asynchronous completion. So the call itself will perform the bitbang
action before returning.
| On Saturday 28 February 2009, Balaji Rao wrote:
|> During the course of development of an accelerometer driver, we saw the
|> necessity to execute spi transfers synchronously within an interrupt
handler.
|
| This sounds like a bad design. How can you know that no other
| transfers are going on ... or are queued in front of the transfer
| you're requesting?
|
| You'd need to wait for all the other transfers to work their
| way through the transfer queue. There are *much* better things
| to do in interrupt handlers.
In our case, the sync and async SPI things are mutually exclusive, you
either talk to your thing with interrupt-lockout protected sync
transfers entirely or use the existing API. It can be enforced if
that's the concern.
I think it's a little fast to call down the airstrike of "bad design" on
being able to use bitbang SPI inside an ISR. Clearly, adding this
ability does not replace the existing system and exists parallel but
compatibly with it; but the existing system cannot provide the same
capability as it stands. With two lis302dl motion sensors in GTA02,
they can spam up to 800 interrupts a second that need servicing each
with a 7-byte bitbang read. The existing SPI setup in Linux does not
provide a way to deal with that in a CPU-friendly and low latency way
(there's no FIFO in these devices either).
|> When using a workqueue instead, we observed a huge number of overruns
|> with very high cpu utlization, which is unacceptable.
|
| Sure, but at least part of that seems to be caused by some
| broken design assumptions.
|
| Why are you even trying to touch SPI devices from hardirq
| context? That's never going to be OK; "can't-sleep" contexts
| don't mix with "must-sleep" calls.
With the "sync" API addition it is OK, given the mutually exclusive
aspect of it.
|> This series adds a new interface for this and modifies no existing ones.
|
| NAK on these two patches.
I hope this maybe gives some extra context about why we use this system,
and why it can be an interesting option for others in the same situation.
- -Andy
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iEYEARECAAYFAkmqPdYACgkQOjLpvpq7dMop/ACfUGHL+BilbzlN4E7rACmlC48N
uv0AnjqgzshtVeYdIVz/14OGq4krCn7+
=Qveo
-----END PGP SIGNATURE-----
On Saturday 28 February 2009, Andy Green wrote:
> | This sounds like a bad design. ?How can you know that no other
> | transfers are going on ... or are queued in front of the transfer
> | you're requesting?
> |
> | You'd need to wait for all the other transfers to work their
> | way through the transfer queue. ?There are *much* better things
> | to do in interrupt handlers.
>
> In our case, the sync and async SPI things are mutually exclusive, you
Come up with some new terminology for what you're proposing,
instead of trying to repurpose the existing terms. The
spi_sync() and spi_async() calls already exist, and they
are *NOT* mutually exclusive.
I suggest you talk about "non-sleeping" calls, since that
seems to be close to what you're thinking about. (Though
it does beg the question of why you're not using the async
calls, if sleeping makes trouble...)
> either talk to your thing with interrupt-lockout protected sync
> transfers entirely or use the existing API. ?It can be enforced if
> that's the concern.
The API proposal is a generic one, and the reference
implementation has deep issues as I noted. At the
level of data corruption. How would that be "enforced"
generally?
> I think it's a little fast to call down the airstrike of "bad design"
> on being able to use bitbang SPI inside an ISR.
The way to access SPI inside an ISR is spi_async().
No exceptions for bitbang are necessary.
Neither you nor Balaji have mentioned any issue that
prevents using that. Or, for that matter, that you
even tried using the interface as designed. You're
defending something that -- so far -- is not defensible;
it introduces data corruption. I'm not sure what else
to call it but "bad design". Regardless, the key point
is to call your attention to the fact that you're doing
something quite wrong. (The nonsensical $SUBJECT was
the first clue...)
> Clearly, adding this
> ability does not replace the existing system and exists parallel but
> compatibly with it;
No, it's not compatible. Start by answering the question
I asked above (quoted at the top of this email), and you'll
surely begin to understand.
> but the existing system cannot provide the same
> capability as it stands.
Erm, the existing system is clearly self-compatible.
I don't think that word means what you think it means. ;)
> With two lis302dl motion sensors in GTA02,
> they can spam up to 800 interrupts a second that need servicing each
> with a 7-byte bitbang read.
Hmm, I'm looking at a schematic with a lis302dl hooked up
using I2C ... at 400 KHz max. So those chips have useful
use cases that don't need much of a data rate at all.
Now, 800 IRQ/sec * 7 bytes * 8 bits/byte == 44800 bits/sec,
which is easy to achieve. At least, in drivers that are
written sensibly. (Even if that's 45 Kbit/sec *each*...)
> The existing SPI setup in Linux does not
> provide a way to deal with that in a CPU-friendly and low latency way
> (there's no FIFO in these devices either).
If you can't get 45 Kbit/second you're doing something
extremely odd. I've bitbanged SPI at over 2 MHz on
ARM chips less than half the speed of your OpenMoko
hardware.
So to recap: the $SUBJECT proposal has serious issues;
and simple math suggests that the performance issues you
are seeing are not fundamentally due to the code at or
below the layer being patched.
- Dave