[PATCH 0/5] Fix the parse_dt of exynos dsi

[PATCH 0/5] Fix the parse_dt of exynos dsi

[Hoegeun Kwon]

This patch fixed the parse_dt function of exynos dsi and fixed the
related exynos3250, exynos4210, exynos4412, exynos5433 dts.

Hoegeun Kwon (5):
  drm/exynos: dsi: Fix the parse_dt function
  arm64: dts: exynos: Move the clock-frequency property
  arm: dts: Move the clock-frequency property for exynos3250 dts
  arm: dts: Move the clock-frequency property for exynos4412 dts
  arm: dts: Move the clock-frequency property for exynos4210 dts

 arch/arm/boot/dts/exynos3250-rinato.dts            | 23 ++--------------
 arch/arm/boot/dts/exynos4210-trats.dts             | 23 ++--------------
 arch/arm/boot/dts/exynos4412-trats2.dts            | 23 ++--------------
 .../boot/dts/exynos/exynos5433-tm2-common.dtsi     | 16 ++---------
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            | 32 ++++++----------------
 5 files changed, 16 insertions(+), 101 deletions(-)

-- 
1.9.1

[PATCH 1/5] drm/exynos: dsi: Fix the parse_dt function

[Hoegeun Kwon]

The bridge_node may or may not be required.
For example, mic and dsi are connected to OF graph, but fimd and dsi
are not connected. Also the OF graph is not needed because the panel
is a child of dsi. So not have to go to the endpoint and parse the
burst, esc clock-frequency.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index e07cb1f..214d486 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1652,39 +1652,23 @@ static int exynos_dsi_parse_dt(struct exynos_dsi *dsi)
 	if (ret < 0)
 		return ret;
 
-	ep = of_graph_get_endpoint_by_regs(node, DSI_PORT_OUT, 0);
-	if (!ep) {
-		dev_err(dev, "no output port with endpoint specified\n");
-		return -EINVAL;
-	}
-
-	ret = exynos_dsi_of_read_u32(ep, "samsung,burst-clock-frequency",
+	ret = exynos_dsi_of_read_u32(node, "samsung,burst-clock-frequency",
 				     &dsi->burst_clk_rate);
 	if (ret < 0)
-		goto end;
+		return ret;
 
-	ret = exynos_dsi_of_read_u32(ep, "samsung,esc-clock-frequency",
+	ret = exynos_dsi_of_read_u32(node, "samsung,esc-clock-frequency",
 				     &dsi->esc_clk_rate);
 	if (ret < 0)
-		goto end;
-
-	of_node_put(ep);
+		return ret;
 
 	ep = of_graph_get_next_endpoint(node, NULL);
-	if (!ep) {
-		ret = -EINVAL;
-		goto end;
-	}
-
-	dsi->bridge_node = of_graph_get_remote_port_parent(ep);
-	if (!dsi->bridge_node) {
-		ret = -EINVAL;
-		goto end;
+	if (ep) {
+		dsi->bridge_node = of_graph_get_remote_port_parent(ep);
+		of_node_put(ep);
 	}
-end:
-	of_node_put(ep);
 
-	return ret;
+	return 0;
 }
 
 static int exynos_dsi_bind(struct device *dev, struct device *master,
-- 
1.9.1

[PATCH 2/5] arm64: dts: exynos: Move the clock-frequency property

[Hoegeun Kwon]

The OF graph is not needed because the panel is a child of dsi. So
removed the ports and moved burst, esc clock-frequency property to the
top.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index 6ce93a3..77ba238 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -298,23 +298,11 @@
 	status = "okay";
 	vddcore-supply = <&ldo6_reg>;
 	vddio-supply = <&ldo7_reg>;
+	samsung,burst-clock-frequency = <512000000>;
+	samsung,esc-clock-frequency = <16000000>;
 	samsung,pll-clock-frequency = <24000000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&te_irq>;
-
-	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port@1 {
-			reg = <1>;
-
-			dsi_out: endpoint {
-				samsung,burst-clock-frequency = <512000000>;
-				samsung,esc-clock-frequency = <16000000>;
-			};
-		};
-	};
 };
 
 &hdmi {
-- 
1.9.1

[PATCH 3/5] arm: dts: Move the clock-frequency property for exynos3250 dts

[Hoegeun Kwon]

The OF graph is not needed because the panel is a child of dsi. So
removed the ports and moved burst, esc clock-frequency property to the
top.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm/boot/dts/exynos3250-rinato.dts | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts
index 548413e..82e676a 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -215,24 +215,11 @@
 &dsi_0 {
 	vddcore-supply = <&ldo6_reg>;
 	vddio-supply = <&ldo6_reg>;
+	samsung,burst-clock-frequency = <250000000>;
+	samsung,esc-clock-frequency = <20000000>;
 	samsung,pll-clock-frequency = <24000000>;
 	status = "okay";
 
-	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port@1 {
-			reg = <1>;
-
-			dsi_out: endpoint {
-				remote-endpoint = <&dsi_in>;
-				samsung,burst-clock-frequency = <250000000>;
-				samsung,esc-clock-frequency = <20000000>;
-			};
-		};
-	};
-
 	panel@0 {
 		compatible = "samsung,s6e63j0x03";
 		reg = <0>;
@@ -262,12 +249,6 @@
 				vsync-len = <2>;
 			};
 		};
-
-		port {
-			dsi_in: endpoint {
-				remote-endpoint = <&dsi_out>;
-			};
-		};
 	};
 };
 
