Messages in this thread | | | Date | Fri, 13 Nov 2020 08:03:43 +0100 | From | Uwe Kleine-König <> | Subject | About devm_platform_ioremap_resource [Was: Re: [PATCH 01/32] pwm: sun4i: convert to devm_platform_ioremap_resource] |
| |
Hello,
[Added lkml and the people involved in commit 7945f929f1a7 ("drivers: provide devm_platform_ioremap_resource()") to Cc:. For the new readers: This is about patches making use of devm_platform_ioremap_resource() instead of open coding it. Full context at https://lore.kernel.org/r/20201112190649.GA908613@ulmo]
On Thu, Nov 12, 2020 at 10:14:29PM +0100, Uwe Kleine-König wrote: > On Thu, Nov 12, 2020 at 08:06:49PM +0100, Thierry Reding wrote: > > I also think that it's overly narrow is scope, so you can't actually > > "blindly" use this helper and I've seen quite a few cases where this was > > unknowingly used for cases where it shouldn't have been used and then > > broke things (because some drivers must not do the request_mem_region() > > for example). > > You have a link to such an accident?
I got a hint in private here: https://lore.kernel.org/r/1555670144-24220-1-git-send-email-aisheng.dong@nxp.com
devm_platform_ioremap_resource() is platform_get_resource() + devm_ioremap_resource() and here it was used to replace platform_get_resource() + devm_ioremap().
IMHO the unlucky thing in this situation is that devm_ioremap_resource() and devm_ioremap() are different by more than just how they get the area to remap. (i.e. devm_ioremap_resource() also does devm_request_mem_region().)
So the problem is not the added wrapper, but unclear semantics in the functions it uses. In my eyes devm_ioremap() and devm_platform_ioremap_resource() should better be named devm_request_ioremap() and devm_platform_request_ioremap_resource() respectively. Is it worth to rename these for clearity?
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |