[PATCH 1/3] ARM: dts: imx6sx: Remove LDB endpoint

[PATCH 1/3] ARM: dts: imx6sx: Remove LDB endpoint

[Fabio Estevam]

From: Fabio Estevam <festevam@denx.de>

Remove the LDB endpoint description from the common imx6sx.dtsi
as it causes regression for boards that has the LCDIF connected
directly to a parallel display.

Let the LDB endpoint be described in the board devicetree file
instead.

Fixes: b74edf626c4f ("ARM: dts: imx6sx: Add LDB support")
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 arch/arm/boot/dts/imx6sx.dtsi | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 3a4308666552..41c900929758 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -863,7 +863,6 @@ port@0 {
 							reg = <0>;
 
 							ldb_from_lcdif1: endpoint {
-								remote-endpoint = <&lcdif1_to_ldb>;
 							};
 						};
 
@@ -1309,11 +1308,8 @@ lcdif1: lcdif@2220000 {
 					power-domains = <&pd_disp>;
 					status = "disabled";
 
-					ports {
-						port {
-							lcdif1_to_ldb: endpoint {
-								remote-endpoint = <&ldb_from_lcdif1>;
-							};
+					port {
+						lcdif1_to_ldb: endpoint {
 						};
 					};
 				};
-- 
2.34.1

[PATCH 2/3] ARM: dts: imx6sx: Describe the default LCDIF1 parent

[Fabio Estevam]

From: Fabio Estevam <festevam@denx.de>

A suitable default for the LCDIF parent is the PLL5 clock, so
describe it in the device tree.

The imx6sx clock driver harcodes PLL5 as the LCDIF1 parent, but
in preparation for removing such hardcoding, describe the parent
relationship via devicetree.

There are some boards that may want to use a different parent
for the LCDIF due to EMI reasons, for example.

With this approch, the user can change the LCDIF parent in the board
devicetree if needed.

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 arch/arm/boot/dts/imx6sx.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 41c900929758..0d549e1f3ced 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -1305,6 +1305,10 @@ lcdif1: lcdif@2220000 {
 						 <&clks IMX6SX_CLK_LCDIF_APB>,
 						 <&clks IMX6SX_CLK_DISPLAY_AXI>;
 					clock-names = "pix", "axi", "disp_axi";
+					assigned-clocks = <&clks IMX6SX_CLK_LCDIF1_PRE_SEL>,
+							  <&clks IMX6SX_CLK_LCDIF1_SEL>;
+					assigned-clock-parents = <&clks IMX6SX_CLK_PLL5_VIDEO_DIV>,
+								 <&clks IMX6SX_CLK_LCDIF1_PODF>;
 					power-domains = <&pd_disp>;
 					status = "disabled";
 
-- 
2.34.1

[PATCH 3/3] clk: imx: imx6sx: Remove hardcoded LCDIF1 parent

[Fabio Estevam]

From: Fabio Estevam <festevam@denx.de>

It is not a good idea to hardcode the LCDIF1 parent inside the
clock driver because some users may want to use a different clock
parent for LCDIF1. One of the reasons could be related to EMI tests.

Remove the harcoded LCDIF1 parent as this is better described via
devicetree.

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 drivers/clk/imx/clk-imx6sx.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/clk/imx/clk-imx6sx.c b/drivers/clk/imx/clk-imx6sx.c
index 3f1502933e59..b110258b9036 100644
--- a/drivers/clk/imx/clk-imx6sx.c
+++ b/drivers/clk/imx/clk-imx6sx.c
@@ -498,10 +498,6 @@ static void __init imx6sx_clocks_init(struct device_node *ccm_node)
 	clk_set_parent(hws[IMX6SX_CLK_EIM_SLOW_SEL]->clk, hws[IMX6SX_CLK_PLL2_PFD2]->clk);
 	clk_set_rate(hws[IMX6SX_CLK_EIM_SLOW]->clk, 132000000);
 
-	/* set parent clock for LCDIF1 pixel clock */
-	clk_set_parent(hws[IMX6SX_CLK_LCDIF1_PRE_SEL]->clk, hws[IMX6SX_CLK_PLL5_VIDEO_DIV]->clk);
-	clk_set_parent(hws[IMX6SX_CLK_LCDIF1_SEL]->clk, hws[IMX6SX_CLK_LCDIF1_PODF]->clk);
-
 	/* Set the parent clks of PCIe lvds1 and pcie_axi to be pcie ref, axi */
 	if (clk_set_parent(hws[IMX6SX_CLK_LVDS1_SEL]->clk, hws[IMX6SX_CLK_PCIE_REF_125M]->clk))
 		pr_err("Failed to set pcie bus parent clk.\n");
-- 
2.34.1

Re: [PATCH 3/3] clk: imx: imx6sx: Remove hardcoded LCDIF1 parent

[Stephen Boyd]

Quoting Fabio Estevam (2023-06-02 15:16:04)
> From: Fabio Estevam <festevam@denx.de>
> 
> It is not a good idea to hardcode the LCDIF1 parent inside the
> clock driver because some users may want to use a different clock
> parent for LCDIF1. One of the reasons could be related to EMI tests.
> 
> Remove the harcoded LCDIF1 parent as this is better described via
> devicetree.
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---

Do you need to check for the DT property and skip these clk_set_parent()
calls if they're present? How does this work if the DT isn't updated at
the same time?

Re: [PATCH 3/3] clk: imx: imx6sx: Remove hardcoded LCDIF1 parent

[Fabio Estevam]

Hi Stephen,

On Thu, Jun 8, 2023 at 9:24 PM Stephen Boyd <sboyd@kernel.org> wrote:

> Do you need to check for the DT property and skip these clk_set_parent()
> calls if they're present? How does this work if the DT isn't updated at
> the same time?

Good point. I've just sent v2 with the DT property check as suggested.

Thanks