[2/2] arm64: dts: exynos: Remove unneeded DSI and DECON address/size cells in Exynos5433
diff mbox

Message ID 20180618174216.24801-2-krzk@kernel.org
State Under Review
Headers show

Commit Message

Krzysztof Kozlowski June 18, 2018, 5:42 p.m. UTC
The decon, decon_tv and dsi nodes have only one child port so
address/size mappings are not necessary.  This fixes DTC warnings like:

    Warning (graph_child_address): /soc/decon@13800000/ports:
        graph node has single child node 'port@0', #address-cells/#size-cells are not necessary

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi |  6 +-----
 arch/arm64/boot/dts/exynos/exynos5433.dtsi            | 12 ++----------
 2 files changed, 3 insertions(+), 15 deletions(-)

Comments

Krzysztof Kozlowski June 18, 2018, 6:37 p.m. UTC | #1
On Mon, Jun 18, 2018 at 07:42:16PM +0200, Krzysztof Kozlowski wrote:
> The decon, decon_tv and dsi nodes have only one child port so
> address/size mappings are not necessary.  This fixes DTC warnings like:
> 
>     Warning (graph_child_address): /soc/decon@13800000/ports:
>         graph node has single child node 'port@0', #address-cells/#size-cells are not necessary
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi |  6 +-----
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi            | 12 ++----------
>  2 files changed, 3 insertions(+), 15 deletions(-)

The patch 1/2 should not have any impact but this one could have and I forgot
to mention that it was not tested.  If someone could provide testing, it
would be highly appreciated.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrzej Hajda June 19, 2018, 7:25 a.m. UTC | #2
On 18.06.2018 19:42, Krzysztof Kozlowski wrote:
> The decon, decon_tv and dsi nodes have only one child port so
> address/size mappings are not necessary.  This fixes DTC warnings like:
>
>     Warning (graph_child_address): /soc/decon@13800000/ports:
>         graph node has single child node 'port@0', #address-cells/#size-cells are not necessary

DECON nodes according to documentation have only one port so it is OK.
DSI has two ports, but since on all Exynos platforms DSI panels/bridges
are subnodes of ExynosDSI the 2nd port is not used.
So if there will be a platform with DSI panel/bridge controlled via I2C,
it should be reverted., but this is theoretical problem.



>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej


> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi |  6 +-----
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi            | 12 ++----------
>  2 files changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index a1e3194b7483..0a15ee513f5c 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -321,11 +321,7 @@
>  	status = "okay";
>  
>  	ports {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		port@0 {
> -			reg = <0>;
> +		port {
>  			tv_to_hdmi: endpoint {
>  				remote-endpoint = <&hdmi_to_tv>;
>  			};
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 3a9b4c4b9c63..e4367fd39120 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -850,11 +850,7 @@
>  			iommu-names = "m0", "m1";
>  
>  			ports {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -
> -				port@0 {
> -					reg = <0>;
> +				port {
>  					decon_to_mic: endpoint {
>  						remote-endpoint =
>  							<&mic_to_decon>;
> @@ -914,11 +910,7 @@
>  			#size-cells = <0>;
>  
>  			ports {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -
> -				port@0 {
> -					reg = <0>;
> +				port {
>  					dsi_to_mic: endpoint {
>  						remote-endpoint = <&mic_to_dsi>;
>  					};


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marek Szyprowski June 19, 2018, 7:26 a.m. UTC | #3
Hi Krzysztof,

On 2018-06-18 19:42, Krzysztof Kozlowski wrote:
> The decon, decon_tv and dsi nodes have only one child port so
> address/size mappings are not necessary.  This fixes DTC warnings like:
>
>      Warning (graph_child_address): /soc/decon@13800000/ports:
>          graph node has single child node 'port@0', #address-cells/#size-cells are not necessary
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Works fine with current Exynos DRM Decon/MIC/DSI drivers.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi |  6 +-----
>   arch/arm64/boot/dts/exynos/exynos5433.dtsi            | 12 ++----------
>   2 files changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index a1e3194b7483..0a15ee513f5c 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -321,11 +321,7 @@
>   	status = "okay";
>   
>   	ports {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		port@0 {
> -			reg = <0>;
> +		port {
>   			tv_to_hdmi: endpoint {
>   				remote-endpoint = <&hdmi_to_tv>;
>   			};
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 3a9b4c4b9c63..e4367fd39120 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -850,11 +850,7 @@
>   			iommu-names = "m0", "m1";
>   
>   			ports {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -
> -				port@0 {
> -					reg = <0>;
> +				port {
>   					decon_to_mic: endpoint {
>   						remote-endpoint =
>   							<&mic_to_decon>;
> @@ -914,11 +910,7 @@
>   			#size-cells = <0>;
>   
>   			ports {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -
> -				port@0 {
> -					reg = <0>;
> +				port {
>   					dsi_to_mic: endpoint {
>   						remote-endpoint = <&mic_to_dsi>;
>   					};

Best regards
Krzysztof Kozlowski June 19, 2018, 7:59 a.m. UTC | #4
On 19 June 2018 at 09:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Hi Krzysztof,
>
> On 2018-06-18 19:42, Krzysztof Kozlowski wrote:
>> The decon, decon_tv and dsi nodes have only one child port so
>> address/size mappings are not necessary.  This fixes DTC warnings like:
>>
>>      Warning (graph_child_address): /soc/decon@13800000/ports:
>>          graph node has single child node 'port@0', #address-cells/#size-cells are not necessary
>>
>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> Works fine with current Exynos DRM Decon/MIC/DSI drivers.
>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Thanks for review and testing!

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski June 20, 2018, 7:34 p.m. UTC | #5
On Tue, Jun 19, 2018 at 09:59:04AM +0200, Krzysztof Kozlowski wrote:
> On 19 June 2018 at 09:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > Hi Krzysztof,
> >
> > On 2018-06-18 19:42, Krzysztof Kozlowski wrote:
> >> The decon, decon_tv and dsi nodes have only one child port so
> >> address/size mappings are not necessary.  This fixes DTC warnings like:
> >>
> >>      Warning (graph_child_address): /soc/decon@13800000/ports:
> >>          graph node has single child node 'port@0', #address-cells/#size-cells are not necessary
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> >
> > Works fine with current Exynos DRM Decon/MIC/DSI drivers.
> >
> > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> 
> Thanks for review and testing!

I have second thoughs whether this patch is correct. AFAIU, the drivers
get the remote endpoints by reg==0 (for example the
of_graph_get_remote_node() in exynos_dsi_parse_dt()). If the port shall
be ignored, then reg==-1 should be passed.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrzej Hajda June 21, 2018, 5:59 a.m. UTC | #6
On 20.06.2018 21:34, Krzysztof Kozlowski wrote:
> On Tue, Jun 19, 2018 at 09:59:04AM +0200, Krzysztof Kozlowski wrote:
>> On 19 June 2018 at 09:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> Hi Krzysztof,
>>>
>>> On 2018-06-18 19:42, Krzysztof Kozlowski wrote:
>>>> The decon, decon_tv and dsi nodes have only one child port so
>>>> address/size mappings are not necessary.  This fixes DTC warnings like:
>>>>
>>>>      Warning (graph_child_address): /soc/decon@13800000/ports:
>>>>          graph node has single child node 'port@0', #address-cells/#size-cells are not necessary
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>>> Works fine with current Exynos DRM Decon/MIC/DSI drivers.
>>>
>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Thanks for review and testing!
> I have second thoughs whether this patch is correct. AFAIU, the drivers
> get the remote endpoints by reg==0 (for example the
> of_graph_get_remote_node() in exynos_dsi_parse_dt()). If the port shall
> be ignored, then reg==-1 should be passed.

All this is about purity, DECON bindings says explicitly that there
should be a port with reg=0.
So your patch and DTC warnings are incorrect from bindings PoV.
On the other side graph bindings are too bloated ( so many lines to
describe one connection ) so I am happy if there are shrinking attempts :)
Functionally nothing changes, of graph helpers assume reg=0 if it is not
present in port/endpoint node.

And regarding real issues, DECON could have more ports, possible candidates:
- GSCALER0/1/2,
- GSD/DSD - interconnect between GSCALERs and DECONs,
- SMIES - image enhancer (not implemented),
- MIC0/1 - image enhancers,
- DSIM0/1 - DSI encoders,
- HDMI - HDMI encoder.

But since all these connections can be configured dynamically, and more
importantly are inside specific SoC I dont think they need of_graphs. In
fact I think current of graph is also not necessary, but this is
different story, removal is on my long TODO list :)

Regards
Andrzej

>
> Best regards,
> Krzysztof
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Krzysztof Kozlowski June 21, 2018, 7:21 a.m. UTC | #7
On 21 June 2018 at 07:59, Andrzej Hajda <a.hajda@samsung.com> wrote:
> On 20.06.2018 21:34, Krzysztof Kozlowski wrote:
>> On Tue, Jun 19, 2018 at 09:59:04AM +0200, Krzysztof Kozlowski wrote:
>>> On 19 June 2018 at 09:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>>> Hi Krzysztof,
>>>>
>>>> On 2018-06-18 19:42, Krzysztof Kozlowski wrote:
>>>>> The decon, decon_tv and dsi nodes have only one child port so
>>>>> address/size mappings are not necessary.  This fixes DTC warnings like:
>>>>>
>>>>>      Warning (graph_child_address): /soc/decon@13800000/ports:
>>>>>          graph node has single child node 'port@0', #address-cells/#size-cells are not necessary
>>>>>
>>>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>>>> Works fine with current Exynos DRM Decon/MIC/DSI drivers.
>>>>
>>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Thanks for review and testing!
>> I have second thoughs whether this patch is correct. AFAIU, the drivers
>> get the remote endpoints by reg==0 (for example the
>> of_graph_get_remote_node() in exynos_dsi_parse_dt()). If the port shall
>> be ignored, then reg==-1 should be passed.
>
> All this is about purity, DECON bindings says explicitly that there
> should be a port with reg=0.

The warnings themself are indeed about purity.... but not having
warnings is quite useful. When you have already warnings either in C
code or in DTC, it is quite easy to skip new ones.

We successfully get rid of all DTC warnings from ARM part. It would be
nice to have the same for ARM64... but not with a cost of making bogus
changes.

> So your patch and DTC warnings are incorrect from bindings PoV.

Good point so this patch should come along with changing of bindings
(and maybe the code as well to use -1 as port/reg).

> On the other side graph bindings are too bloated ( so many lines to
> describe one connection ) so I am happy if there are shrinking attempts :)
> Functionally nothing changes, of graph helpers assume reg=0 if it is not
> present in port/endpoint node.
>
> And regarding real issues, DECON could have more ports, possible candidates:
> - GSCALER0/1/2,
> - GSD/DSD - interconnect between GSCALERs and DECONs,
> - SMIES - image enhancer (not implemented),
> - MIC0/1 - image enhancers,
> - DSIM0/1 - DSI encoders,
> - HDMI - HDMI encoder.
>
> But since all these connections can be configured dynamically, and more
> importantly are inside specific SoC I dont think they need of_graphs. In
> fact I think current of graph is also not necessary, but this is
> different story, removal is on my long TODO list :)

I keep my fingers crossed for this :)

Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index a1e3194b7483..0a15ee513f5c 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -321,11 +321,7 @@ 
 	status = "okay";
 
 	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port@0 {
-			reg = <0>;
+		port {
 			tv_to_hdmi: endpoint {
 				remote-endpoint = <&hdmi_to_tv>;
 			};
diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 3a9b4c4b9c63..e4367fd39120 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -850,11 +850,7 @@ 
 			iommu-names = "m0", "m1";
 
 			ports {
-				#address-cells = <1>;
-				#size-cells = <0>;
-
-				port@0 {
-					reg = <0>;
+				port {
 					decon_to_mic: endpoint {
 						remote-endpoint =
 							<&mic_to_decon>;
@@ -914,11 +910,7 @@ 
 			#size-cells = <0>;
 
 			ports {
-				#address-cells = <1>;
-				#size-cells = <0>;
-
-				port@0 {
-					reg = <0>;
+				port {
 					dsi_to_mic: endpoint {
 						remote-endpoint = <&mic_to_dsi>;
 					};