-- 
1.9.1

[PATCH 4/5] arm: dts: Move the clock-frequency property for exynos4412 dts

[Hoegeun Kwon]

The OF graph is not needed because the panel is a child of dsi. So
removed the ports and moved burst, esc clock-frequency property to the
top.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm/boot/dts/exynos4412-trats2.dts | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
index 41ecd6d..86ce5e5 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -385,24 +385,11 @@
 &dsi_0 {
 	vddcore-supply = <&ldo8_reg>;
 	vddio-supply = <&ldo10_reg>;
+	samsung,burst-clock-frequency = <500000000>;
+	samsung,esc-clock-frequency = <20000000>;
 	samsung,pll-clock-frequency = <24000000>;
 	status = "okay";
 
-	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port@1 {
-			reg = <1>;
-
-			dsi_out: endpoint {
-				remote-endpoint = <&dsi_in>;
-				samsung,burst-clock-frequency = <500000000>;
-				samsung,esc-clock-frequency = <20000000>;
-			};
-		};
-	};
-
 	panel@0 {
 		compatible = "samsung,s6e8aa0";
 		reg = <0>;
@@ -430,12 +417,6 @@
 				vsync-len = <2>;
 			};
 		};
-
-		port {
-			dsi_in: endpoint {
-				remote-endpoint = <&dsi_out>;
-			};
-		};
 	};
 };
 
-- 
1.9.1

[PATCH 5/5] arm: dts: Move the clock-frequency property for exynos4210 dts

[Hoegeun Kwon]

The OF graph is not needed because the panel is a child of dsi. So
removed the ports and moved burst, esc clock-frequency property to the
top.

Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
---
 arch/arm/boot/dts/exynos4210-trats.dts | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts
index 0ca1b4d..9452bed 100644
--- a/arch/arm/boot/dts/exynos4210-trats.dts
+++ b/arch/arm/boot/dts/exynos4210-trats.dts
@@ -197,24 +197,11 @@
 &dsi_0 {
 	vddcore-supply = <&vusb_reg>;
 	vddio-supply = <&vmipi_reg>;
+	samsung,burst-clock-frequency = <500000000>;
+	samsung,esc-clock-frequency = <20000000>;
 	samsung,pll-clock-frequency = <24000000>;
 	status = "okay";
 
-	ports {
-		#address-cells = <1>;
-		#size-cells = <0>;
-
-		port@1 {
-			reg = <1>;
-
-			dsi_out: endpoint {
-				remote-endpoint = <&dsi_in>;
-				samsung,burst-clock-frequency = <500000000>;
-				samsung,esc-clock-frequency = <20000000>;
-			};
-		};
-	};
-
 	panel@0 {
 		reg = <0>;
 		compatible = "samsung,s6e8aa0";
@@ -242,12 +229,6 @@
 				vsync-len = <2>;
 			};
 		};
-
-		port {
-			dsi_in: endpoint {
-				remote-endpoint = <&dsi_out>;
-			};
-		};
 	};
 };
 
-- 
1.9.1

Re: [PATCH 2/5] arm64: dts: exynos: Move the clock-frequency property

[Krzysztof Kozlowski]

Hi,

I think the subject is not really matching the real work. You are rather
removing the OF graph from DSI node.

On Mon, Feb 06, 2017 at 11:19:41AM +0900, Hoegeun Kwon wrote:
> The OF graph is not needed because the panel is a child of dsi. So
> removed the ports and moved burst, esc clock-frequency property to the
> top.

