[PATCH 0/2] provide isr for card-detect interrupts

[PATCH 0/2] provide isr for card-detect interrupts

[Jisheng Zhang]

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

[PATCH 0/2] provide isr for card-detect interrupts

[Jisheng Zhang]

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

[PATCH 0/2] provide isr for card-detect interrupts

[Jisheng Zhang]

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

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

[Jisheng Zhang]

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 <jszhang@marvell.com>
---
 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

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

[Jisheng Zhang]

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 <jszhang@marvell.com>
---
 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

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

[Jisheng Zhang]

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 <jszhang@marvell.com>
---
 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

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

[Jisheng Zhang]

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 <jszhang@marvell.com>
---
 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

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

[Jisheng Zhang]

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 <jszhang@marvell.com>
---
 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

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

[Jisheng Zhang]

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 <jszhang@marvell.com>
---
 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

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

[Adrian Hunter]

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 <jszhang@marvell.com>
> ---
>  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) {
> 

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

[Adrian Hunter]

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 <jszhang@marvell.com>
> ---
>  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) {
> 

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

[Jisheng Zhang]

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

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

[Jisheng Zhang]

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

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

[Jisheng Zhang]

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