Re: [PATCH 1/2] spi/mmc_spi: SPI bus locking API, using mutex

[Mike Frysinger <vapier.adi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Wed, Feb 17, 2010 at 13:17, Ernst Schwab wrote:
> The current runtime API looks like this:
> spi_async(struct spi_device*, struct spi_message*);
> spi_sync(struct spi_device*, struct spi_message*);
>
> The API needs to be extended to this:
> spi_async(struct spi_device*, struct spi_message*)
> spi_sync(struct spi_device*, struct spi_message*)
> spi_bus_lock(struct spi_master*)  /* although struct spi_device* might
> be easier */
> spi_bus_unlock(struct spi_master*)
> spi_async_locked(struct spi_device*, struct spi_message*)
> spi_sync_locked(struct spi_device*, struct spi_message*)
>
> Drivers can only call the last two if they already hold the spi_master_lock().
>
> spi_bus_lock() obtains the mutex, obtains the spin lock, sets the
> flag, and releases the spin lock before returning.  It doesn't even
> need to sleep while waiting for "in-flight" spi_transactions to
> complete because its purpose is to guarantee no additional
> transactions are added.  It does not guarantee that the bus is idle.
>
> spi_bus_unlock() clears the flag and releases the mutex, which will
> wake up any waiters.
>
> The difference between spi_async() and spi_async_locked() is that the
> locked version bypasses the check of the lock flag.  Both versions
> need to obtain the spinlock.
>
> The difference between spi_sync() and spi_sync_locked() is that
> spi_sync() must hold the mutex while enqueuing a new transfer.
> spi_sync_locked() doesn't because the mutex is already held.  Note
> however that spi_sync must *not* continue to hold the mutex while
> waiting for the transfer to complete, otherwise only one transfer
> could be queued up at a time!
>
> Almost no code needs to be written.  The current spi_async() and
> spi_sync() can probably be renamed to __spi_async() and __spi_sync()
> so that spi_async(), spi_sync(), spi_async_locked() and
> spi_sync_locked() can just become wrappers around the common code.
>
> spi_sync() is protected by a mutex because it can sleep
> spi_async() needs to be protected with a flag and a spinlock because
> it can be called atomically and must not sleep

i dont think these new "_locked" versions are a good idea.  why cant
it be handled transparently to the caller in the core ?  the spi core
already requires the CS field of the spi device to be unique per bus.
re-use that to check ownership of the mutex.
-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
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general