From: Michael Opdenacker <michael.opdenacker@free-electrons.com>
To: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>,
Andrew Lunn <andrew@lunn.ch>
Cc: dwmw2@infradead.org, computersforpeace@gmail.com,
jg1.han@samsung.com, linux-mtd@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mtd: orion_nand: fix error code path in probe
Date: Thu, 16 Oct 2014 06:43:28 +0200 [thread overview]
Message-ID: <543F4CF0.8080404@free-electrons.com> (raw)
In-Reply-To: <20141015213953.GB23155@arch.hh.imgtec.org>
Andrew, Ezequiel,
Many thanks for your review!
On 10/15/2014 11:39 PM, Ezequiel Garcia wrote:
> On 14 Oct 11:35 PM, Andrew Lunn wrote:
>
>> Hi Michael
>>
>> It is quite a common pattern to use:
>>
>> res = platform_get_resource(dev, IORESOURCE_MEM, 0);
>> c->membase = devm_ioremap_resource(&dev->dev, res);
>> if (IS_ERR(c->membase))
>> return PTR_ERR(c->membase)
>>
>> which is more compact.
I like it, thanks for the suggestion!
>>
> Be careful with this. devm_ioremap and devm_ioremap_resource are not
> the same thing, as the former requests the region as well.
>
> It can break things if the region is shared across several drivers.
> I don't think this is the case, so in fact adding the request is correct,
> but it's a more intrusive change than just "code cleanup".
Right. If I understand correctly, requesting the region should always be
done anyway, so this should be a welcome change.
What Andrew suggests also changes the return value: -ENOMEM instead of
-EIO, though it should be more standard. This could have side effects too!
I'll post a V2 right away.
Thanks again!
Cheers,
Michael.
--
Michael Opdenacker, CEO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
+33 484 258 098
next prev parent reply other threads:[~2014-10-16 4:43 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-14 14:16 [PATCH] mtd: orion_nand: fix error code path in probe Michael Opdenacker
2014-10-14 21:35 ` Andrew Lunn
2014-10-15 21:39 ` Ezequiel Garcia
2014-10-16 4:43 ` Michael Opdenacker [this message]
2014-10-16 4:58 ` [PATCH v2] " Michael Opdenacker
2014-10-16 7:39 ` Jingoo Han
2014-10-18 19:35 ` Andrew Lunn
2014-10-22 8:49 ` Brian Norris
2014-10-16 5:06 ` [PATCH] " Michael Opdenacker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=543F4CF0.8080404@free-electrons.com \
--to=michael.opdenacker@free-electrons.com \
--cc=andrew@lunn.ch \
--cc=computersforpeace@gmail.com \
--cc=dwmw2@infradead.org \
--cc=ezequiel.garcia@free-electrons.com \
--cc=jg1.han@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox