diff mbox series

[2/3] clk: imx6q: optionally get CCM inputs via standard clock handles

Message ID 20180731102009.6710-2-l.stach@pengutronix.de (mailing list archive)
State Changes Requested, archived
Headers show
Series [1/3] clk: imx6q: reset exclusive gates on init | expand

Commit Message

Lucas Stach July 31, 2018, 10:20 a.m. UTC
When specifying external clock inputs to the CCM the current code
requires the clocks to be in a "clocks" child node of the DT root.
This is not really conformant with DT best practices.

To avoid the need to deviate from those best practices, allow the
clock inputs to be specifies via standard clock handles. This is
in line with how drivers of the later CCM driver revisions on
newer i.MX SoCs handle this.

As we can't retroactively change the DT binding, allow this as an
option with a fallback to the old way of how this has been handled.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../devicetree/bindings/clock/imx6q-clock.txt |  5 +++++
 drivers/clk/imx/clk-imx6q.c                   | 22 ++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

Comments

Rob Herring (Arm) Aug. 14, 2018, 4:08 p.m. UTC | #1
On Tue, Jul 31, 2018 at 12:20:08PM +0200, Lucas Stach wrote:
> When specifying external clock inputs to the CCM the current code
> requires the clocks to be in a "clocks" child node of the DT root.
> This is not really conformant with DT best practices.
> 
> To avoid the need to deviate from those best practices, allow the
> clock inputs to be specifies via standard clock handles. This is
> in line with how drivers of the later CCM driver revisions on
> newer i.MX SoCs handle this.
> 
> As we can't retroactively change the DT binding, allow this as an
> option with a fallback to the old way of how this has been handled.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  .../devicetree/bindings/clock/imx6q-clock.txt |  5 +++++

Ignoring what my bot said if there's no other changes needed,

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/clk/imx/clk-imx6q.c                   | 22 ++++++++++++++-----
>  2 files changed, 22 insertions(+), 5 deletions(-)
Aisheng Dong Aug. 21, 2018, 8:56 a.m. UTC | #2
Hi Lucas,

> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: Tuesday, July 31, 2018 6:20 PM

[...]

> When specifying external clock inputs to the CCM the current code requires
> the clocks to be in a "clocks" child node of the DT root.
> This is not really conformant with DT best practices.
> 
> To avoid the need to deviate from those best practices, allow the clock inputs
> to be specifies via standard clock handles. This is in line with how drivers of
> the later CCM driver revisions on newer i.MX SoCs handle this.
> 
> As we can't retroactively change the DT binding, allow this as an option with a
> fallback to the old way of how this has been handled.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  .../devicetree/bindings/clock/imx6q-clock.txt |  5 +++++
>  drivers/clk/imx/clk-imx6q.c                   | 22 ++++++++++++++-----
>  2 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> index a45ca67a9d5f..ce6aa9920c05 100644
> --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> @@ -6,6 +6,11 @@ Required properties:
>  - interrupts: Should contain CCM interrupt
>  - #clock-cells: Should be <1>
> 
> +Optional properties:
> +- clocks: list of clock specifiers, must contain an entry for each entry
> +          in clock-names
> +- clock-names: valid names are "osc", "ckil", "ckil", "anaclk1" and "anaclk2"

A minor issue: 
The third name should be "ckih1".

Otherwise:
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Dong Aisheng
Stephen Boyd Oct. 15, 2018, 5:01 p.m. UTC | #3
Quoting A.s. Dong (2018-08-21 01:56:09)
> Hi Lucas,
> 
> > -----Original Message-----
> > From: Lucas Stach <l.stach@pengutronix.de>
> > Sent: Tuesday, July 31, 2018 6:20 PM
> 
> [...]
> 
> > When specifying external clock inputs to the CCM the current code requires
> > the clocks to be in a "clocks" child node of the DT root.
> > This is not really conformant with DT best practices.
> > 
> > To avoid the need to deviate from those best practices, allow the clock inputs
> > to be specifies via standard clock handles. This is in line with how drivers of
> > the later CCM driver revisions on newer i.MX SoCs handle this.
> > 
> > As we can't retroactively change the DT binding, allow this as an option with a
> > fallback to the old way of how this has been handled.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  .../devicetree/bindings/clock/imx6q-clock.txt |  5 +++++
> >  drivers/clk/imx/clk-imx6q.c                   | 22 ++++++++++++++-----
> >  2 files changed, 22 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > index a45ca67a9d5f..ce6aa9920c05 100644
> > --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > @@ -6,6 +6,11 @@ Required properties:
> >  - interrupts: Should contain CCM interrupt
> >  - #clock-cells: Should be <1>
> > 
> > +Optional properties:
> > +- clocks: list of clock specifiers, must contain an entry for each entry
> > +          in clock-names
> > +- clock-names: valid names are "osc", "ckil", "ckil", "anaclk1" and "anaclk2"
> 
> A minor issue: 
> The third name should be "ckih1".
> 
> Otherwise:
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> 

