diff mbox

[v2] mmc: sdhci-s3c: fix the card detection in runtime-pm

Message ID 201210181302.30843.heiko@sntech.de (mailing list archive)
State New, archived
Headers show

Commit Message

Heiko Stübner Oct. 18, 2012, 11:02 a.m. UTC 
If host clock is disabled, host cannot detect a card
in case of using the internal or gpio card-detect for detection.

Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---
I've added the SDHCI_CD_GPIO to the conditional. With this change it works
on my machine. But I'm not sure if this would also be necessary for the 
external card detect.

 drivers/mmc/host/sdhci-s3c.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

Comments

Seungwon Jeon Oct. 19, 2012, 8:04 a.m. UTC | #1 
On Thursday, October 18, 2012, Heiko Stuebner <heiko@sntech.de>
> If host clock is disabled, host cannot detect a card
> in case of using the internal or gpio card-detect for detection.
> 
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> ---
> I've added the SDHCI_CD_GPIO to the conditional. With this change it works
> on my machine. But I'm not sure if this would also be necessary for the
> external card detect.

Card detection will be asserted out of host in case of using SDHCI_CD_GPIO.
Adding SDHCI_CD_GPIO to the conditional seems unnecessary.

Thanks.
Seungwon Jeon

> 
>  drivers/mmc/host/sdhci-s3c.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 0cabf18..649dc32 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -750,7 +750,9 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  		sdhci_s3c_setup_card_detect_gpio(sc);
> 
>  #ifdef CONFIG_PM_RUNTIME
> -	clk_disable(sc->clk_io);
> +	if (pdata->cd_type != S3C_SDHCI_CD_INTERNAL &&
> +	    pdata->cd_type != S3C_SDHCI_CD_GPIO)
> +		clk_disable(sc->clk_io);
>  #endif
>  	return 0;
> 
> @@ -797,7 +799,9 @@ static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  		gpio_free(sc->ext_cd_gpio);
> 
>  #ifdef CONFIG_PM_RUNTIME
> -	clk_enable(sc->clk_io);
> +	if (pdata->cd_type != S3C_SDHCI_CD_INTERNAL &&
> +	    pdata->cd_type != S3C_SDHCI_CD_GPIO)
> +		clk_enable(sc->clk_io);
>  #endif
>  	sdhci_remove_host(host, 1);
> 
> --
> 1.7.2.3
> 
> --
> 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

--
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
Heiko Stübner Oct. 19, 2012, 8:10 a.m. UTC | #2 
Am Freitag, 19. Oktober 2012, 10:04:14 schrieb Seungwon Jeon:
> On Thursday, October 18, 2012, Heiko Stuebner <heiko@sntech.de>
> 
> > If host clock is disabled, host cannot detect a card
> > in case of using the internal or gpio card-detect for detection.
> > 
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > Tested-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > I've added the SDHCI_CD_GPIO to the conditional. With this change it
> > works on my machine. But I'm not sure if this would also be necessary
> > for the external card detect.
> 
> Card detection will be asserted out of host in case of using SDHCI_CD_GPIO.
> Adding SDHCI_CD_GPIO to the conditional seems unnecessary.

But it _was_ necessary :-) . Because only then did the problem go away. You 
might be right, that there exists a better solution for this, but something in 
the original patch is at least still missing to fix the problem.

Heiko