Keep the commit style and tense - imperative mode (see
submitting-patches.rstsubmitting-patches.rst), so last sentence could
look like:

"Remove the ports node abd move burst and esc clock frequency properties
to the parent (DSI node)."

The information which is missing is the answer for WHY? Why are you
doing this?

Does the patch depends on 1/5?

Best regards,
Krzysztof

> 
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> index 6ce93a3..77ba238 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
> @@ -298,23 +298,11 @@
>  	status = "okay";
>  	vddcore-supply = <&ldo6_reg>;
>  	vddio-supply = <&ldo7_reg>;
> +	samsung,burst-clock-frequency = <512000000>;
> +	samsung,esc-clock-frequency = <16000000>;
>  	samsung,pll-clock-frequency = <24000000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&te_irq>;
> -
> -	ports {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -
> -		port@1 {
> -			reg = <1>;
> -
> -			dsi_out: endpoint {
> -				samsung,burst-clock-frequency = <512000000>;
> -				samsung,esc-clock-frequency = <16000000>;
> -			};
> -		};
> -	};
>  };
>  
>  &hdmi {
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] arm64: dts: exynos: Move the clock-frequency property

[Hoegeun Kwon]

Hi krzysztof,



On 02/08/2017 05:13 AM, Krzysztof Kozlowski wrote:
> Hi,
>
> I think the subject is not really matching the real work. You are rather
> removing the OF graph from DSI node.
>
> On Mon, Feb 06, 2017 at 11:19:41AM +0900, Hoegeun Kwon wrote:
>> The OF graph is not needed because the panel is a child of dsi. So
>> removed the ports and moved burst, esc clock-frequency property to the
>> top.
> Keep the commit style and tense - imperative mode (see
> submitting-patches.rstsubmitting-patches.rst), so last sentence could
> look like:

Hi Krzysztof,

Thanks for your review.
I will write a clear subject on the next version.

>
> "Remove the ports node abd move burst and esc clock frequency properties
> to the parent (DSI node)."
>
> The information which is missing is the answer for WHY? Why are you
> doing this?

The current mipi-dsi must have at least one OF graph.
However, for example, dsi and panel are parent-child relationships,
so OF graph is not needed, and fimd and dsi are not connected to the OF 
graph.
In this case, an error occurred in dsi_parse in the code before patch (1/5).
So I modified dsi_parse_dt.

>
> Does the patch depends on 1/5?

Yes, it is.
The 2/5 to 5/5 patches depend on the 1/5 patch.

Best regards,
Hoegeun

