linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mike Frysinger <vapier.adi@gmail.com>
To: H Hartley Sweeten <hartleys@visionengravers.com>
Cc: spi-devel-general@lists.sourceforge.net,
	David Brownell <dbrownell@users.sourceforge.net>,
	Yi Li <yi.li@analog.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [spi-devel-general] [PATCH 1/2] spi: new SPI bus lock/unlockfunctions
Date: Sat, 19 Sep 2009 15:11:59 -0400	[thread overview]
Message-ID: <8bd0f97a0909191211q4170febfj316d1c0cae31ef5d@mail.gmail.com> (raw)
In-Reply-To: <BD79186B4FD85F4B8E60E381CAEE190901D41FB1@mi8nycmail19.Mi8.com>

On Fri, Sep 18, 2009 at 19:00, H Hartley Sweeten wrote:
> On Thursday, September 17, 2009 3:54 PM, Mike Frysinger wrote:
>>> I assume the spi master driver must supply the {lock/unlock}_bus methods
>>> to properly support the locking.
>>
>> currently, yes.  a common solution would be nice.  ideas/patches welcome ;).
>>
>>> But, by returning 0 when the methods
>>> are not supplied you are basically saying all the current master drivers
>>> in mainline support bus locking.  I think this is really only "true" if
>>> spi->master->num_chipselect == 1.
>>
>> right, but that is no different from what we have today.  there is no
>> way for a client to guarantee exclusive access, so you cant write code
>> assuming it in the first place.  the only consumer thus far (mmc_spi)
>> actually errors out if such conditions exist.
>>
>> in other words, we arent breaking anything.
>
> Actually you are breaking the mmc_spi driver.

no we arent.  the current code does some sloppy checking for possible
concurrent access and then aborts.  the new code drops that sloppy
checking in place of the bus master doing it right.  any setup that is
working today will continue to work fine.  and setup *that is already
broken* will continue to be broken.

> By returning 0 when the methods are not supplied you are saying that the
> master driver supports and locked the bus.  At a minimum, I think spi_lock_bus()
> should return an error code if locking is not supported.
>
> Also, as Andrew Morton pointed out, calling spi_unlock_bus() without having
> a valid lock by calling spi_lock_bus() is a bug.
>
> In addition your patch to mmc_spi should check the return code from
> spi_lock_bus().  If the driver "requires" that the bus be locked it should
> trigger an error path if it cannot be locked.
>
>>> Also, do you have a master driver that does have the {lock/unlock}_bus
>>> methods?  I would like to see how you handled it.
>>
>> http://git.kernel.org/?p=linux/kernel/git/vapier/blackfin.git;a=commitdiff;h=cc54fa8ed63e11a000031bc650d41d57b441803d
>
> Oiy... The lock/unlock functions are simple enough but the change needed
> to bfin_spi_pump_messages() is a bit complicated.
>
> What happens to next_msg if it is for other devices on the bus?

Yi can take a stab at addressing these issues since he is the guy who
has been putting together in the first place.
-mike

  reply	other threads:[~2009-09-19 19:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-17 22:03 [PATCH 1/2] spi: new SPI bus lock/unlock functions Mike Frysinger
2009-09-17 22:03 ` [PATCH 2/2] mmc_spi: lock the SPI bus when accessing the card Mike Frysinger
2009-09-18  0:29   ` [spi-devel-general] [PATCH 2/2] mmc_spi: lock the SPI bus whenaccessing " H Hartley Sweeten
2009-09-18  0:52     ` [PATCH 2/2 v2] mmc_spi: lock the SPI bus when accessing " Mike Frysinger
2009-09-18 21:30       ` Andrew Morton
2009-09-17 22:45 ` [spi-devel-general] [PATCH 1/2] spi: new SPI bus lock/unlockfunctions H Hartley Sweeten
2009-09-17 22:53   ` Mike Frysinger
2009-09-18 23:00     ` H Hartley Sweeten
2009-09-19 19:11       ` Mike Frysinger [this message]
2009-09-21  8:08       ` Li Yi
2009-09-18 21:29 ` [PATCH 1/2] spi: new SPI bus lock/unlock functions Andrew Morton
2009-09-21  6:33   ` Li Yi
2009-09-21 13:36     ` Mike Frysinger
2009-09-22 14:15 ` Mike Frysinger
2009-12-16 22:49 ` [spi-devel-general] [PATCH 1/2] spi: new SPI bus lock/unlockfunctions H Hartley Sweeten
2009-12-16 23:00   ` Mike Frysinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8bd0f97a0909191211q4170febfj316d1c0cae31ef5d@mail.gmail.com \
    --to=vapier.adi@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=dbrownell@users.sourceforge.net \
    --cc=hartleys@visionengravers.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=spi-devel-general@lists.sourceforge.net \
    --cc=yi.li@analog.com \
    --subject='Re: [spi-devel-general] [PATCH 1/2] spi: new SPI bus lock/unlockfunctions' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).