On Tue, Feb 16, 2010 at 15:43, Grant Likely wrote:
> On Tue, Feb 16, 2010 at 12:57 PM, Ernst Schwab wrote:
>> From: Yi Li <[email protected]>
>>
>> For some MMC cards over SPI bus, it needs to lock the SPI bus for its own
>> use. The SPI transfer must not be interrupted by other SPI devices that
>> share the SPI bus with SPI MMC card.
>>
>> This patch introduces 2 APIs for SPI bus locking operation.
>>
>> Signed-off-by: Yi Li <[email protected]>
>> Signed-off-by: Bryan Wu <[email protected]>
>> Signed-off-by: Mike Frysinger <[email protected]>
>> ---
>> Andrew: we've posted these in the past with no response. could you pick
>> them up please ?
>> drivers/spi/spi.c | 48
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/spi/spi.h | 7 ++++++
>> 2 files changed, 55 insertions(+), 0 deletions(-)
>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>> index 70845cc..b82b8ad 100644
>> --- a/drivers/spi/spi.c
>> +++ b/drivers/spi/spi.c
>> @@ -653,6 +653,54 @@ static void spi_complete(void *arg)
>> }
>>
>> /**
>> + * spi_lock_bus - lock SPI bus for exclusive access
>> + * @spi: device which want to lock the bus
>> + * Context: any
>> + *
>> + * Once the caller owns exclusive access to the SPI bus,
>> + * only messages for this device will be transferred.
>> + * Messages for other devices are queued but not transferred until
>> + * the bus owner unlock the bus.
>> + *
>> + * The caller may call spi_lock_bus() before spi_sync() or spi_async().
>> + * So this call may be used in irq and other contexts which can't sleep,
>> + * as well as from task contexts which can sleep.
>> + *
>> + * It returns zero on success, else a negative error code.
>> + */
>> +int spi_lock_bus(struct spi_device *spi)
>> +{
>> + if (spi->master->lock_bus)
>> + return spi->master->lock_bus(spi);
>> + else
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(spi_lock_bus);
>
> This series seems to try and solve the problem the hard way, and by
> creating a new locking scheme (and as history shows, new locking
> schemes are *alwasy* broken).
>
> Why is the locking getting pushed down to the bus driver level? It
> seems to me that the whole thing could be handled with common code and
> a mutex in the spi_master structure. spi_sync would be easy to handle
> by putting a mutex around the spi_message submission. spi_async would
> be a little harder since it needs to be atomic, but that could also be
> handled with a flag protected by a spinlock.
>
> Basically, the idea is that existing drivers continue to use the API as-is
>
> Drivers that want to lock the bus for exclusive access must call
> spi_lock_bus() which should take the mutex and then sleep until all
> in-flight spi_messages are processed. After that, anyone calling
> spi_async() will simply sleep until the locker unlocks the bus again.
>
> To handle spi_sync() would probably require a flag protected by a
> spinlock. If the flag is set, then spi_sync() would simply fail.
>
> Finally, the locking driver would need locked versions of spi_sync()
> and spi_async() that sidestep the lock checks. It would only be valid
> to call these versions when holding the SPI bus lock.
>
> There is no need to specify the spi_device in the lock request. Since
> the lock is exclusive, it is known that the only driver calling the
> locked API version must already hold the lock.
this is what i proposed last time, but we havent gotten around to
implementing it:
there's nothing Blackfin-specific in the implementation of these
functions. i think the way we should be handling these is by doing:
- remove {lock,unlock}_bus functions from spi_master
- move the {lock,unlock}_bus code from spi_bfin5xx.c to spi.c
- drop the SPI_BFIN_LOCK Kconfig
- add a new spi_master flag to spi.h like SPI_MASTER_HALF_DUPLEX --
SPI_MASTER_LOCK_BUS
- have spi_bfin5xx.c/bfin_sport_spi.c add that flag to its master setup
- have the common spi code key off of that flag to return ENOSYS
- have the mmc_spi code check that bit in the master before falling
back to its hack
-mike
------------------------------------------------------------------------------
SOLARIS 10 is the OS for Data Centers - provides features such as DTrace,
Predictive Self Healing and Award Winning ZFS. Get Solaris 10 NOW
http://p.sf.net/sfu/solaris-dev2dev
_______________________________________________
spi-devel-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
