2016-03-15 21:38:51

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] iommu: provide of_xlate pointer unconditionally

iommu drivers that support the standard DT bindings use a of_xlate
callback pointer, but that is only part of struct iommu_ops when
CONFIG_OF_IOMMU is enabled, leading to build errors in randconfig
builds when that is not provided:

drivers/iommu/mtk_iommu.c:497:2: error: unknown field 'of_xlate' specified in initializer
.of_xlate = mtk_iommu_of_xlate,
^
drivers/iommu/mtk_iommu.c:497:14: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.of_xlate = mtk_iommu_of_xlate,
^~~~~~~~~~~~~~~~~~
drivers/iommu/mtk_iommu.c:497:14: note: (near initialization for 'mtk_iommu_ops.domain_get_attr')

We can work around it by adding more #ifdefs in each driver, but
it seems nicer to just allow setting the pointer even if it is
unused. This makes the driver code look nicer, and it gives better
compile-time coverage when test building on other architectures.

Signed-off-by: Arnd Bergmann <[email protected]>
Fixes: 0df4fabe208d ("iommu/mediatek: Add mt8173 IOMMU driver")
---
include/linux/iommu.h | 2 --
1 file changed, 2 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a5c539fa5d2b..ef7a6ecd8584 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -195,9 +195,7 @@ struct iommu_ops {
/* Get the number of windows per domain */
u32 (*domain_get_windows)(struct iommu_domain *domain);

-#ifdef CONFIG_OF_IOMMU
int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
-#endif

unsigned long pgsize_bitmap;
void *priv;
--
2.7.0


2016-03-16 10:47:14

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH] iommu: provide of_xlate pointer unconditionally

On 15/03/16 21:37, Arnd Bergmann wrote:
> iommu drivers that support the standard DT bindings use a of_xlate
> callback pointer, but that is only part of struct iommu_ops when
> CONFIG_OF_IOMMU is enabled, leading to build errors in randconfig
> builds when that is not provided:
>
> drivers/iommu/mtk_iommu.c:497:2: error: unknown field 'of_xlate' specified in initializer
> .of_xlate = mtk_iommu_of_xlate,
> ^
> drivers/iommu/mtk_iommu.c:497:14: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
> .of_xlate = mtk_iommu_of_xlate,
> ^~~~~~~~~~~~~~~~~~
> drivers/iommu/mtk_iommu.c:497:14: note: (near initialization for 'mtk_iommu_ops.domain_get_attr')
>
> We can work around it by adding more #ifdefs in each driver, but
> it seems nicer to just allow setting the pointer even if it is
> unused. This makes the driver code look nicer, and it gives better
> compile-time coverage when test building on other architectures.

Makes sense. I don't know what the exact plan is for ACPI IORT, but I
would imagine that that could also tie in to the same mechanism.

> Signed-off-by: Arnd Bergmann <[email protected]>
> Fixes: 0df4fabe208d ("iommu/mediatek: Add mt8173 IOMMU driver")

Reviewed-by: Robin Murphy <[email protected]>

FWIW the Exynos IOMMU driver has also been setting .of_xlate
unconditionally for a while, but that isn't covered by COMPILE_TEST.

> ---
> include/linux/iommu.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index a5c539fa5d2b..ef7a6ecd8584 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -195,9 +195,7 @@ struct iommu_ops {
> /* Get the number of windows per domain */
> u32 (*domain_get_windows)(struct iommu_domain *domain);
>
> -#ifdef CONFIG_OF_IOMMU
> int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
> -#endif
>
> unsigned long pgsize_bitmap;
> void *priv;
>

2016-04-05 11:25:44

by Jörg Rödel

[permalink] [raw]
Subject: Re: [PATCH] iommu: provide of_xlate pointer unconditionally

On Tue, Mar 15, 2016 at 10:37:17PM +0100, Arnd Bergmann wrote:
> iommu drivers that support the standard DT bindings use a of_xlate
> callback pointer, but that is only part of struct iommu_ops when
> CONFIG_OF_IOMMU is enabled, leading to build errors in randconfig
> builds when that is not provided:
>
> drivers/iommu/mtk_iommu.c:497:2: error: unknown field 'of_xlate' specified in initializer
> .of_xlate = mtk_iommu_of_xlate,
> ^
> drivers/iommu/mtk_iommu.c:497:14: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
> .of_xlate = mtk_iommu_of_xlate,
> ^~~~~~~~~~~~~~~~~~
> drivers/iommu/mtk_iommu.c:497:14: note: (near initialization for 'mtk_iommu_ops.domain_get_attr')
>
> We can work around it by adding more #ifdefs in each driver, but
> it seems nicer to just allow setting the pointer even if it is
> unused. This makes the driver code look nicer, and it gives better
> compile-time coverage when test building on other architectures.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Fixes: 0df4fabe208d ("iommu/mediatek: Add mt8173 IOMMU driver")

Applied to iommu/fixes, thanks Arnd.