mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* (no subject)
@ 2012-09-28 10:28 yongd
  2012-09-28 10:28 ` [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself yongd
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: yongd @ 2012-09-28 10:28 UTC (permalink / raw)
  To: Chris Ball, Anton Vorontsov, Marek Szyprowski, Shawn Guo,
	Wolfram Sang, Daniel Drake, Sascha Hauer, Wilson Callan,
	Ben Dooks
  Cc: zhangfei.gao, kevin.liu, jlfu, linux-mmc, linux-kernel

ngd@marvell.com> # This line is ignored.
From: yongd <yongd@marvell.com>
Subject:[PATCH 0/3] mmc: enable card-detection polling in host driver rather than in sdhci_add_host
In-Reply-To:


Hi all,
As checked, SDHCI_QUICK_BROKEN_CARD_DETECTION was previously introduced by Anton Vorontsov in commit
68d1fb7e229c6f95be4fbbe3eb46b24e41184924. For a removable card (MMC_CAP_NONREMOVABLE is not set),
if this QUICK is set, sdhci_add_host will set MMC_CAP_NEEDS_POLL to enable card-detection polling method,
and sdhci_set_card_detection will not enable host controller card insert/remove interrupts (the host
internal card detection method).

However, we can have some other card detection methods besides host internal card detection and polling
methods. For example, we might use an external GPIO pin for detection. In such case, we will not set
MMC_CAP_NONREMOVABLE since removable, and set SDHCI_QUICK_BROKEN_CARD_DETECTION since we don't use controller
card insert/remove interrupts. But unexpectedly, sdhci_add_host will still enable polling for us. This
is redundant.

So, here comes the following solution.
1st, enable card-detection polling in vendor host driver itself so that it will not rely on sdhci_add_host.
 [PATCH 1/3]: for sdhci-esdhc-imx.c
 [PATCH 2/3]: for sdhci-s3c.c
 And for other vendor host drivers(sdhci-pxav2/3.c, sdhci-pci.c, sdhci-of-esdhc.c), as checked, those who
set SDHCI_QUICK_BROKEN_CARD_DETECTION actually do not set this for enabling polling. So no similar change is          
needed.

2nd, remove MMC_CAP_NEEDS_POLL setting in sdhci_add_host, as in [PATCH 3/3].

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Actually I have had some discussion with Anton, and now added more reviewers here since my proposal influences
others. I will appreciate your review and comments. Thanks a lot:-)
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself
  2012-09-28 10:28 yongd
@ 2012-09-28 10:28 ` yongd
  2012-10-07 14:06   ` Shawn Guo
  2012-09-28 10:28 ` [PATCH 2/3] mmc: sdhci-s3c: " yongd
  2012-09-28 10:28 ` [PATCH 3/3] mmc: remove MMC_CAP_NEEDS_POLL setting in sdhci_add_host yongd
  2 siblings, 1 reply; 10+ messages in thread
From: yongd @ 2012-09-28 10:28 UTC (permalink / raw)
  To: Chris Ball, Anton Vorontsov, Marek Szyprowski, Shawn Guo,
	Wolfram Sang, Daniel Drake, Sascha Hauer, Wilson Callan,
	Ben Dooks
  Cc: zhangfei.gao, kevin.liu, jlfu, linux-mmc, linux-kernel, yongd

In the current code logic, sdhci_add_host() will enable the polling
method (set MMC_CAP_NEEDS_POLL) for a removable card (MMC_CAP_
NONREMOVABLE is not set) whose host's internal card detection method
is disabled for some reason (SDHCI_QUIRK_BROKEN_CARD_DETECTION is set).

However, this is improper since we can have some other card detection
methods besides host internal card detection and polling method. For
example, if the card detection type is ESDHC_CD_GPIO (external gpio pin
for CD), we will keep SDHCI_QUIRK_BROKEN_CARD_DETECTION set. This is right.
But, just as above said, sdhci_add_host() will also enable polling for such
a card. This is redundant.

On the other hand, for the card with ESDHC_CD_NONE detection type(no CD, neither
controller nor gpio, so use polling), we currently rely on sdhci_add_host() to
enable polling for us.

Here proposed a solution for such an embarrassing case. 1st, this patch will
de-couple polling enabling with sdhci_add_host() by doing this in host driver
itself, just as some other vendors. 2nd, one more patch will remove such improper
MMC_CAP_NEEDS_POLL enabling in sdhci_add_host().

Change-Id: Ia7525009d8fd188e3f0169f225e4a76ff9e94b47
Signed-off-by: yongd <yongd@marvell.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index e23f813..f70079c 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -569,6 +569,7 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		break;
 
 	case ESDHC_CD_NONE:
+		host->mmc->caps = MMC_CAP_NEEDS_POLL;
 		break;
 	}
 
-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] mmc: sdhci-s3c: enable polling to detect card by itself
  2012-09-28 10:28 yongd
  2012-09-28 10:28 ` [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself yongd
@ 2012-09-28 10:28 ` yongd
  2012-09-28 10:28 ` [PATCH 3/3] mmc: remove MMC_CAP_NEEDS_POLL setting in sdhci_add_host yongd
  2 siblings, 0 replies; 10+ messages in thread
From: yongd @ 2012-09-28 10:28 UTC (permalink / raw)
  To: Chris Ball, Anton Vorontsov, Marek Szyprowski, Shawn Guo,
	Wolfram Sang, Daniel Drake, Sascha Hauer, Wilson Callan,
	Ben Dooks
  Cc: zhangfei.gao, kevin.liu, jlfu, linux-mmc, linux-kernel, yongd

In the current code logic, sdhci_add_host() will enable the polling
method (set MMC_CAP_NEEDS_POLL) for a removable card (MMC_CAP_
NONREMOVABLE is not set) whose host's internal card detection method
is disabled for some reason (SDHCI_QUIRK_BROKEN_CARD_DETECTION is set).

However, this is improper since we can have some other card detection
methods besides host internal card detection and polling method. As a
result, sdhci_add_host() will redundantly enable polling for a card
which actually uses other methods, eg external GPIO.

So, we plan to remove such improper setting in sdhci_add_host(). But
before this, this patch is a MUST, which enables polling in host driver
itself when the detection type is S3C_SDHCI_CD_NONE.

Change-Id: I28bd6765e6c6a9dd6c288ff4ca6ecf7268a2d8ed
Signed-off-by: yongd <yongd@marvell.com>
---
 drivers/mmc/host/sdhci-s3c.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 2903949..0701f08 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -687,6 +687,9 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	    pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
 		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
 
+	if (pdata->cd_type == S3C_SDHCI_CD_NONE)
+		host->mmc->caps = MMC_CAP_NEEDS_POLL;
+
 	if (pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
 		host->mmc->caps = MMC_CAP_NONREMOVABLE;
 
-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] mmc: remove MMC_CAP_NEEDS_POLL setting in sdhci_add_host
  2012-09-28 10:28 yongd
  2012-09-28 10:28 ` [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself yongd
  2012-09-28 10:28 ` [PATCH 2/3] mmc: sdhci-s3c: " yongd
@ 2012-09-28 10:28 ` yongd
  2 siblings, 0 replies; 10+ messages in thread
From: yongd @ 2012-09-28 10:28 UTC (permalink / raw)
  To: Chris Ball, Anton Vorontsov, Marek Szyprowski, Shawn Guo,
	Wolfram Sang, Daniel Drake, Sascha Hauer, Wilson Callan,
	Ben Dooks
  Cc: zhangfei.gao, kevin.liu, jlfu, linux-mmc, linux-kernel, yongd

In the current code logic, sdhci_add_host() will enable the polling
method (set MMC_CAP_NEEDS_POLL) for a removable card (MMC_CAP_
NONREMOVABLE is not set) whose host's internal card detection method
is disabled for some reason (SDHCI_QUIRK_BROKEN_CARD_DETECTION is set).

However, this is improper since we can have some other card detection
methods besides host internal card detection and the polling method.
For example, we might use an external GPIO pin's interrupt to detect
a removable SD card, so we shall set SDHCI_QUIRK_BROKEN_CARD_DETECTION
since we don't use the internal card detection, and not set MMC_CAP_
NONREMOVABLE since the SD card is physically removable. As a result,
with the current code, the polling method will also be enabled.
Apparently, this is redundant and not what we want.

And the better one to decide whether we use polling or not should be
the host driver itself. Actually, some host driver has already been
like this. Eg, in drivers/mmc/host/Au1xmmc.c, polling will be enabled
only after the board-specific card detection can't be set up successfully.

Change-Id: I27774488a7b9191d7bc39699fd7d62ee21bbf157
Signed-off-by: yongd <yongd@marvell.com>
---
 drivers/mmc/host/sdhci.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e15c79..900d5f4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2840,10 +2840,6 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (caps[0] & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
 
-	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
-	    !(host->mmc->caps & MMC_CAP_NONREMOVABLE))
-		mmc->caps |= MMC_CAP_NEEDS_POLL;
-
 	/* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
 	host->vqmmc = regulator_get(mmc_dev(mmc), "vqmmc");
 	if (IS_ERR(host->vqmmc)) {
-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself
  2012-09-28 10:28 ` [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself yongd
@ 2012-10-07 14:06   ` Shawn Guo
  2012-10-17  4:01     ` Yong Ding
  0 siblings, 1 reply; 10+ messages in thread
From: Shawn Guo @ 2012-10-07 14:06 UTC (permalink / raw)
  To: yongd
  Cc: Chris Ball, Anton Vorontsov, Marek Szyprowski, Wolfram Sang,
	Daniel Drake, Sascha Hauer, Wilson Callan, Ben Dooks,
	zhangfei.gao, kevin.liu, jlfu, linux-mmc, linux-kernel

On Fri, Sep 28, 2012 at 06:28:31PM +0800, yongd wrote:
> In the current code logic, sdhci_add_host() will enable the polling
> method (set MMC_CAP_NEEDS_POLL) for a removable card (MMC_CAP_
> NONREMOVABLE is not set) whose host's internal card detection method
> is disabled for some reason (SDHCI_QUIRK_BROKEN_CARD_DETECTION is set).
> 
> However, this is improper since we can have some other card detection
> methods besides host internal card detection and polling method. For
> example, if the card detection type is ESDHC_CD_GPIO (external gpio pin
> for CD), we will keep SDHCI_QUIRK_BROKEN_CARD_DETECTION set. This is right.
> But, just as above said, sdhci_add_host() will also enable polling for such
> a card. This is redundant.
> 
At least for sdhci-esdhc-imx, SDHCI_QUIRK_BROKEN_CARD_DETECTION will
be set only when neither ESDHC_CD_GPIO nor ESDHC_CD_CONTROLLER works.

Shawn

> On the other hand, for the card with ESDHC_CD_NONE detection type(no CD, neither
> controller nor gpio, so use polling), we currently rely on sdhci_add_host() to
> enable polling for us.
> 
> Here proposed a solution for such an embarrassing case. 1st, this patch will
> de-couple polling enabling with sdhci_add_host() by doing this in host driver
> itself, just as some other vendors. 2nd, one more patch will remove such improper
> MMC_CAP_NEEDS_POLL enabling in sdhci_add_host().
> 
> Change-Id: Ia7525009d8fd188e3f0169f225e4a76ff9e94b47
> Signed-off-by: yongd <yongd@marvell.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index e23f813..f70079c 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -569,6 +569,7 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  		break;
>  
>  	case ESDHC_CD_NONE:
> +		host->mmc->caps = MMC_CAP_NEEDS_POLL;
>  		break;
>  	}
>  
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself
  2012-10-07 14:06   ` Shawn Guo
@ 2012-10-17  4:01     ` Yong Ding
  2012-10-18  5:50       ` Shawn Guo
  0 siblings, 1 reply; 10+ messages in thread
From: Yong Ding @ 2012-10-17  4:01 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Chris Ball, Anton Vorontsov, Marek Szyprowski, Wolfram Sang,
	Daniel Drake, Sascha Hauer, Wilson Callan, Ben Dooks,
	Zhangfei Gao, Kevin Liu, Jialing Fu, linux-mmc, linux-kernel

Shawn,
Thanks for your comment. And sorry for my so late due to illness:-)
SDHCI_QUIRK_BROKEN_CARD_DETECTION is used for notifying we don't use the host internal card detection method so that we don't need enable/disable those relevant interrupt bits of host(sdhci_set_card_detection in sdhci.c). 
And as I double-checked the latest kernel code, actually sdhci-esdhc-imx sets this flag by default, and then will clear it only when the detection type is ESDHC_CD_CONTROLLER. So this aligns with my understanding. 
What I want to do is that 1st we shall set MMC_CAP_NEEDS_POLL by our host driver itself and 2nd remove the improper logic in sdhci_add_host() . How do u think? Thanks.

________________________________________
From: Shawn Guo [shawn.guo@linaro.org]
Sent: Sunday, October 07, 2012 22:06
To: Yong Ding
Cc: Chris Ball; Anton Vorontsov; Marek Szyprowski; Wolfram Sang; Daniel Drake; Sascha Hauer; Wilson Callan; Ben Dooks; Zhangfei Gao; Kevin Liu; Jialing Fu; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself

On Fri, Sep 28, 2012 at 06:28:31PM +0800, yongd wrote:
> In the current code logic, sdhci_add_host() will enable the polling
> method (set MMC_CAP_NEEDS_POLL) for a removable card (MMC_CAP_
> NONREMOVABLE is not set) whose host's internal card detection method
> is disabled for some reason (SDHCI_QUIRK_BROKEN_CARD_DETECTION is set).
>
> However, this is improper since we can have some other card detection
> methods besides host internal card detection and polling method. For
> example, if the card detection type is ESDHC_CD_GPIO (external gpio pin
> for CD), we will keep SDHCI_QUIRK_BROKEN_CARD_DETECTION set. This is right.
> But, just as above said, sdhci_add_host() will also enable polling for such
> a card. This is redundant.
>
At least for sdhci-esdhc-imx, SDHCI_QUIRK_BROKEN_CARD_DETECTION will
be set only when neither ESDHC_CD_GPIO nor ESDHC_CD_CONTROLLER works.

Shawn

> On the other hand, for the card with ESDHC_CD_NONE detection type(no CD, neither
> controller nor gpio, so use polling), we currently rely on sdhci_add_host() to
> enable polling for us.
>
> Here proposed a solution for such an embarrassing case. 1st, this patch will
> de-couple polling enabling with sdhci_add_host() by doing this in host driver
> itself, just as some other vendors. 2nd, one more patch will remove such improper
> MMC_CAP_NEEDS_POLL enabling in sdhci_add_host().
>
> Change-Id: Ia7525009d8fd188e3f0169f225e4a76ff9e94b47
> Signed-off-by: yongd <yongd@marvell.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index e23f813..f70079c 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -569,6 +569,7 @@ static int __devinit sdhci_esdhc_imx_probe(struct platform_device *pdev)
>               break;
>
>       case ESDHC_CD_NONE:
> +             host->mmc->caps = MMC_CAP_NEEDS_POLL;
>               break;
>       }
>
> --
> 1.7.9.5
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself
  2012-10-17  4:01     ` Yong Ding
@ 2012-10-18  5:50       ` Shawn Guo
  2012-10-18  6:27         ` Yong Ding
  0 siblings, 1 reply; 10+ messages in thread
From: Shawn Guo @ 2012-10-18  5:50 UTC (permalink / raw)
  To: Yong Ding
  Cc: Chris Ball, Anton Vorontsov, Marek Szyprowski, Wolfram Sang,
	Daniel Drake, Sascha Hauer, Wilson Callan, Ben Dooks,
	Zhangfei Gao, Kevin Liu, Jialing Fu, linux-mmc, linux-kernel

On Tue, Oct 16, 2012 at 09:01:40PM -0700, Yong Ding wrote:
> Shawn,
> Thanks for your comment. And sorry for my so late due to illness:-)
> SDHCI_QUIRK_BROKEN_CARD_DETECTION is used for notifying we don't use the host internal card detection method so that we don't need enable/disable those relevant interrupt bits of host(sdhci_set_card_detection in sdhci.c). 
> And as I double-checked the latest kernel code, actually sdhci-esdhc-imx sets this flag by default, and then will clear it only when the detection type is ESDHC_CD_CONTROLLER. So this aligns with my understanding. 

