Skip to contentSkip to conversation

serial/amba-pl011: use devm_* allocators

PATCH Superseded Linus Walleij 5 participants 6 messages Jun 17, 2012 – Jun 20, 2012

Subsystems (2)
TTY layer and serial drivers Arm primecell UART PL010 and PL011 drivers
lore
Share & tools
lore

Linus Walleij

author
#0 lore raw

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

From: Linus Walleij <[email protected]>

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 <[email protected]>
---
 drivers/tty/serial/amba-pl011.c |   20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)
drivers/tty/serial/amba-pl011.c +8 -12
index c17923e..3ce984f 100644
@@ -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;
 }
Signature · 1 line
1.7.9.2
#1 lore raw

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

Review static int pl011_probe(struct amba_device *dev, const struct amba_id *id) · line 1917

Quoted 10 lines · Linus Walleij

? ? ? ?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);

(quoted diff) +0 -0
@@ -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.

#2 lore raw

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

Quoted 15 lines · Fabio Estevam

Review static int pl011_probe(struct amba_device *dev, const struct amba_id *id) · line 1917

Quoted 10 lines · Linus Walleij

? ? ? ?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);

(quoted diff) +0 -0
@@ -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

#3 lore raw

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

Quoted 17 lines · Linus Walleij
Quoted 15 lines · Fabio Estevam

Review static int pl011_probe(struct amba_device *dev, const struct amba_id *id) · line 1917

Quoted 10 lines · Linus Walleij

? ? ? ?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);

(quoted diff) +0 -0
@@ -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.

Quoted 4 lines

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.

maintainer
#4 lore raw

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

In reply to Linus Walleij
Quoted 6 lines · Linus Walleij

From: Linus Walleij <[email protected]>

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.

Quoted 8 lines

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

Signed-off-by Linus Walleij <[email protected]>
---
 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

#5 lore raw

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

Quoted 8 lines · Greg Kroah-Hartman
Quoted 5 lines · Linus Walleij

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