[PATCH] mmc: core: remove waiting time when clkgate_delay is set

[PATCH] mmc: core: remove waiting time when clkgate_delay is set

[Chanho Park]

Since recent commit("mmc: core: Use delayed work in clock gating
framework":597dd9d79cfbbb1), we always wait "unnecessary" default
clock delay(8 cycles). Actually, we don't need it if clkgate_delay
(unit:ms) is set because we already wait sufficient time to change
the clock due to delayed_workqueue.
This patch removes duplicated waiting time when clkgate_delay is set.

Signed-off-by: Chanho Park <chanho61.park@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mmc/core/host.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index c3704e2..d710ce0 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -109,8 +109,11 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
 	 */
 	if (!host->clk_requests) {
 		spin_unlock_irqrestore(&host->clk_lock, flags);
-		tick_ns = DIV_ROUND_UP(1000000000, freq);
-		ndelay(host->clk_delay * tick_ns);
+		/* wait only when clk_gate_delay is 0*/
+		if (!host->clkgate_delay) {
+			tick_ns = DIV_ROUND_UP(1000000000, freq);
+			ndelay(host->clk_delay * tick_ns);
+		}
 	} else {
 		/* New users appeared while waiting for this work */
 		spin_unlock_irqrestore(&host->clk_lock, flags);
-- 
1.7.5.4

Re: [PATCH] mmc: core: remove waiting time when clkgate_delay is set

[Sujit Reddy Thumma]

On 2/29/2012 11:22 AM, Chanho Park wrote:
> Since recent commit("mmc: core: Use delayed work in clock gating
> framework":597dd9d79cfbbb1), we always wait "unnecessary" default
> clock delay(8 cycles). Actually, we don't need it if clkgate_delay
> (unit:ms) is set because we already wait sufficient time to change
> the clock due to delayed_workqueue.
> This patch removes duplicated waiting time when clkgate_delay is set.
>
> Signed-off-by: Chanho Park<chanho61.park@samsung.com>
> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
> ---
>   drivers/mmc/core/host.c |    7 +++++--
>   1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index c3704e2..d710ce0 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -109,8 +109,11 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
>   	 */
>   	if (!host->clk_requests) {
>   		spin_unlock_irqrestore(&host->clk_lock, flags);
> -		tick_ns = DIV_ROUND_UP(1000000000, freq);
> -		ndelay(host->clk_delay * tick_ns);
> +		/* wait only when clk_gate_delay is 0*/

Actually, we should check whether the clkgate_delay is giving sufficient 
clock cycles instead of just >0 value. But since min. f_min in freq. 
table is 100KHz (8CLK cyles is ~80us) and clkgate_delay can only 
specified in milliseconds as of now, I think this should be fine.

> +		if (!host->clkgate_delay) {
> +			tick_ns = DIV_ROUND_UP(1000000000, freq);
> +			ndelay(host->clk_delay * tick_ns);
> +		}
>   	} else {
>   		/* New users appeared while waiting for this work */
>   		spin_unlock_irqrestore(&host->clk_lock, flags);

Reviewed-by: Sujit Reddy Thumma <sthumma@codeaurora.org>

Thanks,
Sujit

Re: [PATCH] mmc: core: remove waiting time when clkgate_delay is set

[Chris Ball]

Hi,

On Wed, Feb 29 2012, Chanho Park wrote:
> Since recent commit("mmc: core: Use delayed work in clock gating
> framework":597dd9d79cfbbb1), we always wait "unnecessary" default
> clock delay(8 cycles). Actually, we don't need it if clkgate_delay
> (unit:ms) is set because we already wait sufficient time to change
> the clock due to delayed_workqueue.
> This patch removes duplicated waiting time when clkgate_delay is set.
>
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/mmc/core/host.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index c3704e2..d710ce0 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -109,8 +109,11 @@ static void mmc_host_clk_gate_delayed(struct mmc_host *host)
>  	 */
>  	if (!host->clk_requests) {
>  		spin_unlock_irqrestore(&host->clk_lock, flags);
> -		tick_ns = DIV_ROUND_UP(1000000000, freq);
> -		ndelay(host->clk_delay * tick_ns);
> +		/* wait only when clk_gate_delay is 0*/
> +		if (!host->clkgate_delay) {
> +			tick_ns = DIV_ROUND_UP(1000000000, freq);
> +			ndelay(host->clk_delay * tick_ns);
> +		}
>  	} else {
>  		/* New users appeared while waiting for this work */
>  		spin_unlock_irqrestore(&host->clk_lock, flags);

Have you seen Guennadi's patch?

http://git.kernel.org/?p=linux/kernel/git/cjb/mmc.git;a=commitdiff;h=63871af54a0f0053de6534afe8da101b988b86b6

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

RE: [PATCH] mmc: core: remove waiting time when clkgate_delay is set

[Chanho Park]

Hi,

> -----Original Message-----
> From: Chris Ball [mailto:cjb@laptop.org]
> Sent: Saturday, March 03, 2012 6:12 AM
> To: Chanho Park
> Cc: linux-mmc@vger.kernel.org; Kyungmin Park; Guennadi Liakhovetski
> Subject: Re: [PATCH] mmc: core: remove waiting time when clkgate_delay is
> set
> 
> Hi,
> 
> On Wed, Feb 29 2012, Chanho Park wrote:
> > Since recent commit("mmc: core: Use delayed work in clock gating
> > framework":597dd9d79cfbbb1), we always wait "unnecessary" default
> > clock delay(8 cycles). Actually, we don't need it if clkgate_delay
> > (unit:ms) is set because we already wait sufficient time to change the
> > clock due to delayed_workqueue.
> > This patch removes duplicated waiting time when clkgate_delay is set.
> >
> > Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/mmc/core/host.c |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c index
> > c3704e2..d710ce0 100644
> > --- a/drivers/mmc/core/host.c
> > +++ b/drivers/mmc/core/host.c
> > @@ -109,8 +109,11 @@ static void mmc_host_clk_gate_delayed(struct
> mmc_host *host)
> >  	 */
> >  	if (!host->clk_requests) {
> >  		spin_unlock_irqrestore(&host->clk_lock, flags);
> > -		tick_ns = DIV_ROUND_UP(1000000000, freq);
> > -		ndelay(host->clk_delay * tick_ns);
> > +		/* wait only when clk_gate_delay is 0*/
> > +		if (!host->clkgate_delay) {
> > +			tick_ns = DIV_ROUND_UP(1000000000, freq);
> > +			ndelay(host->clk_delay * tick_ns);
> > +		}
> >  	} else {
> >  		/* New users appeared while waiting for this work */
> >  		spin_unlock_irqrestore(&host->clk_lock, flags);
> 
> Have you seen Guennadi's patch?
> 
> http://git.kernel.org/?p=linux/kernel/git/cjb/mmc.git;a=commitdiff;h=63871
> af54a0f0053de6534afe8da101b988b86b6

I'd already seen the patch. There is no problem if a user didn't set a clkgate_delay.
However, if he wants to set a "clkgate_delay", a "clk_delay" is no longer needed.
Because, clk_delay(basically 8 cycles) always fall within "clkgate_delay".
I remove this "duplicated" wait.

Best regards,
Chanho Park

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