All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
To: Rob Herring <robh@kernel.org>
Cc: Hans Verkuil <hans.verkuil@cisco.com>,
	daniel.thompson@linaro.org,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Daniel Kurtz <djkurtz@chromium.org>,
	Pawel Osciak <posciak@chromium.org>,
	Houlong Wei <houlong.wei@mediatek.com>,
	srv_heupstream@mediatek.com,
	Eddie Huang <eddie.huang@mediatek.com>,
	Yingjoe Chen <yingjoe.chen@mediatek.com>,
	Wu-Cheng Li <wuchengli@google.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org, linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 1/3] dt-bindings: mt8173: Fix mdp device tree
Date: Fri, 21 Apr 2017 12:06:10 +0800	[thread overview]
Message-ID: <1492747570.25908.36.camel@mtksdaap41> (raw)
In-Reply-To: <20170419213540.4vigeed3mx24ie4e@rob-hp-laptop>

On Wed, 2017-04-19 at 16:35 -0500, Rob Herring wrote:
> On Thu, Apr 13, 2017 at 03:33:05PM +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.
> 
> It may simplify the DT, but it also breaks compatibility with old DT. 
> Not sure if that's a problem on Mediatek platforms, but please be 
> explicit here that you are breaking compatibility and why that is okay.
> 

I will add the following description for more information.
"
Although it fixes iommu assignment issue, it also break compatibility
with old device tree, so driver patch[1] is needed to iterate over
sibling mdp device nodes, not child ones, to keep driver work properly.
In mtk_mdp_probe()
Old: for_each_child_of_node(dev->of_node, node)
New: for_each_child_of_node(dev->of_node->parent, node)

[1]https://patchwork.kernel.org/patch/9678833/
"

> > 
> > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> > Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> 
> Should this have Daniel as the author?

This patch is provided by Daniel, so I keep he is the author.
I just split his patch into two parts. One is dts only, and the other is
for driver. I also provide another patch to modify dts bindings
according to this patch.


> 
> > 
> > ---
> >  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
> > 

WARNING: multiple messages have this Message-ID (diff)
From: minghsiu.tsai@mediatek.com (Minghsiu Tsai)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] dt-bindings: mt8173: Fix mdp device tree
Date: Fri, 21 Apr 2017 12:06:10 +0800	[thread overview]
Message-ID: <1492747570.25908.36.camel@mtksdaap41> (raw)
In-Reply-To: <20170419213540.4vigeed3mx24ie4e@rob-hp-laptop>

On Wed, 2017-04-19 at 16:35 -0500, Rob Herring wrote:
> On Thu, Apr 13, 2017 at 03:33:05PM +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.
> 
> It may simplify the DT, but it also breaks compatibility with old DT. 
> Not sure if that's a problem on Mediatek platforms, but please be 
> explicit here that you are breaking compatibility and why that is okay.
> 

I will add the following description for more information.
"
Although it fixes iommu assignment issue, it also break compatibility
with old device tree, so driver patch[1] is needed to iterate over
sibling mdp device nodes, not child ones, to keep driver work properly.
In mtk_mdp_probe()
Old: for_each_child_of_node(dev->of_node, node)
New: for_each_child_of_node(dev->of_node->parent, node)

[1]https://patchwork.kernel.org/patch/9678833/
"

> > 
> > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> > Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> 
> Should this have Daniel as the author?

This patch is provided by Daniel, so I keep he is the author.
I just split his patch into two parts. One is dts only, and the other is
for driver. I also provide another patch to modify dts bindings
according to this patch.


> 
> > 
> > ---
> >  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 at 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 at 14002000 {
> > @@ -106,4 +101,3 @@ mdp {
> >  		iommus = <&iommu M4U_PORT_MDP_WROT1>;
> >  		mediatek,larb = <&larb4>;
> >  	};
> > -};
> > -- 
> > 1.9.1
> > 

