On Tue, Feb 16, 2010 at 22:48, Grant Likely wrote: > On Tue, Feb 16, 2010 at 5:07 PM, Mike Frysinger wrote: >> 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 > > Am I missing something. I cannot find any lock related functions in > the spi code. Is this stuff in mainline?
i'm proposing what should be done. there is no locking logic anywhere atm. -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
