Guennadi Liakhovetski lg@denx.de wrote:
On Thu, 8 May 2008, Haavard Skinnemoen wrote:
But I have to say I like the idea about passing a spi_slave "handle" around...
How about something like this?
/*
- Set up communication parameters for a SPI slave. This doesn't
- necessarily enable the controller.
*/ struct spi_slave *spi_create_slave(int bus, int cs, unsigned int max_hz, unsigned int mode);
Don't quite like the "create" name. To me it sounds like "we just create a slave object for you, do what you want with it." Whereas, I was thinking about an "attach" method, where the host driver adds the slave pointer to its driver list, possibly initializes the hardware, and can always verify whether the slave pointer it is handed in by one of further method is indeed one of the drivers on the list.
Ok. spi_setup_slave()? spi_init_slave()? I'm not a huge fan of "attach" since it implies that some global state changes. It should be perfectly acceptable for a driver to simply initialize a spi_slave struct and return it, without updating any internal lists or hardware state.
/*
- Get exclusive access to the SPI bus for communicating with the given
- slave. Returns a negative value if the bus is busy (drivers may omit
- such checks if they don't want the extra code/data overhead.)
*/ int spi_claim_bus(struct spi_slave *slave);
Is this needed? Getting the spi_slave handle you gain exclusive access to the spi device, but claiming the whole bus? Drivers may be lazy releasing the chip-select between transfers, but are there cases where you _really_ must have exclusive access to the bus or cannot release the cs?
For some controllers, I believe claiming/releasing the bus explicitly makes a lot of sense. For example, the SPI controller on AT91 and AVR32 allows you to select a given slave by writing to a register, and then start transferring data, toggling chip selects as needed.
If a driver is sloppy with the chip select control, it's buggy. Many SPI devices have very strict (and often strange) requirements about when the chip select should be asserted and when it should not.
I think a claim/release interface is the best way to ensure that a device driver can do multiple transfers more or less atomically (i.e. without releasing the chip select and without having any traffic on the bus in between) in a flexible way.
/*
- Release the SPI bus. This may disable the controller and/or put it
- in low-power mode
*/ void spi_release_bus(struct spi_slave *slave);
/*
- Transfer data on the SPI bus.
- flags can be a combination of any of the following bits:
- SPI_XFER_BEGIN: Assert CS before starting the transfer
- SPI_XFER_END: Deassert CS after the transfer
*/ int spi_xfer(struct spi_slave *slave, int bitlen, const void *dout, void *din, unsigned long flags);
See above - would anything break if we just deassert the cs between transfers?
Yes.
We've had so many chipselect-related problems in Linux that it's not even funny. Controllers, drivers, devices, everything seems to be making assumptions about how the chip select is supposed to behave -- I think we should at least try not to make any assumptions on the interface level.
Maybe we can do it in a different way, but simply saying that the chip select should be deasserted between transfers is broken.
Haavard