WARNING: multiple messages have this Message-ID (diff)
From: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
To: Rob Herring <robh@kernel.org>
Cc: Hans Verkuil <hans.verkuil@cisco.com>,
	<daniel.thompson@linaro.org>,
	"Mauro Carvalho Chehab" <mchehab@osg.samsung.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Daniel Kurtz <djkurtz@chromium.org>,
	Pawel Osciak <posciak@chromium.org>,
	Houlong Wei <houlong.wei@mediatek.com>,
	<srv_heupstream@mediatek.com>,
	Eddie Huang <eddie.huang@mediatek.com>,
	Yingjoe Chen <yingjoe.chen@mediatek.com>,
	Wu-Cheng Li <wuchengli@google.com>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-media@vger.kernel.org>,
	<linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH 1/3] dt-bindings: mt8173: Fix mdp device tree
Date: Fri, 21 Apr 2017 12:06:10 +0800	[thread overview]
Message-ID: <1492747570.25908.36.camel@mtksdaap41> (raw)
In-Reply-To: <20170419213540.4vigeed3mx24ie4e@rob-hp-laptop>

On Wed, 2017-04-19 at 16:35 -0500, Rob Herring wrote:
> On Thu, Apr 13, 2017 at 03:33:05PM +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.
> 
> It may simplify the DT, but it also breaks compatibility with old DT. 
> Not sure if that's a problem on Mediatek platforms, but please be 
> explicit here that you are breaking compatibility and why that is okay.
> 

I will add the following description for more information.
"
Although it fixes iommu assignment issue, it also break compatibility
with old device tree, so driver patch[1] is needed to iterate over
sibling mdp device nodes, not child ones, to keep driver work properly.
In mtk_mdp_probe()
Old: for_each_child_of_node(dev->of_node, node)
New: for_each_child_of_node(dev->of_node->parent, node)

[1]https://patchwork.kernel.org/patch/9678833/
"

> > 
> > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> > Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> 
> Should this have Daniel as the author?

This patch is provided by Daniel, so I keep he is the author.
I just split his patch into two parts. One is dts only, and the other is
for driver. I also provide another patch to modify dts bindings
according to this patch.


> 
> > 
> > ---
> >  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
> > 

  reply	other threads:[~2017-04-21  4:06 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-13  7:33 [PATCH 0/3] Fix mdp device tree Minghsiu Tsai
2017-04-13  7:33 ` Minghsiu Tsai
2017-04-13  7:33 ` Minghsiu Tsai
2017-04-13  7:33 ` [PATCH 1/3] dt-bindings: mt8173: " Minghsiu Tsai
2017-04-13  7:33   ` Minghsiu Tsai
2017-04-13  7:33   ` Minghsiu Tsai
     [not found]   ` <1492068787-17838-2-git-send-email-minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
2017-04-19 21:35     ` Rob Herring
2017-04-19 21:35       ` Rob Herring
2017-04-19 21:35       ` Rob Herring
2017-04-21  4:06       ` Minghsiu Tsai [this message]
2017-04-21  4:06         ` Minghsiu Tsai
2017-04-21  4:06         ` Minghsiu Tsai
2017-04-13  7:33 ` [PATCH 2/3] arm64: dts: " Minghsiu Tsai
2017-04-13  7:33   ` Minghsiu Tsai
2017-04-13  7:33   ` Minghsiu Tsai
     [not found] ` <1492068787-17838-1-git-send-email-minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
2017-04-13  7:33   ` [PATCH 3/3] media: mtk-mdp: " Minghsiu Tsai
2017-04-13  7:33     ` Minghsiu Tsai
2017-04-13  7:33     ` Minghsiu Tsai
  -- strict thread matches above, loose matches on Subject: below --
2017-05-12  3:15 [PATCH v2 0/3] " Minghsiu Tsai
     [not found] ` <1494558914-41591-1-git-send-email-minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
2017-05-12  3:15   ` [PATCH 1/3] dt-bindings: mt8173: " Minghsiu Tsai
2017-05-12  3:15     ` Minghsiu Tsai
2017-05-12  3:15     ` Minghsiu Tsai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1492747570.25908.36.camel@mtksdaap41 \
    --to=minghsiu.tsai@mediatek.com \
    --cc=daniel.thompson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=djkurtz@chromium.org \
    --cc=eddie.huang@mediatek.com \
    --cc=hans.verkuil@cisco.com \
    --cc=houlong.wei@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=mchehab@osg.samsung.com \
    --cc=posciak@chromium.org \
    --cc=robh@kernel.org \
    --cc=srv_heupstream@mediatek.com \
    --cc=wuchengli@google.com \
    --cc=yingjoe.chen@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.