Re: [RFC} sdhci.c set_ios -- disable global interrupts and Question on Power Management

[Philip Rakity <prakity@marvell.com>]

On Feb 14, 2011, at 1:29 AM, Pierre Tardy wrote:

> On Mon, Feb 14, 2011 at 6:50 AM, Philip Rakity <prakity@marvell.com> wrote:
>> 
>> Pierre,
>> 
>> are you comfortable with this ?
> The mmc_delay() looks hackish at the first sight. Looks like Pierre
> Ossman was the author, he must have his reasons.
> 
> It looks like the mdelay are not in the fastpath, so why bother.
> 
> And, more important, you will do cond_resched while holding you
> spinlock, which is *bad*.
> What if the mmc stack will call you again from another thread? deadlock...


Assumptions -- Please Confirm 
-------------------------------------------

There is a one to one relationship
between the calls in the core layer into the SD driver.

Each thread is independent of the other.

Each thread maps into exactly one register instance for the SD driver

eg --> 2 threads cannot be active at the same time that use the same
registers in the SD driver.

Question
------------

If these assumptions are correct how can the deadlock happen ?
host->lock is held on a per thread basis.

======

> 
> 
>> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
>> -static inline void mmc_delay(unsigned int ms)
> why do you want to remove it from here ?
> It is still used by the core code.

moved to include/linux/mmc so okay for sdhci driver.  Did not want to duplicate code.

> 
> 
>> @@ -1043,7 +1043,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
>>                        return;
>>                }
>>                timeout--;
>> -               mdelay(1);
>> +               mmc_delay(1);
> This looks like timeout code that never happen in normal condition.
> 
> 
>> @@ -1108,7 +1108,7 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
>>         * can apply clock after applying power
>>         */
>>        if (host->quirks & SDHCI_QUIRK_DELAY_AFTER_POWER)
>> -               mdelay(10);
>> +               mmc_delay(10);
> Do you need this quirk in your platform?

No

> 
> 
> Pierre