> >  drivers/mmc/host/sdhci-s3c.c |    8 ++++++--
> >  1 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> > index 0cabf18..649dc32 100644
> > --- a/drivers/mmc/host/sdhci-s3c.c
> > +++ b/drivers/mmc/host/sdhci-s3c.c
> > @@ -750,7 +750,9 @@ static int __devinit sdhci_s3c_probe(struct
> > platform_device *pdev)
> > 
> >  		sdhci_s3c_setup_card_detect_gpio(sc);
> >  
> >  #ifdef CONFIG_PM_RUNTIME
> > 
> > -	clk_disable(sc->clk_io);
> > +	if (pdata->cd_type != S3C_SDHCI_CD_INTERNAL &&
> > +	    pdata->cd_type != S3C_SDHCI_CD_GPIO)
> > +		clk_disable(sc->clk_io);
> > 
> >  #endif
> >  
> >  	return 0;
> > 
> > @@ -797,7 +799,9 @@ static int __devexit sdhci_s3c_remove(struct
> > platform_device *pdev)
> > 
> >  		gpio_free(sc->ext_cd_gpio);
> >  
> >  #ifdef CONFIG_PM_RUNTIME
> > 
> > -	clk_enable(sc->clk_io);
> > +	if (pdata->cd_type != S3C_SDHCI_CD_INTERNAL &&
> > +	    pdata->cd_type != S3C_SDHCI_CD_GPIO)
> > +		clk_enable(sc->clk_io);
> > 
> >  #endif
> >  
> >  	sdhci_remove_host(host, 1);
> > 
> > --
> > 1.7.2.3
> > 
> > --
> > 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

--
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
Chris Ball Oct. 29, 2012, 9:16 p.m. UTC | #3 
Hi,

On Fri, Oct 19 2012, Heiko Stübner wrote:
> Am Freitag, 19. Oktober 2012, 10:04:14 schrieb Seungwon Jeon:
>> On Thursday, October 18, 2012, Heiko Stuebner <heiko@sntech.de>
>> 
>> > If host clock is disabled, host cannot detect a card
>> > in case of using the internal or gpio card-detect for detection.
>> > 
>> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>> > Tested-by: Heiko Stuebner <heiko@sntech.de>
>> > ---
>> > I've added the SDHCI_CD_GPIO to the conditional. With this change it
>> > works on my machine. But I'm not sure if this would also be necessary
>> > for the external card detect.
>> 
>> Card detection will be asserted out of host in case of using SDHCI_CD_GPIO.
>> Adding SDHCI_CD_GPIO to the conditional seems unnecessary.
>
> But it _was_ necessary :-) . Because only then did the problem go away. You 
> might be right, that there exists a better solution for this, but something in 
> the original patch is at least still missing to fix the problem.

Seungwon, what would you like to do here?  (I'm unhappy taking a patch
with your name on it that's been modified in a way you disagree with.)

Thanks,

- Chris.
Heiko Stübner Oct. 29, 2012, 10:37 p.m. UTC | #4 
Hi Chris,

Am Montag, 29. Oktober 2012, 22:16:47 schrieb Chris Ball:
> Hi,
> 
> On Fri, Oct 19 2012, Heiko Stübner wrote:
> > Am Freitag, 19. Oktober 2012, 10:04:14 schrieb Seungwon Jeon:
> >> On Thursday, October 18, 2012, Heiko Stuebner <heiko@sntech.de>
> >> 
> >> > If host clock is disabled, host cannot detect a card
> >> > in case of using the internal or gpio card-detect for detection.
> >> > 
> >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> >> > Tested-by: Heiko Stuebner <heiko@sntech.de>
> >> > ---
> >> > I've added the SDHCI_CD_GPIO to the conditional. With this change it
> >> > works on my machine. But I'm not sure if this would also be necessary
> >> > for the external card detect.
> >> 
> >> Card detection will be asserted out of host in case of using
> >> SDHCI_CD_GPIO. Adding SDHCI_CD_GPIO to the conditional seems
> >> unnecessary.
> > 
> > But it _was_ necessary :-) . Because only then did the problem go away.
> > You might be right, that there exists a better solution for this, but
> > something in the original patch is at least still missing to fix the
> > problem.
> 
> Seungwon, what would you like to do here?  (I'm unhappy taking a patch
> with your name on it that's been modified in a way you disagree with.)

I agree with Seungwon, that the solution in my v2 is not the correct one :-) - 
should've called it RFC or so. The gpio-card-detect code _should_ be able to 
handle the resume of the host itself in the card detect case, but it seems 
it's not able to.

