serial driver cleanups v2

serial driver cleanups v2

[Enrico Weigelt, metux IT consult]

Hello folks,


here's v2 of my serial cleanups queue - part I:

essentially using helpers to code more compact and switching to
devm_*() functions for mmio management.

Part II will be about moving the mmio range from mapbase and
mapsize (which are used quite inconsistently) to a struct resource
and using helpers for that. But this one isn't finished yet.
(if somebody likes to have a look at it, I can send it, too)


happy hacking


--mtx

[PATCH v2 01/45] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()

[Enrico Weigelt, metux IT consult]

For the common platform_get_resource()+devm_platform_ioremap() combination,
there is a helper, so use it and make the code a bit more compact.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
index bd53661..0738d14 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -25,7 +25,6 @@ struct bcm2835aux_data {
 static int bcm2835aux_serial_probe(struct platform_device *pdev)
 {
 	struct bcm2835aux_data *data;
-	struct resource *res;
 	int ret;
 
 	/* allocate the custom structure */
@@ -63,15 +62,12 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
 	data->uart.port.irq = ret;
 
 	/* map the main registers */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "memory resource not found");
-		return -EINVAL;
-	}
-	data->uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
+	data->uart.port.membase = devm_platform_ioremap_resource(pdev, 0);
 	ret = PTR_ERR_OR_ZERO(data->uart.port.membase);
-	if (ret)
+	if (ret) {
+		dev_err(&pdev->dev, "could not map memory resource");
 		return ret;
+	}
 
 	/* Check for a fixed line number */
 	ret = of_alias_get_id(pdev->dev.of_node, "serial");
-- 
1.9.1

[PATCH v2 02/45] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_dw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d31b975..f0a294d 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
 	p->set_ldisc	= dw8250_set_ldisc;
 	p->set_termios	= dw8250_set_termios;
 
-	p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
+	p->membase = devm_ioremap_resource(dev, regs);
 	if (!p->membase)
 		return -ENOMEM;
 
-- 
1.9.1

Re: [PATCH v2 02/45] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()

[Andy Shevchenko]

On Fri, Mar 15, 2019 at 12:41 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Instead of fetching out data from a struct resource for passing
> it to devm_ioremap(), directly use devm_ioremap_resource()

I don't see any advantage of this change.
See also below.

> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
>         p->set_ldisc    = dw8250_set_ldisc;
>         p->set_termios  = dw8250_set_termios;
>
> -       p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
> +       p->membase = devm_ioremap_resource(dev, regs);
>         if (!p->membase)

And how did you test this? devm_ioremap_resource() returns error
pointer in case of error.

>                 return -ENOMEM;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 02/45] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

On 15.03.19 10:04, Andy Shevchenko wrote:
> On Fri, Mar 15, 2019 at 12:41 AM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
>>
>> Instead of fetching out data from a struct resource for passing
>> it to devm_ioremap(), directly use devm_ioremap_resource()
> 
> I don't see any advantage of this change.
> See also below.

I see that the whole story wasn't clear. Please see my reply to Greg,
hope that clears it up a little bit.

>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
>>         p->set_ldisc    = dw8250_set_ldisc;
>>         p->set_termios  = dw8250_set_termios;
>>
>> -       p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
>> +       p->membase = devm_ioremap_resource(dev, regs);
>>         if (!p->membase)
> 
> And how did you test this? devm_ioremap_resource() returns error
> pointer in case of error.

hmm, devm_ioremap_resource() does so, but devm_ioremap() does not ?
I really didn't expect that. Thanks for pointing that out.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

[PATCH v2 03/45] drivers: tty: serial: 8250_ingenic: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_ingenic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
index 424c07c..90ae3e2 100644
--- a/drivers/tty/serial/8250/8250_ingenic.c
+++ b/drivers/tty/serial/8250/8250_ingenic.c
@@ -249,8 +249,7 @@ static int ingenic_uart_probe(struct platform_device *pdev)
 	if (line >= 0)
 		uart.port.line = line;
 
-	uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
-					 resource_size(regs));
+	uart.port.membase = devm_ioremap_resource(&pdev->dev, regs);
 	if (!uart.port.membase)
 		return -ENOMEM;
 
-- 
1.9.1

[PATCH v2 04/45] drivers: tty: serial: amba-pl010: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/amba-pl010.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index 2c37d11..109b8df 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -713,8 +713,7 @@ static int pl010_probe(struct amba_device *dev, const struct amba_id *id)
 	if (!uap)
 		return -ENOMEM;
 
-	base = devm_ioremap(&dev->dev, dev->res.start,
-			    resource_size(&dev->res));
+	base = devm_ioremap_resource(&dev->dev, &dev->res);
 	if (!base)
 		return -ENOMEM;
 
-- 
1.9.1

[PATCH v2 05/45] drivers: tty: serial: sirfsoc_uart: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/sirfsoc_uart.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index 38622f2..db2e08d 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -1359,8 +1359,7 @@ static int sirfsoc_uart_probe(struct platform_device *pdev)
 		goto err;
 	}
 	port->mapbase = res->start;
-	port->membase = devm_ioremap(&pdev->dev,
-			res->start, resource_size(res));
+	port->membase = devm_ioremap_resource(&pdev->dev, res);
 	if (!port->membase) {
 		dev_err(&pdev->dev, "Cannot remap resource.\n");
 		ret = -ENOMEM;
-- 
1.9.1

[PATCH v2 06/45] drivers: tty: serial: samsung: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/samsung.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 83fd516..a49cf15 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1775,7 +1775,7 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
 
 	dbg("resource %pR)\n", res);
 
-	port->membase = devm_ioremap(port->dev, res->start, resource_size(res));
+	port->membase = devm_ioremap_resource(port->dev, res);
 	if (!port->membase) {
 		dev_err(port->dev, "failed to remap controller address\n");
 		return -EBUSY;
-- 
1.9.1

[PATCH v2 07/45] drivers: tty: serial: 8250_uniphier: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_uniphier.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
index 164ba13..9c1244e 100644
--- a/drivers/tty/serial/8250/8250_uniphier.c
+++ b/drivers/tty/serial/8250/8250_uniphier.c
@@ -171,7 +171,7 @@ static int uniphier_uart_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	membase = devm_ioremap(dev, regs->start, resource_size(regs));
+	membase = devm_ioremap_resource(dev, regs);
 	if (!membase)
 		return -ENOMEM;
 
-- 
1.9.1

Re: [PATCH v2 07/45] drivers: tty: serial: 8250_uniphier: use devm_ioremap_resource()

[Masahiro Yamada]

On Fri, Mar 15, 2019 at 7:35 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Instead of fetching out data from a struct resource for passing
> it to devm_ioremap(), directly use devm_ioremap_resource()
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---


NACK.


This patch would break my driver.

Probably, all the 8250* drivers would be broken in the same way.


For 8250 drivers, request_mem_region() is called
from 8250_port.c


Let's not touch around the code you do not
understand how it works.


Thanks.

Masahiro Yamada



>  drivers/tty/serial/8250/8250_uniphier.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
> index 164ba13..9c1244e 100644
> --- a/drivers/tty/serial/8250/8250_uniphier.c
> +++ b/drivers/tty/serial/8250/8250_uniphier.c
> @@ -171,7 +171,7 @@ static int uniphier_uart_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>
> -       membase = devm_ioremap(dev, regs->start, resource_size(regs));
> +       membase = devm_ioremap_resource(dev, regs);
>         if (!membase)
>                 return -ENOMEM;
>
> --
> 1.9.1
>

[PATCH v2 08/45] drivers: tty: serial: 8250_lpc18xx: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_lpc18xx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index eddf119..f36902b 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -119,8 +119,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
 
 	memset(&uart, 0, sizeof(uart));
 
-	uart.port.membase = devm_ioremap(&pdev->dev, res->start,
-					 resource_size(res));
+	uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
 	if (!uart.port.membase)
 		return -ENOMEM;
 
-- 
1.9.1

[PATCH v2 09/45] drivers: tty: serial: 8250_mtk: use devm_ioremap_resource()

[Enrico Weigelt, metux IT consult]

Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_mtk.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index c1fdbc0..bf6eb8d 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -383,8 +383,7 @@ static int mtk8250_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
-					 resource_size(regs));
+	uart.port.membase = devm_ioremap_resource(&pdev->dev, regs);
 	if (!uart.port.membase)
 		return -ENOMEM;
 
-- 
1.9.1

[PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/zs.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index b03d3e4..0b1ec2f 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -984,16 +984,17 @@ static const char *zs_type(struct uart_port *uport)
 
 static void zs_release_port(struct uart_port *uport)
 {
-	iounmap(uport->membase);
+	devm_iounmap(uport->dev, uport->membase);
 	uport->membase = 0;
-	release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+	devm_release_mem_region(uport->dev, uport->mapbase, ZS_CHAN_IO_SIZE);
 }
 
 static int zs_map_port(struct uart_port *uport)
 {
 	if (!uport->membase)
-		uport->membase = ioremap_nocache(uport->mapbase,
-						 ZS_CHAN_IO_SIZE);
+		uport->membase = devm_ioremap_nocache(uport->dev,
+						      uport->mapbase,
+						      ZS_CHAN_IO_SIZE);
 	if (!uport->membase) {
 		printk(KERN_ERR "zs: Cannot map MMIO\n");
 		return -ENOMEM;
@@ -1005,13 +1006,16 @@ static int zs_request_port(struct uart_port *uport)
 {
 	int ret;
 
-	if (!request_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE, "scc")) {
+	if (!devm_request_mem_region(uport->mapbase,
+				     ZS_CHAN_IO_SIZE, "scc")) {
 		printk(KERN_ERR "zs: Unable to reserve MMIO resource\n");
 		return -EBUSY;
 	}
 	ret = zs_map_port(uport);
 	if (ret) {
-		release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+		devm_release_mem_region(uport-dev,
+					uport->mapbase,
+					ZS_CHAN_IO_SIZE);
 		return ret;
 	}
 	return 0;
-- 
1.9.1

Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

[Greg KH]

On Thu, Mar 14, 2019 at 11:33:40PM +0100, Enrico Weigelt, metux IT consult wrote:
> Use the safer devm versions of memory mapping functions.

What is "safer" about them?

> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---
>  drivers/tty/serial/zs.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
> index b03d3e4..0b1ec2f 100644
> --- a/drivers/tty/serial/zs.c
> +++ b/drivers/tty/serial/zs.c
> @@ -984,16 +984,17 @@ static const char *zs_type(struct uart_port *uport)
>  
>  static void zs_release_port(struct uart_port *uport)
>  {
> -	iounmap(uport->membase);
> +	devm_iounmap(uport->dev, uport->membase);
>  	uport->membase = 0;
> -	release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
> +	devm_release_mem_region(uport->dev, uport->mapbase, ZS_CHAN_IO_SIZE);

Isn't the whole goal of the devm* functions such that you are not
required to call "release" on them?

If so, are you sure this patchset is correct?

And also, why make the change, you aren't changing any functionality for
these old drivers at all from what I can tell (for the devm calls).
What am I missing here?

thanks,

greg k-h

Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

[Enrico Weigelt, metux IT consult]

On 14.03.19 23:52, Greg KH wrote:
> On Thu, Mar 14, 2019 at 11:33:40PM +0100, Enrico Weigelt, metux IT consult wrote:
>> Use the safer devm versions of memory mapping functions.
> 
> What is "safer" about them?

Garbage collection :)

Several drivers didn't seem to clean up properly (maybe these're just
used compiled-in, so nobody noticed yet).

In general, I think devm_* should be the standard case, unless there's
a really good reason to do otherwise.

<snip>

> Isn't the whole goal of the devm* functions such that you are not
> required to call "release" on them?

Looks that one slipped through, when I was doing that big bulk change
in the middle of the night and not enough coffe ;-)

One problem here is that many drivers do this stuff in request/release
port, instead of probe/remove. I'm not sure yet, whether we should
rewrite that. There're also cases which do request/release, but no
ioremap (doing hardcoded register accesses), others do ioremap w/o
request/release.

IMHO, we should have a closer look at those and check whether that's
really okay (just adding request/release blindly could cause trouble)

> And also, why make the change, you aren't changing any functionality for
> these old drivers at all from what I can tell (for the devm calls).
> What am I missing here?

Okay, there's a bigger story behind, you can't know yet. Finally, I'd
like to move everything to using struct resource and corresponding
helpers consistently, so most of the drivers would be pretty simple
at that point. (there're of course special cases, like devices w/
multiple register spaces, etc)

Here's my wip branch:

https://github.com/metux/linux/commits/wip/serial-res

In this consolidation process, I'm trying to move everything to
devm_*, to have it more generic (for now, I still need two versions
of the request/release/ioremap/iounmap helpers - one w/ and one
w/o devm).

My idea was moving to devm first, so it can be reviewed/tested
independently, before moving forward. Smaller, easily digestable
pieces should minimize the risk of breaking anything. But if you
prefer having this things squashed together, just let me know.

In the queue are also other minor cleanups like using dev_err()
instead of printk(), etc. Should I send these separately ?

By the way: do you have some public branch where you're collecting
accepted patches, which I could base mine on ? (tty.git/tty-next ?)


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

[Greg KH]

On Fri, Mar 15, 2019 at 10:06:16AM +0100, Enrico Weigelt, metux IT consult wrote:
> On 14.03.19 23:52, Greg KH wrote:
> > On Thu, Mar 14, 2019 at 11:33:40PM +0100, Enrico Weigelt, metux IT consult wrote:
> >> Use the safer devm versions of memory mapping functions.
> > 
> > What is "safer" about them?
> 
> Garbage collection :)

At times, but not when you do not use the api correctly :)

> Several drivers didn't seem to clean up properly (maybe these're just
> used compiled-in, so nobody noticed yet).

