diff mbox series

[v4] clk: imx: imx6sx: Allow a different LCDIF1 clock parent

Message ID 20230815130923.775117-1-festevam@gmail.com (mailing list archive)
State Awaiting Upstream, archived
Headers show
Series [v4] clk: imx: imx6sx: Allow a different LCDIF1 clock parent | expand

Commit Message

Fabio Estevam Aug. 15, 2023, 1:09 p.m. UTC 
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 when the LCDIF1 parent is described
via devicetree.

Old dtb's that do not describe the LCDIF1 parent via devicetree will
use the same PLL5 clock as parent to keep the original behavior.

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v3:
- Check for the presence of 'assigned-clock-parents'. (Stephen)

 drivers/clk/imx/clk-imx6sx.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Fabio Estevam Oct. 4, 2023, 11:33 a.m. UTC | #1 
Hi Abel,

Gentle ping.

On Tue, Aug 15, 2023 at 10:09 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> 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 when the LCDIF1 parent is described
> via devicetree.
>
> Old dtb's that do not describe the LCDIF1 parent via devicetree will
> use the same PLL5 clock as parent to keep the original behavior.
>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
> Changes since v3:
> - Check for the presence of 'assigned-clock-parents'. (Stephen)
>
>  drivers/clk/imx/clk-imx6sx.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx6sx.c b/drivers/clk/imx/clk-imx6sx.c
> index 3f1502933e59..69f8f6f9ca49 100644
> --- a/drivers/clk/imx/clk-imx6sx.c
> +++ b/drivers/clk/imx/clk-imx6sx.c
> @@ -121,6 +121,7 @@ static void __init imx6sx_clocks_init(struct device_node *ccm_node)
>  {
>         struct device_node *np;
>         void __iomem *base;
> +       bool lcdif1_assigned_clk;
>
>         clk_hw_data = kzalloc(struct_size(clk_hw_data, hws,
>                                           IMX6SX_CLK_CLK_END), GFP_KERNEL);
> @@ -498,9 +499,16 @@ 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);
> +       np = of_find_node_by_path("/soc/bus@2200000/spba-bus@2240000/lcdif@2220000");
> +       lcdif1_assigned_clk = of_find_property(np, "assigned-clock-parents", NULL);
> +
> +       /* Set parent clock for LCDIF1 pixel clock if not done via devicetree */
> +       if (!lcdif1_assigned_clk) {
> +               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))
> --
> 2.34.1
>
Abel Vesa Oct. 4, 2023, 12:25 p.m. UTC | #2 
On 23-08-15 10:09:23, Fabio Estevam wrote:
> 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 when the LCDIF1 parent is described
> via devicetree.
> 
> Old dtb's that do not describe the LCDIF1 parent via devicetree will
> use the same PLL5 clock as parent to keep the original behavior.
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>

Sorry, this hasn't made it to my clk-imx inbox due to linux-imx ML not
being CC'ed. My clk-imx inbox filters on my kernel.org email, the linux-clk
ML and the linux-imx ML.

Please use get_maintainer.pl script since that will also point to Peng Fan
who is a reviewer.

Patch looks good to me.

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

> ---
> Changes since v3:
> - Check for the presence of 'assigned-clock-parents'. (Stephen)
> 
>  drivers/clk/imx/clk-imx6sx.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx6sx.c b/drivers/clk/imx/clk-imx6sx.c
> index 3f1502933e59..69f8f6f9ca49 100644
> --- a/drivers/clk/imx/clk-imx6sx.c
> +++ b/drivers/clk/imx/clk-imx6sx.c
> @@ -121,6 +121,7 @@ static void __init imx6sx_clocks_init(struct device_node *ccm_node)
>  {
>  	struct device_node *np;
>  	void __iomem *base;
> +	bool lcdif1_assigned_clk;
>  
>  	clk_hw_data = kzalloc(struct_size(clk_hw_data, hws,
>  					  IMX6SX_CLK_CLK_END), GFP_KERNEL);
> @@ -498,9 +499,16 @@ 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);
> +	np = of_find_node_by_path("/soc/bus@2200000/spba-bus@2240000/lcdif@2220000");
> +	lcdif1_assigned_clk = of_find_property(np, "assigned-clock-parents", NULL);
> +
> +	/* Set parent clock for LCDIF1 pixel clock if not done via devicetree */
> +	if (!lcdif1_assigned_clk) {
> +		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))
> -- 
> 2.34.1
> 
>
Abel Vesa Oct. 4, 2023, 12:36 p.m. UTC | #3 
On Tue, 15 Aug 2023 10:09:23 -0300, Fabio Estevam wrote:
> 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 when the LCDIF1 parent is described
> via devicetree.
> 
> [...]

Applied, thanks!

[1/1] clk: imx: imx6sx: Allow a different LCDIF1 clock parent
      commit: 0a22b3a6f446223aff5bcdcc06003ef6e412bfd8

Best regards,
diff mbox series

Patch

diff --git a/drivers/clk/imx/clk-imx6sx.c b/drivers/clk/imx/clk-imx6sx.c
index 3f1502933e59..69f8f6f9ca49 100644
--- a/drivers/clk/imx/clk-imx6sx.c
+++ b/drivers/clk/imx/clk-imx6sx.c
@@ -121,6 +121,7 @@  static void __init imx6sx_clocks_init(struct device_node *ccm_node)
 {
 	struct device_node *np;
 	void __iomem *base;
+	bool lcdif1_assigned_clk;
 
 	clk_hw_data = kzalloc(struct_size(clk_hw_data, hws,
 					  IMX6SX_CLK_CLK_END), GFP_KERNEL);
@@ -498,9 +499,16 @@  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);
+	np = of_find_node_by_path("/soc/bus@2200000/spba-bus@2240000/lcdif@2220000");
+	lcdif1_assigned_clk = of_find_property(np, "assigned-clock-parents", NULL);
+
+	/* Set parent clock for LCDIF1 pixel clock if not done via devicetree */
+	if (!lcdif1_assigned_clk) {
+		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))