SPI, _sz fields in struct spi_command

[Bernd Walter]

On Thu, Feb 21, 2013 at 02:26:55AM +0200, Aleksandr Rybalko wrote:
> On Wed, 20 Feb 2013 13:57:52 -0500
> Patrick Kelsey <kelsey at ieee.org> wrote:
> 
> > On Wed, Feb 20, 2013 at 12:44 PM, Bernd Walter
> > <ticso at cicely7.cicely.de> wrote:
> > > On Wed, Feb 20, 2013 at 10:40:35AM -0500, Patrick Kelsey wrote:
> > 
> > >> In (1) and (3) above, the drivers for those chips could usefully
> > >> tell the SPI bus interface that they aren't interested in received
> > >> data when transmitting to the SPI-attached devices, and certainly
> > >> it would be at least a small convenience to such SPI interface
> > >> consumers to have the lower level driver handle it.  Since not all
> > >> host-side SPI interface hardware supports discarding received
> > >> data, to allow a zero-sized receive buffer in the command passed
> > >> down, at least some SPI interface hardware drivers will have to
> > >> dynamically allocate or maintain a static discard buffer and
> > >> provide error responses for allocation-failure or
> > >> transfer-too-large.
> > >
> > > If it is SPI hardware drivers decision you at least nee to take into
> > > account that in some RX-only cases you need 0xff dummy data.
> > > The slave chip driver knows if the device parses received data, but
> > > the master chip driver won't.
> > >
> > >> Example (2) is equally easy to handle on either side of the SPI bus
> > >> interface by pointing the transmit buffer pointer at the receive
> > >> buffer.
> > >
> > > This destroys the send data, which is not always welcome behavour.
> > >
> > 
> > My comments are in the overall context of extending the functionality
> > of the existing SPI bus interface in ways that provide programming
> > conveniences to those who wish to use them.  I am not suggesting in
> > any way that the current interface be narrowed based on any set of
> > assumptions, so in my view, there will always be a facility for fully
> > independent transmit and receive buffers specified by the caller.
> > 
> > My example (2) was for a slave device that does not have a data input
> > pin and thus is oblivious to any data sent.  Certainly, if you have a
> > device that can receive data sent by the master and your code design
> > calls for preserving the send data across bus transactions, you would
> > continue to use two separate buffers.
> > 
> > -Patrick
> 
> Thanks to all for interesting comments, some of my keys for that topic
> you all already said but I say it again anyway :)
> So why I ask about that:
> 1. Do we really need to allocate second unused buffer?

For AT91 yes, unless:
- RX-only and you know that sending the RX buffers undefined content is Ok,
  which is not if the device needs 0xff
- TX-only and you know that trashing the TX buffer is Ok

> 2. Do we really need to spend clock cycles on that buffer?

For AT91 it is required, others maybe not.

> 3. Ralink-s SPI can't do bidirectional transfer (IIRC I've tested it
> properly, only 1 8bits reg for both RX and TX and same data on read
> after TX byte) (Sorry for not commited yet, will do, hope soon :) )
> 4. Since we have sz for both direction and both types (cmd, data) why
> controller drivers force us to make it equal? (at91_spi_transfer
> KASSERT)

They can't otherwise.

> What I think we need to do with that:
> 1. remove KASSERT in controller drivers, make driver to accept at least
> cases when (tx_sz == rx_sz), (tx_sz && !rx_sz), (!tx_sz && rx_sz). Plus
> if controller require some buffer to drain into it (don't know how to
> correctly say in English what men do after many beers :) ) allocate it
> or make some static buffer.

Sounds good.
Static buffers however may require multiple DMA setup if the transfer
request ist bigger.
With AT91 this can be done via *empty interrupt to replay the same buffer.
It is just not wise to interrupt on each single byte, which would be required
without a DMA buffer at all.
Sending 0xff as default dummy data requires filling the TX buffer with
this value, so maybe if doing static then have 2 buffers, one with 0xff
dummy data and one bit-bucket buffer to RX into.
Otherwise use the same dummy data and if a driver requires sending dummy
0xff it needs to supply its own buffer, but there are many devices requiring
this.

> 2. teach consumers to give only correct numbers (very nice we have only
> two SPI devices in tree)
> 
> After that we will be able to make drivers for some (potential) devices
> which will require bidirectional communication. And controllers which
> can't do that, will just report error in that. I believe peoples thinks
> before attach such devices to controllers, so we will not have such
> incompatibility.

I don't think there are many devices requiring RX/TX at the same time.
And if a controller won't support it there is nothing to be done anyway
beside bit bang-ing.
But I usually do RX/TX at the same transfer with devices wanting a comand
word and then sending data at the next data word.
In many cases you must not deassert CS during this.
I don't know how CS is handled in our SPI as I never did SPI with FreeBSD.
If CS is handled automatically on each transaction then it must support
bidirectional transactions.
If CS however are done with separate calls it is Ok to do a TX and then
an TX call before deasserting.
The AT91 hardware has automatic CS support - in my own controller code
outside of FreeBSD I usually prefer handling CS with GPIO for that reason.

> Hope I'm not forget something important :-D
> 
> Thanks to all!
> 
> WBW
> -- 
> Aleksandr Rybalko <ray at freebsd.org>

-- 
B.Walter <bernd at bwct.de> http://www.bwct.de
Modbus/TCP Ethernet I/O Baugruppen, ARM basierte FreeBSD Rechner uvm.