> -----Original Message----- > From: Nicolas Pitre [mailto:nico@xxxxxxxxxxx] > Sent: Wednesday, February 16, 2011 2:48 AM > To: Nath, Arindam > Cc: cjb@xxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; Su, Henry; Lu, Aaron; > anath.amd@xxxxxxxxx > Subject: Re: [PATCH 02/12] mmc: sd: add support for signal voltage > switch procedure > > On Tue, 15 Feb 2011, Arindam Nath wrote: > > > @@ -1340,11 +1342,76 @@ out: > > spin_unlock_irqrestore(&host->lock, flags); > > } > > > > +static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc) > > +{ > > + struct sdhci_host *host; > > + u8 pwr; > > + u16 clk, ctrl; > > + u32 present_state; > > + unsigned long flags; > > + > > + host = mmc_priv(mmc); > > + > > + spin_lock_irqsave(&host->lock, flags); > > + > > + /* Stop SDCLK */ > > + host = mmc_priv(mmc); > > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > > + clk &= ~SDHCI_CLOCK_CARD_EN; > > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); > > + > > + /* Check whether DAT[3:0] is 0000 */ > > + present_state = sdhci_readl(host, SDHCI_PRESENT_STATE); > > + if (!((present_state & SDHCI_DATA_LVL_MASK) >> > SDHCI_DATA_LVL_SHIFT)) { > > + /* Enable 1.8V Signal Enable in the Host Control2 register > */ > > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); > > + ctrl |= SDHCI_CTRL_VDD_180; > > + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); > > + > > + /* Wait for 5ms */ > > + mdelay(5); > > You're busy-waiting for 5 ms while holding a spinlock and with > interrupts masked off. This is totally unacceptable. > > There shouldn't be any other concurrent access to the controller here > as > the > core code should have claimed the host through mmc_claim_host(), if not > this is a bug. So if the only thing you're worried about is some sdhci > specific interrupts then simply mask interrupts at the controller > level, > do your business including this 5 ms delay using msleep() and _not_ > mdelay(), and unmask them afterwards. If this is called in a non > sleepable context then please make it so. Yes, it makes sense. I will use msleep() instead of mdelay() and the changes will reflect in V2 of the patchset. Regards, Arindam -- 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