What I was saying is the bit will be cleared when the detection type is
ESDHC_CD_CONTROLLE or ESDHC_CD_GPIO.  You may have missed the fact that
there is no "break" in case ESDHC_CD_GPIO but a "fall through" comment.

        switch (boarddata->cd_type) {
        case ESDHC_CD_GPIO:
                err = gpio_request_one(boarddata->cd_gpio, GPIOF_IN, "ESDHC_CD");
                if (err) {
                        dev_err(mmc_dev(host->mmc),
                                "no card-detect pin available!\n");
                        goto no_card_detect_pin;
                }

                err = request_irq(gpio_to_irq(boarddata->cd_gpio), cd_irq,
                                 IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
                                 mmc_hostname(host->mmc), host);
                if (err) {
                        dev_err(mmc_dev(host->mmc), "request irq error\n");
                        goto no_card_detect_irq;
                }
                /* fall through */

        case ESDHC_CD_CONTROLLER:
                /* we have a working card_detect back */
                host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
                break;

        case ESDHC_CD_PERMANENT:
                host->mmc->caps = MMC_CAP_NONREMOVABLE;
                break;

        case ESDHC_CD_NONE:
                break;
        }

Shawn

> What I want to do is that 1st we shall set MMC_CAP_NEEDS_POLL by our host driver itself and 2nd remove the improper logic in sdhci_add_host() . How do u think? Thanks.
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself
  2012-10-18  5:50       ` Shawn Guo
@ 2012-10-18  6:27         ` Yong Ding
  2012-10-18  6:38           ` Shawn Guo
  0 siblings, 1 reply; 10+ messages in thread
From: Yong Ding @ 2012-10-18  6:27 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Chris Ball, Anton Vorontsov, Marek Szyprowski, Wolfram Sang,
	Daniel Drake, Sascha Hauer, Wilson Callan, Ben Dooks,
	Zhangfei Gao, Kevin Liu, Jialing Fu, linux-mmc, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 3369 bytes --]

Shawn,
Thanks. Oh, sorry I really have missed the fact u mentioned. U are right in the current code, the bit will also be cleared for ESDHC_CD_GPIO. 
But I think this is improper since for GPIO detection type, we don't use the host controller internal card detection(ESDHC_CD_CONTROLLER), but with SDHCI_QUIRK_BROKEN_CARD_DETECTION cleared, we'll still enable/disable relevant INT bits (in sdhci_set_card_detection in sdhci.c). This is my biggest concern. And I think the SDHCI_QUIRK_BROKEN_CARD_DETECTION shall be purely used to notify whether the host controller detection method is used or not. So even for the ESDHC_CD_GPIO type, we should still set this flag. How do u think?

-----Original Message-----
From: Shawn Guo [mailto:shawn.guo@linaro.org] 
Sent: 2012Äê10ÔÂ18ÈÕ 13:51
To: Yong Ding
Cc: Chris Ball; Anton Vorontsov; Marek Szyprowski; Wolfram Sang; Daniel Drake; Sascha Hauer; Wilson Callan; Ben Dooks; Zhangfei Gao; Kevin Liu; Jialing Fu; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself

On Tue, Oct 16, 2012 at 09:01:40PM -0700, Yong Ding wrote:
> Shawn,
> Thanks for your comment. And sorry for my so late due to illness:-)
> SDHCI_QUIRK_BROKEN_CARD_DETECTION is used for notifying we don't use the host internal card detection method so that we don't need enable/disable those relevant interrupt bits of host(sdhci_set_card_detection in sdhci.c). 
> And as I double-checked the latest kernel code, actually sdhci-esdhc-imx sets this flag by default, and then will clear it only when the detection type is ESDHC_CD_CONTROLLER. So this aligns with my understanding. 

What I was saying is the bit will be cleared when the detection type is
ESDHC_CD_CONTROLLE or ESDHC_CD_GPIO.  You may have missed the fact that
there is no "break" in case ESDHC_CD_GPIO but a "fall through" comment.

        switch (boarddata->cd_type) {
        case ESDHC_CD_GPIO:
                err = gpio_request_one(boarddata->cd_gpio, GPIOF_IN, "ESDHC_CD");
                if (err) {
                        dev_err(mmc_dev(host->mmc),
                                "no card-detect pin available!\n");
                        goto no_card_detect_pin;
                }

                err = request_irq(gpio_to_irq(boarddata->cd_gpio), cd_irq,
                                 IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
                                 mmc_hostname(host->mmc), host);
                if (err) {
                        dev_err(mmc_dev(host->mmc), "request irq error\n");
                        goto no_card_detect_irq;
                }
                /* fall through */

        case ESDHC_CD_CONTROLLER:
                /* we have a working card_detect back */
                host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
                break;

        case ESDHC_CD_PERMANENT:
                host->mmc->caps = MMC_CAP_NONREMOVABLE;
                break;

        case ESDHC_CD_NONE:
                break;
        }

Shawn

> What I want to do is that 1st we shall set MMC_CAP_NEEDS_POLL by our host driver itself and 2nd remove the improper logic in sdhci_add_host() . How do u think? Thanks.
> 
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself
  2012-10-18  6:27         ` Yong Ding
