Re: One bug of SDHCI driver

From: Fu, Zhonghui
Date: Mon Aug 11 2014 - 01:53:20 EST

Next message: Zhang Rui: "[GIT PULL] thermal management updates for v3.17-rc1"
Previous message: Stephen Rothwell: "linux-next: Tree for Aug 11"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi, everyone

I have posted this new patch in a separate mail. Many thanks for your comments.

Thanks,
Zhonghui

On 2014/8/7 14:58, Fu, Zhonghui wrote:
> kindly reminder.
>
> Thanks,
> Zhonghui
>
> On 2014/8/5 12:56, Fu, Zhonghui wrote:
>> Hi, Jaehoon
>>
>> According to your comments, I created a new patch for this issue as follows:
>>
>>
>> Thanks,
>> Zhonghui
>>
>>
>> From 6cee984e1d76ba0a3320430f8cf4318ab65fcf06 Mon Sep 17 00:00:00 2001
>> From: Fu Zhonghui <zhonghui.fu@xxxxxxxxxxxxxxx>
>> Date: Tue, 5 Aug 2014 12:44:38 +0800
>> Subject: [PATCH] mmc: core: sdio: Fix unconditional wake_up_process() on sdio thread
>>
>> 781e989cf59 ("mmc: sdhci: convert to new SDIO IRQ handling") and
>> bf3b5ec66bd ("mmc: sdio_irq: rework sdio irq handling") disabled
>> the use of our own custom threaded IRQ handler, but left in an
>> unconditional wake_up_process() on that handler at resume-time.
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=80151
>>
>> In addition, the check for MMC_CAP_SDIO_IRQ capability is added
>> before enable sdio IRQ.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
>> Signed-off-by: Chris Ball <chris@xxxxxxxxxx>
>> Signed-off-by: Fu Zhonghui <zhonghui.fu@xxxxxxxxxxxxxxx>
>> ---
>> drivers/mmc/core/sdio.c | 14 ++++++++++++--
>> drivers/mmc/core/sdio_irq.c | 4 ++--
>> 2 files changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>> index e636d9e..e04a540 100644
>> --- a/drivers/mmc/core/sdio.c
>> +++ b/drivers/mmc/core/sdio.c
>> @@ -992,8 +992,18 @@ static int mmc_sdio_resume(struct mmc_host *host)
>> }
>> }
>>
>> - if (!err && host->sdio_irqs)
>> - wake_up_process(host->sdio_irq_thread);
>> + if (!err && host->sdio_irqs) {
>> + if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD)) {
>> + wake_up_process(host->sdio_irq_thread);
>> + } else if (host->caps & MMC_CAP_SDIO_IRQ) {
>> + mmc_release_host(host);
>> + mmc_host_clk_hold(host);
>> + host->ops->enable_sdio_irq(host, 1);
>> + mmc_host_clk_release(host);
>> + mmc_claim_host(host);
>> + }
>> + }
>> +
>> mmc_release_host(host);
>>
>> host->pm_flags &= ~MMC_PM_KEEP_POWER;
>> diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
>> index 5cc13c8..696eca4 100644
>> --- a/drivers/mmc/core/sdio_irq.c
>> +++ b/drivers/mmc/core/sdio_irq.c
>> @@ -208,7 +208,7 @@ static int sdio_card_irq_get(struct mmc_card *card)
>> host->sdio_irqs--;
>> return err;
>> }
>> - } else {
>> + } else if (host->caps & MMC_CAP_SDIO_IRQ) {
>> mmc_host_clk_hold(host);
>> host->ops->enable_sdio_irq(host, 1);
>> mmc_host_clk_release(host);
>> @@ -229,7 +229,7 @@ static int sdio_card_irq_put(struct mmc_card *card)
>> if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD)) {
>> atomic_set(&host->sdio_irq_thread_abort, 1);
>> kthread_stop(host->sdio_irq_thread);
>> - } else {
>> + } else if (host->caps & MMC_CAP_SDIO_IRQ) {
>> mmc_host_clk_hold(host);
>> host->ops->enable_sdio_irq(host, 0);
>> mmc_host_clk_release(host);
>> -- 1.7.1
>>
>>
>>
>>
>>
>>
>> On 2014/7/30 11:40, Jaehoon Chung wrote:
>>> Hi, Zhonghui.
>>>
>>> On 07/30/2014 11:47 AM, Fu, Zhonghui wrote:
>>>> Hi,
>>>>
>>>> In the resume function, SDIO irq must be enabled, or the interrupts from devices on SDIO bus can't be acknowledged. I also uploaded this new patch to https://bugzilla.kernel.org/show_bug.cgi?id=80151.
>>>> Could you please help to review it?
>>>>
>>>>
>>>>
>>>>
>>>> Thanks,
>>>> Zhonghui
>>>>
>>>> On 2014/7/24 23:27, Fu, Zhonghui wrote:
>>>>> Hi,
>>>>>
>>>>> Any comments for this new patch?
>>>>>
>>>>> Thanks,
>>>>> Zhonghui
>>>>> On 2014/7/20 22:51, Fu, Zhonghui wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Chris' patch is not enough to fix this bug. I made a patch as follows and verified it can work. Could you please give out some comments about this patch?
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Zhonghui
>>>>>>
>>>>>> >From 72d6f5b56fa04290fd3a055a3333de1d89e7c8d4 Mon Sep 17 00:00:00 2001
>>>>>> From: Fu Zhonghui <zhonghui.fu@xxxxxxxxxxxxxxx>
>>>>>> Date: Sun, 20 Jul 2014 22:29:53 +0800
>>>>>> Subject: [PATCH] mmc: core: sdio: Fix unconditional wake_up_process() on sdio thread
>>>>>>
>>>>>> 781e989cf59 ("mmc: sdhci: convert to new SDIO IRQ handling") and
>>>>>> bf3b5ec66bd ("mmc: sdio_irq: rework sdio irq handling") disabled
>>>>>> the use of our own custom threaded IRQ handler, but left in an
>>>>>> unconditional wake_up_process() on that handler at resume-time.
>>>>>>
>>>>>> Signed-off-by: Fu Zhonghui <zhonghui.fu@xxxxxxxxxxxxxxx>
>>>>>> ---
>>>>>> drivers/mmc/core/sdio.c | 14 ++++++++++++--
>>>>>> 1 files changed, 12 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>>>> index e636d9e..8369e56 100644
>>>>>> --- a/drivers/mmc/core/sdio.c
>>>>>> +++ b/drivers/mmc/core/sdio.c
>>>>>> @@ -992,8 +992,18 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> - if (!err && host->sdio_irqs)
>>>>>> - wake_up_process(host->sdio_irq_thread);
>>>>>> + if (!err && host->sdio_irqs) {
>>>>>> + if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD)) {
>>>>>> + wake_up_process(host->sdio_irq_thread);
>>>>>> + } else {
>>>>>> + mmc_release_host(host);
>>>>>> + mmc_host_clk_hold(host);
>>>>>> + host->ops->enable_sdio_irq(host, 1);
>>>>>> + mmc_host_clk_release(host);
>>>>>> + mmc_claim_host(host);
>>>>>> + }
>>>>>> + }
>>> If you enable the sdio_irq, I think it needs to check whether MMC_CAP_SDIO_IRQ is set or not.
>>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>>>> +
>>>>>> mmc_release_host(host);
>>>>>>
>>>>>> host->pm_flags &= ~MMC_PM_KEEP_POWER;
>>>>>> -- 1.7.1
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 2014/7/15 12:40, Jaehoon Chung wrote:
>>>>>>> From: Chris Ball <chris@xxxxxxxxxx>
>>>>>>> Subject: [PATCH] mmc: core: sdio: Fix unconditional wake_up_process() on sdio thread
>>>>>>>
>>>>>>> 781e989cf59 ("mmc: sdhci: convert to new SDIO IRQ handling") and
>>>>>>> bf3b5ec66bd ("mmc: sdio_irq: rework sdio irq handling") disabled
>>>>>>> the use of our own custom threaded IRQ handler, but left in an
>>>>>>> unconditional wake_up_process() on that handler at resume-time.
>>>>>>>
>>>>>>> Reported-by: Fu, Zhonghui <zhonghui.fu@xxxxxxxxxxxxxxx>
>>>>>>> [Patch suggested by Jaehoon Chung]
>>>>>>> Signed-off-by: Chris Ball <chris@xxxxxxxxxx>
>>>>>>> ---
>>>>>>> drivers/mmc/core/sdio.c | 3 ++-
>>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>>>>>> index e636d9e..11cc4e0 100644
>>>>>>> --- a/drivers/mmc/core/sdio.c
>>>>>>> +++ b/drivers/mmc/core/sdio.c
>>>>>>> @@ -992,7 +992,8 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>> - if (!err && host->sdio_irqs)
>>>>>>> + if (!err && host->sdio_irqs &&
>>>>>>> + !(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD))
>>>>>>> wake_up_process(host->sdio_irq_thread);
>>>>>>> mmc_release_host(host);
>>>>>>>
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>>> Please read the FAQ at http://www.tux.org/lkml/
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> Please read the FAQ at http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Zhang Rui: "[GIT PULL] thermal management updates for v3.17-rc1"
Previous message: Stephen Rothwell: "linux-next: Tree for Aug 11"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]