Re: [PATCH] iommu: rockchip: fix building without CONFIG_OF
On Wed, Apr 04, 2018 at 12:23:53PM +0200, Arnd Bergmann wrote: > We get a build error when compiling the iommu driver without CONFIG_OF: > > drivers/iommu/rockchip-iommu.c: In function 'rk_iommu_of_xlate': > drivers/iommu/rockchip-iommu.c:1101:2: error: implicit declaration of > function 'of_dev_put'; did you mean 'of_node_put'? > [-Werror=implicit-function-declaration] > > This replaces the of_dev_put() with the equivalent platform_device_put(), > and adds an error check for of_find_device_by_node() returning NULL, > which seems to be appropriate here given that we pass the device into > platform_get_drvdata() next, and that of_find_device_by_node() might > theoretically return a NULL pointer. > > Fixes: 5fd577c3eac3 ("iommu/rockchip: Use OF_IOMMU to attach devices > automatically") > Signed-off-by: Arnd Bergmann > --- > This warning appears to only have been introduced in linux-next after > the merge window opened. > --- > drivers/iommu/rockchip-iommu.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) Changed patch according to Robins comments and applied it, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: rockchip: fix building without CONFIG_OF
Hi Arnd, On 04/04/18 11:23, Arnd Bergmann wrote: We get a build error when compiling the iommu driver without CONFIG_OF: drivers/iommu/rockchip-iommu.c: In function 'rk_iommu_of_xlate': drivers/iommu/rockchip-iommu.c:1101:2: error: implicit declaration of function 'of_dev_put'; did you mean 'of_node_put'? [-Werror=implicit-function-declaration] This replaces the of_dev_put() with the equivalent platform_device_put(), and adds an error check for of_find_device_by_node() returning NULL, which seems to be appropriate here given that we pass the device into platform_get_drvdata() next, and that of_find_device_by_node() might theoretically return a NULL pointer. Fixes: 5fd577c3eac3 ("iommu/rockchip: Use OF_IOMMU to attach devices automatically") Signed-off-by: Arnd Bergmann --- This warning appears to only have been introduced in linux-next after the merge window opened. --- drivers/iommu/rockchip-iommu.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 5fc8656c60f9..fe9c9cc1a9d4 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1094,11 +1094,15 @@ static int rk_iommu_of_xlate(struct device *dev, return -ENOMEM; iommu_dev = of_find_device_by_node(args->np); + if (!iommu_dev) { Actually, though the logic is perhaps a bit subtle, this cannot reasonably happen - .of_xlate is only called via a set of iommu_ops associated with the IOMMU node (args->np here); those ops are only registered for the given node at the end of rk_iommu_probe(), by which point the IOMMU device exists and is otherwise successfully probed, so the platform bus lookup should never be able to fail. + kfree(data); + return -ENODEV; + } data->iommu = platform_get_drvdata(iommu_dev); dev->archdata.iommu = data; - of_dev_put(iommu_dev); + platform_device_put(iommu_dev); This bit looks reasonable, thanks for the fix. Robin. return 0; } ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: rockchip: fix building without CONFIG_OF
Hi Amd, Thanks for your patch. On 04/04/2018 06:23 PM, Arnd Bergmann wrote: We get a build error when compiling the iommu driver without CONFIG_OF: drivers/iommu/rockchip-iommu.c: In function 'rk_iommu_of_xlate': drivers/iommu/rockchip-iommu.c:1101:2: error: implicit declaration of function 'of_dev_put'; did you mean 'of_node_put'? [-Werror=implicit-function-declaration] oops, didn't notice that. and checking other iommu drivers which call of_find_device_by_node() too, seems they didn't put() it? maybe we should fix them too This replaces the of_dev_put() with the equivalent platform_device_put(), and adds an error check for of_find_device_by_node() returning NULL, which seems to be appropriate here given that we pass the device into platform_get_drvdata() next, and that of_find_device_by_node() might theoretically return a NULL pointer. hmm, i thought the of_iommu_xlate() checked that before calling us: static int of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec) { ... if ((ops && !ops->of_xlate) || !of_device_is_available(iommu_spec->np) || (!ops && !of_iommu_driver_present(iommu_spec->np))) return NO_IOMMU; ... return ops->of_xlate(dev, iommu_spec); but it's better to check it ourself :) Fixes: 5fd577c3eac3 ("iommu/rockchip: Use OF_IOMMU to attach devices automatically") Signed-off-by: Arnd Bergmann --- This warning appears to only have been introduced in linux-next after the merge window opened. --- drivers/iommu/rockchip-iommu.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 5fc8656c60f9..fe9c9cc1a9d4 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1094,11 +1094,15 @@ static int rk_iommu_of_xlate(struct device *dev, return -ENOMEM; iommu_dev = of_find_device_by_node(args->np); + if (!iommu_dev) { + kfree(data); + return -ENODEV; + } data->iommu = platform_get_drvdata(iommu_dev); dev->archdata.iommu = data; - of_dev_put(iommu_dev); + platform_device_put(iommu_dev); return 0; } ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: rockchip: fix building without CONFIG_OF
We get a build error when compiling the iommu driver without CONFIG_OF: drivers/iommu/rockchip-iommu.c: In function 'rk_iommu_of_xlate': drivers/iommu/rockchip-iommu.c:1101:2: error: implicit declaration of function 'of_dev_put'; did you mean 'of_node_put'? [-Werror=implicit-function-declaration] This replaces the of_dev_put() with the equivalent platform_device_put(), and adds an error check for of_find_device_by_node() returning NULL, which seems to be appropriate here given that we pass the device into platform_get_drvdata() next, and that of_find_device_by_node() might theoretically return a NULL pointer. Fixes: 5fd577c3eac3 ("iommu/rockchip: Use OF_IOMMU to attach devices automatically") Signed-off-by: Arnd Bergmann --- This warning appears to only have been introduced in linux-next after the merge window opened. --- drivers/iommu/rockchip-iommu.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 5fc8656c60f9..fe9c9cc1a9d4 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -1094,11 +1094,15 @@ static int rk_iommu_of_xlate(struct device *dev, return -ENOMEM; iommu_dev = of_find_device_by_node(args->np); + if (!iommu_dev) { + kfree(data); + return -ENODEV; + } data->iommu = platform_get_drvdata(iommu_dev); dev->archdata.iommu = data; - of_dev_put(iommu_dev); + platform_device_put(iommu_dev); return 0; } -- 2.9.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu