On Tue, Feb 21, 2017 at 3:49 PM, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: > Hi All, > > On 2017-02-13 15:49, Linus Walleij wrote: >> >> On Sun, Feb 5, 2017 at 4:58 PM, Wei Yongjun <weiyj.lk@xxxxxxxxx> wrote: >> >>> From: Wei Yongjun <weiyongjun1@xxxxxxxxxx> >>> >>> In case of error, the function devm_ioremap() returns NULL pointer not >>> ERR_PTR(). Fix by using devm_ioremap_resource instead of devm_ioremap. >>> >>> Fixes: 8b1bd11c1f8f ("pinctrl: samsung: Add the support the multiple >>> IORESOURCE_MEM for one pin-bank") >>> Signed-off-by: Wei Yongjun <weiyongjun1@xxxxxxxxxx> >>> --- >>> v1 -> v2: use devm_ioremap_resource instead of devm_ioremap >> >> Patch applied with Krzysztof's ACK. > > > Sadly this patch breaks support for IMEM pinctrl block on Exynos5433/TM2 > and it took us some time to find the source of the problem. > > devm_ioremap_resource() is not functionally a full equivalent of > devm_ioremap(). The problem here is that registers for IMEM and ALIVE > pin controllers are shared and both devices have <0x11090000 0x1000> > range in their reg property. devm_ioremap_resource() maps given > resource exclusively for the device, while devm_ioremap() allows > non-exclusive mappings. > > This patch has to be reverted asap. Damn, the additional request_mem_region() raised my concerns but I didn't compare it with actual DTS layout... Which brings us to two important lessons: 1. Do not accept untested code. 2. Do not fix two things at the same time. I vote for the revert as well + original version of patch (these two could be squashed) + a need of Tested-by. Otherwise an untested fix might not be a fix at all. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html