Yes, there are lots of drivers for devices that are never unloaded or
removed from the system.  The fact that no one has reported any problems
with them means that they are never used in situations like this.

> In general, I think devm_* should be the standard case, unless there's
> a really good reason to do otherwise.

No, you need to have a good reason why it needs to be changed, when you
can not verify that this actually resolves a problem.  As this patch
shows, you just replaced one api call with another, so nothing changed
at all, except you actually took up more memory and logic to do the same
thing :(

> > Isn't the whole goal of the devm* functions such that you are not
> > required to call "release" on them?
> 
> Looks that one slipped through, when I was doing that big bulk change
> in the middle of the night and not enough coffe ;-)
> 
> One problem here is that many drivers do this stuff in request/release
> port, instead of probe/remove. I'm not sure yet, whether we should
> rewrite that. There're also cases which do request/release, but no
> ioremap (doing hardcoded register accesses), others do ioremap w/o
> request/release.
> 
> IMHO, we should have a closer look at those and check whether that's
> really okay (just adding request/release blindly could cause trouble)

I agree, please feel free to audit these to verify they are all correct.
But that's not what you did here, so that's not a valid justification
for these patches to be accepted.

> > And also, why make the change, you aren't changing any functionality for
> > these old drivers at all from what I can tell (for the devm calls).
> > What am I missing here?
> 
> Okay, there's a bigger story behind, you can't know yet. Finally, I'd
> like to move everything to using struct resource and corresponding
> helpers consistently, so most of the drivers would be pretty simple
> at that point. (there're of course special cases, like devices w/
> multiple register spaces, etc)
> 
> Here's my wip branch:
> 
> https://github.com/metux/linux/commits/wip/serial-res
> 
> In this consolidation process, I'm trying to move everything to
> devm_*, to have it more generic (for now, I still need two versions
> of the request/release/ioremap/iounmap helpers - one w/ and one
> w/o devm).

Move everything in what part of the kernel?  The whole kernel or just
one subsystem?

> My idea was moving to devm first, so it can be reviewed/tested
> independently, before moving forward. Smaller, easily digestable
> pieces should minimize the risk of breaking anything. But if you
> prefer having this things squashed together, just let me know.

Small pieces are required, that's fine, but those pieces need to have a
justification for why they should be accepted at all points along the
way.

> In the queue are also other minor cleanups like using dev_err()
> instead of printk(), etc. Should I send these separately ?

Of course.

> By the way: do you have some public branch where you're collecting
> accepted patches, which I could base mine on ? (tty.git/tty-next ?)

Yes, that is the tree and branch, but remember that none of my trees can
open up until after -rc1 is out.

thanks,

greg k-h

Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

[Enrico Weigelt, metux IT consult]

On 15.03.19 15:26, Greg KH wrote:
> On Fri, Mar 15, 2019 at 10:06:16AM +0100, Enrico Weigelt, metux IT consult wrote:
>> On 14.03.19 23:52, Greg KH wrote:
>>> On Thu, Mar 14, 2019 at 11:33:40PM +0100, Enrico Weigelt, metux IT consult wrote:
>>>> Use the safer devm versions of memory mapping functions.
>>>
>>> What is "safer" about them?
>>
>> Garbage collection :)
> 
> At times, but not when you do not use the api correctly :)

Okay, my fault that I didn't read the code careful enough :o

But still, I think the name is a bit misleading as it *sounds* as just
a wrapper around devm_ioremap() that just picks the params from a
struct resource. I guess we can't change the name easily ?

> Yes, there are lots of drivers for devices that are never unloaded or
> removed from the system.  The fact that no one has reported any problems
> with them means that they are never used in situations like this.

So, never touch a running system ?

> No, you need to have a good reason why it needs to be changed, when you
> can not verify that this actually resolves a problem.  As this patch
> shows, you just replaced one api call with another, so nothing changed
> at all, except you actually took up more memory and logic to do the same
> thing :(

Okay, I was on a wrong track here - I had the silly idea that it would
make things easier if we'd do it the same way everywhere.

>> IMHO, we should have a closer look at those and check whether that's
>> really okay (just adding request/release blindly could cause trouble)
> 
> I agree, please feel free to audit these to verify they are all correct.
> But that's not what you did here, so that's not a valid justification
> for these patches to be accepted.

Understood. Assuming I've found some of these cases, shall I use devm
oder just add the missing release ?

>> In this consolidation process, I'm trying to move everything to
>> devm_*, to have it more generic (for now, I still need two versions
>> of the request/release/ioremap/iounmap helpers - one w/ and one
>> w/o devm).
> 
> Move everything in what part of the kernel?  The whole kernel or just
> one subsystem?

Just talking about the serial drivers.

>> My idea was moving to devm first, so it can be reviewed/tested
>> independently, before moving forward. Smaller, easily digestable
>> pieces should minimize the risk of breaking anything. But if you
>> prefer having this things squashed together, just let me know.
> 
> Small pieces are required, that's fine, but those pieces need to have a
> justification for why they should be accepted at all points along the
> way.

Hmm, okay, in these cases, I agree there's no real justification if
we're not seeing it as an intermediate step to the upcoming stuff.
Having thought a bit more about this, my underlying dumbness was
setting everything on the devm horse when converting introducing the
helpers, and then splitted out the change to devm in even more patches
... Silly me, I better should have catched some sleep instead :o

>> In the queue are also other minor cleanups like using dev_err()
>> instead of printk(), etc. Should I send these separately ?
> 
> Of course.

Ok. I'll collect those things in a separate branch and send out the
queue from time to time:

https://github.com/metux/linux/tree/wip/serial/charlady

>> By the way: do you have some public branch where you're collecting
>> accepted patches, which I could base mine on ? (tty.git/tty-next ?)
> 
> Yes, that is the tree and branch, but remember that none of my trees can
> open up until after -rc1 is out.

So, within a merge window, you put everything else on hold ?


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

[Greg KH]

On Fri, Mar 15, 2019 at 08:12:47PM +0100, Enrico Weigelt, metux IT consult wrote:
> On 15.03.19 15:26, Greg KH wrote:
> > Yes, there are lots of drivers for devices that are never unloaded or
> > removed from the system.  The fact that no one has reported any problems
> > with them means that they are never used in situations like this.
> 
> So, never touch a running system ?

No, it's just that those systems do not allow those devices to be
removed because they are probably not on a removable bus.

> > No, you need to have a good reason why it needs to be changed, when you
> > can not verify that this actually resolves a problem.  As this patch
> > shows, you just replaced one api call with another, so nothing changed
> > at all, except you actually took up more memory and logic to do the same
> > thing :(
> 
> Okay, I was on a wrong track here - I had the silly idea that it would
> make things easier if we'd do it the same way everywhere.

"Consistent" is good, and valid, but touching old drivers that have few
users is always risky, and you need a solid reason to do so.

> >> IMHO, we should have a closer look at those and check whether that's
> >> really okay (just adding request/release blindly could cause trouble)
> > 
> > I agree, please feel free to audit these to verify they are all correct.
> > But that's not what you did here, so that's not a valid justification
> > for these patches to be accepted.
> 
> Understood. Assuming I've found some of these cases, shall I use devm
> oder just add the missing release ?

If it actually makes the code "simpler" or "more obvious", sure, that's
fine.  But churn for churns sake is not ok.

> >> By the way: do you have some public branch where you're collecting
> >> accepted patches, which I could base mine on ? (tty.git/tty-next ?)
> > 
> > Yes, that is the tree and branch, but remember that none of my trees can
> > open up until after -rc1 is out.
> 
> So, within a merge window, you put everything else on hold ?

I put the review of new patch submissions on hold, yes.  Almost all
maintainers do that as we can not add new patches to our trees at that
point in time.

And I do have other things I do during that period so it's not like I'm
just sitting around doing nothing :)

thanks,

greg k-h

Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

[Enrico Weigelt, metux IT consult]

On 16.03.19 04:26, Greg KH wrote:

> No, it's just that those systems do not allow those devices to be
> removed because they are probably not on a removable bus.

Ok, devices (hw) might not be removable - that also the case for uarts
builtin some SoCs, or the good old PC w/ 8250. But does that also mean
that the driver should not be removable ?

IMHO, even if that's the case, it's still inconsistent. The driver then
shouldn't support a remove at all (or even builtin only), not just
incomplete remove.

>> Okay, I was on a wrong track here - I had the silly idea that it would
>> make things easier if we'd do it the same way everywhere.
> 
> "Consistent" is good, and valid, but touching old drivers that have few
> users is always risky, and you need a solid reason to do so.

Understood.

By the way: do we have some people who have those old hw and could test?
Should we (try to) create some ? Perhaps some "tester" entry in
MAINTAINERS file ? (I could ask around several people who might have
lots of old / rare hardware.)

>> Understood. Assuming I've found some of these cases, shall I use devm
>> oder just add the missing release ?
> 
> If it actually makes the code "simpler" or "more obvious", sure, that's
> fine.  But churn for churns sake is not ok.

Ok.

> I put the review of new patch submissions on hold, yes.  Almost all
> maintainers do that as we can not add new patches to our trees at that
> point in time.

hmm, looks like a pipeline stall ;-)
why not collecting in a separate branch, which later gets rebased to
mainline when rc is out ?

