diff mbox

[v2,4/8] mmc: sdhci: do not enable card detect interrupt for gpio cd type

Message ID 1411377131-19741-5-git-send-email-b29396@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aisheng Dong Sept. 22, 2014, 9:12 a.m. UTC
Except SDHCI_QUIRK_BROKEN_CARD_DETECTION and MMC_CAP_NONREMOVABLE,
we also do not need to handle controller native card detect interrupt
for gpio as card detect case.
If we wrong enabled the card detect interrupt for gpio case,
it will cause a lot of unexpected card detect interrupts during data transfer
which should not happen.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Comments

Aisheng Dong Sept. 23, 2014, 7:10 a.m. UTC | #1
On Tue, Sep 23, 2014 at 09:27:49AM +0200, Ulf Hansson wrote:
> On 22 September 2014 11:12, Dong Aisheng <b29396@freescale.com> wrote:
> > Except SDHCI_QUIRK_BROKEN_CARD_DETECTION and MMC_CAP_NONREMOVABLE,
> > we also do not need to handle controller native card detect interrupt
> > for gpio as card detect case.
> > If we wrong enabled the card detect interrupt for gpio case,
> > it will cause a lot of unexpected card detect interrupts during data transfer
> > which should not happen.
> >
> > Signed-off-by: Dong Aisheng <b29396@freescale.com>
> > ---
> >  drivers/mmc/host/sdhci.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 7481bd8..85cbf45 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -136,9 +136,10 @@ static void sdhci_dumpregs(struct sdhci_host *host)
> >  static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
> >  {
> >         u32 present;
> > +       int gpio_cd = mmc_gpio_get_cd(host->mmc);
> >
> >         if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
> > -           (host->mmc->caps & MMC_CAP_NONREMOVABLE))
> > +           (host->mmc->caps & MMC_CAP_NONREMOVABLE) || !IS_ERR_VALUE(gpio_cd))
> 
> If you have a properly working gpio_cd, then why does you also have
> SDHCI_QUIRK_BROKEN_CARD_DETECTION set? Isn't that the actual problem?
> 

SDHCI_QUIRK_BROKEN_CARD_DETECTION seems only for controller CD function.
Thus even we're using gpio_cd, we still have this quirk set for controller
to avoid enable controller cd interrupts.
Does it make sense?

Regards
Dong Aisheng

> Kind regards
> Uffe
> 
> >                 return;
> >
> >         if (enable) {
> > --
> > 1.7.8
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Sept. 23, 2014, 7:27 a.m. UTC | #2
On 22 September 2014 11:12, Dong Aisheng <b29396@freescale.com> wrote:
> Except SDHCI_QUIRK_BROKEN_CARD_DETECTION and MMC_CAP_NONREMOVABLE,
> we also do not need to handle controller native card detect interrupt
> for gpio as card detect case.
> If we wrong enabled the card detect interrupt for gpio case,
> it will cause a lot of unexpected card detect interrupts during data transfer
> which should not happen.
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
>  drivers/mmc/host/sdhci.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 7481bd8..85cbf45 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -136,9 +136,10 @@ static void sdhci_dumpregs(struct sdhci_host *host)
>  static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
>  {
>         u32 present;
> +       int gpio_cd = mmc_gpio_get_cd(host->mmc);
>
>         if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
> -           (host->mmc->caps & MMC_CAP_NONREMOVABLE))
> +           (host->mmc->caps & MMC_CAP_NONREMOVABLE) || !IS_ERR_VALUE(gpio_cd))

If you have a properly working gpio_cd, then why does you also have
SDHCI_QUIRK_BROKEN_CARD_DETECTION set? Isn't that the actual problem?

Kind regards
Uffe

>                 return;
>
>         if (enable) {
> --
> 1.7.8
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Sept. 23, 2014, 8:17 a.m. UTC | #3
On 23 September 2014 09:10, Dong Aisheng <b29396@freescale.com> wrote:
> On Tue, Sep 23, 2014 at 09:27:49AM +0200, Ulf Hansson wrote:
>> On 22 September 2014 11:12, Dong Aisheng <b29396@freescale.com> wrote:
>> > Except SDHCI_QUIRK_BROKEN_CARD_DETECTION and MMC_CAP_NONREMOVABLE,
>> > we also do not need to handle controller native card detect interrupt
>> > for gpio as card detect case.
>> > If we wrong enabled the card detect interrupt for gpio case,
>> > it will cause a lot of unexpected card detect interrupts during data transfer
>> > which should not happen.

I see. But shouldn't you then check for MMC_CAP_NEEDS_POLL instead of
reading the current state of the gpio_cd pin?

Kind regards
Uffe

>> >
>> > Signed-off-by: Dong Aisheng <b29396@freescale.com>
>> > ---
>> >  drivers/mmc/host/sdhci.c |    3 ++-
>> >  1 files changed, 2 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> > index 7481bd8..85cbf45 100644
>> > --- a/drivers/mmc/host/sdhci.c
>> > +++ b/drivers/mmc/host/sdhci.c
>> > @@ -136,9 +136,10 @@ static void sdhci_dumpregs(struct sdhci_host *host)
>> >  static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
>> >  {
>> >         u32 present;
>> > +       int gpio_cd = mmc_gpio_get_cd(host->mmc);
>> >
>> >         if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
>> > -           (host->mmc->caps & MMC_CAP_NONREMOVABLE))
>> > +           (host->mmc->caps & MMC_CAP_NONREMOVABLE) || !IS_ERR_VALUE(gpio_cd))
>>
>> If you have a properly working gpio_cd, then why does you also have
>> SDHCI_QUIRK_BROKEN_CARD_DETECTION set? Isn't that the actual problem?
>>
>
> SDHCI_QUIRK_BROKEN_CARD_DETECTION seems only for controller CD function.
> Thus even we're using gpio_cd, we still have this quirk set for controller
> to avoid enable controller cd interrupts.
> Does it make sense?
>
> Regards
> Dong Aisheng
>
>> Kind regards
>> Uffe
>>
>> >                 return;
>> >
>> >         if (enable) {
>> > --
>> > 1.7.8
>> >
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 7481bd8..85cbf45 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -136,9 +136,10 @@  static void sdhci_dumpregs(struct sdhci_host *host)
 static void sdhci_set_card_detection(struct sdhci_host *host, bool enable)
 {
 	u32 present;
+	int gpio_cd = mmc_gpio_get_cd(host->mmc);
 
 	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) ||
-	    (host->mmc->caps & MMC_CAP_NONREMOVABLE))
+	    (host->mmc->caps & MMC_CAP_NONREMOVABLE) || !IS_ERR_VALUE(gpio_cd))
 		return;
 
 	if (enable) {