[PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

[PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

[Tony Lin]

1ms is enough for hardware to change the clock to stable.
100ms is too long in the tasklet.

Signed-off-by: Tony Lin <tony.lin@freescale.com>
CC: Xiaobo Xie <X.Xie@freescale.com>
CC: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci-esdhc.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index c3b08f1..b97b2f5 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -73,7 +73,7 @@ static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock)
 		| (div << ESDHC_DIVIDER_SHIFT)
 		| (pre_div << ESDHC_PREDIV_SHIFT));
 	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
-	mdelay(100);
+	mdelay(1);
 out:
 	host->clock = clock;
 }
-- 
1.7.0.4

[PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

[Chris Ball]

Hi,

On Tue, Nov 22 2011, Tony Lin wrote:
> 1ms is enough for hardware to change the clock to stable.
> 100ms is too long in the tasklet.
>
> Signed-off-by: Tony Lin <tony.lin@freescale.com>
> CC: Xiaobo Xie <X.Xie@freescale.com>
> CC: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>  drivers/mmc/host/sdhci-esdhc.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
> index c3b08f1..b97b2f5 100644
> --- a/drivers/mmc/host/sdhci-esdhc.h
> +++ b/drivers/mmc/host/sdhci-esdhc.h
> @@ -73,7 +73,7 @@ static inline void esdhc_set_clock(struct sdhci_host
> *host, unsigned int clock)
>  		| (div << ESDHC_DIVIDER_SHIFT)
>  		| (pre_div << ESDHC_PREDIV_SHIFT));
>  	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
> -	mdelay(100);
> +	mdelay(1);
>  out:
>  	host->clock = clock;
>  }

I don't know if 1ms is actually long enough for the clock to stabilize
on all boards, but I'll push this change to mmc-next and we can see if
we get any regression reports.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

[Lin Tony-B19295]

Okay
Thanks

> -----Original Message-----
> From: linux-mmc-owner at vger.kernel.org [mailto:linux-mmc-
> owner at vger.kernel.org] On Behalf Of Chris Ball
> Sent: Friday, December 02, 2011 2:11 AM
> To: Lin Tony-B19295
> Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Xie
> Xiaobo-R63061; avorontsov at ru.mvista.com
> Subject: Re: [PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting
> clock from 100ms to 1ms
> 
> Hi,
> 
> On Tue, Nov 22 2011, Tony Lin wrote:
> > 1ms is enough for hardware to change the clock to stable.
> > 100ms is too long in the tasklet.
> >
> > Signed-off-by: Tony Lin <tony.lin@freescale.com>
> > CC: Xiaobo Xie <X.Xie@freescale.com>
> > CC: Anton Vorontsov <avorontsov@ru.mvista.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc.h |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc.h
> > b/drivers/mmc/host/sdhci-esdhc.h index c3b08f1..b97b2f5 100644
> > --- a/drivers/mmc/host/sdhci-esdhc.h
> > +++ b/drivers/mmc/host/sdhci-esdhc.h
> > @@ -73,7 +73,7 @@ static inline void esdhc_set_clock(struct sdhci_host
> > *host, unsigned int clock)
> >  		| (div << ESDHC_DIVIDER_SHIFT)
> >  		| (pre_div << ESDHC_PREDIV_SHIFT));
> >  	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
> > -	mdelay(100);
> > +	mdelay(1);
> >  out:
> >  	host->clock = clock;
> >  }
> 
> I don't know if 1ms is actually long enough for the clock to stabilize on
> all boards, but I'll push this change to mmc-next and we can see if we
> get any regression reports.
> 
> Thanks,
> 
> - Chris.
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

[Huang Changming-R66093]

I don't think it make sense to change the delay time.
Which will affect all PowerPC eSDHC controller, could you confirm which can work on all PowerPC platform?