Is this going to be resent? I don't see anything new and there are
review comments on this patch series so I'm dropping it from the clk
review queue. Please resend.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
index a45ca67a9d5f..ce6aa9920c05 100644
--- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
+++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
@@ -6,6 +6,11 @@  Required properties:
 - interrupts: Should contain CCM interrupt
 - #clock-cells: Should be <1>
 
+Optional properties:
+- clocks: list of clock specifiers, must contain an entry for each entry
+          in clock-names
+- clock-names: valid names are "osc", "ckil", "ckil", "anaclk1" and "anaclk2"
+
 The clock consumer should specify the desired clock by having the clock
 ID in its "clocks" phandle cell.  See include/dt-bindings/clock/imx6qdl-clock.h
 for the full list of i.MX6 Quad and DualLite clock IDs.
diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index 9059a369ae95..f64f0fe76658 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -421,12 +421,24 @@  static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	int ret;
 
 	clk[IMX6QDL_CLK_DUMMY] = imx_clk_fixed("dummy", 0);
-	clk[IMX6QDL_CLK_CKIL] = imx_obtain_fixed_clock("ckil", 0);
-	clk[IMX6QDL_CLK_CKIH] = imx_obtain_fixed_clock("ckih1", 0);
-	clk[IMX6QDL_CLK_OSC] = imx_obtain_fixed_clock("osc", 0);
+	clk[IMX6QDL_CLK_CKIL] = of_clk_get_by_name(ccm_node, "ckil");
+	if (IS_ERR(clk[IMX6QDL_CLK_CKIL]))
+		clk[IMX6QDL_CLK_CKIL] = imx_obtain_fixed_clock("ckil", 0);
+	clk[IMX6QDL_CLK_CKIH] = of_clk_get_by_name(ccm_node, "ckih1");
+	if (IS_ERR(clk[IMX6QDL_CLK_CKIH]))
+		clk[IMX6QDL_CLK_CKIH] = imx_obtain_fixed_clock("ckih1", 0);
+	clk[IMX6QDL_CLK_OSC] = of_clk_get_by_name(ccm_node, "osc");
+	if (IS_ERR(clk[IMX6QDL_CLK_OSC]))
+		clk[IMX6QDL_CLK_OSC] = imx_obtain_fixed_clock("osc", 0);
+
 	/* Clock source from external clock via CLK1/2 PADs */
-	clk[IMX6QDL_CLK_ANACLK1] = imx_obtain_fixed_clock("anaclk1", 0);
-	clk[IMX6QDL_CLK_ANACLK2] = imx_obtain_fixed_clock("anaclk2", 0);
+	clk[IMX6QDL_CLK_ANACLK1] = of_clk_get_by_name(ccm_node, "anaclk1");
+	if (IS_ERR(clk[IMX6QDL_CLK_ANACLK1]))
+		clk[IMX6QDL_CLK_ANACLK1] = imx_obtain_fixed_clock("anaclk1", 0);
+
+	clk[IMX6QDL_CLK_ANACLK2] = of_clk_get_by_name(ccm_node, "anaclk2");
+	if (IS_ERR(clk[IMX6QDL_CLK_ANACLK2]))
+		clk[IMX6QDL_CLK_ANACLK2] = imx_obtain_fixed_clock("anaclk2", 0);
 
 	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-anatop");
 	anatop_base = base = of_iomap(np, 0);