[PATCH 7/13] drivers/mmc/host/mmci.c: use clk_prepare_enable and clk_disable_unprepare

[PATCH 7/13] drivers/mmc/host/mmci.c: use clk_prepare_enable and clk_disable_unprepare

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

Clk_prepare_enable and clk_disable_unprepare combine clk_prepare and
clk_enable, and clk_disable and clk_unprepare.  They make the code more
concise, and ensure that clk_unprepare is called when clk_enable fails.

A simplified version of the semantic patch that introduces calls to these
functions is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@

- clk_prepare(e);
- clk_enable(e);
+ clk_prepare_enable(e);

@@
expression e;
@@

- clk_disable(e);
- clk_unprepare(e);
+ clk_disable_unprepare(e);
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/mmc/host/mmci.c |   13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 50ff19a..edc3e9b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1309,14 +1309,10 @@ static int __devinit mmci_probe(struct amba_device *dev,
 		goto host_free;
 	}
 
-	ret = clk_prepare(host->clk);
+	ret = clk_prepare_enable(host->clk);
 	if (ret)
 		goto clk_free;
 
-	ret = clk_enable(host->clk);
-	if (ret)
-		goto clk_unprep;
-
 	host->plat = plat;
 	host->variant = variant;
 	host->mclk = clk_get_rate(host->clk);
