[PATCH v3 0/3] Fix mdp device tree

[PATCH v3 0/3] Fix mdp device tree

[Minghsiu Tsai]

Changes in v3:
- Upload patches again because forget to add v2 in title

Changes in v2:
- Update commit message

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems.  This also simplifies the
device tree.

Although it fixes iommu assignment issue, it also break compatibility
with old device tree. So, the patch in driver is needed to iterate over
sibling mdp device nodes, not child ones, to keep driver work properly.

Daniel Kurtz (2):
  arm64: dts: mt8173: Fix mdp device tree
  media: mtk-mdp: Fix mdp device tree

Minghsiu Tsai (1):
  dt-bindings: mt8173: Fix mdp device tree

 .../devicetree/bindings/media/mediatek-mdp.txt     |  12 +-
 arch/arm64/boot/dts/mediatek/mt8173.dtsi           | 126 ++++++++++-----------
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c      |   2 +-
 3 files changed, 64 insertions(+), 76 deletions(-)

-- 
1.9.1

[PATCH v3 1/3] dt-bindings: mt8173: Fix mdp device tree

[Minghsiu Tsai]

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems.  This also simplifies the
device tree.

Although it fixes iommu assignment issue, it also break compatibility
with old device tree. So, the patch in driver is needed to iterate over
sibling mdp device nodes, not child ones, to keep driver work properly.

Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>

---
 Documentation/devicetree/bindings/media/mediatek-mdp.txt | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek-mdp.txt b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
index 4182063..0d03e3a 100644
--- a/Documentation/devicetree/bindings/media/mediatek-mdp.txt
+++ b/Documentation/devicetree/bindings/media/mediatek-mdp.txt
@@ -2,7 +2,7 @@
 
 Media Data Path is used for scaling and color space conversion.
 
-Required properties (controller (parent) node):
+Required properties (controller node):
 - compatible: "mediatek,mt8173-mdp"
 - mediatek,vpu: the node of video processor unit, see
   Documentation/devicetree/bindings/media/mediatek-vpu.txt for details.
@@ -32,21 +32,16 @@ Required properties (DMA function blocks, child node):
   for details.
 
 Example:
-mdp {
-	compatible = "mediatek,mt8173-mdp";
-	#address-cells = <2>;
-	#size-cells = <2>;
-	ranges;
-	mediatek,vpu = <&vpu>;
-
 	mdp_rdma0: rdma@14001000 {
 		compatible = "mediatek,mt8173-mdp-rdma";
+			     "mediatek,mt8173-mdp";
 		reg = <0 0x14001000 0 0x1000>;
 		clocks = <&mmsys CLK_MM_MDP_RDMA0>,
 			 <&mmsys CLK_MM_MUTEX_32K>;
 		power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
 		iommus = <&iommu M4U_PORT_MDP_RDMA0>;
 		mediatek,larb = <&larb0>;
+		mediatek,vpu = <&vpu>;
 	};
 
 	mdp_rdma1: rdma@14002000 {
@@ -106,4 +101,3 @@ mdp {
 		iommus = <&iommu M4U_PORT_MDP_WROT1>;
 		mediatek,larb = <&larb4>;
 	};
-};
-- 
1.9.1

[PATCH v3 2/3] arm64: dts: mt8173: Fix mdp device tree

[Minghsiu Tsai]

From: Daniel Kurtz <djkurtz@chromium.org>

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems.  This also simplifies the
device tree.

Although it fixes iommu assignment issue, it also break compatibility
with old device tree. So, the patch in driver is needed to iterate over
sibling mdp device nodes, not child ones, to keep driver work properly.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>

---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 126 +++++++++++++++----------------
 1 file changed, 60 insertions(+), 66 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 6922252..d28a363 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -792,80 +792,74 @@
 			#clock-cells = <1>;
 		};
 
