[PATCH 0/8] mtk_iommu: Specify phandles to infracfg and pericfg

[PATCH 0/8] mtk_iommu: Specify phandles to infracfg and pericfg

[AngeloGioacchino Del Regno]

The IOMMU has registers in the infracfg and/or pericfg iospaces: as
for the currently supported SoCs, MT2712 and MT8173 need a phandle to
infracfg, while MT8195 needs one to pericfg.

Before this change, the driver was checking for a SoC-specific infra/peri
compatible but, sooner or later, these lists are going to grow a lot...
...and this is why it was chosen to add phandles (as it was done with
some other drivers already - look at mtk-pm-domains, mt8192-afe

Please note that, while it was necessary to update the devicetrees for
MT8173 and MT2712e, there was no update for MT8195 because there is no
IOMMU node in there yet.

AngeloGioacchino Del Regno (8):
  dt-bindings: iommu: mediatek: Add mediatek,infracfg phandle
  iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg
  dt-bindings: iommu: mediatek: Add mediatek,pericfg phandle
  iommu: mtk_iommu: Lookup phandle to retrieve syscon to pericfg
  arm64: dts: mediatek: mt8173: Add mediatek,infracfg phandle for IOMMU
  arm64: dts: mediatek: mt2712e: Add mediatek,infracfg phandle for IOMMU
  dt-bindings: iommu: mediatek: Require mediatek,infracfg for
    mt2712/8173
  dt-bindings: iommu: mediatek: Require mediatek,pericfg for
    mt8195-infra

 .../bindings/iommu/mediatek,iommu.yaml        | 30 +++++++++
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi     |  2 +
 arch/arm64/boot/dts/mediatek/mt8173.dtsi      |  1 +
 drivers/iommu/mtk_iommu.c                     | 66 ++++++++++++-------
 4 files changed, 75 insertions(+), 24 deletions(-)

-- 
2.35.1

[PATCH 1/8] dt-bindings: iommu: mediatek: Add mediatek,infracfg phandle

[AngeloGioacchino Del Regno]

Add property "mediatek,infracfg" to let the mtk_iommu driver retrieve
a phandle to the infracfg syscon instead of performing a per-soc
compatible lookup.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index 2ae3bbad7f1a..78c72c22740b 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -101,6 +101,10 @@ properties:
     items:
       - const: bclk
 
+  mediatek,infracfg:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+    description: The phandle to the mediatek infracfg syscon
+
   mediatek,larbs:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     minItems: 1
-- 
2.35.1

[PATCH 2/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg

[AngeloGioacchino Del Regno]

This driver will get support for more SoCs and the list of infracfg
compatibles is expected to grow: in order to prevent getting this
situation out of control and see a long list of compatible strings,
add support to retrieve a handle to infracfg's regmap through a
new "mediatek,infracfg" phandle.

In order to keep retrocompatibility with older devicetrees, the old
way is kept in place, but also a dev_warn() was added to advertise
this change in hope that the user will see it and eventually update
the devicetree if this is possible.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/iommu/mtk_iommu.c | 40 +++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 71b2ace74cd6..cfaaa98d2b50 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1134,22 +1134,34 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
 
 	if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
-		switch (data->plat_data->m4u_plat) {
-		case M4U_MT2712:
-			p = "mediatek,mt2712-infracfg";
-			break;
-		case M4U_MT8173:
-			p = "mediatek,mt8173-infracfg";
-			break;
-		default:
-			p = NULL;
+		infracfg = syscon_regmap_lookup_by_phandle(dev->of_node, "mediatek,infracfg");
+		if (IS_ERR(infracfg)) {
+			dev_warn(dev, "Cannot find phandle to mediatek,infracfg:"
+				      " Please update your devicetree.\n");
+			/*
+			 * Legacy devicetrees will not specify a phandle to
+			 * mediatek,infracfg: in that case, we use the older
+			 * way to retrieve a syscon to infra.
+			 *
+			 * This is for retrocompatibility purposes only, hence
+			 * no more compatibles shall be added to this.
+			 */
+			switch (data->plat_data->m4u_plat) {
+			case M4U_MT2712:
+				p = "mediatek,mt2712-infracfg";
+				break;
+			case M4U_MT8173:
+				p = "mediatek,mt8173-infracfg";
+				break;
+			default:
+				p = NULL;
+			}
+
+			infracfg = syscon_regmap_lookup_by_compatible(p);
+			if (IS_ERR(infracfg))
+				return PTR_ERR(infracfg);
 		}
 
-		infracfg = syscon_regmap_lookup_by_compatible(p);
-
-		if (IS_ERR(infracfg))
-			return PTR_ERR(infracfg);
-
 		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
 		if (ret)
 			return ret;
-- 
2.35.1

[PATCH 3/8] dt-bindings: iommu: mediatek: Add mediatek,pericfg phandle

[AngeloGioacchino Del Regno]

Add property "mediatek,pericfg" to let the mtk_iommu driver retrieve
a phandle to the pericfg syscon instead of performing a per-soc
compatible lookup, as it was also done with infracfg.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index 78c72c22740b..a6cf9678271f 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -116,6 +116,10 @@ properties:
       Refer to bindings/memory-controllers/mediatek,smi-larb.yaml. It must sort
       according to the local arbiter index, like larb0, larb1, larb2...
 
+  mediatek,pericfg:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+    description: The phandle to the mediatek pericfg syscon
+
   '#iommu-cells':
     const: 1
     description: |
-- 
2.35.1

[PATCH 4/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to pericfg

[AngeloGioacchino Del Regno]

On some SoCs (of which only MT8195 is supported at the time of writing),
the "R" and "W" (I/O) enable bits for the IOMMUs are in the pericfg_ao
register space and not in the IOMMU space: as it happened already with
infracfg, it is expected that this list will grow.

Instead of specifying pericfg compatibles on a per-SoC basis, following
what was done with infracfg, let's lookup the syscon by phandle instead.
Also following the previous infracfg change, add a warning for outdated
devicetrees, in hope that the user will take action.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/iommu/mtk_iommu.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index cfaaa98d2b50..c7e2d836199e 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -138,6 +138,8 @@
 /* PM and clock always on. e.g. infra iommu */
 #define PM_CLK_AO			BIT(15)
 #define IFA_IOMMU_PCIE_SUPPORT		BIT(16)
+/* IOMMU I/O (r/w) is enabled using PERICFG_IOMMU_1 register */
+#define HAS_PERI_IOMMU1_REG		BIT(17)
 
 #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask)	\
 				((((pdata)->flags) & (mask)) == (_x))
@@ -187,7 +189,6 @@ struct mtk_iommu_plat_data {
 	u32			flags;
 	u32			inv_sel_reg;
 
-	char			*pericfg_comp_str;
 	struct list_head	*hw_list;
 	unsigned int		iova_region_nr;
 	const struct mtk_iommu_iova_region	*iova_region;
@@ -1214,14 +1215,19 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 			goto out_runtime_disable;
 		}
 	} else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
-		   data->plat_data->pericfg_comp_str) {
-		infracfg = syscon_regmap_lookup_by_compatible(data->plat_data->pericfg_comp_str);
-		if (IS_ERR(infracfg)) {
-			ret = PTR_ERR(infracfg);
-			goto out_runtime_disable;
-		}
+		   MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_PERI_IOMMU1_REG)) {
+		data->pericfg = syscon_regmap_lookup_by_phandle(dev->of_node, "mediatek,pericfg");
+		if (IS_ERR(data->pericfg)) {
+			dev_warn(dev, "Cannot find phandle to mediatek,pericfg:"
+				      " Please update your devicetree.\n");
 
-		data->pericfg = infracfg;
+			p = "mediatek,mt8195-pericfg_ao";
+			data->pericfg = syscon_regmap_lookup_by_compatible(p);
+			if (IS_ERR(data->pericfg)) {
+				ret = PTR_ERR(data->pericfg);
+				goto out_runtime_disable;
+			}
+		}
 	}
 
 	platform_set_drvdata(pdev, data);
@@ -1480,8 +1486,8 @@ static const struct mtk_iommu_plat_data mt8192_data = {
 static const struct mtk_iommu_plat_data mt8195_data_infra = {
 	.m4u_plat	  = M4U_MT8195,
 	.flags            = WR_THROT_EN | DCM_DISABLE | STD_AXI_MODE | PM_CLK_AO |
-			    MTK_IOMMU_TYPE_INFRA | IFA_IOMMU_PCIE_SUPPORT,
-	.pericfg_comp_str = "mediatek,mt8195-pericfg_ao",
+			    HAS_PERI_IOMMU1_REG | MTK_IOMMU_TYPE_INFRA |
+			    IFA_IOMMU_PCIE_SUPPORT,
 	.inv_sel_reg      = REG_MMU_INV_SEL_GEN2,
 	.banks_num	  = 5,
 	.banks_enable     = {true, false, false, false, true},
-- 
2.35.1

[PATCH 5/8] arm64: dts: mediatek: mt8173: Add mediatek,infracfg phandle for IOMMU

[AngeloGioacchino Del Regno]

The IOMMU driver now looks for the "mediatek,infracfg" phandle as a
new way to retrieve a syscon to that:
even though the old way is retained, it has been deprecated and the
driver will write a message in kmsg advertising to use the phandle
way instead.

For this reason, assign the right phandle to mediatek,infracfg in
the iommu node.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 40d7b47fc52e..825a3c670373 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -588,6 +588,7 @@ iommu: iommu@10205000 {
 			interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_LOW>;
 			clocks = <&infracfg CLK_INFRA_M4U>;
 			clock-names = "bclk";
+			mediatek,infracfg = <&infracfg>;
 			mediatek,larbs = <&larb0>, <&larb1>, <&larb2>,
 					 <&larb3>, <&larb4>, <&larb5>;
 			#iommu-cells = <1>;
-- 
2.35.1

[PATCH 6/8] arm64: dts: mediatek: mt2712e: Add mediatek,infracfg phandle for IOMMU

[AngeloGioacchino Del Regno]

The IOMMU driver now looks for the "mediatek,infracfg" phandle as a
new way to retrieve a syscon to that:
even though the old way is retained, it has been deprecated and the
driver will write a message in kmsg advertising to use the phandle
way instead.

For this reason, assign the right phandle to mediatek,infracfg in
the iommu node.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 623eb3beabf2..4797537cb368 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -329,6 +329,7 @@ iommu0: iommu@10205000 {
 		interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&infracfg CLK_INFRA_M4U>;
 		clock-names = "bclk";
+		mediatek,infracfg = <&infracfg>;
 		mediatek,larbs = <&larb0>, <&larb1>, <&larb2>,
 				 <&larb3>, <&larb6>;
 		#iommu-cells = <1>;
@@ -346,6 +347,7 @@ iommu1: iommu@1020a000 {
 		interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_LOW>;
 		clocks = <&infracfg CLK_INFRA_M4U>;
 		clock-names = "bclk";
+		mediatek,infracfg = <&infracfg>;
 		mediatek,larbs = <&larb4>, <&larb5>, <&larb7>;
 		#iommu-cells = <1>;
 	};
-- 
2.35.1

[PATCH 7/8] dt-bindings: iommu: mediatek: Require mediatek,infracfg for mt2712/8173

[AngeloGioacchino Del Regno]

Both MT2712 and MT8173 got a mediatek,infracfg phandle: add that to
the required properties for these SoCs to deprecate the old way of
looking for SoC-specific infracfg compatible in the entire devicetree.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../devicetree/bindings/iommu/mediatek,iommu.yaml    | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index a6cf9678271f..17d78b17027a 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -175,6 +175,18 @@ allOf:
       required:
         - power-domains
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt2712-m4u
+              - mediatek,mt8173-m4u
+
+    then:
+      required:
+        - mediatek,infracfg
+
   - if: # The IOMMUs don't have larbs.
       not:
         properties:
-- 
2.35.1

[PATCH 8/8] dt-bindings: iommu: mediatek: Require mediatek,pericfg for mt8195-infra

[AngeloGioacchino Del Regno]

The MT8195 SoC has IOMMU related registers in the pericfg_ao iospace:
require a phandle to that.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---

Note for Rob: as of now, there's no iommu node in upstream mt8195 devicetrees yet.

 .../devicetree/bindings/iommu/mediatek,iommu.yaml      | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index 17d78b17027a..2441c2e8e55d 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -187,6 +187,16 @@ allOf:
       required:
         - mediatek,infracfg
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt8195-iommu-infra
+
+    then:
+      required:
+        - mediatek,pericfg
+
   - if: # The IOMMUs don't have larbs.
       not:
         properties:
-- 
2.35.1

Re: [PATCH 2/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg

[Robin Murphy]

On 2022-05-17 14:21, AngeloGioacchino Del Regno wrote:
> This driver will get support for more SoCs and the list of infracfg
> compatibles is expected to grow: in order to prevent getting this
> situation out of control and see a long list of compatible strings,
> add support to retrieve a handle to infracfg's regmap through a
> new "mediatek,infracfg" phandle.
> 
> In order to keep retrocompatibility with older devicetrees, the old
> way is kept in place, but also a dev_warn() was added to advertise
> this change in hope that the user will see it and eventually update
> the devicetree if this is possible.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>   drivers/iommu/mtk_iommu.c | 40 +++++++++++++++++++++++++--------------
>   1 file changed, 26 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 71b2ace74cd6..cfaaa98d2b50 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -1134,22 +1134,34 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>   
>   	if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
> -		switch (data->plat_data->m4u_plat) {
> -		case M4U_MT2712:
> -			p = "mediatek,mt2712-infracfg";
> -			break;
> -		case M4U_MT8173:
> -			p = "mediatek,mt8173-infracfg";
> -			break;
> -		default:
> -			p = NULL;
> +		infracfg = syscon_regmap_lookup_by_phandle(dev->of_node, "mediatek,infracfg");
> +		if (IS_ERR(infracfg)) {
> +			dev_warn(dev, "Cannot find phandle to mediatek,infracfg:"
> +				      " Please update your devicetree.\n");

Is this really a dev_warn-level problem? There's no functional impact, 
given that we can't stop supporting the original binding any time soon, 
if ever, so I suspect this is more likely to just annoy users and CI 
systems than effect any significant change.

> +			/*
> +			 * Legacy devicetrees will not specify a phandle to
> +			 * mediatek,infracfg: in that case, we use the older
> +			 * way to retrieve a syscon to infra.
> +			 *
> +			 * This is for retrocompatibility purposes only, hence
> +			 * no more compatibles shall be added to this.
> +			 */
> +			switch (data->plat_data->m4u_plat) {
> +			case M4U_MT2712:
> +				p = "mediatek,mt2712-infracfg";
> +				break;
> +			case M4U_MT8173:
> +				p = "mediatek,mt8173-infracfg";
> +				break;
> +			default:
> +				p = NULL;
> +			}
> +
> +			infracfg = syscon_regmap_lookup_by_compatible(p);

Would it not make sense to punt this over to the same mechanism as for 
pericfg, such that it simplifies down to something like:

	if (IS_ERR(infracfg) && plat_data->infracfg) {
		infracfg = syscon_regmap_lookup_by_compatible(plat_data->infracfg);
		...
	}

?

TBH if we're still going to have a load of per-SoC data in the driver 
anyway then I don't see that we really gain much by delegating one 
aspect of it to DT, but meh. I would note that with the phandle 
approach, you still need some *other* flag in the driver to know whether 
a phandle is expected to be present or not, whereas a NULL vs. non-NULL 
string is at least neatly self-describing.

Robin.

> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);
>   		}
>   
> -		infracfg = syscon_regmap_lookup_by_compatible(p);
> -
> -		if (IS_ERR(infracfg))
> -			return PTR_ERR(infracfg);
> -
>   		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
>   		if (ret)
>   			return ret;

Re: [PATCH 1/8] dt-bindings: iommu: mediatek: Add mediatek,infracfg phandle

[Krzysztof Kozlowski]

On 17/05/2022 15:21, AngeloGioacchino Del Regno wrote:
> Add property "mediatek,infracfg" to let the mtk_iommu driver retrieve
> a phandle to the infracfg syscon instead of performing a per-soc
> compatible lookup.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> index 2ae3bbad7f1a..78c72c22740b 100644
> --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> @@ -101,6 +101,10 @@ properties:
>      items:
>        - const: bclk
>  
> +  mediatek,infracfg:
> +    $ref: "/schemas/types.yaml#/definitions/phandle"

No need for quotes. They are not present in other places.


> +    description: The phandle to the mediatek infracfg syscon
> +
>    mediatek,larbs:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      minItems: 1


Best regards,
Krzysztof

Re: [PATCH 3/8] dt-bindings: iommu: mediatek: Add mediatek,pericfg phandle

[Krzysztof Kozlowski]

On 17/05/2022 15:21, AngeloGioacchino Del Regno wrote:
> Add property "mediatek,pericfg" to let the mtk_iommu driver retrieve
> a phandle to the pericfg syscon instead of performing a per-soc
> compatible lookup, as it was also done with infracfg.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> index 78c72c22740b..a6cf9678271f 100644
> --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> @@ -116,6 +116,10 @@ properties:
>        Refer to bindings/memory-controllers/mediatek,smi-larb.yaml. It must sort
>        according to the local arbiter index, like larb0, larb1, larb2...
>  
> +  mediatek,pericfg:
> +    $ref: "/schemas/types.yaml#/definitions/phandle"

No need for quotes.


Best regards,
Krzysztof

Re: [PATCH 7/8] dt-bindings: iommu: mediatek: Require mediatek,infracfg for mt2712/8173

[Rob Herring]

On Tue, May 17, 2022 at 03:21:06PM +0200, AngeloGioacchino Del Regno wrote:
> Both MT2712 and MT8173 got a mediatek,infracfg phandle: add that to
> the required properties for these SoCs to deprecate the old way of
> looking for SoC-specific infracfg compatible in the entire devicetree.

Wait, what? If there's only one possible node that can match, I prefer 
the 'old way'. Until we implemented a phandle cache, searching the 
entire tree was how phandle lookups worked too, so not any better.

But if this makes things more consistent,

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 8/8] dt-bindings: iommu: mediatek: Require mediatek,pericfg for mt8195-infra

[Rob Herring]

On Tue, 17 May 2022 15:21:07 +0200, AngeloGioacchino Del Regno wrote:
> The MT8195 SoC has IOMMU related registers in the pericfg_ao iospace:
> require a phandle to that.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> 
> Note for Rob: as of now, there's no iommu node in upstream mt8195 devicetrees yet.
> 
>  .../devicetree/bindings/iommu/mediatek,iommu.yaml      | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 7/8] dt-bindings: iommu: mediatek: Require mediatek,infracfg for mt2712/8173

[AngeloGioacchino Del Regno]

Il 18/05/22 03:41, Rob Herring ha scritto:
> On Tue, May 17, 2022 at 03:21:06PM +0200, AngeloGioacchino Del Regno wrote:
>> Both MT2712 and MT8173 got a mediatek,infracfg phandle: add that to
>> the required properties for these SoCs to deprecate the old way of
>> looking for SoC-specific infracfg compatible in the entire devicetree.
> 
> Wait, what? If there's only one possible node that can match, I prefer
> the 'old way'. Until we implemented a phandle cache, searching the
> entire tree was how phandle lookups worked too, so not any better.
> 
> But if this makes things more consistent,
> 
> Acked-by: Rob Herring <robh@kernel.org>


Hello Rob,

This makes things definitely more consistent, as it's done like that on
mtk-pm-domains and other mtk drivers as well.

The main reason why this phandle is useful, here and in other drivers, is
that we're seeing a list of compatibles that is growing more and more, so
you see stuff like (mockup names warning):

switch (some_model)
case MT1000:
	p = "mediatek,mt1000-infracfg";
	break;
case MT1001:
	p = "mediatek,mt1001-infracfg";
	break;
case MT1002:
	p = "mediatek,mt1002-infracfg";
	break;
.....add another 20 SoCs, replicate this switch for 4/5 drivers....

and this is why I want the mtk_iommu driver to also get that phandle like
some other drivers are already doing.

By the way, thanks for the ack!

Regards,
Angelo

Re: [PATCH 2/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg

[AngeloGioacchino Del Regno]

Il 17/05/22 16:12, Robin Murphy ha scritto:
> On 2022-05-17 14:21, AngeloGioacchino Del Regno wrote:
>> This driver will get support for more SoCs and the list of infracfg
>> compatibles is expected to grow: in order to prevent getting this
>> situation out of control and see a long list of compatible strings,
>> add support to retrieve a handle to infracfg's regmap through a
>> new "mediatek,infracfg" phandle.
>>
>> In order to keep retrocompatibility with older devicetrees, the old
>> way is kept in place, but also a dev_warn() was added to advertise
>> this change in hope that the user will see it and eventually update
>> the devicetree if this is possible.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/iommu/mtk_iommu.c | 40 +++++++++++++++++++++++++--------------
>>   1 file changed, 26 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>> index 71b2ace74cd6..cfaaa98d2b50 100644
>> --- a/drivers/iommu/mtk_iommu.c
>> +++ b/drivers/iommu/mtk_iommu.c
>> @@ -1134,22 +1134,34 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>       data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>>       if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
>> -        switch (data->plat_data->m4u_plat) {
>> -        case M4U_MT2712:
>> -            p = "mediatek,mt2712-infracfg";
>> -            break;
>> -        case M4U_MT8173:
>> -            p = "mediatek,mt8173-infracfg";
>> -            break;
>> -        default:
>> -            p = NULL;
>> +        infracfg = syscon_regmap_lookup_by_phandle(dev->of_node, 
>> "mediatek,infracfg");
>> +        if (IS_ERR(infracfg)) {
>> +            dev_warn(dev, "Cannot find phandle to mediatek,infracfg:"
>> +                      " Please update your devicetree.\n");
> 
> Is this really a dev_warn-level problem? There's no functional impact, given that 
> we can't stop supporting the original binding any time soon, if ever, so I suspect 
> this is more likely to just annoy users and CI systems than effect any significant 
> change.
> 

The upstream devicetrees were updated to use the new handle and this is a way to
warn about having outdated DTs... besides, I believe that CIs will always get the
devicetree from the same tree that the kernel was compiled from (hence no message
will be thrown).

In any case, if you think that a dev_info would be more appropriate, I can change
that no problem.

>> +            /*
>> +             * Legacy devicetrees will not specify a phandle to
>> +             * mediatek,infracfg: in that case, we use the older
>> +             * way to retrieve a syscon to infra.
>> +             *
>> +             * This is for retrocompatibility purposes only, hence
>> +             * no more compatibles shall be added to this.
>> +             */
>> +            switch (data->plat_data->m4u_plat) {
>> +            case M4U_MT2712:
>> +                p = "mediatek,mt2712-infracfg";
>> +                break;
>> +            case M4U_MT8173:
>> +                p = "mediatek,mt8173-infracfg";
>> +                break;
>> +            default:
>> +                p = NULL;
>> +            }
>> +
>> +            infracfg = syscon_regmap_lookup_by_compatible(p);
> 
> Would it not make sense to punt this over to the same mechanism as for pericfg, 
> such that it simplifies down to something like:
> 
>      if (IS_ERR(infracfg) && plat_data->infracfg) {
>          infracfg = syscon_regmap_lookup_by_compatible(plat_data->infracfg);
>          ...
>      }
> 
> ?
> 
> TBH if we're still going to have a load of per-SoC data in the driver anyway then I 
> don't see that we really gain much by delegating one aspect of it to DT, but meh. I 
> would note that with the phandle approach, you still need some *other* flag in the 
> driver to know whether a phandle is expected to be present or not, whereas a NULL 
> vs. non-NULL string is at least neatly self-describing.
> 

That would be possible but, as Yong also pointed out, we should try to reduce the
per-SoC data in the driver by commonizing as much as possible, because this driver
supports a very long list of SoCs (even though they're not all upstreamed yet),
and the list is going to grow even more with time: this is also why I have changed
the MT8195 pericfg regmap lookup with a phandle like I've done for infra.

There would also be another way, which would imply adding a generic compatible
"mediatek,infracfg" to the infra syscon node, but I really don't like that for
more than one reason, one of which is that this poses an issue, for which it's
not guaranteed that the registers are in infracfg and not infracfg_ao (even
though the offsets are the same), so then we would be back to ground zero.

Regards,
Angelo

Re: [PATCH 1/8] dt-bindings: iommu: mediatek: Add mediatek,infracfg phandle

[Matthias Brugger]

On 17/05/2022 15:21, AngeloGioacchino Del Regno wrote:
> Add property "mediatek,infracfg" to let the mtk_iommu driver retrieve
> a phandle to the infracfg syscon instead of performing a per-soc
> compatible lookup.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>   Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> index 2ae3bbad7f1a..78c72c22740b 100644
> --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> @@ -101,6 +101,10 @@ properties:
>       items:
>         - const: bclk
>   
> +  mediatek,infracfg:
> +    $ref: "/schemas/types.yaml#/definitions/phandle"
> +    description: The phandle to the mediatek infracfg syscon
> +
>     mediatek,larbs:
>       $ref: /schemas/types.yaml#/definitions/phandle-array
>       minItems: 1

I think we can squash patch 7 in here. Same holds for pericfg

Regards,
Matthias

Re: [PATCH 4/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to pericfg

[Matthias Brugger]

On 17/05/2022 15:21, AngeloGioacchino Del Regno wrote:
> On some SoCs (of which only MT8195 is supported at the time of writing),
> the "R" and "W" (I/O) enable bits for the IOMMUs are in the pericfg_ao
> register space and not in the IOMMU space: as it happened already with
> infracfg, it is expected that this list will grow.
> 
> Instead of specifying pericfg compatibles on a per-SoC basis, following
> what was done with infracfg, let's lookup the syscon by phandle instead.
> Also following the previous infracfg change, add a warning for outdated
> devicetrees, in hope that the user will take action.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> ---
>   drivers/iommu/mtk_iommu.c | 26 ++++++++++++++++----------
>   1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index cfaaa98d2b50..c7e2d836199e 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -138,6 +138,8 @@
>   /* PM and clock always on. e.g. infra iommu */
>   #define PM_CLK_AO			BIT(15)
>   #define IFA_IOMMU_PCIE_SUPPORT		BIT(16)
> +/* IOMMU I/O (r/w) is enabled using PERICFG_IOMMU_1 register */
> +#define HAS_PERI_IOMMU1_REG		BIT(17)
>   
>   #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask)	\
>   				((((pdata)->flags) & (mask)) == (_x))
> @@ -187,7 +189,6 @@ struct mtk_iommu_plat_data {
>   	u32			flags;
>   	u32			inv_sel_reg;
>   
> -	char			*pericfg_comp_str;
>   	struct list_head	*hw_list;
>   	unsigned int		iova_region_nr;
>   	const struct mtk_iommu_iova_region	*iova_region;
> @@ -1214,14 +1215,19 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   			goto out_runtime_disable;
>   		}
>   	} else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
> -		   data->plat_data->pericfg_comp_str) {
> -		infracfg = syscon_regmap_lookup_by_compatible(data->plat_data->pericfg_comp_str);
> -		if (IS_ERR(infracfg)) {
> -			ret = PTR_ERR(infracfg);
> -			goto out_runtime_disable;
> -		}
> +		   MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_PERI_IOMMU1_REG)) {
> +		data->pericfg = syscon_regmap_lookup_by_phandle(dev->of_node, "mediatek,pericfg");
> +		if (IS_ERR(data->pericfg)) {
> +			dev_warn(dev, "Cannot find phandle to mediatek,pericfg:"
> +				      " Please update your devicetree.\n");
>   
> -		data->pericfg = infracfg;
> +			p = "mediatek,mt8195-pericfg_ao";
> +			data->pericfg = syscon_regmap_lookup_by_compatible(p);
> +			if (IS_ERR(data->pericfg)) {
> +				ret = PTR_ERR(data->pericfg);
> +				goto out_runtime_disable;
> +			}
> +		}
>   	}
>   
>   	platform_set_drvdata(pdev, data);
> @@ -1480,8 +1486,8 @@ static const struct mtk_iommu_plat_data mt8192_data = {
>   static const struct mtk_iommu_plat_data mt8195_data_infra = {
>   	.m4u_plat	  = M4U_MT8195,
>   	.flags            = WR_THROT_EN | DCM_DISABLE | STD_AXI_MODE | PM_CLK_AO |
> -			    MTK_IOMMU_TYPE_INFRA | IFA_IOMMU_PCIE_SUPPORT,
> -	.pericfg_comp_str = "mediatek,mt8195-pericfg_ao",
> +			    HAS_PERI_IOMMU1_REG | MTK_IOMMU_TYPE_INFRA |
> +			    IFA_IOMMU_PCIE_SUPPORT,
>   	.inv_sel_reg      = REG_MMU_INV_SEL_GEN2,
>   	.banks_num	  = 5,
>   	.banks_enable     = {true, false, false, false, true},

Re: [PATCH 2/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg

[Robin Murphy]

On 2022-05-18 09:29, AngeloGioacchino Del Regno wrote:
> Il 17/05/22 16:12, Robin Murphy ha scritto:
>> On 2022-05-17 14:21, AngeloGioacchino Del Regno wrote:
>>> This driver will get support for more SoCs and the list of infracfg
>>> compatibles is expected to grow: in order to prevent getting this
>>> situation out of control and see a long list of compatible strings,
>>> add support to retrieve a handle to infracfg's regmap through a
>>> new "mediatek,infracfg" phandle.
>>>
>>> In order to keep retrocompatibility with older devicetrees, the old
>>> way is kept in place, but also a dev_warn() was added to advertise
>>> this change in hope that the user will see it and eventually update
>>> the devicetree if this is possible.
>>>
>>> Signed-off-by: AngeloGioacchino Del Regno 
>>> <angelogioacchino.delregno@collabora.com>
>>> ---
>>>   drivers/iommu/mtk_iommu.c | 40 +++++++++++++++++++++++++--------------
>>>   1 file changed, 26 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>>> index 71b2ace74cd6..cfaaa98d2b50 100644
>>> --- a/drivers/iommu/mtk_iommu.c
>>> +++ b/drivers/iommu/mtk_iommu.c
>>> @@ -1134,22 +1134,34 @@ static int mtk_iommu_probe(struct 
>>> platform_device *pdev)
>>>       data->protect_base = ALIGN(virt_to_phys(protect), 
>>> MTK_PROTECT_PA_ALIGN);
>>>       if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
>>> -        switch (data->plat_data->m4u_plat) {
>>> -        case M4U_MT2712:
>>> -            p = "mediatek,mt2712-infracfg";
>>> -            break;
>>> -        case M4U_MT8173:
>>> -            p = "mediatek,mt8173-infracfg";
>>> -            break;
>>> -        default:
>>> -            p = NULL;
>>> +        infracfg = syscon_regmap_lookup_by_phandle(dev->of_node, 
>>> "mediatek,infracfg");
>>> +        if (IS_ERR(infracfg)) {
>>> +            dev_warn(dev, "Cannot find phandle to mediatek,infracfg:"
>>> +                      " Please update your devicetree.\n");
>>
>> Is this really a dev_warn-level problem? There's no functional impact, 
>> given that we can't stop supporting the original binding any time 
>> soon, if ever, so I suspect this is more likely to just annoy users 
>> and CI systems than effect any significant change.
>>
> 
> The upstream devicetrees were updated to use the new handle and this is 
> a way to
> warn about having outdated DTs... besides, I believe that CIs will 
> always get the
> devicetree from the same tree that the kernel was compiled from (hence 
> no message
> will be thrown).
> 
> In any case, if you think that a dev_info would be more appropriate, I 
> can change
> that no problem.

If there's some functional impact from using the old binding vs. the new 
one then it's reasonable to inform the user of that (as we do in 
arm-smmu, for example).

However if you change an established binding for non-functional reasons, 
then you get to support both bindings, and it's not the end user's 
problem at all. There seems to be zero reason to update an existing DTB 
for this difference alone, so TBH I don't think it deserves a message at 
all.

>>> +            /*
>>> +             * Legacy devicetrees will not specify a phandle to
>>> +             * mediatek,infracfg: in that case, we use the older
>>> +             * way to retrieve a syscon to infra.
>>> +             *
>>> +             * This is for retrocompatibility purposes only, hence
>>> +             * no more compatibles shall be added to this.
>>> +             */
>>> +            switch (data->plat_data->m4u_plat) {
>>> +            case M4U_MT2712:
>>> +                p = "mediatek,mt2712-infracfg";
>>> +                break;
>>> +            case M4U_MT8173:
>>> +                p = "mediatek,mt8173-infracfg";
>>> +                break;
>>> +            default:
>>> +                p = NULL;
>>> +            }
>>> +
>>> +            infracfg = syscon_regmap_lookup_by_compatible(p);
>>
>> Would it not make sense to punt this over to the same mechanism as for 
>> pericfg, such that it simplifies down to something like:
>>
>>      if (IS_ERR(infracfg) && plat_data->infracfg) {
>>          infracfg = 
>> syscon_regmap_lookup_by_compatible(plat_data->infracfg);
>>          ...
>>      }
>>
>> ?
>>
>> TBH if we're still going to have a load of per-SoC data in the driver 
>> anyway then I don't see that we really gain much by delegating one 
>> aspect of it to DT, but meh. I would note that with the phandle 
>> approach, you still need some *other* flag in the driver to know 
>> whether a phandle is expected to be present or not, whereas a NULL vs. 
>> non-NULL string is at least neatly self-describing.
>>
> 
> That would be possible but, as Yong also pointed out, we should try to 
> reduce the
> per-SoC data in the driver by commonizing as much as possible, because 
> this driver
> supports a very long list of SoCs (even though they're not all 
> upstreamed yet),
> and the list is going to grow even more with time: this is also why I 
> have changed
> the MT8195 pericfg regmap lookup with a phandle like I've done for infra.

That's fair enough, but it's not what the commit message says. The "long 
list of compatible strings" complaint could be addressed at face value 
by refactoring without changing the DT binding at all.

However, I didn't think I'd have to point out why that argument doesn't 
apply to existing SoCs which we have to support with the original 
binding too; take another look at the switch statement above and have a 
think...

If we have to maintain infracfg compatible data *somewhere* in the 
driver, which we do, then it seems more logical to keep it with the rest 
of the data rather than scattered through the code, that's the main 
point I wanted to make here.

> There would also be another way, which would imply adding a generic 
> compatible
> "mediatek,infracfg" to the infra syscon node, but I really don't like 
> that for
> more than one reason, one of which is that this poses an issue, for 
> which it's
> not guaranteed that the registers are in infracfg and not infracfg_ao (even
> though the offsets are the same), so then we would be back to ground zero.

No, we still have to support the existing binding in the context of the 
existing binding. There *are* some reasonable arguments for moving 
future SoCs to the phandle binding, they just haven't been presented in 
these patches ;)

Thanks,
Robin.

Re: [PATCH 7/8] dt-bindings: iommu: mediatek: Require mediatek,infracfg for mt2712/8173

[Rob Herring]

On Wed, May 18, 2022 at 10:14:43AM +0200, AngeloGioacchino Del Regno wrote:
> Il 18/05/22 03:41, Rob Herring ha scritto:
> > On Tue, May 17, 2022 at 03:21:06PM +0200, AngeloGioacchino Del Regno wrote:
> > > Both MT2712 and MT8173 got a mediatek,infracfg phandle: add that to
> > > the required properties for these SoCs to deprecate the old way of
> > > looking for SoC-specific infracfg compatible in the entire devicetree.
> > 
> > Wait, what? If there's only one possible node that can match, I prefer
> > the 'old way'. Until we implemented a phandle cache, searching the
> > entire tree was how phandle lookups worked too, so not any better.
> > 
> > But if this makes things more consistent,
> > 
> > Acked-by: Rob Herring <robh@kernel.org>
> 
> 
> Hello Rob,
> 
> This makes things definitely more consistent, as it's done like that on
> mtk-pm-domains and other mtk drivers as well.
> 
> The main reason why this phandle is useful, here and in other drivers, is
> that we're seeing a list of compatibles that is growing more and more, so
> you see stuff like (mockup names warning):
> 
> switch (some_model)
> case MT1000:
> 	p = "mediatek,mt1000-infracfg";
> 	break;
> case MT1001:
> 	p = "mediatek,mt1001-infracfg";
> 	break;
> case MT1002:
> 	p = "mediatek,mt1002-infracfg";
> 	break;
> .....add another 20 SoCs, replicate this switch for 4/5 drivers....

This type of property is used for poking random bits in another block 
(that's usually a collection of random bits). These interfaces don't 
tend to be that stable across many SoC generations. As there's no 
abstraction beyond perhaps what the offset is, the client side ends up 
needing to know the specifics of that block anyways. If the block is 
that stable, then perhaps it needs a common fallback compatible.

Sometimes these instances are also just places we haven't created a 
common subsystem for.

> and this is why I want the mtk_iommu driver to also get that phandle like
> some other drivers are already doing.

As I said, fine.

Rob

Re: [PATCH 2/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg

[Rob Herring]

On Wed, May 18, 2022 at 12:07:58PM +0100, Robin Murphy wrote:
> On 2022-05-18 09:29, AngeloGioacchino Del Regno wrote:
> > Il 17/05/22 16:12, Robin Murphy ha scritto:
> > > On 2022-05-17 14:21, AngeloGioacchino Del Regno wrote:
> > > > This driver will get support for more SoCs and the list of infracfg
> > > > compatibles is expected to grow: in order to prevent getting this
> > > > situation out of control and see a long list of compatible strings,
> > > > add support to retrieve a handle to infracfg's regmap through a
> > > > new "mediatek,infracfg" phandle.
> > > > 
> > > > In order to keep retrocompatibility with older devicetrees, the old
> > > > way is kept in place, but also a dev_warn() was added to advertise
> > > > this change in hope that the user will see it and eventually update
> > > > the devicetree if this is possible.
> > > > 
> > > > Signed-off-by: AngeloGioacchino Del Regno
> > > > <angelogioacchino.delregno@collabora.com>
> > > > ---
> > > >   drivers/iommu/mtk_iommu.c | 40 +++++++++++++++++++++++++--------------
> > > >   1 file changed, 26 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > > > index 71b2ace74cd6..cfaaa98d2b50 100644
> > > > --- a/drivers/iommu/mtk_iommu.c
> > > > +++ b/drivers/iommu/mtk_iommu.c
> > > > @@ -1134,22 +1134,34 @@ static int mtk_iommu_probe(struct
> > > > platform_device *pdev)
> > > >       data->protect_base = ALIGN(virt_to_phys(protect),
> > > > MTK_PROTECT_PA_ALIGN);
> > > >       if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
> > > > -        switch (data->plat_data->m4u_plat) {
> > > > -        case M4U_MT2712:
> > > > -            p = "mediatek,mt2712-infracfg";
> > > > -            break;
> > > > -        case M4U_MT8173:
> > > > -            p = "mediatek,mt8173-infracfg";
> > > > -            break;
> > > > -        default:
> > > > -            p = NULL;
> > > > +        infracfg =
> > > > syscon_regmap_lookup_by_phandle(dev->of_node,
> > > > "mediatek,infracfg");
> > > > +        if (IS_ERR(infracfg)) {
> > > > +            dev_warn(dev, "Cannot find phandle to mediatek,infracfg:"
> > > > +                      " Please update your devicetree.\n");
> > > 
> > > Is this really a dev_warn-level problem? There's no functional
> > > impact, given that we can't stop supporting the original binding any
> > > time soon, if ever, so I suspect this is more likely to just annoy
> > > users and CI systems than effect any significant change.
> > > 
> > 
> > The upstream devicetrees were updated to use the new handle and this is
> > a way to
> > warn about having outdated DTs... besides, I believe that CIs will
> > always get the
> > devicetree from the same tree that the kernel was compiled from (hence
> > no message
> > will be thrown).
> > 
> > In any case, if you think that a dev_info would be more appropriate, I
> > can change
> > that no problem.
> 
> If there's some functional impact from using the old binding vs. the new one
> then it's reasonable to inform the user of that (as we do in arm-smmu, for
> example).
> 
> However if you change an established binding for non-functional reasons,
> then you get to support both bindings, and it's not the end user's problem
> at all. There seems to be zero reason to update an existing DTB for this
> difference alone, so TBH I don't think it deserves a message at all.

It's also not the kernel's job to validate the DT. It's horrible at it 
and we have something else now.

Rob