The patch in its v1 form does not fix the problem commit 2abeb5c5ded2 (mmc: 
sdhci-s3c: Add clk_(enable/disable) in runtime suspend/resume) has caused for 
Samsung devices using the gpio card-detect - it's broken in 3.7 currently.


So for 3.7 the easiest way would of course be to revert the commit above and 
find a correct solution for 3.8.


Heiko
--
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
Seungwon Jeon Oct. 30, 2012, 3:54 a.m. UTC | #5 
Tuesday, October 30, 2012, Heiko Stuebner <heiko@sntech.de> wrote:
> Hi Chris,
> 
> Am Montag, 29. Oktober 2012, 22:16:47 schrieb Chris Ball:
> > Hi,
> >
> > On Fri, Oct 19 2012, Heiko Stübner wrote:
> > > Am Freitag, 19. Oktober 2012, 10:04:14 schrieb Seungwon Jeon:
> > >> On Thursday, October 18, 2012, Heiko Stuebner <heiko@sntech.de>
> > >>
> > >> > If host clock is disabled, host cannot detect a card
> > >> > in case of using the internal or gpio card-detect for detection.
> > >> >
> > >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > >> > Tested-by: Heiko Stuebner <heiko@sntech.de>
> > >> > ---
> > >> > I've added the SDHCI_CD_GPIO to the conditional. With this change it
> > >> > works on my machine. But I'm not sure if this would also be necessary
> > >> > for the external card detect.
> > >>
> > >> Card detection will be asserted out of host in case of using
> > >> SDHCI_CD_GPIO. Adding SDHCI_CD_GPIO to the conditional seems
> > >> unnecessary.
> > >
> > > But it _was_ necessary :-) . Because only then did the problem go away.
> > > You might be right, that there exists a better solution for this, but
> > > something in the original patch is at least still missing to fix the
> > > problem.
> >
> > Seungwon, what would you like to do here?  (I'm unhappy taking a patch
> > with your name on it that's been modified in a way you disagree with.)
> 
> I agree with Seungwon, that the solution in my v2 is not the correct one :-) -
> should've called it RFC or so. The gpio-card-detect code _should_ be able to
> handle the resume of the host itself in the card detect case, but it seems
> it's not able to.
> 
> The patch in its v1 form does not fix the problem commit 2abeb5c5ded2 (mmc:
> sdhci-s3c: Add clk_(enable/disable) in runtime suspend/resume) has caused for
> Samsung devices using the gpio card-detect - it's broken in 3.7 currently.
> 
> 
> So for 3.7 the easiest way would of course be to revert the commit above and
> find a correct solution for 3.8.

Hi Chris,

This patch doesn't aim Heiko's problem but guarantee card detection for using internal CD.
Clock should be enabled for internal card detection of host.

Thanks,
Seungwon Jeon
> 
> 
> Heiko
> --
> 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

--
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-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 0cabf18..649dc32 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -750,7 +750,9 @@  static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 		sdhci_s3c_setup_card_detect_gpio(sc);
 
 #ifdef CONFIG_PM_RUNTIME
-	clk_disable(sc->clk_io);
+	if (pdata->cd_type != S3C_SDHCI_CD_INTERNAL &&
+	    pdata->cd_type != S3C_SDHCI_CD_GPIO)
+		clk_disable(sc->clk_io);
 #endif
 	return 0;
 
@@ -797,7 +799,9 @@  static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 		gpio_free(sc->ext_cd_gpio);
 
 #ifdef CONFIG_PM_RUNTIME
-	clk_enable(sc->clk_io);
+	if (pdata->cd_type != S3C_SDHCI_CD_INTERNAL &&
+	    pdata->cd_type != S3C_SDHCI_CD_GPIO)
+		clk_enable(sc->clk_io);
 #endif
 	sdhci_remove_host(host, 1);