[PATCH 1/2] serial/amba-pl011: use devm_* allocators

[PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Linus Walleij]

From: Linus Walleij <linus.walleij@linaro.org>

This switches a bunch of allocation and remapping to use the
devm_* garbage collected methods and cleans up the error
path and remove() paths consequently.

devm_ioremap() is only in <linux/io.h> so fix up the
erroneous <asm/*> include as well.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/tty/serial/amba-pl011.c |   20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index c17923e..3ce984f 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -53,8 +53,8 @@
 #include <linux/delay.h>
 #include <linux/types.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/io.h>
 
-#include <asm/io.h>
 #include <asm/sizes.h>
 
 #define UART_NR			14
@@ -1885,22 +1885,24 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 		goto out;
 	}
 
-	uap = kzalloc(sizeof(struct uart_amba_port), GFP_KERNEL);
+	uap = devm_kzalloc(&dev->dev, sizeof(struct uart_amba_port),
+			   GFP_KERNEL);
 	if (uap == NULL) {
 		ret = -ENOMEM;
 		goto out;
 	}
 
-	base = ioremap(dev->res.start, resource_size(&dev->res));
+	base = devm_ioremap(&dev->dev, dev->res.start,
+			    resource_size(&dev->res));
 	if (!base) {
 		ret = -ENOMEM;
-		goto free;
+		goto out;
 	}
 
 	uap->pinctrl = devm_pinctrl_get(&dev->dev);
 	if (IS_ERR(uap->pinctrl)) {
 		ret = PTR_ERR(uap->pinctrl);
-		goto unmap;
+		goto out;
 	}
 	uap->pins_default = pinctrl_lookup_state(uap->pinctrl,
 						 PINCTRL_STATE_DEFAULT);
@@ -1915,7 +1917,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 	uap->clk = clk_get(&dev->dev, NULL);
 	if (IS_ERR(uap->clk)) {
 		ret = PTR_ERR(uap->clk);
-		goto unmap;
+		goto out;
 	}
 
 	uap->vendor = vendor;
@@ -1950,10 +1952,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 		amba_ports[i] = NULL;
 		pl011_dma_remove(uap);
 		clk_put(uap->clk);
- unmap:
-		iounmap(base);
- free:
-		kfree(uap);
 	}
  out:
 	return ret;
@@ -1973,9 +1971,7 @@ static int pl011_remove(struct amba_device *dev)
 			amba_ports[i] = NULL;
 
 	pl011_dma_remove(uap);
-	iounmap(uap->port.membase);
 	clk_put(uap->clk);
-	kfree(uap);
 	return 0;
 }
 
-- 
1.7.9.2

[PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Linus Walleij]

From: Linus Walleij <linus.walleij@linaro.org>

This switches a bunch of allocation and remapping to use the
devm_* garbage collected methods and cleans up the error
path and remove() paths consequently.

devm_ioremap() is only in <linux/io.h> so fix up the
erroneous <asm/*> include as well.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/tty/serial/amba-pl011.c |   20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index c17923e..3ce984f 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -53,8 +53,8 @@
 #include <linux/delay.h>
 #include <linux/types.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/io.h>
 
-#include <asm/io.h>
 #include <asm/sizes.h>
 
 #define UART_NR			14
@@ -1885,22 +1885,24 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 		goto out;
 	}
 
-	uap = kzalloc(sizeof(struct uart_amba_port), GFP_KERNEL);
+	uap = devm_kzalloc(&dev->dev, sizeof(struct uart_amba_port),
+			   GFP_KERNEL);
 	if (uap == NULL) {
 		ret = -ENOMEM;
 		goto out;
 	}
 
-	base = ioremap(dev->res.start, resource_size(&dev->res));
+	base = devm_ioremap(&dev->dev, dev->res.start,
+			    resource_size(&dev->res));
 	if (!base) {
 		ret = -ENOMEM;
-		goto free;
+		goto out;
 	}
 
 	uap->pinctrl = devm_pinctrl_get(&dev->dev);
 	if (IS_ERR(uap->pinctrl)) {
 		ret = PTR_ERR(uap->pinctrl);
-		goto unmap;
+		goto out;
 	}
 	uap->pins_default = pinctrl_lookup_state(uap->pinctrl,
 						 PINCTRL_STATE_DEFAULT);
@@ -1915,7 +1917,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 	uap->clk = clk_get(&dev->dev, NULL);
 	if (IS_ERR(uap->clk)) {
 		ret = PTR_ERR(uap->clk);
-		goto unmap;
+		goto out;
 	}
 
 	uap->vendor = vendor;
@@ -1950,10 +1952,6 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
 		amba_ports[i] = NULL;
 		pl011_dma_remove(uap);
 		clk_put(uap->clk);
- unmap:
-		iounmap(base);
- free:
-		kfree(uap);
 	}
  out:
 	return ret;
@@ -1973,9 +1971,7 @@ static int pl011_remove(struct amba_device *dev)
 			amba_ports[i] = NULL;
 
 	pl011_dma_remove(uap);
-	iounmap(uap->port.membase);
 	clk_put(uap->clk);
-	kfree(uap);
 	return 0;
 }
 
-- 
1.7.9.2

Re: [PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Fabio Estevam]

On Sun, Jun 17, 2012 at 10:44 AM, Linus Walleij
<linus.walleij@stericsson.com> wrote:

>        uap->pinctrl = devm_pinctrl_get(&dev->dev);
>        if (IS_ERR(uap->pinctrl)) {
>                ret = PTR_ERR(uap->pinctrl);
> -               goto unmap;
> +               goto out;
>        }
>        uap->pins_default = pinctrl_lookup_state(uap->pinctrl,
>                                                 PINCTRL_STATE_DEFAULT);
> @@ -1915,7 +1917,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>        uap->clk = clk_get(&dev->dev, NULL);

Maybe you can use devm_clk_get here too.

[PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Fabio Estevam]

On Sun, Jun 17, 2012 at 10:44 AM, Linus Walleij
<linus.walleij@stericsson.com> wrote:

> ? ? ? ?uap->pinctrl = devm_pinctrl_get(&dev->dev);
> ? ? ? ?if (IS_ERR(uap->pinctrl)) {
> ? ? ? ? ? ? ? ?ret = PTR_ERR(uap->pinctrl);
> - ? ? ? ? ? ? ? goto unmap;
> + ? ? ? ? ? ? ? goto out;
> ? ? ? ?}
> ? ? ? ?uap->pins_default = pinctrl_lookup_state(uap->pinctrl,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PINCTRL_STATE_DEFAULT);
> @@ -1915,7 +1917,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
> ? ? ? ?uap->clk = clk_get(&dev->dev, NULL);

Maybe you can use devm_clk_get here too.

Re: [PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Linus Walleij]

On Sun, Jun 17, 2012 at 4:13 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Sun, Jun 17, 2012 at 10:44 AM, Linus Walleij
> <linus.walleij@stericsson.com> wrote:
>
>>        uap->pinctrl = devm_pinctrl_get(&dev->dev);
>>        if (IS_ERR(uap->pinctrl)) {
>>                ret = PTR_ERR(uap->pinctrl);
>> -               goto unmap;
>> +               goto out;
>>        }
>>        uap->pins_default = pinctrl_lookup_state(uap->pinctrl,
>>                                                 PINCTRL_STATE_DEFAULT);
>> @@ -1915,7 +1917,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>>        uap->clk = clk_get(&dev->dev, NULL);
>
> Maybe you can use devm_clk_get here too.

That requires all users of the driver to be using CONFIG_CLKDEV, which
I *think* is safe to do, but better check: Russell, do you know if
it's safe to use
this feature from clkdev in the PL011 driver?

It seems so to me and most likely works on the systems I have (Ux500, U300,
Nomadik, Integrator, RealView) but the world of PL011 is so big...

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Linus Walleij]

On Sun, Jun 17, 2012 at 4:13 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Sun, Jun 17, 2012 at 10:44 AM, Linus Walleij
> <linus.walleij@stericsson.com> wrote:
>
>> ? ? ? ?uap->pinctrl = devm_pinctrl_get(&dev->dev);
>> ? ? ? ?if (IS_ERR(uap->pinctrl)) {
>> ? ? ? ? ? ? ? ?ret = PTR_ERR(uap->pinctrl);
>> - ? ? ? ? ? ? ? goto unmap;
>> + ? ? ? ? ? ? ? goto out;
>> ? ? ? ?}
>> ? ? ? ?uap->pins_default = pinctrl_lookup_state(uap->pinctrl,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PINCTRL_STATE_DEFAULT);
>> @@ -1915,7 +1917,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>> ? ? ? ?uap->clk = clk_get(&dev->dev, NULL);
>
> Maybe you can use devm_clk_get here too.

That requires all users of the driver to be using CONFIG_CLKDEV, which
I *think* is safe to do, but better check: Russell, do you know if
it's safe to use
this feature from clkdev in the PL011 driver?

It seems so to me and most likely works on the systems I have (Ux500, U300,
Nomadik, Integrator, RealView) but the world of PL011 is so big...

Yours,
Linus Walleij

Re: [PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Greg Kroah-Hartman]

On Sun, Jun 17, 2012 at 03:44:08PM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This switches a bunch of allocation and remapping to use the
> devm_* garbage collected methods and cleans up the error
> path and remove() paths consequently.
> 
> devm_ioremap() is only in <linux/io.h> so fix up the
> erroneous <asm/*> include as well.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c |   20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 

This patch doesn't apply, what tree did you make it against?

greg k-h

[PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Greg Kroah-Hartman]

On Sun, Jun 17, 2012 at 03:44:08PM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This switches a bunch of allocation and remapping to use the
> devm_* garbage collected methods and cleans up the error
> path and remove() paths consequently.
> 
> devm_ioremap() is only in <linux/io.h> so fix up the
> erroneous <asm/*> include as well.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c |   20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 

This patch doesn't apply, what tree did you make it against?

greg k-h

Re: [PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Linus Walleij]

On Tue, Jun 19, 2012 at 2:34 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Sun, Jun 17, 2012 at 03:44:08PM +0200, Linus Walleij wrote:
>>
>> This switches a bunch of allocation and remapping to use the
>> devm_* garbage collected methods and cleans up the error
>> path and remove() paths consequently.
>> (...)
>
> This patch doesn't apply, what tree did you make it against?

It was on top of the patch pending for inclusion into the -rc:s,
I can sure wait for that to be upstream and resend this then,
no hurry.

Yours,
Linus Walleij

[PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Linus Walleij]

On Tue, Jun 19, 2012 at 2:34 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Sun, Jun 17, 2012 at 03:44:08PM +0200, Linus Walleij wrote:
>>
>> This switches a bunch of allocation and remapping to use the
>> devm_* garbage collected methods and cleans up the error
>> path and remove() paths consequently.
>> (...)
>
> This patch doesn't apply, what tree did you make it against?

It was on top of the patch pending for inclusion into the -rc:s,
I can sure wait for that to be upstream and resend this then,
no hurry.

Yours,
Linus Walleij

Re: [PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Russell King - ARM Linux]

On Mon, Jun 18, 2012 at 08:37:20AM +0200, Linus Walleij wrote:
> On Sun, Jun 17, 2012 at 4:13 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Sun, Jun 17, 2012 at 10:44 AM, Linus Walleij
> > <linus.walleij@stericsson.com> wrote:
> >
> >>        uap->pinctrl = devm_pinctrl_get(&dev->dev);
> >>        if (IS_ERR(uap->pinctrl)) {
> >>                ret = PTR_ERR(uap->pinctrl);
> >> -               goto unmap;
> >> +               goto out;
> >>        }
> >>        uap->pins_default = pinctrl_lookup_state(uap->pinctrl,
> >>                                                 PINCTRL_STATE_DEFAULT);
> >> @@ -1915,7 +1917,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
> >>        uap->clk = clk_get(&dev->dev, NULL);
> >
> > Maybe you can use devm_clk_get here too.
> 
> That requires all users of the driver to be using CONFIG_CLKDEV, which
> I *think* is safe to do, but better check: Russell, do you know if
> it's safe to use
> this feature from clkdev in the PL011 driver?

No idea.  What I have said is that the devm_* clk_get interfaces should
not be dependent on clkdev being built.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] serial/amba-pl011: use devm_* allocators

[Russell King - ARM Linux]

On Mon, Jun 18, 2012 at 08:37:20AM +0200, Linus Walleij wrote:
> On Sun, Jun 17, 2012 at 4:13 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Sun, Jun 17, 2012 at 10:44 AM, Linus Walleij
> > <linus.walleij@stericsson.com> wrote:
> >
> >> ? ? ? ?uap->pinctrl = devm_pinctrl_get(&dev->dev);
> >> ? ? ? ?if (IS_ERR(uap->pinctrl)) {
> >> ? ? ? ? ? ? ? ?ret = PTR_ERR(uap->pinctrl);
> >> - ? ? ? ? ? ? ? goto unmap;
> >> + ? ? ? ? ? ? ? goto out;
> >> ? ? ? ?}
> >> ? ? ? ?uap->pins_default = pinctrl_lookup_state(uap->pinctrl,
> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PINCTRL_STATE_DEFAULT);
> >> @@ -1915,7 +1917,7 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
> >> ? ? ? ?uap->clk = clk_get(&dev->dev, NULL);
> >
> > Maybe you can use devm_clk_get here too.
> 
> That requires all users of the driver to be using CONFIG_CLKDEV, which
> I *think* is safe to do, but better check: Russell, do you know if
> it's safe to use
> this feature from clkdev in the PL011 driver?

No idea.  What I have said is that the devm_* clk_get interfaces should
not be dependent on clkdev being built.