@ 2012-10-18  6:38           ` Shawn Guo
  2012-10-18  7:19             ` Yong Ding
  0 siblings, 1 reply; 10+ messages in thread
From: Shawn Guo @ 2012-10-18  6:38 UTC (permalink / raw)
  To: Yong Ding
  Cc: Chris Ball, Anton Vorontsov, Marek Szyprowski, Wolfram Sang,
	Daniel Drake, Sascha Hauer, Wilson Callan, Ben Dooks,
	Zhangfei Gao, Kevin Liu, Jialing Fu, linux-mmc, linux-kernel

Can you fix your mailer to use bottom posting rather than top posting,
and have texts wrap around column 70?  Otherwise, you message stands
a good chance to be ignored by people.

On Wed, Oct 17, 2012 at 11:27:17PM -0700, Yong Ding wrote:
> Shawn,
> Thanks. Oh, sorry I really have missed the fact u mentioned. U are right in the current code, the bit will also be cleared for ESDHC_CD_GPIO. 
> But I think this is improper since for GPIO detection type, we don't use the host controller internal card detection(ESDHC_CD_CONTROLLER), but with SDHCI_QUIRK_BROKEN_CARD_DETECTION cleared, we'll still enable/disable relevant INT bits (in sdhci_set_card_detection in sdhci.c). This is my biggest concern. And I think the SDHCI_QUIRK_BROKEN_CARD_DETECTION shall be purely used to notify whether the host controller detection method is used or not. So even for the ESDHC_CD_GPIO type, we should still set this flag. How do u think?
> 
I'm fine with that.  Just remind you a fact you seem missed in case
you need to change sdhci-esdhc-imx.c to adapt the changes you are going
to make on sdhci.c.

