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/ |