> And I do have other things I do during that period so it's not like I'm
> just sitting around doing nothing :)

So it's also a fixed schedule for your other work. Understood.

It seems that this workflow can confuse people. Few days ago, somebody
became nervous about missing reactions on patches. Your autoresponder
worked for me, but maybe not for everybody.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

[Greg KH]

On Sat, Mar 16, 2019 at 10:17:11AM +0100, Enrico Weigelt, metux IT consult wrote:
> On 16.03.19 04:26, Greg KH wrote:
> 
> > No, it's just that those systems do not allow those devices to be
> > removed because they are probably not on a removable bus.
> 
> Ok, devices (hw) might not be removable - that also the case for uarts
> builtin some SoCs, or the good old PC w/ 8250. But does that also mean
> that the driver should not be removable ?

No, but 'rmmod' is not a normal operation that anyone ever does in a
working system.  It is only for developer's ease-of-use.

> IMHO, even if that's the case, it's still inconsistent. The driver then
> shouldn't support a remove at all (or even builtin only), not just
> incomplete remove.

Cleaning up properly when the module is unloaded is a good idea, but so
far the patches you submitted did not change anything from a logic point
of view.  They all just cleaned up memory the same way it was cleaned up
before, so I really do not understand what you are trying to do here.

> >> Okay, I was on a wrong track here - I had the silly idea that it would
> >> make things easier if we'd do it the same way everywhere.
> > 
> > "Consistent" is good, and valid, but touching old drivers that have few
> > users is always risky, and you need a solid reason to do so.
> 
> Understood.
> 
> By the way: do we have some people who have those old hw and could test?
> Should we (try to) create some ? Perhaps some "tester" entry in
> MAINTAINERS file ? (I could ask around several people who might have
> lots of old / rare hardware.)

Let's not clutter up MAINTAINERS with anything else please.

> >> Understood. Assuming I've found some of these cases, shall I use devm
> >> oder just add the missing release ?
> > 
> > If it actually makes the code "simpler" or "more obvious", sure, that's
> > fine.  But churn for churns sake is not ok.
> 
> Ok.
> 
> > I put the review of new patch submissions on hold, yes.  Almost all
> > maintainers do that as we can not add new patches to our trees at that
> > point in time.
> 
> hmm, looks like a pipeline stall ;-)
> why not collecting in a separate branch, which later gets rebased to
> mainline when rc is out ?

I do do that for subsystems that actually have a high patch rate.  The
tty/serial subsystem is not such a thing, and it can handle 2 weeks of
delay just fine.

> > And I do have other things I do during that period so it's not like I'm
> > just sitting around doing nothing :)
> 
> So it's also a fixed schedule for your other work. Understood.
> 
> It seems that this workflow can confuse people. Few days ago, somebody
> became nervous about missing reactions on patches. Your autoresponder
> worked for me, but maybe not for everybody.

Why would it not work for everybody?  Kernel development has been done
in this manner for over a decade.  Having a 2 week window like this is
good for the maintainers, remember they are the most limited resource we
have, not developers.

thanks,

greg k-h

Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

[Maciej W. Rozycki]

On Sat, 16 Mar 2019, Enrico Weigelt, metux IT consult wrote:

