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.
Nicolas
--
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