-----Original Message-----
From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-owner at vger.kernel.org] On Behalf Of Lin Tony-B19295
Sent: Friday, December 02, 2011 1:37 PM
To: Chris Ball
Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Xie Xiaobo-R63061; avorontsov at ru.mvista.com
Subject: RE: [PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

Okay
Thanks

> -----Original Message-----
> From: linux-mmc-owner at vger.kernel.org [mailto:linux-mmc- 
> owner at vger.kernel.org] On Behalf Of Chris Ball
> Sent: Friday, December 02, 2011 2:11 AM
> To: Lin Tony-B19295
> Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org; 
> Xie Xiaobo-R63061; avorontsov at ru.mvista.com
> Subject: Re: [PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting 
> clock from 100ms to 1ms
> 
> Hi,
> 
> On Tue, Nov 22 2011, Tony Lin wrote:
> > 1ms is enough for hardware to change the clock to stable.
> > 100ms is too long in the tasklet.
> >
> > Signed-off-by: Tony Lin <tony.lin@freescale.com>
> > CC: Xiaobo Xie <X.Xie@freescale.com>
> > CC: Anton Vorontsov <avorontsov@ru.mvista.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc.h |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc.h 
> > b/drivers/mmc/host/sdhci-esdhc.h index c3b08f1..b97b2f5 100644
> > --- a/drivers/mmc/host/sdhci-esdhc.h
> > +++ b/drivers/mmc/host/sdhci-esdhc.h
> > @@ -73,7 +73,7 @@ static inline void esdhc_set_clock(struct 
> > sdhci_host *host, unsigned int clock)
> >  		| (div << ESDHC_DIVIDER_SHIFT)
> >  		| (pre_div << ESDHC_PREDIV_SHIFT));
> >  	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
> > -	mdelay(100);
> > +	mdelay(1);
> >  out:
> >  	host->clock = clock;
> >  }
> 
> I don't know if 1ms is actually long enough for the clock to stabilize 
> on all boards, but I'll push this change to mmc-next and we can see if 
> we get any regression reports.
> 
> Thanks,
> 
> - Chris.
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" 
> in the body of a message to majordomo at 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 at vger.kernel.org More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

[Chris Ball]

Hi,

On Fri, Dec 02 2011, Huang Changming-R66093 wrote:
> I don't think it make sense to change the delay time.
> Which will affect all PowerPC eSDHC controller, could you confirm
> which can work on all PowerPC platform?

No, of course not, but that's why I said we can put the patch in
mmc-next and see if anyone complains; mdelay(100) is clearly broken.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

[Tony Lin]

1ms is enough for hardware to change the clock to stable.
100ms is too long.

Signed-off-by: Tony Lin <tony.lin@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index c3b08f1..b97b2f5 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -73,7 +73,7 @@ static inline void esdhc_set_clock(struct sdhci_host *host, unsigned int clock)
 		| (div << ESDHC_DIVIDER_SHIFT)
 		| (pre_div << ESDHC_PREDIV_SHIFT));
 	sdhci_writel(host, temp, ESDHC_SYSTEM_CONTROL);
-	mdelay(100);
+	mdelay(1);
 out:
 	host->clock = clock;
 }
-- 
1.7.0.4

[PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

[Wolfram Sang]

On Thu, Sep 01, 2011 at 01:51:15PM +0800, Tony Lin wrote:
> 1ms is enough for hardware to change the clock to stable.
> 100ms is too long.

How do you know that? Can you be sure for PowerPC as well? Have you researched
why the original author of the code has chosen that value? If so, please update
your commit message with such infos.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110901/5ae74122/attachment.sig>

[PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting clock from 100ms to 1ms

[Lin Tony-B19295]

> -----Original Message-----
> From: Wolfram Sang [mailto:w.sang at pengutronix.de]
> Sent: Thursday, September 01, 2011 1:55 PM
> To: Lin Tony-B19295
> Cc: linux-mmc at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> cjb at laptop.org
> Subject: Re: [PATCH 1/1] mmc: sdhci-esdhc: Change delay after setting
> clock from 100ms to 1ms
> 
> On Thu, Sep 01, 2011 at 01:51:15PM +0800, Tony Lin wrote:
> > 1ms is enough for hardware to change the clock to stable.
> > 100ms is too long.
> 
> How do you know that? Can you be sure for PowerPC as well? Have you
> researched why the original author of the code has chosen that value? If
> so, please update your commit message with such infos.
> 
I got this confirmation by IC simulation, but sorry I just noticed that's also used for PPC.
I can update the information after got confirmation from author. 

> --
> Pengutronix e.K.                           | Wolfram Sang
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |