2017-04-11 10:27:37

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH 0/2] provide isr for card-detect interrupts

We found one issue on BG4CT platforms with GPIO for the card detect
interrupt: remove sdcard when there's read write access to the sdcard,
sometimes the card remove event can't be handled for a long time, so
the system still think the sdcard is still plugged in. It turns out
that the sdhci_card_event() is missing in common slot gpio's card
detect isr. This should be common for all sdhci platforms.

We fix this issue by providing sdhci's own isr for card-detect
interrupts. In this own isr, we call sdhci_card_event() then process
the change of state.

patch1 is to prepare the fix in slot-gpio
patch2 is the real fix

Jisheng Zhang (2):
mmc: slot-gpio: check cd_gpio before setting up cd_gpio_isr
mmc: sdhci: provide isr for card-detect interrupts

drivers/mmc/core/slot-gpio.c | 3 +++
drivers/mmc/host/sdhci.c | 12 ++++++++++++
2 files changed, 15 insertions(+)

--
2.11.0


2017-04-11 10:27:38

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH 2/2] mmc: sdhci: provide isr for card-detect interrupts

We found one issue on BG4CT platforms with GPIO for sdcar card detect
interrupt: remove sdcard when there's read write access to the sdcard,
sometimes the card remove event can't be handled for a long time, so
the system still think the sdcard is still plugged in. It turns out
that the sdhci_card_event() is missing in common slot gpio's card
detect isr.

We fix this issue by providing sdhci's own isr for card-detect
interrupts. In this own isr, we call sdhci_card_event() then process
the change of state.

Signed-off-by: Jisheng Zhang <[email protected]>
---
drivers/mmc/host/sdhci.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 63bc33a54d0d..b6c5021a5892 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2777,6 +2777,16 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
return result;
}

+static irqreturn_t sdhci_cd_irq(int irq, void *dev_id)
+{
+ struct mmc_host *mmc = dev_id;
+ struct sdhci_host *host = mmc_priv(mmc);
+
+ mmc->ops->card_event(mmc);
+ mmc_detect_change(host->mmc, msecs_to_jiffies(200));
+ return IRQ_HANDLED;
+}
+
static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
{
struct sdhci_host *host = dev_id;
@@ -3617,6 +3627,8 @@ int __sdhci_add_host(struct sdhci_host *host)

sdhci_init(host, 0);

+ mmc_gpio_set_cd_isr(mmc, sdhci_cd_irq);
+
ret = request_threaded_irq(host->irq, sdhci_irq, sdhci_thread_irq,
IRQF_SHARED, mmc_hostname(mmc), host);
if (ret) {
--
2.11.0

2017-04-11 10:27:36

by Jisheng Zhang

[permalink] [raw]
Subject: [PATCH 1/2] mmc: slot-gpio: check cd_gpio before setting up cd_gpio_isr

slot-gpio knows more information than its user, such as whether there
is cd gpio_desc or not. While one of its user -- the common sdhci
driver doesn't know it. To prepare for next patch which will setup
sdhci's own isr for card-detect interrupts, we add the check before
setting up cd_gpio_isr.

Signed-off-by: Jisheng Zhang <[email protected]>
---
drivers/mmc/core/slot-gpio.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index a8450a8701e4..42b260593955 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -162,6 +162,9 @@ void mmc_gpio_set_cd_isr(struct mmc_host *host,
{
struct mmc_gpio *ctx = host->slot.handler_priv;

+ if (!ctx->cd_gpio)
+ return;
+
WARN_ON(ctx->cd_gpio_isr);
ctx->cd_gpio_isr = isr;
}
--
2.11.0

2017-04-11 10:38:04

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 2/2] mmc: sdhci: provide isr for card-detect interrupts

On 11/04/17 13:20, Jisheng Zhang wrote:
> We found one issue on BG4CT platforms with GPIO for sdcar card detect
> interrupt: remove sdcard when there's read write access to the sdcard,
> sometimes the card remove event can't be handled for a long time, so
> the system still think the sdcard is still plugged in. It turns out
> that the sdhci_card_event() is missing in common slot gpio's card
> detect isr.

->card_event() is called by mmc_rescan(). Why is that too late?

>
> We fix this issue by providing sdhci's own isr for card-detect
> interrupts. In this own isr, we call sdhci_card_event() then process
> the change of state.

Why doesn't every other host controller have this problem?

>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> drivers/mmc/host/sdhci.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 63bc33a54d0d..b6c5021a5892 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2777,6 +2777,16 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
> return result;
> }
>
> +static irqreturn_t sdhci_cd_irq(int irq, void *dev_id)
> +{
> + struct mmc_host *mmc = dev_id;
> + struct sdhci_host *host = mmc_priv(mmc);
> +
> + mmc->ops->card_event(mmc);
> + mmc_detect_change(host->mmc, msecs_to_jiffies(200));
> + return IRQ_HANDLED;
> +}
> +
> static irqreturn_t sdhci_thread_irq(int irq, void *dev_id)
> {
> struct sdhci_host *host = dev_id;
> @@ -3617,6 +3627,8 @@ int __sdhci_add_host(struct sdhci_host *host)
>
> sdhci_init(host, 0);
>
> + mmc_gpio_set_cd_isr(mmc, sdhci_cd_irq);
> +
> ret = request_threaded_irq(host->irq, sdhci_irq, sdhci_thread_irq,
> IRQF_SHARED, mmc_hostname(mmc), host);
> if (ret) {
>

2017-04-13 09:23:12

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/2] mmc: sdhci: provide isr for card-detect interrupts

On Tue, 11 Apr 2017 13:31:16 +0300 Adrian Hunter wrote:

> On 11/04/17 13:20, Jisheng Zhang wrote:
> > We found one issue on BG4CT platforms with GPIO for sdcar card detect
> > interrupt: remove sdcard when there's read write access to the sdcard,
> > sometimes the card remove event can't be handled for a long time, so
> > the system still think the sdcard is still plugged in. It turns out
> > that the sdhci_card_event() is missing in common slot gpio's card
> > detect isr.
>
> ->card_event() is called by mmc_rescan(). Why is that too late?

Indeed, maybe the "too late" here is not accurate. Compared with host's
CD interrupt handling, the GPIO CD interrupt handling differs in when to
call sdhci_card_event(): in previous case, it's called in ISR, in later
it's in workqueue.

Now, I agree it's not necessary to put into ISR. So let's ignore the
two patches.

Thanks for your review,
Jisheng