On Mon, Feb 10, 2014 at 6:33 PM, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxxx> wrote:
> On Mon, Feb 10, 2014 at 11:31:40AM +0800, Dong Aisheng wrote:
>> On Sun, Feb 9, 2014 at 11:14 PM, Russell King - ARM Linux
>> <linux@xxxxxxxxxxxxxxxx> wrote:
>> > Okay, I have this working now - there was a hardware problem. However,
>> > it's obvious that this has never been tested with lockdep enabled beacuse
>> > of the following:
>> >
>> > disable_irq(host->irq);
>> > spin_lock(&host->lock);
>> > host->mrq = &mrq;
>> >
>> > sdhci_send_command(host, mrq.cmd);
>> >
>> > spin_unlock(&host->lock);
>> > enable_irq(host->irq);
>> >
>> > You can't "work around" stuff like this. Use spin_lock_irq()..
>> > spin_unlock_irq(). You can't just disable the interrupt line and
>> > expect lockdep to know that it's safe.
>> >
>>
>> Yes, i missed this.
>> Such kind of code was originally referenced from sdhci_execute_tuning.
>> It was formerly fixed in commit 2b35bd83.
>> But i did miss sdhci-esdhci-imx also has such issue.
>> Thanks for the reminder.
>>
>> You can try the following fix and I will send out a patch for it later.
>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
>> b/drivers/mmc/host/sdhci-esdhc-imx.c
>> index b841bb7..0372baf 100644
>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> @@ -720,6 +720,7 @@ static int esdhc_send_tuning_cmd(struct sdhci_host
>> *host, u32 opcode)
>> struct mmc_data data = {0};
>> struct scatterlist sg;
>> char tuning_pattern[ESDHC_TUNING_BLOCK_PATTERN_LEN];
>> + unsigned long flags;
>>
>> cmd.opcode = opcode;
>> cmd.arg = 0;
>> @@ -742,14 +743,12 @@ static int esdhc_send_tuning_cmd(struct
>> sdhci_host *host, u32 opcode)
>> mrq.done = esdhc_request_done;
>> init_completion(&(mrq.completion));
>>
>> - disable_irq(host->irq);
>> - spin_lock(&host->lock);
>> + spin_lock_irqsave(&host->lock, flags);
>> host->mrq = &mrq;
>>
>> sdhci_send_command(host, mrq.cmd);
>>
>> - spin_unlock(&host->lock);
>> - enable_irq(host->irq);
>> + spin_unlock_irqrestore(&host->lock, flags);
>>
>> wait_for_completion(&mrq.completion);
>
> You don't need to use irqsave/irqrestore here - IRQs must be enabled here
> otherwise wait_for_completion() will scream - scheduling with interrupts
> disabled is not allowed.
>
Right.
Thanks for the suggestion.
Regards
Dong Aisheng
> --
> FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
> in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
> Estimate before purchase was "up to 13.2Mbit".
--
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
