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

login
register
mail settings
Submitter Enrico Weigelt, metux IT consult
Date March 14, 2019, 10:33 p.m.
Message ID <1552602855-26086-3-git-send-email-info@metux.net>
Download mbox | patch
Permalink /patch/749177/
State New
Headers show

Comments

Enrico Weigelt, metux IT consult - March 14, 2019, 10:33 p.m.
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(-)
Andy Shevchenko - March 15, 2019, 9:04 a.m.
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;
Enrico Weigelt, metux IT consult - March 15, 2019, 9:37 a.m.
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

Patch

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;