Linus, please pull from
git://git.kernel.org/pub/scm/linux/kernel/git/drzeus/mmc.git for-linus
to receive the following updates:
drivers/misc/tifm_7xx1.c | 27 ++++++++++++++++++++++++---
drivers/mmc/Kconfig | 10 +++++-----
drivers/mmc/card/Kconfig | 3 +--
drivers/mmc/core/Kconfig | 1 -
drivers/mmc/core/core.c | 10 ++++++----
drivers/mmc/host/Kconfig | 19 +++++++++----------
drivers/mmc/host/tifm_sd.c | 13 +------------
include/linux/tifm.h | 1 +
8 files changed, 47 insertions(+), 37 deletions(-)
Alex Dubov (1):
disable socket power in adapter driver instead of media one
Jan Engelhardt (1):
mmc: Use menuconfig objects
Pierre Ossman (1):
mmc: use lock instead of claim in debug check
Pierre Ossman wrote:
> Linus, please pull from
>
> git://git.kernel.org/pub/scm/linux/kernel/git/drzeus/mmc.git for-linus
>
fsck! I pushed the wrong branch :/
This fix should have been in the last commit.
Sorry,
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
Pierre Ossman wrote:
> Pierre Ossman wrote:
>
>>Linus, please pull from
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/drzeus/mmc.git for-linus
>>
>
>
> fsck! I pushed the wrong branch :/
>
> This fix should have been in the last commit.
>
> Sorry,
>
>
> ------------------------------------------------------------------------
>
> commit 3b9a6d78eb439016728c598a1373b50328f5e9fe
> Author: Pierre Ossman <[email protected]>
> Date: Wed May 9 07:53:28 2007 +0200
>
> mmc: fix wrong call to spinlock
>
> Fix silly typo in spinlock calls.
>
> Signed-off-by: Pierre Ossman <[email protected]>
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index b6c1670..7385acf 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -501,9 +501,9 @@ void mmc_detect_change(struct mmc_host *host, unsigned long delay)
> {
> #ifdef CONFIG_MMC_DEBUG
> unsigned long flags;
> - spin_lock_irqsave(host->lock, flags);
> + spin_lock_irqsave(&host->lock, flags);
> BUG_ON(host->removed);
> - spin_unlock_irqrestore(host->lock, flags);
> + spin_unlock_irqrestore(&host->lock, flags);
> #endif
Do you actually need the lock there at all? What is it protecting?
--
SUSE Labs, Novell Inc.
Nick Piggin wrote:
>> @@ -501,9 +501,9 @@ void mmc_detect_change(struct mmc_host *host,
>> unsigned long delay)
>> {
>> #ifdef CONFIG_MMC_DEBUG
>> unsigned long flags;
>> - spin_lock_irqsave(host->lock, flags);
>> + spin_lock_irqsave(&host->lock, flags);
>> BUG_ON(host->removed);
>> - spin_unlock_irqrestore(host->lock, flags);
>> + spin_unlock_irqrestore(&host->lock, flags);
>> #endif
>
> Do you actually need the lock there at all? What is it protecting?
>
It makes sure we don't have any race when it comes to modifying
host->removed. We had some problems where controllers reported card
insertion events even after they'd signaled to be removed, causing all
kind of odd problems. This check was added to easier spot it should it
arise again.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
Pierre Ossman wrote:
> Nick Piggin wrote:
>
>>>@@ -501,9 +501,9 @@ void mmc_detect_change(struct mmc_host *host,
>>>unsigned long delay)
>>> {
>>> #ifdef CONFIG_MMC_DEBUG
>>> unsigned long flags;
>>>- spin_lock_irqsave(host->lock, flags);
>>>+ spin_lock_irqsave(&host->lock, flags);
>>> BUG_ON(host->removed);
>>>- spin_unlock_irqrestore(host->lock, flags);
>>>+ spin_unlock_irqrestore(&host->lock, flags);
>>> #endif
>>
>>Do you actually need the lock there at all? What is it protecting?
>>
>
>
> It makes sure we don't have any race when it comes to modifying
> host->removed.
If you want to ensure you always only modify host->removed from under
the spinlock, it would be enforcable by introducing an accessor function
and doing a BUG_ON(!spin_is_locked()) in there.
If you just want to ensure that host->removed is 0 at this point, you
shouldn't need any spinlocks AFAIKS... that way you can probably afford
to move it out from CONFIG_MMC_DEBUG and get wider testing.
--
SUSE Labs, Novell Inc.
Nick Piggin wrote:
>
> If you want to ensure you always only modify host->removed from under
> the spinlock, it would be enforcable by introducing an accessor function
> and doing a BUG_ON(!spin_is_locked()) in there.
>
> If you just want to ensure that host->removed is 0 at this point, you
> shouldn't need any spinlocks AFAIKS... that way you can probably afford
> to move it out from CONFIG_MMC_DEBUG and get wider testing.
>
The host->removed member is only used for this simple test. It is set in
mmc_host_remove() to indicate that the removal process has begun. At
this point it is invalid to call mmc_detect_change() (the place this
patch fixes). So the spinlocks are mostly there so that things are
properly ordered when we go SMP. Some creative barriers would probably
work as well, but I find spinlocks more "normal" and hence more readable.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
Pierre Ossman wrote:
> Nick Piggin wrote:
>
>>If you want to ensure you always only modify host->removed from under
>>the spinlock, it would be enforcable by introducing an accessor function
>>and doing a BUG_ON(!spin_is_locked()) in there.
>>
>>If you just want to ensure that host->removed is 0 at this point, you
>>shouldn't need any spinlocks AFAIKS... that way you can probably afford
>>to move it out from CONFIG_MMC_DEBUG and get wider testing.
>>
>
>
> The host->removed member is only used for this simple test. It is set in
> mmc_host_remove() to indicate that the removal process has begun. At
> this point it is invalid to call mmc_detect_change() (the place this
> patch fixes). So the spinlocks are mostly there so that things are
> properly ordered when we go SMP. Some creative barriers would probably
> work as well, but I find spinlocks more "normal" and hence more readable.
Fair enough. No big deal :)
--
SUSE Labs, Novell Inc.
Pierre Ossman wrote:
> The host->removed member is only used for this simple test. It is set in
> mmc_host_remove() to indicate that the removal process has begun. At
> this point it is invalid to call mmc_detect_change() (the place this
> patch fixes). So the spinlocks are mostly there so that things are
> properly ordered when we go SMP. Some creative barriers would probably
> work as well, but I find spinlocks more "normal" and hence more readable.
Sounds to me like either struct xyz_host { atomic_t removed; } would do
the job, or that actually wider regions of mmc_host_remove() and
mmc_detect_change() need to be serialized.
--
Stefan Richter
-=====-=-=== -=-= -=--=
http://arcgraph.de/sr/
Stefan Richter wrote:
> Sounds to me like either struct xyz_host { atomic_t removed; } would do
> the job, or that actually wider regions of mmc_host_remove() and
> mmc_detect_change() need to be serialized.
>
AFAIK, an atomic_t doesn't guarantee any ordering, just atomicity. So an
atomic_t with a barrier would be sufficient. But barriers are mostly
voodoo that few people understand ;)
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org