[PATCH 1/2] arm64: dts: exynos: Remove leading 0x from unit addresses in Exynos5433

[PATCH 1/2] arm64: dts: exynos: Remove leading 0x from unit addresses in Exynos5433

[Krzysztof Kozlowski]

Remove leading 0x from recently introduced unit addresses to fix DTC
warnings:

    Warning (unit_address_format): /soc/sysmmu@0x15040000: unit name should not have leading "0x"

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 038c99792ccb..3a9b4c4b9c63 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -1171,7 +1171,7 @@
 			power-domains = <&pd_gscl>;
 		};
 
-		sysmmu_scaler_0: sysmmu@0x15040000 {
+		sysmmu_scaler_0: sysmmu@15040000 {
 			compatible = "samsung,exynos-sysmmu";
 			reg = <0x15040000 0x1000>;
 			interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>;
@@ -1182,7 +1182,7 @@
 			power-domains = <&pd_mscl>;
 		};
 
-		sysmmu_scaler_1: sysmmu@0x15050000 {
+		sysmmu_scaler_1: sysmmu@15050000 {
 			compatible = "samsung,exynos-sysmmu";
 			reg = <0x15050000 0x1000>;
 			interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.14.1

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

[Krzysztof Kozlowski]

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

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>;
 					};
-- 
2.14.1

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

[Krzysztof Kozlowski]

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

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

[Andrzej Hajda]

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

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

[Marek Szyprowski]

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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

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

[Krzysztof Kozlowski]

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

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

[Krzysztof Kozlowski]

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

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

[Andrzej Hajda]

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

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

[Krzysztof Kozlowski]

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

Re: [PATCH 1/2] arm64: dts: exynos: Remove leading 0x from unit addresses in Exynos5433

[Joe Perches]

On Mon, 2018-06-18 at 19:42 +0200, Krzysztof Kozlowski wrote:
> Remove leading 0x from recently introduced unit addresses to fix DTC
> warnings:
> 
>     Warning (unit_address_format): /soc/sysmmu@0x15040000: unit name should not have leading "0x"
[]
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
[]
> @@ -1171,7 +1171,7 @@
>  			power-domains = <&pd_gscl>;
>  		};
>  
> -		sysmmu_scaler_0: sysmmu@0x15040000 {
> +		sysmmu_scaler_0: sysmmu@15040000 {
>  			compatible = "samsung,exynos-sysmmu";
>  			reg = <0x15040000 0x1000>;
>  			interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>;
> @@ -1182,7 +1182,7 @@
>  			power-domains = <&pd_mscl>;
>  		};
>  
> -		sysmmu_scaler_1: sysmmu@0x15050000 {
> +		sysmmu_scaler_1: sysmmu@15050000 {
>  			compatible = "samsung,exynos-sysmmu";
>  			reg = <0x15050000 0x1000>;
>  			interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;

Presumably these 3 other entries too?

$ git grep -P "\b\w+:\s+\w+@0x" -- "*.dt*"
arch/arc/boot/dts/abilis_tb10x.dtsi:            spi0: spi@0xFE010000 {
arch/arc/boot/dts/abilis_tb10x.dtsi:            spi1: spi@0xFE011000 {
arch/arc/boot/dts/vdk_axs10x_mb.dtsi:   uio_ev: uio@0xD0000000 {
arch/arm64/boot/dts/exynos/exynos5433.dtsi:             sysmmu_scaler_0: sysmmu@0x15040000 {
arch/arm64/boot/dts/exynos/exynos5433.dtsi:             sysmmu_scaler_1: sysmmu@0x15050000 {

Re: [PATCH 1/2] arm64: dts: exynos: Remove leading 0x from unit addresses in Exynos5433

[Krzysztof Kozlowski]

On 18 June 2018 at 19:57, Joe Perches <joe@perches.com> wrote:
> On Mon, 2018-06-18 at 19:42 +0200, Krzysztof Kozlowski wrote:
>> Remove leading 0x from recently introduced unit addresses to fix DTC
>> warnings:
>>
>>     Warning (unit_address_format): /soc/sysmmu@0x15040000: unit name should not have leading "0x"
> []
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> []
>> @@ -1171,7 +1171,7 @@
>>                       power-domains = <&pd_gscl>;
>>               };
>>
>> -             sysmmu_scaler_0: sysmmu@0x15040000 {
>> +             sysmmu_scaler_0: sysmmu@15040000 {
>>                       compatible = "samsung,exynos-sysmmu";
>>                       reg = <0x15040000 0x1000>;
>>                       interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>;
>> @@ -1182,7 +1182,7 @@
>>                       power-domains = <&pd_mscl>;
>>               };
>>
>> -             sysmmu_scaler_1: sysmmu@0x15050000 {
>> +             sysmmu_scaler_1: sysmmu@15050000 {
>>                       compatible = "samsung,exynos-sysmmu";
>>                       reg = <0x15050000 0x1000>;
>>                       interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;
>
> Presumably these 3 other entries too?
>
> $ git grep -P "\b\w+:\s+\w+@0x" -- "*.dt*"
> arch/arc/boot/dts/abilis_tb10x.dtsi:            spi0: spi@0xFE010000 {
> arch/arc/boot/dts/abilis_tb10x.dtsi:            spi1: spi@0xFE011000 {
> arch/arc/boot/dts/vdk_axs10x_mb.dtsi:   uio_ev: uio@0xD0000000 {

True. That's an "arc" but  I can fix it as well with your reported-by.

Best regards,
Krzysztof