@@ -1515,9 +1511,7 @@ static int __devinit mmci_probe(struct amba_device *dev,
  err_gpio_cd:
 	iounmap(host->base);
  clk_disable:
-	clk_disable(host->clk);
- clk_unprep:
-	clk_unprepare(host->clk);
+	clk_disable_unprepare(host->clk);
  clk_free:
 	clk_put(host->clk);
  host_free:
@@ -1564,8 +1558,7 @@ static int __devexit mmci_remove(struct amba_device *dev)
 			gpio_free(host->gpio_cd);
 
 		iounmap(host->base);
-		clk_disable(host->clk);
-		clk_unprepare(host->clk);
+		clk_disable_unprepare(host->clk);
 		clk_put(host->clk);
 
 		if (host->vcc)

Re: [PATCH 7/13] drivers/mmc/host/mmci.c: use clk_prepare_enable and clk_disable_unprepare

[Ulf Hansson]

Hi Julia,

Patches for mmci should normally be sent on the arm list as well. mmci
is maintained by Russell King, even if the MAINTAINER file does not
say so.

Some minor comment below.

On 26 August 2012 18:00, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>

I think can simplify the commit msg a lot. Just say something with
"simplify clock code in probe".

> Clk_prepare_enable and clk_disable_unprepare combine clk_prepare and
> clk_enable, and clk_disable and clk_unprepare.  They make the code more
> concise, and ensure that clk_unprepare is called when clk_enable fails.
>
> A simplified version of the semantic patch that introduces calls to these
> functions is as follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression e;
> @@
>
> - clk_prepare(e);
> - clk_enable(e);
> + clk_prepare_enable(e);
>
> @@
> expression e;
> @@
>
> - clk_disable(e);
> - clk_unprepare(e);
> + clk_disable_unprepare(e);
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
>  drivers/mmc/host/mmci.c |   13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 50ff19a..edc3e9b 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -1309,14 +1309,10 @@ static int __devinit mmci_probe(struct amba_device *dev,
>                 goto host_free;
>         }
>
> -       ret = clk_prepare(host->clk);
> +       ret = clk_prepare_enable(host->clk);

white space? Did you run checkpatch?

>         if (ret)
>                 goto clk_free;
>
> -       ret = clk_enable(host->clk);
> -       if (ret)
> -               goto clk_unprep;
> -
>         host->plat = plat;
>         host->variant = variant;
>         host->mclk = clk_get_rate(host->clk);
> @@ -1515,9 +1511,7 @@ static int __devinit mmci_probe(struct amba_device *dev,
>   err_gpio_cd:
>         iounmap(host->base);
>   clk_disable:
> -       clk_disable(host->clk);
> - clk_unprep:
> -       clk_unprepare(host->clk);
> +       clk_disable_unprepare(host->clk);

white space? Did you run checkpatch?

>   clk_free:
>         clk_put(host->clk);
>   host_free:
> @@ -1564,8 +1558,7 @@ static int __devexit mmci_remove(struct amba_device *dev)
>                         gpio_free(host->gpio_cd);
>
>                 iounmap(host->base);
> -               clk_disable(host->clk);
> -               clk_unprepare(host->clk);
> +               clk_disable_unprepare(host->clk);

white space? Did you run checkpatch?

>                 clk_put(host->clk);
>
>                 if (host->vcc)
>
> --
> 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

Kind regards
Ulf Hansson

Re: [PATCH 7/13] drivers/mmc/host/mmci.c: use clk_prepare_enable and clk_disable_unprepare

[Julia Lawall]

On Mon, 27 Aug 2012, Ulf Hansson wrote:

> Hi Julia,
>
> Patches for mmci should normally be sent on the arm list as well. mmci
> is maintained by Russell King, even if the MAINTAINER file does not
> say so.

Would it be possible to update the MAINTAINER file?

> Some minor comment below.
>
> On 26 August 2012 18:00, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
>
> I think can simplify the commit msg a lot. Just say something with
> "simplify clock code in probe".

OK, I wasn't familiar with these functions before, so I wanted to explain
my rationale for making the change, in case I was totally off base.

> > Clk_prepare_enable and clk_disable_unprepare combine clk_prepare and
> > clk_enable, and clk_disable and clk_unprepare.  They make the code more
> > concise, and ensure that clk_unprepare is called when clk_enable fails.
> >
> > A simplified version of the semantic patch that introduces calls to these
> > functions is as follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression e;
> > @@
> >
> > - clk_prepare(e);
> > - clk_enable(e);
> > + clk_prepare_enable(e);
> >
> > @@
> > expression e;
> > @@
> >
> > - clk_disable(e);
> > - clk_unprepare(e);
> > + clk_disable_unprepare(e);
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
> >  drivers/mmc/host/mmci.c |   13 +++----------
> >  1 file changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> > index 50ff19a..edc3e9b 100644
> > --- a/drivers/mmc/host/mmci.c
> > +++ b/drivers/mmc/host/mmci.c
> > @@ -1309,14 +1309,10 @@ static int __devinit mmci_probe(struct amba_device *dev,
> >                 goto host_free;
> >         }
> >
> > -       ret = clk_prepare(host->clk);
> > +       ret = clk_prepare_enable(host->clk);
>
> white space? Did you run checkpatch?

Yes, I did.  And I did so again just now:

palace:linux-next: scripts/checkpatch.pl ~/patches/prepare2/send7
total: 0 errors, 0 warnings, 34 lines checked

/home/julia/patches/prepare2/send7 has no obvious style problems and is
ready for submission.  I can see that in your reply there are no tabs,
only space.  If you received it that way, perhaps it is a mailer problem
on my side, although I didn't do anything unusual for this patch.

I will try again.

julia


> >         if (ret)
> >                 goto clk_free;
> >
> > -       ret = clk_enable(host->clk);
> > -       if (ret)
> > -               goto clk_unprep;
> > -
> >         host->plat = plat;
> >         host->variant = variant;
> >         host->mclk = clk_get_rate(host->clk);
> > @@ -1515,9 +1511,7 @@ static int __devinit mmci_probe(struct amba_device *dev,
> >   err_gpio_cd:
> >         iounmap(host->base);
> >   clk_disable:
> > -       clk_disable(host->clk);
> > - clk_unprep:
> > -       clk_unprepare(host->clk);
> > +       clk_disable_unprepare(host->clk);
>
> white space? Did you run checkpatch?
>
> >   clk_free:
> >         clk_put(host->clk);
> >   host_free:
> > @@ -1564,8 +1558,7 @@ static int __devexit mmci_remove(struct amba_device *dev)
> >                         gpio_free(host->gpio_cd);
> >
> >                 iounmap(host->base);
> > -               clk_disable(host->clk);
> > -               clk_unprepare(host->clk);
> > +               clk_disable_unprepare(host->clk);
>
> white space? Did you run checkpatch?
>
> >                 clk_put(host->clk);
> >
> >                 if (host->vcc)
> >
> > --
> > 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
>
> Kind regards
> Ulf Hansson
>

Re: [PATCH 7/13] drivers/mmc/host/mmci.c: use clk_prepare_enable and clk_disable_unprepare

[Julia Lawall]

From: Julia Lawall <Julia.Lawall@lip6.fr>

Simplify clock code in probe.

A simplified version of the semantic patch that introduces calls to these
functions is as follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e;
@@

- clk_prepare(e);
- clk_enable(e);
+ clk_prepare_enable(e);

@@
expression e;
@@

- clk_disable(e);
- clk_unprepare(e);
+ clk_disable_unprepare(e);
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
v2, try to avoid whitespace problem

 drivers/mmc/host/mmci.c |   13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 50ff19a..edc3e9b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1309,14 +1309,10 @@ static int __devinit mmci_probe(struct amba_device *dev,
 		goto host_free;
 	}

-	ret = clk_prepare(host->clk);
+	ret = clk_prepare_enable(host->clk);
 	if (ret)
 		goto clk_free;

-	ret = clk_enable(host->clk);
-	if (ret)
-		goto clk_unprep;
-
 	host->plat = plat;
 	host->variant = variant;
 	host->mclk = clk_get_rate(host->clk);
@@ -1515,9 +1511,7 @@ static int __devinit mmci_probe(struct amba_device *dev,
  err_gpio_cd:
 	iounmap(host->base);
  clk_disable:
-	clk_disable(host->clk);
- clk_unprep:
-	clk_unprepare(host->clk);
+	clk_disable_unprepare(host->clk);
  clk_free:
 	clk_put(host->clk);
  host_free:
@@ -1564,8 +1558,7 @@ static int __devexit mmci_remove(struct amba_device *dev)
 			gpio_free(host->gpio_cd);

 		iounmap(host->base);
-		clk_disable(host->clk);
-		clk_unprepare(host->clk);
+		clk_disable_unprepare(host->clk);
 		clk_put(host->clk);

 		if (host->vcc)

Re: [PATCH 7/13] drivers/mmc/host/mmci.c: use clk_prepare_enable and clk_disable_unprepare

[Chris Ball]

Hi Julia,

On Sun, Aug 26 2012, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Clk_prepare_enable and clk_disable_unprepare combine clk_prepare and
> clk_enable, and clk_disable and clk_unprepare.  They make the code more
> concise, and ensure that clk_unprepare is called when clk_enable fails.
>
> A simplified version of the semantic patch that introduces calls to these
> functions is as follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression e;
> @@
>
> - clk_prepare(e);
> - clk_enable(e);
> + clk_prepare_enable(e);
>
> @@
> expression e;
> @@
>
> - clk_disable(e);
> - clk_unprepare(e);
> + clk_disable_unprepare(e);
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
>  drivers/mmc/host/mmci.c |   13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 50ff19a..edc3e9b 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -1309,14 +1309,10 @@ static int __devinit mmci_probe(struct amba_device *dev,
>  		goto host_free;
>  	}
>  
> -	ret = clk_prepare(host->clk);
> +	ret = clk_prepare_enable(host->clk);
>  	if (ret)
>  		goto clk_free;
>  
> -	ret = clk_enable(host->clk);
> -	if (ret)
> -		goto clk_unprep;
> -
>  	host->plat = plat;
>  	host->variant = variant;
>  	host->mclk = clk_get_rate(host->clk);
> @@ -1515,9 +1511,7 @@ static int __devinit mmci_probe(struct amba_device *dev,
>   err_gpio_cd:
>  	iounmap(host->base);
>   clk_disable:
> -	clk_disable(host->clk);
> - clk_unprep:
> -	clk_unprepare(host->clk);
> +	clk_disable_unprepare(host->clk);
>   clk_free:
>  	clk_put(host->clk);
>   host_free:
> @@ -1564,8 +1558,7 @@ static int __devexit mmci_remove(struct amba_device *dev)
>  			gpio_free(host->gpio_cd);
>  
>  		iounmap(host->base);
> -		clk_disable(host->clk);
> -		clk_unprepare(host->clk);
> +		clk_disable_unprepare(host->clk);
>  		clk_put(host->clk);
>  
>  		if (host->vcc)

Thanks, pushed to mmc-next for 3.7.

(I merged v1 instead of v2 because I don't see any whitespace problems
with it, and I liked the verbosity in the original commit message.)

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