-		mdp {
-			compatible = "mediatek,mt8173-mdp";
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges;
+		mdp_rdma0: rdma@14001000 {
+			compatible = "mediatek,mt8173-mdp-rdma",
+				     "mediatek,mt8173-mdp";
+			reg = <0 0x14001000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RDMA0>,
+				 <&mmsys CLK_MM_MUTEX_32K>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_RDMA0>;
+			mediatek,larb = <&larb0>;
 			mediatek,vpu = <&vpu>;
+		};
 
-			mdp_rdma0: rdma@14001000 {
-				compatible = "mediatek,mt8173-mdp-rdma";
-				reg = <0 0x14001000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RDMA0>,
-					 <&mmsys CLK_MM_MUTEX_32K>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_RDMA0>;
-				mediatek,larb = <&larb0>;
-			};
-
-			mdp_rdma1: rdma@14002000 {
-				compatible = "mediatek,mt8173-mdp-rdma";
-				reg = <0 0x14002000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RDMA1>,
-					 <&mmsys CLK_MM_MUTEX_32K>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_RDMA1>;
-				mediatek,larb = <&larb4>;
-			};
+		mdp_rdma1: rdma@14002000 {
+			compatible = "mediatek,mt8173-mdp-rdma";
+			reg = <0 0x14002000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RDMA1>,
+				 <&mmsys CLK_MM_MUTEX_32K>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_RDMA1>;
+			mediatek,larb = <&larb4>;
+		};
 
-			mdp_rsz0: rsz@14003000 {
-				compatible = "mediatek,mt8173-mdp-rsz";
-				reg = <0 0x14003000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RSZ0>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-			};
+		mdp_rsz0: rsz@14003000 {
+			compatible = "mediatek,mt8173-mdp-rsz";
+			reg = <0 0x14003000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RSZ0>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+		};
 
-			mdp_rsz1: rsz@14004000 {
-				compatible = "mediatek,mt8173-mdp-rsz";
-				reg = <0 0x14004000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RSZ1>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-			};
+		mdp_rsz1: rsz@14004000 {
+			compatible = "mediatek,mt8173-mdp-rsz";
+			reg = <0 0x14004000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RSZ1>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+		};
 
-			mdp_rsz2: rsz@14005000 {
-				compatible = "mediatek,mt8173-mdp-rsz";
-				reg = <0 0x14005000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_RSZ2>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-			};
+		mdp_rsz2: rsz@14005000 {
+			compatible = "mediatek,mt8173-mdp-rsz";
+			reg = <0 0x14005000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_RSZ2>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+		};
 
-			mdp_wdma0: wdma@14006000 {
-				compatible = "mediatek,mt8173-mdp-wdma";
-				reg = <0 0x14006000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_WDMA>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_WDMA>;
-				mediatek,larb = <&larb0>;
-			};
+		mdp_wdma0: wdma@14006000 {
+			compatible = "mediatek,mt8173-mdp-wdma";
+			reg = <0 0x14006000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_WDMA>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_WDMA>;
+			mediatek,larb = <&larb0>;
+		};
 
-			mdp_wrot0: wrot@14007000 {
-				compatible = "mediatek,mt8173-mdp-wrot";
-				reg = <0 0x14007000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_WROT0>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_WROT0>;
-				mediatek,larb = <&larb0>;
-			};
+		mdp_wrot0: wrot@14007000 {
+			compatible = "mediatek,mt8173-mdp-wrot";
+			reg = <0 0x14007000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_WROT0>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_WROT0>;
+			mediatek,larb = <&larb0>;
+		};
 
-			mdp_wrot1: wrot@14008000 {
-				compatible = "mediatek,mt8173-mdp-wrot";
-				reg = <0 0x14008000 0 0x1000>;
-				clocks = <&mmsys CLK_MM_MDP_WROT1>;
-				power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-				iommus = <&iommu M4U_PORT_MDP_WROT1>;
-				mediatek,larb = <&larb4>;
-			};
+		mdp_wrot1: wrot@14008000 {
+			compatible = "mediatek,mt8173-mdp-wrot";
+			reg = <0 0x14008000 0 0x1000>;
+			clocks = <&mmsys CLK_MM_MDP_WROT1>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			iommus = <&iommu M4U_PORT_MDP_WROT1>;
+			mediatek,larb = <&larb4>;
 		};
 
 		ovl0: ovl@1400c000 {
-- 
1.9.1

[PATCH v3 3/3] media: mtk-mdp: Fix mdp device tree

[Minghsiu Tsai]

From: Daniel Kurtz <djkurtz@chromium.org>

If the mdp_* nodes are under an mdp sub-node, their corresponding
platform device does not automatically get its iommu assigned properly.

Fix this by moving the mdp component nodes up a level such that they are
siblings of mdp and all other SoC subsystems.  This also simplifies the
device tree.

Although it fixes iommu assignment issue, it also break compatibility
with old device tree. So, the patch in driver is needed to iterate over
sibling mdp device nodes, not child ones, to keep driver work properly.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>

---
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index 9e4eb7d..a5ad586 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -118,7 +118,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
 	mutex_init(&mdp->vpulock);
 
 	/* Iterate over sibling MDP function blocks */
-	for_each_child_of_node(dev->of_node, node) {
+	for_each_child_of_node(dev->of_node->parent, node) {
 		const struct of_device_id *of_id;
 		enum mtk_mdp_comp_type comp_type;
 		int comp_id;
-- 
1.9.1

Re: [PATCH v3 3/3] media: mtk-mdp: Fix mdp device tree

[Matthias Brugger]

On 12/05/17 05:22, Minghsiu Tsai wrote:
> From: Daniel Kurtz <djkurtz@chromium.org>
> 
> If the mdp_* nodes are under an mdp sub-node, their corresponding
> platform device does not automatically get its iommu assigned properly.
> 
> Fix this by moving the mdp component nodes up a level such that they are
> siblings of mdp and all other SoC subsystems.  This also simplifies the
> device tree.
> 
> Although it fixes iommu assignment issue, it also break compatibility
> with old device tree. So, the patch in driver is needed to iterate over
> sibling mdp device nodes, not child ones, to keep driver work properly.
> 

Couldn't we preserve backwards compatibility by doing something like this:
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c 
b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index 9e4eb7dcc424..277d8fe6eb76 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -103,7 +103,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
  {
  	struct mtk_mdp_dev *mdp;
  	struct device *dev = &pdev->dev;
-	struct device_node *node;
+	struct device_node *node, *parent;
  	int i, ret = 0;

  	mdp = devm_kzalloc(dev, sizeof(*mdp), GFP_KERNEL);
@@ -117,8 +117,14 @@ static int mtk_mdp_probe(struct platform_device *pdev)
  	mutex_init(&mdp->lock);
  	mutex_init(&mdp->vpulock);

+	/* Old dts had the components as child nodes */
+	if (of_get_next_child(dev->of_node, NULL))
+		parent = dev->of_node;
+	else
+		parent = dev->of_node->parent;
+
  	/* Iterate over sibling MDP function blocks */
-	for_each_child_of_node(dev->of_node, node) {
+	for_each_child_of_node(parent, node) {
  		const struct of_device_id *of_id;
  		enum mtk_mdp_comp_type comp_type;
  		int comp_id;

Maybe even by putting a warning in the if branch to make sure, people 
are aware of their out-of-date device tree blobs.

Regards,
Matthias

> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> 
> ---
>   drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
> index 9e4eb7d..a5ad586 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
> @@ -118,7 +118,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
>   	mutex_init(&mdp->vpulock);
>   
>   	/* Iterate over sibling MDP function blocks */
> -	for_each_child_of_node(dev->of_node, node) {
> +	for_each_child_of_node(dev->of_node->parent, node) {
>   		const struct of_device_id *of_id;
>   		enum mtk_mdp_comp_type comp_type;
>   		int comp_id;
> 

Re: [PATCH v3 3/3] media: mtk-mdp: Fix mdp device tree

[Minghsiu Tsai]

On Fri, 2017-05-12 at 17:05 +0200, Matthias Brugger wrote:
> 
> On 12/05/17 05:22, Minghsiu Tsai wrote:
> > From: Daniel Kurtz <djkurtz@chromium.org>
> > 
> > If the mdp_* nodes are under an mdp sub-node, their corresponding
> > platform device does not automatically get its iommu assigned properly.
> > 
> > Fix this by moving the mdp component nodes up a level such that they are
> > siblings of mdp and all other SoC subsystems.  This also simplifies the
> > device tree.
> > 
> > Although it fixes iommu assignment issue, it also break compatibility
> > with old device tree. So, the patch in driver is needed to iterate over
> > sibling mdp device nodes, not child ones, to keep driver work properly.
> > 
> 
> Couldn't we preserve backwards compatibility by doing something like this:
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c 
> b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
> index 9e4eb7dcc424..277d8fe6eb76 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
> @@ -103,7 +103,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
>   {
>   	struct mtk_mdp_dev *mdp;
>   	struct device *dev = &pdev->dev;
> -	struct device_node *node;
> +	struct device_node *node, *parent;
>   	int i, ret = 0;
> 
>   	mdp = devm_kzalloc(dev, sizeof(*mdp), GFP_KERNEL);
> @@ -117,8 +117,14 @@ static int mtk_mdp_probe(struct platform_device *pdev)
>   	mutex_init(&mdp->lock);
>   	mutex_init(&mdp->vpulock);
> 
> +	/* Old dts had the components as child nodes */
> +	if (of_get_next_child(dev->of_node, NULL))
> +		parent = dev->of_node;
> +	else
> +		parent = dev->of_node->parent;
> +
>   	/* Iterate over sibling MDP function blocks */
> -	for_each_child_of_node(dev->of_node, node) {
> +	for_each_child_of_node(parent, node) {
>   		const struct of_device_id *of_id;
>   		enum mtk_mdp_comp_type comp_type;
>   		int comp_id;
> 
> Maybe even by putting a warning in the if branch to make sure, people 
> are aware of their out-of-date device tree blobs.
> 
> Regards,
> Matthias
> 

Hi Matthias,

It is a good idea to do compatible in such a way and put a warning the
device tree is out of date. People can find out cause soon if device
tree is old.

I modify the code as below:

+	/* Old dts had the components as child nodes */
+	if (of_get_next_child(dev->of_node, NULL)) {
+		parent = dev->of_node;
+		dev_warn(dev, "device tree is out of date\n");
+	} else {
+		parent = dev->of_node->parent;
+	}

Will you upload it in a separate patch? 
If not, I can merge it in my patch series and upload v4.


Best Regards,

Ming Hsiu

> > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> > Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> > 
> > ---
> >   drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
> > index 9e4eb7d..a5ad586 100644
> > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
> > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
> > @@ -118,7 +118,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
> >   	mutex_init(&mdp->vpulock);
> >   
> >   	/* Iterate over sibling MDP function blocks */
> > -	for_each_child_of_node(dev->of_node, node) {
> > +	for_each_child_of_node(dev->of_node->parent, node) {
> >   		const struct of_device_id *of_id;
> >   		enum mtk_mdp_comp_type comp_type;
> >   		int comp_id;
> > 

Re: [PATCH v3 3/3] media: mtk-mdp: Fix mdp device tree

[Matthias Brugger]

On 15/05/17 04:31, Minghsiu Tsai wrote:
> On Fri, 2017-05-12 at 17:05 +0200, Matthias Brugger wrote:
>>
>> On 12/05/17 05:22, Minghsiu Tsai wrote:
>>> From: Daniel Kurtz <djkurtz@chromium.org>
>>>
>>> If the mdp_* nodes are under an mdp sub-node, their corresponding
>>> platform device does not automatically get its iommu assigned properly.
>>>
>>> Fix this by moving the mdp component nodes up a level such that they are
>>> siblings of mdp and all other SoC subsystems.  This also simplifies the
>>> device tree.
>>>
>>> Although it fixes iommu assignment issue, it also break compatibility
>>> with old device tree. So, the patch in driver is needed to iterate over
>>> sibling mdp device nodes, not child ones, to keep driver work properly.
>>>
>>
>> Couldn't we preserve backwards compatibility by doing something like this:
>> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
>> b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
>> index 9e4eb7dcc424..277d8fe6eb76 100644
>> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
>> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
>> @@ -103,7 +103,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
>>    {
>>    	struct mtk_mdp_dev *mdp;
>>    	struct device *dev = &pdev->dev;
>> -	struct device_node *node;
>> +	struct device_node *node, *parent;
>>    	int i, ret = 0;
>>
>>    	mdp = devm_kzalloc(dev, sizeof(*mdp), GFP_KERNEL);
>> @@ -117,8 +117,14 @@ static int mtk_mdp_probe(struct platform_device *pdev)
>>    	mutex_init(&mdp->lock);
>>    	mutex_init(&mdp->vpulock);
>>
>> +	/* Old dts had the components as child nodes */
>> +	if (of_get_next_child(dev->of_node, NULL))
>> +		parent = dev->of_node;
>> +	else
>> +		parent = dev->of_node->parent;
>> +
>>    	/* Iterate over sibling MDP function blocks */
>> -	for_each_child_of_node(dev->of_node, node) {
>> +	for_each_child_of_node(parent, node) {
>>    		const struct of_device_id *of_id;
>>    		enum mtk_mdp_comp_type comp_type;
>>    		int comp_id;
>>
>> Maybe even by putting a warning in the if branch to make sure, people
>> are aware of their out-of-date device tree blobs.
>>
>> Regards,
>> Matthias
>>
> 
> Hi Matthias,
> 
> It is a good idea to do compatible in such a way and put a warning the
> device tree is out of date. People can find out cause soon if device
> tree is old.
> 
> I modify the code as below:
> 
> +	/* Old dts had the components as child nodes */
> +	if (of_get_next_child(dev->of_node, NULL)) {
> +		parent = dev->of_node;
> +		dev_warn(dev, "device tree is out of date\n");
> +	} else {
> +		parent = dev->of_node->parent;
> +	}
> 
> Will you upload it in a separate patch?
> If not, I can merge it in my patch series and upload v4.
> 

Please integrate it into your patch series.

Mauro, are you ok with the dev_warn about the out-of-date device-tree?

Regards,
Matthias


> 
> Best Regards,
> 
> Ming Hsiu
> 
>>> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>>> Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
>>>
>>> ---
>>>    drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
>>> index 9e4eb7d..a5ad586 100644
>>> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
>>> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
>>> @@ -118,7 +118,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
>>>    	mutex_init(&mdp->vpulock);
>>>    
>>>    	/* Iterate over sibling MDP function blocks */
>>> -	for_each_child_of_node(dev->of_node, node) {
>>> +	for_each_child_of_node(dev->of_node->parent, node) {
>>>    		const struct of_device_id *of_id;
>>>    		enum mtk_mdp_comp_type comp_type;
>>>    		int comp_id;
>>>
> 
> 

Re: [PATCH v3 1/3] dt-bindings: mt8173: Fix mdp device tree

[Rob Herring]

On Fri, May 12, 2017 at 11:22:39AM +0800, Minghsiu Tsai wrote:
> If the mdp_* nodes are under an mdp sub-node, their corresponding
> platform device does not automatically get its iommu assigned properly.
> 
> Fix this by moving the mdp component nodes up a level such that they are
> siblings of mdp and all other SoC subsystems.  This also simplifies the
> device tree.
> 
> Although it fixes iommu assignment issue, it also break compatibility
> with old device tree. So, the patch in driver is needed to iterate over
> sibling mdp device nodes, not child ones, to keep driver work properly.
> 
> Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> 
> ---
>  Documentation/devicetree/bindings/media/mediatek-mdp.txt | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)

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

Re: [PATCH v3 0/3] Fix mdp device tree

[Hans Verkuil]

On 05/12/2017 05:22 AM, Minghsiu Tsai wrote:

Who should take care of the dtsi changes? I'm not sure who maintains the mdp dts.

The driver change and the dtsi change need to be in sync, so it is probably easiest
to merge this via one tree.

Here is my Acked-by for these three patches:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

I can take all three, provided I have the Ack of the mdp dts maintainer. Or it can
go through him with my Ack.

Regards,

	Hans

> Changes in v3:
> - Upload patches again because forget to add v2 in title
> 
> Changes in v2:
> - Update commit message
> 
> If the mdp_* nodes are under an mdp sub-node, their corresponding
> platform device does not automatically get its iommu assigned properly.
> 
> Fix this by moving the mdp component nodes up a level such that they are
> siblings of mdp and all other SoC subsystems.  This also simplifies the
> device tree.
> 
> Although it fixes iommu assignment issue, it also break compatibility
> with old device tree. So, the patch in driver is needed to iterate over
> sibling mdp device nodes, not child ones, to keep driver work properly.
> 
> Daniel Kurtz (2):
>   arm64: dts: mt8173: Fix mdp device tree
>   media: mtk-mdp: Fix mdp device tree
> 
> Minghsiu Tsai (1):
>   dt-bindings: mt8173: Fix mdp device tree
> 
>  .../devicetree/bindings/media/mediatek-mdp.txt     |  12 +-
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi           | 126 ++++++++++-----------
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.c      |   2 +-
>  3 files changed, 64 insertions(+), 76 deletions(-)
> 

Re: [PATCH v3 0/3] Fix mdp device tree

[Matthias Brugger]

On 22/05/17 11:09, Hans Verkuil wrote:
> On 05/12/2017 05:22 AM, Minghsiu Tsai wrote:
> 
> Who should take care of the dtsi changes? I'm not sure who maintains the mdp dts.

I will take care of the dtsi patches.

> 
> The driver change and the dtsi change need to be in sync, so it is probably easiest
> to merge this via one tree.
> 
> Here is my Acked-by for these three patches:
> 
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> I can take all three, provided I have the Ack of the mdp dts maintainer. Or it can
> go through him with my Ack.
> 

I think we should provide backwards compability instead, as proposed here:
http://lists.infradead.org/pipermail/linux-mediatek/2017-May/008811.html

If this change is ok for you, please let Minghsiu know so that he can 
provide a v4.

Regards,
Matthias

> Regards,
> 
> 	Hans
> 
>> Changes in v3:
>> - Upload patches again because forget to add v2 in title
>>
>> Changes in v2:
>> - Update commit message
>>
>> If the mdp_* nodes are under an mdp sub-node, their corresponding
>> platform device does not automatically get its iommu assigned properly.
>>
>> Fix this by moving the mdp component nodes up a level such that they are
>> siblings of mdp and all other SoC subsystems.  This also simplifies the
>> device tree.
>>
>> Although it fixes iommu assignment issue, it also break compatibility
>> with old device tree. So, the patch in driver is needed to iterate over
>> sibling mdp device nodes, not child ones, to keep driver work properly.
>>
>> Daniel Kurtz (2):
>>    arm64: dts: mt8173: Fix mdp device tree
>>    media: mtk-mdp: Fix mdp device tree
>>
>> Minghsiu Tsai (1):
>>    dt-bindings: mt8173: Fix mdp device tree
>>
>>   .../devicetree/bindings/media/mediatek-mdp.txt     |  12 +-
>>   arch/arm64/boot/dts/mediatek/mt8173.dtsi           | 126 ++++++++++-----------
>>   drivers/media/platform/mtk-mdp/mtk_mdp_core.c      |   2 +-
>>   3 files changed, 64 insertions(+), 76 deletions(-)
>>
> 

Re: [PATCH v3 0/3] Fix mdp device tree

[Hans Verkuil]

On 05/22/2017 04:14 PM, Matthias Brugger wrote:
> 
> 
> On 22/05/17 11:09, Hans Verkuil wrote:
>> On 05/12/2017 05:22 AM, Minghsiu Tsai wrote:
>>
>> Who should take care of the dtsi changes? I'm not sure who maintains the mdp dts.
> 
> I will take care of the dtsi patches.
> 
>>
>> The driver change and the dtsi change need to be in sync, so it is probably easiest
>> to merge this via one tree.
>>
>> Here is my Acked-by for these three patches:
>>
>> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> I can take all three, provided I have the Ack of the mdp dts maintainer. Or it can
>> go through him with my Ack.
>>
> 
> I think we should provide backwards compability instead, as proposed here:
> http://lists.infradead.org/pipermail/linux-mediatek/2017-May/008811.html
> 
> If this change is ok for you, please let Minghsiu know so that he can 
> provide a v4.

That's a lot better. In that case I can take the media patches and you the dts.

I'll wait for the v4.

Regards,

	Hans

> 
> Regards,
> Matthias
> 
>> Regards,
>>
>> 	Hans
>>
>>> Changes in v3:
>>> - Upload patches again because forget to add v2 in title
>>>
>>> Changes in v2:
>>> - Update commit message
>>>
>>> If the mdp_* nodes are under an mdp sub-node, their corresponding
>>> platform device does not automatically get its iommu assigned properly.
>>>
>>> Fix this by moving the mdp component nodes up a level such that they are
>>> siblings of mdp and all other SoC subsystems.  This also simplifies the
>>> device tree.
>>>
>>> Although it fixes iommu assignment issue, it also break compatibility
>>> with old device tree. So, the patch in driver is needed to iterate over
>>> sibling mdp device nodes, not child ones, to keep driver work properly.
>>>
>>> Daniel Kurtz (2):
>>>    arm64: dts: mt8173: Fix mdp device tree
>>>    media: mtk-mdp: Fix mdp device tree
>>>
>>> Minghsiu Tsai (1):
>>>    dt-bindings: mt8173: Fix mdp device tree
>>>
>>>   .../devicetree/bindings/media/mediatek-mdp.txt     |  12 +-
>>>   arch/arm64/boot/dts/mediatek/mt8173.dtsi           | 126 ++++++++++-----------
>>>   drivers/media/platform/mtk-mdp/mtk_mdp_core.c      |   2 +-
>>>   3 files changed, 64 insertions(+), 76 deletions(-)
>>>
>>

Re: [PATCH v3 0/3] Fix mdp device tree

[Minghsiu Tsai]

On Mon, 2017-05-22 at 16:16 +0200, Hans Verkuil wrote:
> On 05/22/2017 04:14 PM, Matthias Brugger wrote:
> > 
> > 
> > On 22/05/17 11:09, Hans Verkuil wrote:
> >> On 05/12/2017 05:22 AM, Minghsiu Tsai wrote:
> >>
> >> Who should take care of the dtsi changes? I'm not sure who maintains the mdp dts.
> > 
> > I will take care of the dtsi patches.
> > 
> >>
> >> The driver change and the dtsi change need to be in sync, so it is probably easiest
> >> to merge this via one tree.
> >>
> >> Here is my Acked-by for these three patches:
> >>
> >> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> >>
> >> I can take all three, provided I have the Ack of the mdp dts maintainer. Or it can
> >> go through him with my Ack.
> >>
> > 
> > I think we should provide backwards compability instead, as proposed here:
> > http://lists.infradead.org/pipermail/linux-mediatek/2017-May/008811.html
> > 
> > If this change is ok for you, please let Minghsiu know so that he can 
> > provide a v4.
> 
> That's a lot better. In that case I can take the media patches and you the dts.
> 
> I'll wait for the v4.
> 

Hi Hans, Matthias,

I have uploaded v4, thanks.


> Regards,
> 
> 	Hans
> 
> > 
> > Regards,
> > Matthias
> > 
> >> Regards,
> >>
> >> 	Hans
> >>
> >>> Changes in v3:
> >>> - Upload patches again because forget to add v2 in title
> >>>
> >>> Changes in v2:
> >>> - Update commit message
> >>>
> >>> If the mdp_* nodes are under an mdp sub-node, their corresponding
> >>> platform device does not automatically get its iommu assigned properly.
> >>>
> >>> Fix this by moving the mdp component nodes up a level such that they are
> >>> siblings of mdp and all other SoC subsystems.  This also simplifies the
> >>> device tree.
> >>>
> >>> Although it fixes iommu assignment issue, it also break compatibility
> >>> with old device tree. So, the patch in driver is needed to iterate over
> >>> sibling mdp device nodes, not child ones, to keep driver work properly.
> >>>
> >>> Daniel Kurtz (2):
> >>>    arm64: dts: mt8173: Fix mdp device tree
> >>>    media: mtk-mdp: Fix mdp device tree
> >>>
> >>> Minghsiu Tsai (1):
> >>>    dt-bindings: mt8173: Fix mdp device tree
> >>>
> >>>   .../devicetree/bindings/media/mediatek-mdp.txt     |  12 +-
> >>>   arch/arm64/boot/dts/mediatek/mt8173.dtsi           | 126 ++++++++++-----------
> >>>   drivers/media/platform/mtk-mdp/mtk_mdp_core.c      |   2 +-
> >>>   3 files changed, 64 insertions(+), 76 deletions(-)
> >>>
> >>
>