On Feb 14, 2011, at 1:29 AM, Pierre Tardy wrote: > On Mon, Feb 14, 2011 at 6:50 AM, Philip Rakity <prakity@xxxxxxxxxxx> 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... The original code used a spin_lock_irqsave and a spin_lock_irqrestore so if the code was called again the deadlock surely would have happened by now. > > >> 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. > > >> @@ -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? > > > Pierre -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