>
> Best regards,
> Krzysztof
>
>> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
>> ---
>>   arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 16 ++--------------
>>   1 file changed, 2 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> index 6ce93a3..77ba238 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
>> @@ -298,23 +298,11 @@
>>   	status = "okay";
>>   	vddcore-supply = <&ldo6_reg>;
>>   	vddio-supply = <&ldo7_reg>;
>> +	samsung,burst-clock-frequency = <512000000>;
>> +	samsung,esc-clock-frequency = <16000000>;
>>   	samsung,pll-clock-frequency = <24000000>;
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&te_irq>;
>> -
>> -	ports {
>> -		#address-cells = <1>;
>> -		#size-cells = <0>;
>> -
>> -		port@1 {
>> -			reg = <1>;
>> -
>> -			dsi_out: endpoint {
>> -				samsung,burst-clock-frequency = <512000000>;
>> -				samsung,esc-clock-frequency = <16000000>;
>> -			};
>> -		};
>> -	};
>>   };
>>   
>>   &hdmi {
>> -- 
>> 1.9.1
>>
>
>

Re: [PATCH 2/5] arm64: dts: exynos: Move the clock-frequency property

[Krzysztof Kozlowski]

On Wed, Feb 8, 2017 at 9:24 AM, Hoegeun Kwon <hoegeun.kwon@samsung.com> wrote:
>
>>
>> "Remove the ports node abd move burst and esc clock frequency properties
>> to the parent (DSI node)."
>>
>> The information which is missing is the answer for WHY? Why are you
>> doing this?
>
>
> The current mipi-dsi must have at least one OF graph.
> However, for example, dsi and panel are parent-child relationships,
> so OF graph is not needed, and fimd and dsi are not connected to the OF
> graph.
> In this case, an error occurred in dsi_parse in the code before patch (1/5).

The error occurred in case of DSI + FIMD? I do not get it whether you
are removing the something which is not needed or fixing something.

> So I modified dsi_parse_dt.
>
>>
>> Does the patch depends on 1/5?
>
>
> Yes, it is.
> The 2/5 to 5/5 patches depend on the 1/5 patch.

So that's a break of DT ABI for no clear (yet) reasons. Please mention
this in 1/5 patch and explain what is really fixed.

Best regards,
Krzysztof

Re: [PATCH 2/5] arm64: dts: exynos: Move the clock-frequency property

[Hoegeun Kwon]

On 02/08/2017 04:32 PM, Krzysztof Kozlowski wrote:
> On Wed, Feb 8, 2017 at 9:24 AM, Hoegeun Kwon <hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>>> "Remove the ports node abd move burst and esc clock frequency properties
>>> to the parent (DSI node)."
>>>
>>> The information which is missing is the answer for WHY? Why are you
>>> doing this?
>>
>> The current mipi-dsi must have at least one OF graph.
>> However, for example, dsi and panel are parent-child relationships,
>> so OF graph is not needed, and fimd and dsi are not connected to the OF
>> graph.
>> In this case, an error occurred in dsi_parse in the code before patch (1/5).
> The error occurred in case of DSI + FIMD? I do not get it whether you
> are removing the something which is not needed or fixing something.
>
>> So I modified dsi_parse_dt.
>>
>>> Does the patch depends on 1/5?
>>
>> Yes, it is.
>> The 2/5 to 5/5 patches depend on the 1/5 patch.
> So that's a break of DT ABI for no clear (yet) reasons. Please mention
> this in 1/5 patch and explain what is really fixed.

I would like to post the s6e63j0x03 panel driver for exynos3250.
However, as Rob Herring noted, dsi + panel is a parental relationship,
so OF grpah is not needed.
Therefore, the current dsi_parse_dt function will throw an error,
because there is no linked OF graph for case such as fimd + dsi + panel.
I think that the OF graph of dsi should be deleted even if it is not
the purpose of the s6e63j0x03 panel driver.

So the 1/5 patch parse the Pll, burst and esc clock frequency properties
in dsi_parse_dt and modified to create a bridge_node only if there is
an OF graph associated with dsi.

Best Regards,
Hoegeun

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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] arm64: dts: exynos: Move the clock-frequency property

[Krzysztof Kozlowski]

On Wed, Feb 8, 2017 at 12:09 PM, Hoegeun Kwon <hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> On 02/08/2017 04:32 PM, Krzysztof Kozlowski wrote:
>>
>> On Wed, Feb 8, 2017 at 9:24 AM, Hoegeun Kwon <hoegeun.kwon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> wrote:
>>>>
>>>> "Remove the ports node abd move burst and esc clock frequency properties
>>>> to the parent (DSI node)."
>>>>
>>>> The information which is missing is the answer for WHY? Why are you
>>>> doing this?
>>>
>>>
>>> The current mipi-dsi must have at least one OF graph.
>>> However, for example, dsi and panel are parent-child relationships,
>>> so OF graph is not needed, and fimd and dsi are not connected to the OF
>>> graph.
>>> In this case, an error occurred in dsi_parse in the code before patch
>>> (1/5).
>>
>> The error occurred in case of DSI + FIMD? I do not get it whether you
>> are removing the something which is not needed or fixing something.
>>
>>> So I modified dsi_parse_dt.
>>>
>>>> Does the patch depends on 1/5?
>>>
>>>
>>> Yes, it is.
>>> The 2/5 to 5/5 patches depend on the 1/5 patch.
>>
>> So that's a break of DT ABI for no clear (yet) reasons. Please mention
>> this in 1/5 patch and explain what is really fixed.
>
>
> I would like to post the s6e63j0x03 panel driver for exynos3250.
> However, as Rob Herring noted, dsi + panel is a parental relationship,
> so OF grpah is not needed.
> Therefore, the current dsi_parse_dt function will throw an error,
> because there is no linked OF graph for case such as fimd + dsi + panel.
> I think that the OF graph of dsi should be deleted even if it is not
> the purpose of the s6e63j0x03 panel driver.
>
> So the 1/5 patch parse the Pll, burst and esc clock frequency properties
> in dsi_parse_dt and modified to create a bridge_node only if there is
> an OF graph associated with dsi.
>

Thanks, now it makes sense. Such clear explanation should be part of
commit 1/5 as a proof that ABI breakage is needed.

BR,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html