> > No, it's just that those systems do not allow those devices to be
> > removed because they are probably not on a removable bus.
> 
> Ok, devices (hw) might not be removable - that also the case for uarts
> builtin some SoCs, or the good old PC w/ 8250. But does that also mean
> that the driver should not be removable ?
> 
> IMHO, even if that's the case, it's still inconsistent. The driver then
> shouldn't support a remove at all (or even builtin only), not just
> incomplete remove.

 This device (as well as `dz') is typically used for the serial console as 
well, so being built-in is the usual configuration.  Nevertheless modular 
operation is supposed to be supported, however it may not have been 
verified for ages.

 A further complication is in the virtual console configuration one of the 
serial lines is dedicated for the keyboard, so again you want the driver 
built-in (although hooking up the virtual console keyboard this way has 
been broken with the conversion to the serial core in the 2.6 timeframe 
and I have never figured it out how it is supposed to be done correctly 
with the new serial infrastructure and SERIO_SERPORT; I believe some 
platforms do it with the use of horrible hacks rather than SERIO_SERPORT).

  Maciej

[PATCH v2 11/45] drivers: tty: serial: zs: use dev_err() instead of printk()

[Enrico Weigelt, metux IT consult]

Using dev_err() instead of printk() for more consistent output.
(prints device name, etc).

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/zs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index 0b1ec2f..c3e23d4 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -767,7 +767,7 @@ static int zs_startup(struct uart_port *uport)
 				  IRQF_SHARED, "scc", scc);
 		if (ret) {
 			atomic_add(-1, &scc->irq_guard);
-			printk(KERN_ERR "zs: can't get irq %d\n",
+			dev_err(uport->dev, "zs: can't get irq %d\n",
 			       zport->port.irq);
 			return ret;
 		}
@@ -996,7 +996,7 @@ static int zs_map_port(struct uart_port *uport)
 						      uport->mapbase,
 						      ZS_CHAN_IO_SIZE);
 	if (!uport->membase) {
-		printk(KERN_ERR "zs: Cannot map MMIO\n");
+		dev_err(port->dev, "zs: Cannot map MMIO\n");
 		return -ENOMEM;
 	}
 	return 0;
@@ -1008,7 +1008,7 @@ static int zs_request_port(struct uart_port *uport)
 
 	if (!devm_request_mem_region(uport->mapbase,
 				     ZS_CHAN_IO_SIZE, "scc")) {
-		printk(KERN_ERR "zs: Unable to reserve MMIO resource\n");
+		dev_err(uport->dev, "zs: Unable to reserve MMIO resource\n");
 		return -EBUSY;
 	}
 	ret = zs_map_port(uport);
-- 
1.9.1

[PATCH v2 12/45] drivers: tty: serial: xilinx_uartps: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/xilinx_uartps.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 74089f5..6684ed7 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -953,15 +953,21 @@ static int cdns_uart_verify_port(struct uart_port *port,
  */
 static int cdns_uart_request_port(struct uart_port *port)
 {
-	if (!request_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE,
-					 CDNS_UART_NAME)) {
+	if (!devm_request_mem_region(port->dev,
+				     port->mapbase,
+				     CDNS_UART_REGISTER_SPACE,
+				     CDNS_UART_NAME)) {
 		return -ENOMEM;
 	}
 
-	port->membase = ioremap(port->mapbase, CDNS_UART_REGISTER_SPACE);
+	port->membase = devm_ioremap(port->dev,
+				     port->mapbase,
+				     CDNS_UART_REGISTER_SPACE);
 	if (!port->membase) {
 		dev_err(port->dev, "Unable to map registers\n");
-		release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+		devm_release_mem_region(port->dev,
+					port->mapbase,
+					CDNS_UART_REGISTER_SPACE);
 		return -ENOMEM;
 	}
 	return 0;
@@ -976,8 +982,10 @@ static int cdns_uart_request_port(struct uart_port *port)
  */
 static void cdns_uart_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
-	iounmap(port->membase);
+	devm_release_mem_region(port->dev,
+				port->mapbase,
+				CDNS_UART_REGISTER_SPACE);
+	devm_iounmap(port->dev, port->membase);
 	port->membase = NULL;
 }
 
-- 
1.9.1

[PATCH v2 13/45] drivers: tty: serial: 21285: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/21285.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 32b3acf..4ce2de2 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -305,12 +305,13 @@ static const char *serial21285_type(struct uart_port *port)
 
 static void serial21285_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, 32);
+	devm_release_mem_region(port->dev, port->mapbase, 32);
 }
 
 static int serial21285_request_port(struct uart_port *port)
 {
-	return request_mem_region(port->mapbase, 32, serial21285_name)
+	return devm_request_mem_region(port->dev, port->mapbase,
+				       32, serial21285_name)
 			 != NULL ? 0 : -EBUSY;
 }
 
-- 
1.9.1

[PATCH v2 14/45] drivers: tty: serial: vr41xx_siu: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/vr41xx_siu.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/vr41xx_siu.c b/drivers/tty/serial/vr41xx_siu.c
index 6d106e3..3f69603 100644
--- a/drivers/tty/serial/vr41xx_siu.c
+++ b/drivers/tty/serial/vr41xx_siu.c
@@ -617,12 +617,12 @@ static void siu_release_port(struct uart_port *port)
 	unsigned long size;
 
 	if (port->flags	& UPF_IOREMAP) {
-		iounmap(port->membase);
+		devm_iounmap(port->dev, port->membase);
 		port->membase = NULL;
 	}
 
 	size = siu_port_size(port);
-	release_mem_region(port->mapbase, size);
+	devm_release_mem_region(port->dev, port->mapbase, size);
 }
 
 static int siu_request_port(struct uart_port *port)
@@ -631,12 +631,15 @@ static int siu_request_port(struct uart_port *port)
 	struct resource *res;
 
 	size = siu_port_size(port);
-	res = request_mem_region(port->mapbase, size, siu_type_name(port));
+	res = devm_request_mem_region(port->dev,
+				      port->mapbase,
+				      size,
+				      siu_type_name(port));
 	if (res == NULL)
 		return -EBUSY;
 
 	if (port->flags & UPF_IOREMAP) {
-		port->membase = ioremap(port->mapbase, size);
+		port->membase = devm_ioremap(port->dev, port->mapbase, size);
 		if (port->membase == NULL) {
 			release_resource(res);
 			return -ENOMEM;
-- 
1.9.1

[PATCH v2 15/45] drivers: tty: serial: uartlite: use dev_err() instead of printk()

[Enrico Weigelt, metux IT consult]

Using dev_err() instead of printk() for more consistent output.
(prints device name, etc).

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/uartlite.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index b8b912b..aa461b8 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -352,7 +352,7 @@ static int ulite_request_port(struct uart_port *port)
 	struct uartlite_data *pdata = port->private_data;
 	int ret;
 
-	pr_debug("ulite console: port=%p; port->mapbase=%llx\n",
+	dev_dbg(port->dev, "ulite console: port=%p; port->mapbase=%llx\n",
 		 port, (unsigned long long) port->mapbase);
 
 	if (!request_mem_region(port->mapbase, ULITE_REGION, "uartlite")) {
@@ -519,7 +519,7 @@ static int ulite_console_setup(struct console *co, char *options)
 
 	/* Has the device been initialized yet? */
 	if (!port->mapbase) {
-		pr_debug("console on ttyUL%i not present\n", co->index);
+		dev_dbg(port->dev, "console on ttyUL%i not present\n", co->index);
 		return -ENODEV;
 	}
 
-- 
1.9.1

[PATCH v2 16/45] drivers: tty: serial: uartlite: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/uartlite.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/uartlite.c b/drivers/tty/serial/uartlite.c
index aa461b8..f068d00 100644
--- a/drivers/tty/serial/uartlite.c
+++ b/drivers/tty/serial/uartlite.c
@@ -342,8 +342,8 @@ static const char *ulite_type(struct uart_port *port)
 
 static void ulite_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, ULITE_REGION);
-	iounmap(port->membase);
+	devm_release_mem_region(port->dev, port->mapbase, ULITE_REGION);
+	devm_iounmap(port->dev, port->membase);
 	port->membase = NULL;
 }
 
@@ -355,15 +355,20 @@ static int ulite_request_port(struct uart_port *port)
 	dev_dbg(port->dev, "ulite console: port=%p; port->mapbase=%llx\n",
 		 port, (unsigned long long) port->mapbase);
 
-	if (!request_mem_region(port->mapbase, ULITE_REGION, "uartlite")) {
+	if (!devm_request_mem_region(port->dev,
+				     port->mapbase,
+				     ULITE_REGION,
+				     "uartlite")) {
 		dev_err(port->dev, "Memory region busy\n");
 		return -EBUSY;
 	}
 
-	port->membase = ioremap(port->mapbase, ULITE_REGION);
+	port->membase = devm_ioremap(port->dev, port->mapbase, ULITE_REGION);
 	if (!port->membase) {
 		dev_err(port->dev, "Unable to map registers\n");
-		release_mem_region(port->mapbase, ULITE_REGION);
+		devm_release_mem_region(port->dev,
+					port->mapbase,
+					ULITE_REGION);
 		return -EBUSY;
 	}
 
-- 
1.9.1

[PATCH v2 17/45] drivers: tty: serial: timuart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/timbuart.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/timbuart.c b/drivers/tty/serial/timbuart.c
index 19d38b5..292354b 100644
--- a/drivers/tty/serial/timbuart.c
+++ b/drivers/tty/serial/timbuart.c
@@ -319,11 +319,11 @@ static void timbuart_release_port(struct uart_port *port)
 		resource_size(platform_get_resource(pdev, IORESOURCE_MEM, 0));
 
 	if (port->flags & UPF_IOREMAP) {
-		iounmap(port->membase);
+		devm_iounmap(port->dev, port->membase);
 		port->membase = NULL;
 	}
 
-	release_mem_region(port->mapbase, size);
+	devm_release_mem_region(port->dev, port->mapbase, size);
 }
 
 static int timbuart_request_port(struct uart_port *port)
@@ -332,13 +332,18 @@ static int timbuart_request_port(struct uart_port *port)
 	int size =
 		resource_size(platform_get_resource(pdev, IORESOURCE_MEM, 0));
 
-	if (!request_mem_region(port->mapbase, size, "timb-uart"))
+	if (!devm_request_mem_region(port->dev,
+				     port->mapbase,
+				     size,
+				     "timb-uart"))
 		return -EBUSY;
 
 	if (port->flags & UPF_IOREMAP) {
 		port->membase = ioremap(port->mapbase, size);
 		if (port->membase == NULL) {
-			release_mem_region(port->mapbase, size);
+			devm-release_mem_region(port->dev,
+						port->mapbase,
+						size);
 			return -ENOMEM;
 		}
 	}
-- 
1.9.1

[PATCH v2 18/45] drivers: tty: serial: sirfsoc_uart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/sirfsoc_uart.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index db2e08d..64ea1e7 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -1042,14 +1042,14 @@ static int sirfsoc_uart_request_port(struct uart_port *port)
 	struct sirfsoc_uart_port *sirfport = to_sirfport(port);
 	struct sirfsoc_uart_param *uart_param = &sirfport->uart_reg->uart_param;
 	void *ret;
-	ret = request_mem_region(port->mapbase,
+	ret = devm_request_mem_region(port->dev, port->mapbase,
 		SIRFUART_MAP_SIZE, uart_param->port_name);
 	return ret ? 0 : -EBUSY;
 }
 
 static void sirfsoc_uart_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, SIRFUART_MAP_SIZE);
+	devm_release_mem_region(port->dev, port->mapbase, SIRFUART_MAP_SIZE);
 }
 
 static void sirfsoc_uart_config_port(struct uart_port *port, int flags)
-- 
1.9.1

[PATCH v2 19/45] drivers: tty: serial: sh-sci: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/sh-sci.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 060fcd4..e40b0d0 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2667,7 +2667,9 @@ static int sci_remap_port(struct uart_port *port)
 		return 0;
 
 	if (port->dev->of_node || (port->flags & UPF_IOREMAP)) {
-		port->membase = ioremap_nocache(port->mapbase, sport->reg_size);
+		port->membase = devm_ioremap_nocache(port->dev,
+						     port->mapbase,
+						     sport->reg_size);
 		if (unlikely(!port->membase)) {
 			dev_err(port->dev, "can't remap port#%d\n", port->line);
 			return -ENXIO;
@@ -2689,11 +2691,11 @@ static void sci_release_port(struct uart_port *port)
 	struct sci_port *sport = to_sci_port(port);
 
 	if (port->dev->of_node || (port->flags & UPF_IOREMAP)) {
-		iounmap(port->membase);
+		devm_iounmap(port->dev, port->membase);
 		port->membase = NULL;
 	}
 
-	release_mem_region(port->mapbase, sport->reg_size);
+	devm_release_mem_region(port->dev, port->mapbase, sport->reg_size);
 }
 
 static int sci_request_port(struct uart_port *port)
@@ -2702,8 +2704,10 @@ static int sci_request_port(struct uart_port *port)
 	struct sci_port *sport = to_sci_port(port);
 	int ret;
 
-	res = request_mem_region(port->mapbase, sport->reg_size,
-				 dev_name(port->dev));
+	res = devm_request_mem_region(port->dev,
+				      port->mapbase,
+				      sport->reg_size,
+				      dev_name(port->dev));
 	if (unlikely(res == NULL)) {
 		dev_err(port->dev, "request_mem_region failed.");
 		return -EBUSY;
-- 
1.9.1

[PATCH v2 20/45] drivers: tty: serial: msm_serial: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/msm_serial.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index 1090960..e8e0c87 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -1324,8 +1324,8 @@ static void msm_release_port(struct uart_port *port)
 		return;
 	size = resource_size(uart_resource);
 
-	release_mem_region(port->mapbase, size);
-	iounmap(port->membase);
+	devm_release_mem_region(port->dev, port->mapbase, size);
+	devm_iounmap(port->dev, port->membase);
 	port->membase = NULL;
 }
 
@@ -1342,10 +1342,13 @@ static int msm_request_port(struct uart_port *port)
 
 	size = resource_size(uart_resource);
 
-	if (!request_mem_region(port->mapbase, size, "msm_serial"))
+	if (!devm_request_mem_region(port->dev,
+				     port->mapbase,
+				     size,
+				     "msm_serial"))
 		return -EBUSY;
 
-	port->membase = ioremap(port->mapbase, size);
+	port->membase = ioremap(port->dev, port->mapbase, size);
 	if (!port->membase) {
 		ret = -EBUSY;
 		goto fail_release_port;
@@ -1354,7 +1357,7 @@ static int msm_request_port(struct uart_port *port)
 	return 0;
 
 fail_release_port:
-	release_mem_region(port->mapbase, size);
+	devm_release_mem_region(port->dev, port->mapbase, size);
 	return ret;
 }
 
-- 
1.9.1

Re: [PATCH v2 20/45] drivers: tty: serial: msm_serial: use devm_* functions

[Bjorn Andersson]

On Thu 14 Mar 15:33 PDT 2019, Enrico Weigelt, metux IT consult wrote:

> Use the safer devm versions of memory mapping functions.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

As pointed out by others, this resource does not follow the life cycle
of the port->dev, so I don't think this improves the code.

Regards,
Bjorn

> ---
>  drivers/tty/serial/msm_serial.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
> index 1090960..e8e0c87 100644
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -1324,8 +1324,8 @@ static void msm_release_port(struct uart_port *port)
>  		return;
>  	size = resource_size(uart_resource);
>  
> -	release_mem_region(port->mapbase, size);
> -	iounmap(port->membase);
> +	devm_release_mem_region(port->dev, port->mapbase, size);
> +	devm_iounmap(port->dev, port->membase);
>  	port->membase = NULL;
>  }
>  
> @@ -1342,10 +1342,13 @@ static int msm_request_port(struct uart_port *port)
>  
>  	size = resource_size(uart_resource);
>  
> -	if (!request_mem_region(port->mapbase, size, "msm_serial"))
> +	if (!devm_request_mem_region(port->dev,
> +				     port->mapbase,
> +				     size,
> +				     "msm_serial"))
>  		return -EBUSY;
>  
> -	port->membase = ioremap(port->mapbase, size);
> +	port->membase = ioremap(port->dev, port->mapbase, size);
>  	if (!port->membase) {
>  		ret = -EBUSY;
>  		goto fail_release_port;
> @@ -1354,7 +1357,7 @@ static int msm_request_port(struct uart_port *port)
>  	return 0;
>  
>  fail_release_port:
> -	release_mem_region(port->mapbase, size);
> +	devm_release_mem_region(port->dev, port->mapbase, size);
>  	return ret;
>  }
>  
> -- 
> 1.9.1
> 

[PATCH v2 21/45] drivers: tty: serial: altera_jtaguart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/altera_jtaguart.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
index c90e503..13cab5d 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -448,7 +448,9 @@ static int altera_jtaguart_probe(struct platform_device *pdev)
 	else
 		return -ENODEV;
 
-	port->membase = ioremap(port->mapbase, ALTERA_JTAGUART_SIZE);
+	port->membase = devm_ioremap(port->dev,
+				     port->mapbase,
+				     ALTERA_JTAGUART_SIZE);
 	if (!port->membase)
 		return -ENOMEM;
 
@@ -474,7 +476,7 @@ static int altera_jtaguart_remove(struct platform_device *pdev)
 
 	port = &altera_jtaguart_ports[i].port;
 	uart_remove_one_port(&altera_jtaguart_driver, port);
-	iounmap(port->membase);
+	devm_iounmap(port->dev, port->membase);
 
 	return 0;
 }
-- 
1.9.1

[PATCH v2 22/45] drivers: tty: serial: altera_uart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.
The iounmap() call isn't necessary anymore, as devm will clean up.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/altera_uart.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 0e487ce..d4f7150 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -595,7 +595,9 @@ static int altera_uart_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	port->membase = ioremap(port->mapbase, ALTERA_UART_SIZE);
+	port->membase = devm_ioremap(port->dev,
+				     port->mapbase,
+				     ALTERA_UART_SIZE);
 	if (!port->membase)
 		return -ENOMEM;
 
@@ -625,7 +627,6 @@ static int altera_uart_remove(struct platform_device *pdev)
 	if (port) {
 		uart_remove_one_port(&altera_uart_driver, port);
 		port->mapbase = 0;
-		iounmap(port->membase);
 	}
 
 	return 0;
-- 
1.9.1

[PATCH v2 23/45] drivers: tty: serial: amba-pl010: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/amba-pl010.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index 109b8df..7abd1f8 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -503,7 +503,7 @@ static const char *pl010_type(struct uart_port *port)
  */
 static void pl010_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, UART_PORT_SIZE);
+	devm_release_mem_region(port->dev, port->mapbase, UART_PORT_SIZE);
 }
 
 /*
@@ -511,7 +511,10 @@ static void pl010_release_port(struct uart_port *port)
  */
 static int pl010_request_port(struct uart_port *port)
 {
-	return request_mem_region(port->mapbase, UART_PORT_SIZE, "uart-pl010")
+	return devm_request_mem_region(port->dev,
+				       port->mapbase,
+				       UART_PORT_SIZE,
+				       "uart-pl010")
 			!= NULL ? 0 : -EBUSY;
 }
 
-- 
1.9.1

[PATCH v2 24/45] drivers: tty: serial: mxs-auart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/mxs-auart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 27235a5..0a43c69 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1685,7 +1685,7 @@ static int mxs_auart_probe(struct platform_device *pdev)
 	}
 
 	s->port.mapbase = r->start;
-	s->port.membase = ioremap(r->start, resource_size(r));
+	s->port.membase = devm_ioremap_resource(s->port.dev, r);
 	s->port.ops = &mxs_auart_ops;
 	s->port.iotype = UPIO_MEM;
 	s->port.fifosize = MXS_AUART_FIFO_SIZE;
-- 
1.9.1

[PATCH v2 25/45] drivers: tty: serial: pxa: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/pxa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 4932b67..9512a9f 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -892,7 +892,7 @@ static int serial_pxa_probe(struct platform_device *dev)
 	}
 	snprintf(sport->name, PXA_NAME_LEN - 1, "UART%d", sport->port.line + 1);
 
-	sport->port.membase = ioremap(mmres->start, resource_size(mmres));
+	sport->port.membase = devm_ioremap_resource(sport->port.dev, mmres);
 	if (!sport->port.membase) {
 		ret = -ENOMEM;
 		goto err_clk;
-- 
1.9.1

[PATCH v2 26/45] drivers: tty: serial: dz: use dev_err() instead of printk()

[Enrico Weigelt, metux IT consult]

Using dev_err() instead of printk() for more consistent output.
(prints device name, etc).

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/dz.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index 7b57e84..96e35af 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -416,7 +416,7 @@ static int dz_startup(struct uart_port *uport)
 			  IRQF_SHARED, "dz", mux);
 	if (ret) {
 		atomic_add(-1, &mux->irq_guard);
-		printk(KERN_ERR "dz: Cannot get IRQ %d!\n", dport->port.irq);
+		dev_err(uport->dev, "Cannot get IRQ %d!\n", dport->port.irq);
 		return ret;
 	}
 
@@ -680,7 +680,7 @@ static int dz_map_port(struct uart_port *uport)
 		uport->membase = ioremap_nocache(uport->mapbase,
 						 dec_kn_slot_size);
 	if (!uport->membase) {
-		printk(KERN_ERR "dz: Cannot map MMIO\n");
+		dev_err(uport->dev, "Cannot map MMIO\n");
 		return -ENOMEM;
 	}
 	return 0;
@@ -697,8 +697,8 @@ static int dz_request_port(struct uart_port *uport)
 		if (!request_mem_region(uport->mapbase, dec_kn_slot_size,
 					"dz")) {
 			atomic_add(-1, &mux->map_guard);
-			printk(KERN_ERR
-			       "dz: Unable to reserve MMIO resource\n");
+			dev_err(uport->dev,
+				"Unable to reserve MMIO resource\n");
 			return -EBUSY;
 		}
 	}
-- 
1.9.1

[PATCH v2 27/45] drivers: tty: serial: dz: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/dz.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index 96e35af..10a75e3 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -666,19 +666,22 @@ static void dz_release_port(struct uart_port *uport)
 	struct dz_mux *mux = to_dport(uport)->mux;
 	int map_guard;
 
-	iounmap(uport->membase);
+	devm_iounmap(uport->dev, uport->membase);
 	uport->membase = NULL;
 
 	map_guard = atomic_add_return(-1, &mux->map_guard);
 	if (!map_guard)
-		release_mem_region(uport->mapbase, dec_kn_slot_size);
+		devm_release_mem_region(uport->dev,
+					uport->mapbase,
+					dec_kn_slot_size);
 }
 
 static int dz_map_port(struct uart_port *uport)
 {
 	if (!uport->membase)
-		uport->membase = ioremap_nocache(uport->mapbase,
-						 dec_kn_slot_size);
+		uport->membase = devm_ioremap_nocache(uport->dev,
+						      uport->mapbase,
+						      dec_kn_slot_size);
 	if (!uport->membase) {
 		dev_err(uport->dev, "Cannot map MMIO\n");
 		return -ENOMEM;
@@ -694,8 +697,10 @@ static int dz_request_port(struct uart_port *uport)
 
 	map_guard = atomic_add_return(1, &mux->map_guard);
 	if (map_guard == 1) {
-		if (!request_mem_region(uport->mapbase, dec_kn_slot_size,
-					"dz")) {
+		if (!devm_request_mem_region(uport->dev,
+					     uport->mapbase,
+					     dec_kn_slot_size,
+					     "dz")) {
 			atomic_add(-1, &mux->map_guard);
 			dev_err(uport->dev,
 				"Unable to reserve MMIO resource\n");
@@ -706,7 +711,9 @@ static int dz_request_port(struct uart_port *uport)
 	if (ret) {
 		map_guard = atomic_add_return(-1, &mux->map_guard);
 		if (!map_guard)
-			release_mem_region(uport->mapbase, dec_kn_slot_size);
+			devm_release_mem_region(uport->dev,
+						uport->mapbase,
+						dec_kn_slot_size);
 		return ret;
 	}
 	return 0;
-- 
1.9.1

[PATCH v2 28/45] drivers: tty: serial: netx-serial: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/netx-serial.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/netx-serial.c b/drivers/tty/serial/netx-serial.c
index b355686..a696d96 100644
--- a/drivers/tty/serial/netx-serial.c
+++ b/drivers/tty/serial/netx-serial.c
@@ -424,12 +424,13 @@ static const char *netx_type(struct uart_port *port)
 
 static void netx_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, UART_PORT_SIZE);
+	devm_release_mem_region(port->dev, port->mapbase, UART_PORT_SIZE);
 }
 
 static int netx_request_port(struct uart_port *port)
 {
-	return request_mem_region(port->mapbase, UART_PORT_SIZE,
+	return devm_request_mem_region(port->dev,
+				       port->mapbase, UART_PORT_SIZE,
 			DRIVER_NAME) != NULL ? 0 : -EBUSY;
 }
 
-- 
1.9.1

[PATCH v2 29/45] drivers: tty: serial: serial_txx9: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/serial_txx9.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c
index 1b4008d..aa01349 100644
--- a/drivers/tty/serial/serial_txx9.c
+++ b/drivers/tty/serial/serial_txx9.c
@@ -757,15 +757,22 @@ static int serial_txx9_request_resource(struct uart_txx9_port *up)
 		if (!up->port.mapbase)
 			break;
 
-		if (!request_mem_region(up->port.mapbase, size, "serial_txx9")) {
+		if (!devm_request_mem_region(up->port.dev,
+					     up->port.mapbase,
+					     size,
+					     "serial_txx9")) {
 			ret = -EBUSY;
 			break;
 		}
 
 		if (up->port.flags & UPF_IOREMAP) {
-			up->port.membase = ioremap(up->port.mapbase, size);
+			up->port.membase = devm_ioremap(up->port.dev,
+							up->port.mapbase,
+							size);
 			if (!up->port.membase) {
-				release_mem_region(up->port.mapbase, size);
+				devm_release_mem_region(up->port.dev,
+							up->port.mapbase,
+							size);
 				ret = -ENOMEM;
 			}
 		}
@@ -789,11 +796,11 @@ static void serial_txx9_release_resource(struct uart_txx9_port *up)
 			break;
 
 		if (up->port.flags & UPF_IOREMAP) {
-			iounmap(up->port.membase);
+			devm_iounmap(up->port.dev, up->port.membase);
 			up->port.membase = NULL;
 		}
 
-		release_mem_region(up->port.mapbase, size);
+		devm_release_mem_region(up->port.dev, up->port.mapbase, size);
 		break;
 
 	case UPIO_PORT:
-- 
1.9.1

[PATCH v2 30/45] drivers: tty: serial: serial_ks8695: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.
---
 drivers/tty/serial/serial_ks8695.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_ks8695.c b/drivers/tty/serial/serial_ks8695.c
index b461d79..f901eea 100644
--- a/drivers/tty/serial/serial_ks8695.c
+++ b/drivers/tty/serial/serial_ks8695.c
@@ -482,7 +482,7 @@ static const char *ks8695uart_type(struct uart_port *port)
  */
 static void ks8695uart_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, UART_PORT_SIZE);
+	devm_release_mem_region(port->dev, port->mapbase, UART_PORT_SIZE);
 }
 
 /*
@@ -490,7 +490,9 @@ static void ks8695uart_release_port(struct uart_port *port)
  */
 static int ks8695uart_request_port(struct uart_port *port)
 {
-	return request_mem_region(port->mapbase, UART_PORT_SIZE,
+	return devm_request_mem_region(port->dev,
+				       port->mapbase,
+				       UART_PORT_SIZE,
 			"serial_ks8695") != NULL ? 0 : -EBUSY;
 }
 
-- 
1.9.1

[PATCH v2 31/45] drivers: tty: serial: amba-pl011: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/amba-pl011.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 89ade21..2a968b2 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2098,7 +2098,7 @@ static const char *pl011_type(struct uart_port *port)
  */
 static void pl011_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, SZ_4K);
+	devm_release_mem_region(port->dev, port->mapbase, SZ_4K);
 }
 
 /*
@@ -2106,7 +2106,10 @@ static void pl011_release_port(struct uart_port *port)
  */
 static int pl011_request_port(struct uart_port *port)
 {
-	return request_mem_region(port->mapbase, SZ_4K, "uart-pl011")
+	return devm_request_mem_region(port->dev,
+				       port->mapbase,
+				       SZ_4K,
+				       "uart-pl011")
 			!= NULL ? 0 : -EBUSY;
 }
 
-- 
1.9.1

[PATCH v2 32/45] drivers: tty: serial: atmel_serial: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/atmel_serial.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 05147fe..084f106 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2389,10 +2389,10 @@ static void atmel_release_port(struct uart_port *port)
 	struct platform_device *mpdev = to_platform_device(port->dev->parent);
 	int size = resource_size(mpdev->resource);
 
-	release_mem_region(port->mapbase, size);
+	devm_release_mem_region(port->dev, port->mapbase, size);
 
 	if (port->flags & UPF_IOREMAP) {
-		iounmap(port->membase);
+		devm_iounmap(port->dev, port->membase);
 		port->membase = NULL;
 	}
 }
@@ -2405,13 +2405,16 @@ static int atmel_request_port(struct uart_port *port)
 	struct platform_device *mpdev = to_platform_device(port->dev->parent);
 	int size = resource_size(mpdev->resource);
 
-	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
+	if (!devm_request_mem_region(port->dev,
+				     port->mapbase,
+				     size,
+				     "atmel_serial"))
 		return -EBUSY;
 
 	if (port->flags & UPF_IOREMAP) {
-		port->membase = ioremap(port->mapbase, size);
+		port->membase = devm_ioremap(port->dev, port->mapbase, size);
 		if (port->membase == NULL) {
-			release_mem_region(port->mapbase, size);
+			devm_release_mem_region(port->dev, port->mapbase, size);
 			return -ENOMEM;
 		}
 	}
-- 
1.9.1

[PATCH v2 33/45] drivers: tty: serial: sb1250-duart: use dev_err() instead of printk()

[Enrico Weigelt, metux IT consult]

Using dev_err() instead of printk() for more consistent output.
(prints device name, etc).

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/sb1250-duart.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
index 329aced..655961c 100644
--- a/drivers/tty/serial/sb1250-duart.c
+++ b/drivers/tty/serial/sb1250-duart.c
@@ -663,7 +663,6 @@ static void sbd_release_port(struct uart_port *uport)
 
 static int sbd_map_port(struct uart_port *uport)
 {
-	const char *err = KERN_ERR "sbd: Cannot map MMIO\n";
 	struct sbd_port *sport = to_sport(uport);
 	struct sbd_duart *duart = sport->duart;
 
@@ -671,7 +670,7 @@ static int sbd_map_port(struct uart_port *uport)
 		uport->membase = ioremap_nocache(uport->mapbase,
 						 DUART_CHANREG_SPACING);
 	if (!uport->membase) {
-		printk(err);
+		dev_err(uport->dev, "Cannot map MMIO (base)\n");
 		return -ENOMEM;
 	}
 
@@ -679,7 +678,7 @@ static int sbd_map_port(struct uart_port *uport)
 		sport->memctrl = ioremap_nocache(duart->mapctrl,
 						 DUART_CHANREG_SPACING);
 	if (!sport->memctrl) {
-		printk(err);
+		dev_err(uport->dev, "Cannot map MMIO (ctrl)\n");
 		iounmap(uport->membase);
 		uport->membase = NULL;
 		return -ENOMEM;
-- 
1.9.1

[PATCH v2 34/45] drivers: tty: serial: sb1250-duart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/sb1250-duart.c | 42 +++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/sb1250-duart.c b/drivers/tty/serial/sb1250-duart.c
index 655961c..e77eef6 100644
--- a/drivers/tty/serial/sb1250-duart.c
+++ b/drivers/tty/serial/sb1250-duart.c
@@ -651,14 +651,18 @@ static void sbd_release_port(struct uart_port *uport)
 	struct sbd_port *sport = to_sport(uport);
 	struct sbd_duart *duart = sport->duart;
 
-	iounmap(sport->memctrl);
+	devm_iounmap(uport->dev, sport->memctrl);
 	sport->memctrl = NULL;
-	iounmap(uport->membase);
+	devm_iounmap(uport->dev, uport->membase);
 	uport->membase = NULL;
 
 	if(refcount_dec_and_test(&duart->map_guard))
-		release_mem_region(duart->mapctrl, DUART_CHANREG_SPACING);
-	release_mem_region(uport->mapbase, DUART_CHANREG_SPACING);
+		devm_release_mem_region(uport->dev,
+					duart->mapctrl,
+					DUART_CHANREG_SPACING);
+	devm_release_mem_region(uport->dev,
+				uport->mapbase,
+				DUART_CHANREG_SPACING);
 }
 
 static int sbd_map_port(struct uart_port *uport)
@@ -667,19 +671,21 @@ static int sbd_map_port(struct uart_port *uport)
 	struct sbd_duart *duart = sport->duart;
 
 	if (!uport->membase)
-		uport->membase = ioremap_nocache(uport->mapbase,
-						 DUART_CHANREG_SPACING);
+		uport->membase = devm_ioremap_nocache(uport->dev,
+						      uport->mapbase,
+						      DUART_CHANREG_SPACING);
 	if (!uport->membase) {
 		dev_err(uport->dev, "Cannot map MMIO (base)\n");
 		return -ENOMEM;
 	}
 
 	if (!sport->memctrl)
-		sport->memctrl = ioremap_nocache(duart->mapctrl,
-						 DUART_CHANREG_SPACING);
+		sport->memctrl = devm_ioremap_nocache(uport->dev,
+						      duart->mapctrl,
+						      DUART_CHANREG_SPACING);
 	if (!sport->memctrl) {
 		dev_err(uport->dev, "Cannot map MMIO (ctrl)\n");
-		iounmap(uport->membase);
+		devm_iounmap(uport->dev, uport->membase);
 		uport->membase = NULL;
 		return -ENOMEM;
 	}
@@ -693,15 +699,18 @@ static int sbd_request_port(struct uart_port *uport)
 	struct sbd_duart *duart = to_sport(uport)->duart;
 	int ret = 0;
 
-	if (!request_mem_region(uport->mapbase, DUART_CHANREG_SPACING,
+	if (!devm_request_mem_region(uport->dev,
+				uport->mapbase, DUART_CHANREG_SPACING,
 				"sb1250-duart")) {
 		printk(err);
 		return -EBUSY;
 	}
 	refcount_inc(&duart->map_guard);
 	if (refcount_read(&duart->map_guard) == 1) {
-		if (!request_mem_region(duart->mapctrl, DUART_CHANREG_SPACING,
-					"sb1250-duart")) {
+		if (!devm_request_mem_region(uport->dev,
+					     duart->mapctrl,
+					     DUART_CHANREG_SPACING,
+					     "sb1250-duart")) {
 			refcount_dec(&duart->map_guard);
 			printk(err);
 			ret = -EBUSY;
@@ -711,12 +720,15 @@ static int sbd_request_port(struct uart_port *uport)
 		ret = sbd_map_port(uport);
 		if (ret) {
 			if (refcount_dec_and_test(&duart->map_guard))
-				release_mem_region(duart->mapctrl,
-						   DUART_CHANREG_SPACING);
+				devm_release_mem_region(uport->dev,
+							duart->mapctrl,
+							DUART_CHANREG_SPACING);
 		}
 	}
 	if (ret) {
-		release_mem_region(uport->mapbase, DUART_CHANREG_SPACING);
+		devm_release_mem_region(uport->dev,
+					uport->mapbase,
+					DUART_CHANREG_SPACING);
 		return ret;
 	}
 	return 0;
-- 
1.9.1

[PATCH v2 35/45] drivers: tty: serial: lpc32xx_hs: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/lpc32xx_hs.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/lpc32xx_hs.c b/drivers/tty/serial/lpc32xx_hs.c
index f4e27d0..0bb86d7 100644
--- a/drivers/tty/serial/lpc32xx_hs.c
+++ b/drivers/tty/serial/lpc32xx_hs.c
@@ -575,11 +575,11 @@ static void serial_lpc32xx_release_port(struct uart_port *port)
 {
 	if ((port->iotype == UPIO_MEM32) && (port->mapbase)) {
 		if (port->flags & UPF_IOREMAP) {
-			iounmap(port->membase);
+			devm_iounmap(port->dev, port->membase);
 			port->membase = NULL;
 		}
 
-		release_mem_region(port->mapbase, SZ_4K);
+		devm_release_mem_region(port->dev, port->mapbase, SZ_4K);
 	}
 }
 
@@ -590,12 +590,19 @@ static int serial_lpc32xx_request_port(struct uart_port *port)
 	if ((port->iotype == UPIO_MEM32) && (port->mapbase)) {
 		ret = 0;
 
-		if (!request_mem_region(port->mapbase, SZ_4K, MODNAME))
+		if (!devm_request_mem_region(port->dev,
+					     port->mapbase,
+					     SZ_4K,
+					     MODNAME))
 			ret = -EBUSY;
 		else if (port->flags & UPF_IOREMAP) {
-			port->membase = ioremap(port->mapbase, SZ_4K);
+			port->membase = devm_ioremap(port->dev,
+						     port->mapbase,
+						     SZ_4K);
 			if (!port->membase) {
-				release_mem_region(port->mapbase, SZ_4K);
+				devm_release_mem_region(port->dev,
+							port->mapbase,
+							SZ_4K);
 				ret = -ENOMEM;
 			}
 		}
-- 
1.9.1

[PATCH v2 36/45] drivers: tty: serial: pic32_uart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/pic32_uart.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/pic32_uart.c b/drivers/tty/serial/pic32_uart.c
index 0bdf168..b4f0b29 100644
--- a/drivers/tty/serial/pic32_uart.c
+++ b/drivers/tty/serial/pic32_uart.c
@@ -614,15 +614,19 @@ static int pic32_uart_request_port(struct uart_port *port)
 	if (unlikely(!res_mem))
 		return -EINVAL;
 
-	if (!request_mem_region(port->mapbase, resource_size(res_mem),
-				"pic32_uart_mem"))
+	if (!devm_request_mem_region(port->dev,
+				     port->mapbase,
+				     resource_size(res_mem),
+				     "pic32_uart_mem"))
 		return -EBUSY;
 
 	port->membase = devm_ioremap_nocache(port->dev, port->mapbase,
 						resource_size(res_mem));
 	if (!port->membase) {
 		dev_err(port->dev, "Unable to map registers\n");
-		release_mem_region(port->mapbase, resource_size(res_mem));
+		devm_release_mem_region(port->dev,
+					port->mapbase,
+					resource_size(res_mem));
 		return -ENOMEM;
 	}
 
@@ -641,7 +645,7 @@ static void pic32_uart_release_port(struct uart_port *port)
 		return;
 	res_size = resource_size(res_mem);
 
-	release_mem_region(port->mapbase, res_size);
+	devm_release_mem_region(port->dev, port->mapbase, res_size);
 }
 
 /* serial core request to do any port required auto-configuration */
-- 
1.9.1

[PATCH v2 37/45] drivers: tty: serial: apbuart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/apbuart.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index 60cd133..5eaaee9 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -293,12 +293,15 @@ static const char *apbuart_type(struct uart_port *port)
 
 static void apbuart_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, 0x100);
+	devm_release_mem_region(port->dev, port->mapbase, 0x100);
 }
 
 static int apbuart_request_port(struct uart_port *port)
 {
-	return request_mem_region(port->mapbase, 0x100, "grlib-apbuart")
+	return devm_request_mem_region(port->dev,
+				       port->mapbase,
+				       0x100,
+				       "grlib-apbuart")
 	    != NULL ? 0 : -EBUSY;
 	return 0;
 }
@@ -622,7 +625,9 @@ static int __init grlib_apbuart_configure(void)
 		port = &grlib_apbuart_ports[line];
 
 		port->mapbase = addr;
-		port->membase = ioremap(addr, sizeof(struct grlib_apbuart_regs_map));
+		port->membase = devm_ioremap(port->dev,
+					     addr,
+					     sizeof(struct grlib_apbuart_regs_map));
 		port->irq = 0;
 		port->iotype = UPIO_MEM;
 		port->ops = &grlib_apbuart_ops;
-- 
1.9.1

[PATCH v2 38/45] drivers: tty: serial: samsung: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/samsung.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index a49cf15..e0bab87 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1436,13 +1436,16 @@ static const char *s3c24xx_serial_type(struct uart_port *port)
 
 static void s3c24xx_serial_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, MAP_SIZE);
+	devm_release_mem_region(port->dev, port->mapbase, MAP_SIZE);
 }
 
 static int s3c24xx_serial_request_port(struct uart_port *port)
 {
 	const char *name = s3c24xx_serial_portname(port);
-	return request_mem_region(port->mapbase, MAP_SIZE, name) ? 0 : -EBUSY;
+	return devm_request_mem_region(port->dev,
+				       port->mapbase,
+				       MAP_SIZE,
+				       name) ? 0 : -EBUSY;
 }
 
 static void s3c24xx_serial_config_port(struct uart_port *port, int flags)
-- 
1.9.1

[PATCH v2 39/45] drivers: tty: serial: efm32-uart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/efm32-uart.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/efm32-uart.c b/drivers/tty/serial/efm32-uart.c
index d6b5e54..a79cadc 100644
--- a/drivers/tty/serial/efm32-uart.c
+++ b/drivers/tty/serial/efm32-uart.c
@@ -437,7 +437,7 @@ static void efm32_uart_release_port(struct uart_port *port)
 
 	clk_unprepare(efm_port->clk);
 	clk_put(efm_port->clk);
-	iounmap(port->membase);
+	devm_iounmap(port->dev, port->membase);
 }
 
 static int efm32_uart_request_port(struct uart_port *port)
@@ -445,7 +445,7 @@ static int efm32_uart_request_port(struct uart_port *port)
 	struct efm32_uart_port *efm_port = to_efm_port(port);
 	int ret;
 
-	port->membase = ioremap(port->mapbase, 60);
+	port->membase = devm_ioremap(port->dev, port->mapbase, 60);
 	if (!efm_port->port.membase) {
 		ret = -ENOMEM;
 		efm_debug(efm_port, "failed to remap\n");
@@ -464,7 +464,7 @@ static int efm32_uart_request_port(struct uart_port *port)
 		clk_put(efm_port->clk);
 err_clk_get:
 
-		iounmap(port->membase);
+		devm_iounmap(port->dev, port->membase);
 err_ioremap:
 		return ret;
 	}
-- 
1.9.1

Re: [PATCH v2 39/45] drivers: tty: serial: efm32-uart: use devm_* functions

[Uwe Kleine-König]

Hello Enrico,

On Thu, Mar 14, 2019 at 11:34:09PM +0100, Enrico Weigelt, metux IT consult wrote:
> Use the safer devm versions of memory mapping functions.

In which aspect is devm_ioremap safer than ioremap?

The only upside I'm aware of is that the memory is automatically
unmapped on device unbind. But we don't benefit from this because an
UART port is "released" before the device is unbound and we call
devm_iounmap() then anyhow. So this patch just adds a memory allocation
(side note: on a platform that is quite tight on RAM) with no added
benefit.

I didn't look at the other patches in this series, but assuming that
they are similar in spirit, the same question applies for them.

Do I miss anything?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v2 40/45] drivers: tty: serial: mpc52xx_uart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/mpc52xx_uart.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 3a75ee0..3e74e44 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -1276,11 +1276,13 @@ static u8 mpc5125_psc_get_mr1(struct uart_port *port)
 
 	/* remapped by us ? */
 	if (port->flags & UPF_IOREMAP) {
-		iounmap(port->membase);
+		devm_iounmap(port->dev, port->membase);
 		port->membase = NULL;
 	}
 
-	release_mem_region(port->mapbase, sizeof(struct mpc52xx_psc));
+	devm_release_mem_region(port->dev,
+				port->mapbase,
+				sizeof(struct mpc52xx_psc));
 }
 
 static int
@@ -1289,13 +1291,14 @@ static u8 mpc5125_psc_get_mr1(struct uart_port *port)
 	int err;
 
 	if (port->flags & UPF_IOREMAP) /* Need to remap ? */
-		port->membase = ioremap(port->mapbase,
+		port->membase = devm_ioremap(port->dev, port->mapbase,
 					sizeof(struct mpc52xx_psc));
 
 	if (!port->membase)
 		return -EINVAL;
 
-	err = request_mem_region(port->mapbase, sizeof(struct mpc52xx_psc),
+	err = devm_request_mem_region(port->mapbase,
+			sizeof(struct mpc52xx_psc),
 			"mpc52xx_psc_uart") != NULL ? 0 : -EBUSY;
 
 	if (err)
@@ -1310,10 +1313,12 @@ static u8 mpc5125_psc_get_mr1(struct uart_port *port)
 	return 0;
 
 out_mapregion:
-	release_mem_region(port->mapbase, sizeof(struct mpc52xx_psc));
+	devm_release_mem_region(port->dev,
+				port->mapbase,
+				sizeof(struct mpc52xx_psc));
 out_membase:
 	if (port->flags & UPF_IOREMAP) {
-		iounmap(port->membase);
+		devm_iounmap(port->dev, port->membase);
 		port->membase = NULL;
 	}
 	return err;
@@ -1653,7 +1658,9 @@ static u8 mpc5125_psc_get_mr1(struct uart_port *port)
 	port->uartclk = uartclk;
 	port->ops	= &mpc52xx_uart_ops;
 	port->mapbase = res.start;
-	port->membase = ioremap(res.start, sizeof(struct mpc52xx_psc));
+	port->membase = devm_ioremap(port->dev,
+				     res.start,
+				     sizeof(struct mpc52xx_psc));
 	port->irq = irq_of_parse_and_map(np, 0);
 
 	if (port->membase == NULL)
-- 
1.9.1

[PATCH v2 41/45] drivers: tty: serial: timuart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/men_z135_uart.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/men_z135_uart.c b/drivers/tty/serial/men_z135_uart.c
index ef89534..349f70e 100644
--- a/drivers/tty/serial/men_z135_uart.c
+++ b/drivers/tty/serial/men_z135_uart.c
@@ -732,7 +732,7 @@ static void men_z135_release_port(struct uart_port *port)
 {
 	struct men_z135_port *uart = to_men_z135(port);
 
-	iounmap(port->membase);
+	devm_iounmap(port->dev, port->membase);
 	port->membase = NULL;
 
 	mcb_release_mem(uart->mem);
@@ -751,7 +751,7 @@ static int men_z135_request_port(struct uart_port *port)
 	port->mapbase = mem->start;
 	uart->mem = mem;
 
-	port->membase = ioremap(mem->start, resource_size(mem));
+	port->membase = devm_ioremap_resource(port->dev, mem);
 	if (port->membase == NULL) {
 		mcb_release_mem(mem);
 		return -ENOMEM;
-- 
1.9.1

[PATCH v2 42/45] drivers: tty: serial: sa1100: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/sa1100.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index a399772b..3aa20a2 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -522,7 +522,7 @@ static void sa1100_release_port(struct uart_port *port)
 	struct sa1100_port *sport =
 		container_of(port, struct sa1100_port, port);
 
-	release_mem_region(sport->port.mapbase, UART_PORT_SIZE);
+	devm_release_mem_region(port->dev, sport->port.mapbase, UART_PORT_SIZE);
 }
 
 /*
@@ -533,7 +533,7 @@ static int sa1100_request_port(struct uart_port *port)
 	struct sa1100_port *sport =
 		container_of(port, struct sa1100_port, port);
 
-	return request_mem_region(sport->port.mapbase, UART_PORT_SIZE,
+	return devm_request_mem_region(port->dev, port->mapbase, UART_PORT_SIZE,
 			"sa11x0-uart") != NULL ? 0 : -EBUSY;
 }
 
-- 
1.9.1

[PATCH v2 43/45] drivers: tty: serial: pmac_zilog: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/pmac_zilog.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index bcb5bf7..bce19b0 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -1411,7 +1411,7 @@ static int __init pmz_init_port(struct uart_pmac_port *uap)
 	if (of_address_to_resource(np, 0, &r_ports))
 		return -ENODEV;
 	uap->port.mapbase = r_ports.start;
-	uap->port.membase = ioremap(uap->port.mapbase, 0x1000);
+	uap->port.membase = devm_ioremap(uap->port->dev, uap->port.mapbase, 0x1000);
 
 	uap->control_reg = uap->port.membase;
 	uap->data_reg = uap->control_reg + 0x10;
@@ -1428,14 +1428,16 @@ static int __init pmz_init_port(struct uart_pmac_port *uap)
 	memset(&r_rxdma, 0, sizeof(struct resource));
 #endif	
 	if (ZS_HAS_DMA(uap)) {
-		uap->tx_dma_regs = ioremap(r_txdma.start, 0x100);
+		uap->tx_dma_regs = devm_ioremap(uap->port.dev,
+					        r_txdma.start, 0x100);
 		if (uap->tx_dma_regs == NULL) {	
 			uap->flags &= ~PMACZILOG_FLAG_HAS_DMA;
 			goto no_dma;
 		}
-		uap->rx_dma_regs = ioremap(r_rxdma.start, 0x100);
+		uap->rx_dma_regs = devm_ioremap(uap->port.dev,
+					        r_rxdma.start, 0x100);
 		if (uap->rx_dma_regs == NULL) {	
-			iounmap(uap->tx_dma_regs);
+			devm_iounmap(uap->port.dev, uap->tx_dma_regs);
 			uap->tx_dma_regs = NULL;
 			uap->flags &= ~PMACZILOG_FLAG_HAS_DMA;
 			goto no_dma;
@@ -1530,9 +1532,9 @@ static void pmz_dispose_port(struct uart_pmac_port *uap)
 	struct device_node *np;
 
 	np = uap->node;
-	iounmap(uap->rx_dma_regs);
-	iounmap(uap->tx_dma_regs);
-	iounmap(uap->control_reg);
+	devm_iounmap(uap->port.dev, uap->rx_dma_regs);
+	devm_iounmap(uap->port.dev, uap->tx_dma_regs);
+	devm_iounmap(uap->port.dev, uap->control_reg);
 	uap->node = NULL;
 	of_node_put(np);
 	memset(uap, 0, sizeof(struct uart_pmac_port));
-- 
1.9.1

[PATCH v2 44/45] drivers: tty: serial: pnx8xxx_uart: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/pnx8xxx_uart.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/pnx8xxx_uart.c b/drivers/tty/serial/pnx8xxx_uart.c
index 223a949..3951226 100644
--- a/drivers/tty/serial/pnx8xxx_uart.c
+++ b/drivers/tty/serial/pnx8xxx_uart.c
@@ -568,10 +568,7 @@ static const char *pnx8xxx_type(struct uart_port *port)
  */
 static void pnx8xxx_release_port(struct uart_port *port)
 {
-	struct pnx8xxx_port *sport =
-		container_of(port, struct pnx8xxx_port, port);
-
-	release_mem_region(sport->port.mapbase, UART_PORT_SIZE);
+	devm_release_mem_region(port->dev, port->mapbase, UART_PORT_SIZE);
 }
 
 /*
@@ -579,9 +576,7 @@ static void pnx8xxx_release_port(struct uart_port *port)
  */
 static int pnx8xxx_request_port(struct uart_port *port)
 {
-	struct pnx8xxx_port *sport =
-		container_of(port, struct pnx8xxx_port, port);
-	return request_mem_region(sport->port.mapbase, UART_PORT_SIZE,
+	return devm_request_mem_region(port->dev, port->mapbase, UART_PORT_SIZE,
 			"pnx8xxx-uart") != NULL ? 0 : -EBUSY;
 }
 
-- 
1.9.1

[PATCH v2 45/45] drivers: tty: serial: mux: use devm_* functions

[Enrico Weigelt, metux IT consult]

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/mux.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/mux.c b/drivers/tty/serial/mux.c
index 00ce31e..cd08f0f 100644
--- a/drivers/tty/serial/mux.c
+++ b/drivers/tty/serial/mux.c
@@ -456,8 +456,9 @@ static int __init mux_probe(struct parisc_device *dev)
 	printk(KERN_INFO "Serial mux driver (%d ports) Revision: 0.6\n", port_count);
 
 	dev_set_drvdata(&dev->dev, (void *)(long)port_count);
-	request_mem_region(dev->hpa.start + MUX_OFFSET,
-                           port_count * MUX_LINE_OFFSET, "Mux");
+	devm_request_mem_region(&dev->dev,
+				dev->hpa.start + MUX_OFFSET,
+				port_count * MUX_LINE_OFFSET, "Mux");
 
 	if(!port_cnt) {
 		mux_driver.cons = MUX_CONSOLE;
@@ -474,7 +475,9 @@ static int __init mux_probe(struct parisc_device *dev)
 		port->iobase	= 0;
 		port->mapbase	= dev->hpa.start + MUX_OFFSET +
 						(i * MUX_LINE_OFFSET);
-		port->membase	= ioremap_nocache(port->mapbase, MUX_LINE_OFFSET);
+		port->membase	= devm_ioremap_nocache(port->dev,
+						       port->mapbase,
+						       MUX_LINE_OFFSET);
 		port->iotype	= UPIO_MEM;
 		port->type	= PORT_MUX;
 		port->irq	= 0;
@@ -517,10 +520,12 @@ static int __exit mux_remove(struct parisc_device *dev)
 
 		uart_remove_one_port(&mux_driver, port);
 		if(port->membase)
-			iounmap(port->membase);
+			devm_iounmap(port->dev, port->membase);
 	}
 
-	release_mem_region(dev->hpa.start + MUX_OFFSET, port_count * MUX_LINE_OFFSET);
+	devm_release_mem_region(&dev->dev,
+				dev->hpa.start + MUX_OFFSET,
+				port_count * MUX_LINE_OFFSET);
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH v2 45/45] drivers: tty: serial: mux: use devm_* functions

[Andy Shevchenko]

On Fri, Mar 15, 2019 at 12:37 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Use the safer devm versions of memory mapping functions.

If you are going to use devm_*_free(), what's the point to have this
change from the beginning?

P.S. Disregard that this is untested series...

> --- a/drivers/tty/serial/mux.c
> +++ b/drivers/tty/serial/mux.c
> @@ -456,8 +456,9 @@ static int __init mux_probe(struct parisc_device *dev)
>         printk(KERN_INFO "Serial mux driver (%d ports) Revision: 0.6\n", port_count);
>
>         dev_set_drvdata(&dev->dev, (void *)(long)port_count);
> -       request_mem_region(dev->hpa.start + MUX_OFFSET,
> -                           port_count * MUX_LINE_OFFSET, "Mux");

> +       devm_request_mem_region(&dev->dev,
> +                               dev->hpa.start + MUX_OFFSET,
> +                               port_count * MUX_LINE_OFFSET, "Mux");

...and on top of this where is error checking?

>
>         if(!port_cnt) {
>                 mux_driver.cons = MUX_CONSOLE;
> @@ -474,7 +475,9 @@ static int __init mux_probe(struct parisc_device *dev)
>                 port->iobase    = 0;
>                 port->mapbase   = dev->hpa.start + MUX_OFFSET +
>                                                 (i * MUX_LINE_OFFSET);
> -               port->membase   = ioremap_nocache(port->mapbase, MUX_LINE_OFFSET);
> +               port->membase   = devm_ioremap_nocache(port->dev,
> +                                                      port->mapbase,
> +                                                      MUX_LINE_OFFSET);
>                 port->iotype    = UPIO_MEM;
>                 port->type      = PORT_MUX;
>                 port->irq       = 0;
> @@ -517,10 +520,12 @@ static int __exit mux_remove(struct parisc_device *dev)
>
>                 uart_remove_one_port(&mux_driver, port);
>                 if(port->membase)
> -                       iounmap(port->membase);
> +                       devm_iounmap(port->dev, port->membase);
>         }
>
> -       release_mem_region(dev->hpa.start + MUX_OFFSET, port_count * MUX_LINE_OFFSET);
> +       devm_release_mem_region(&dev->dev,
> +                               dev->hpa.start + MUX_OFFSET,
> +                               port_count * MUX_LINE_OFFSET);
>         return 0;
>  }


-- 
With Best Regards,
Andy Shevchenko

Re: serial driver cleanups v2

[Andy Shevchenko]

On Fri, Mar 15, 2019 at 12:40 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:

> here's v2 of my serial cleanups queue - part I:
>
> essentially using helpers to code more compact and switching to
> devm_*() functions for mmio management.
>
> Part II will be about moving the mmio range from mapbase and
> mapsize (which are used quite inconsistently) to a struct resource
> and using helpers for that. But this one isn't finished yet.
> (if somebody likes to have a look at it, I can send it, too)

Let's do that way you are preparing a branch somewhere and anounce
here as an RFC, since this was neither tested nor correct.
And selling point for many of them is not true: it doesn't make any
difference in the size in code, but increases a time to run
(devm_ioremap_resource() does more than plain devm_iomap() call).

-- 
With Best Regards,
Andy Shevchenko

Re: serial driver cleanups v2

[Andy Shevchenko]

On Fri, Mar 15, 2019 at 11:12 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Mar 15, 2019 at 12:40 AM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
>
> > here's v2 of my serial cleanups queue - part I:
> >
> > essentially using helpers to code more compact and switching to
> > devm_*() functions for mmio management.
> >
> > Part II will be about moving the mmio range from mapbase and
> > mapsize (which are used quite inconsistently) to a struct resource
> > and using helpers for that. But this one isn't finished yet.
> > (if somebody likes to have a look at it, I can send it, too)
>
> Let's do that way you are preparing a branch somewhere and anounce
> here as an RFC, since this was neither tested nor correct.
> And selling point for many of them is not true: it doesn't make any
> difference in the size in code, but increases a time to run
> (devm_ioremap_resource() does more than plain devm_iomap() call).

And one more thing, perhaps you can run existing and / or contribute
to coccinelle since this all scriptable and maintainers can decide if
this or that coccinelle script is useful.

-- 
With Best Regards,
Andy Shevchenko

Re: serial driver cleanups v2

[Enrico Weigelt, metux IT consult]

On 15.03.19 10:12, Andy Shevchenko wrote:

>> Part II will be about moving the mmio range from mapbase and
>> mapsize (which are used quite inconsistently) to a struct resource
>> and using helpers for that. But this one isn't finished yet.
>> (if somebody likes to have a look at it, I can send it, too)
> 
> Let's do that way you are preparing a branch somewhere and anounce
> here as an RFC, since this was neither tested nor correct.

Okay, here it is:

I. https://github.com/metux/linux/tree/submit/serial-clean-v3
   --> general cleanups, as basis for II

II. https://github.com/metux/linux/tree/wip/serial-res
   --> moving towards using struct resource consistently

III. https://github.com/metux/linux/tree/hack/serial
    --> the final steps, which are yet completely broken
    (more a notepad for things still to do :o)

The actual goal is generalizing the whole iomem handling, so individual
usually just need to call some helpers that do most of the things.
Finally, I also wanted to have all io region information consolidated
in struct resource.

Meanwhile I've learned that I probably was a bit too eager w/ that.
Guess I'll have to rethink my strategy.

> And selling point for many of them is not true: it doesn't make any
> difference in the size in code, but increases a time to run
> (devm_ioremap_resource() does more than plain devm_iomap() call).

Okay, just seen it. Does the the runtime overhead cause any problems ?


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: serial driver cleanups v2

[Andy Shevchenko]

On Fri, Mar 15, 2019 at 11:36:04AM +0100, Enrico Weigelt, metux IT consult wrote:
> On 15.03.19 10:12, Andy Shevchenko wrote:
> 
> >> Part II will be about moving the mmio range from mapbase and
> >> mapsize (which are used quite inconsistently) to a struct resource
> >> and using helpers for that. But this one isn't finished yet.
> >> (if somebody likes to have a look at it, I can send it, too)
> > 
> > Let's do that way you are preparing a branch somewhere and anounce
> > here as an RFC, since this was neither tested nor correct.
> 
> Okay, here it is:
> 
> I. https://github.com/metux/linux/tree/submit/serial-clean-v3
>    --> general cleanups, as basis for II
> 
> II. https://github.com/metux/linux/tree/wip/serial-res
>    --> moving towards using struct resource consistently
> 
> III. https://github.com/metux/linux/tree/hack/serial
>     --> the final steps, which are yet completely broken
>     (more a notepad for things still to do :o)
> 
> The actual goal is generalizing the whole iomem handling, so individual
> usually just need to call some helpers that do most of the things.
> Finally, I also wanted to have all io region information consolidated
> in struct resource.

That's should be a selling point, not just conversion per se.

> > And selling point for many of them is not true: it doesn't make any
> > difference in the size in code, but increases a time to run
> > (devm_ioremap_resource() does more than plain devm_iomap() call).
> 
> Okay, just seen it. Does the the runtime overhead cause any problems ?

You have to explain that in each commit message, that the change does bring a
possible new error message printed.

The performance side of the deal, you are lucky here, is not significant
because it's slow path.

-- 
With Best Regards,
Andy Shevchenko

Re: serial driver cleanups v2

[Enrico Weigelt, metux IT consult]

On 15.03.19 19:11, Andy Shevchenko wrote:

>> The actual goal is generalizing the whole iomem handling, so individual
>> usually just need to call some helpers that do most of the things.
>> Finally, I also wanted to have all io region information consolidated
>> in struct resource.
> 
> That's should be a selling point, not just conversion per se.

hmm, I never was good at selling :o

but I'll try anyways: it shall make the code smaller and easier to
read/understand.

does that count ?

> You have to explain that in each commit message, that the change does bring a
> possible new error message printed.

Ok, I wasn't aware of that. Yes, I missed to read the code of that
function carefully and didn't expect that it does more than just a dumb
wrapper around dev_ioremap() that picks out the params from struct
resource.

OTOH, the name, IMHO, is a bit misleading. Any chance of ever changing
it to a more clear name (eg. devm_request_and_ioremap_resource()) ?

> The performance side of the deal, you are lucky here, is not significant
> because it's slow path.

okay :)


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: serial driver cleanups v2

[Andy Shevchenko]

On Fri, Mar 15, 2019 at 8:44 PM Enrico Weigelt, metux IT consult
<lkml@metux.net> wrote:
> On 15.03.19 19:11, Andy Shevchenko wrote:


> OTOH, the name, IMHO, is a bit misleading. Any chance of ever changing
> it to a more clear name (eg. devm_request_and_ioremap_resource()) ?

Compare  iomap vs. ioREmap.

-- 
With Best Regards,
Andy Shevchenko