Shawn 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself
  2012-10-18  6:38           ` Shawn Guo
@ 2012-10-18  7:19             ` Yong Ding
  0 siblings, 0 replies; 10+ messages in thread
From: Yong Ding @ 2012-10-18  7:19 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Chris Ball, Anton Vorontsov, Marek Szyprowski, Wolfram Sang,
	Daniel Drake, Sascha Hauer, Wilson Callan, Ben Dooks,
	Zhangfei Gao, Kevin Liu, Jialing Fu, linux-mmc, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 1753 bytes --]

Shawn,
Thanks, I will update the patch for sdhci-esdhc-imx.c and also fix my mail ASAP:-)

-----Original Message-----
From: Shawn Guo [mailto:shawn.guo@linaro.org] 
Sent: 2012Äê10ÔÂ18ÈÕ 14:38
To: Yong Ding
Cc: Chris Ball; Anton Vorontsov; Marek Szyprowski; Wolfram Sang; Daniel Drake; Sascha Hauer; Wilson Callan; Ben Dooks; Zhangfei Gao; Kevin Liu; Jialing Fu; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself

Can you fix your mailer to use bottom posting rather than top posting,
and have texts wrap around column 70?  Otherwise, you message stands
a good chance to be ignored by people.

On Wed, Oct 17, 2012 at 11:27:17PM -0700, Yong Ding wrote:
> Shawn,
> Thanks. Oh, sorry I really have missed the fact u mentioned. U are right in the current code, the bit will also be cleared for ESDHC_CD_GPIO. 
> But I think this is improper since for GPIO detection type, we don't use the host controller internal card detection(ESDHC_CD_CONTROLLER), but with SDHCI_QUIRK_BROKEN_CARD_DETECTION cleared, we'll still enable/disable relevant INT bits (in sdhci_set_card_detection in sdhci.c). This is my biggest concern. And I think the SDHCI_QUIRK_BROKEN_CARD_DETECTION shall be purely used to notify whether the host controller detection method is used or not. So even for the ESDHC_CD_GPIO type, we should still set this flag. How do u think?
> 
I'm fine with that.  Just remind you a fact you seem missed in case
you need to change sdhci-esdhc-imx.c to adapt the changes you are going
to make on sdhci.c.

Shawn 
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-10-18  7:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-28 10:28 yongd
2012-09-28 10:28 ` [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself yongd
2012-10-07 14:06   ` Shawn Guo
2012-10-17  4:01     ` Yong Ding
2012-10-18  5:50       ` Shawn Guo
2012-10-18  6:27         ` Yong Ding
2012-10-18  6:38           ` Shawn Guo
2012-10-18  7:19             ` Yong Ding
2012-09-28 10:28 ` [PATCH 2/3] mmc: sdhci-s3c: " yongd
2012-09-28 10:28 ` [PATCH 3/3] mmc: remove MMC_CAP_NEEDS_POLL setting in sdhci_add_host yongd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®