Guennadi Liakhovetski lg@denx.de wrote:
On Thu, 8 May 2008, Haavard Skinnemoen wrote:
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.
Is it compulsory then to claim / release a bus or not? Looks like it is not from your description above. But - even if you don't claim a bus for exclusive access, you still have to release it, right? Otherwise, if someone has setup a slave without claiming the bus, noone will ever be able to claim it afterwards? Because you shouldn't be able to claim it as long as there are even non-exclusive users on the bus? Actually, what's the mode variable in the setup method for? Maybe we could just use it to configure exclusive / non-exclusive access? Then you get a simple API like
I'd say we should either make it compulsory or forget about it. You always have exclusive access to the bus while doing a transfer, so distinguishing between "exclusive" and "non-exclusive" users just complicates things.
So we should just define spi_claim_bus() as a way of letting the controller driver know that "I'm going to start talking to this particular slave now", and let it handle it any way it chooses. We could also use the interface to add debugging checks to verify that we're not trying to access a different slaves while some driver thinks it has exclusive access to the bus.
The "mode" variable in the setup function means exactly the same as it does in your spi_select() function -- a bitwise combination of various communication parameters like clock polarity, clock phase, etc.
Not all drivers may actually _need_ to do anything in the claim/release functions though. I just think the interface makes sense conceptually, and I also think it might improve performance with some drivers.
Another reason to have the claim/release interface is to allow controller drivers to aggressively turn the controller on/off in order to save power, or to ensure that everything is properly switched off when booting the OS.
slave = spi_setup_slave(host, dev, hz, 0 /*| SPI_ACCESS_EXCLUSIVE */); if (!slave) return; /* Now you MUST release the slave / host */ spi_xfer(); ... spi_release();
I think you should set up a slave exactly once before starting to communicate with it. Once you've done that, you just execute sequences of
spi_claim_bus(slave); spi_xfer(slave, ...); /* possibly more transfers */ spi_release_bus(slave);
We could also define commonly-used "canned" sequences, like spi_w8r8() (write 8 bits, read 8 bits).
And you get access to the bus if there are no exclusive users, and you get exclusive access, only if there are no users currently at all.
I don't see how having multiple active users on the bus at the same time can possibly make any sense...once you start a transfer, you better make sure nobody else is doing a transfer at the same time since you're using the same MOSI/MISO/SCK lines...
What do we do, if there are several non-exclusive users on a bus, and one of them doesn't release its cs between transfers? Return an error to others if they try to communicate at this time?
That's a bug, but there are several ways the controller driver can try to recover. The best one is probably to just deselect the CS that was left active, possibly complaining a bit about it to the console.
spi_release_bus() could deactivate CS implicitly, making sure this situation never happens in the first place.
Looks like most of this API implementation is hardware independent, and should go into an spi.c?
Not really...what "claiming" the bus really means is highly hardware dependent. And SPI slave setup is mostly about decoding hardware-independent parameters like SCK rate and mode bits into hardware register values. But any convenience wrappers like spi_w8r8() probably belongs somewhere hardware-independent.
Haavard