diff mbox series

[V3,4/5] dt-bindings: imx8-clock: add a53 and a72 clock id

Message ID 1548335800-6438-5-git-send-email-aisheng.dong@nxp.com (mailing list archive)
State New, archived
Headers show
Series dt-binding: clk: prepare for imx8qm clock support | expand

Commit Message

Dong Aisheng Jan. 24, 2019, 1:22 p.m. UTC
Add a53 and a72 clock id, as there's still no users, we update
IMX_LSIO_MEM_CLK base to start from 6 to allow a53 and a72 clock
id to be continued with a35 clk.

Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
v1->v2:
 * change cpu clock to cpu cluster clock per Rob's suggestion
---
 include/dt-bindings/clock/imx8-clock.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Rob Herring Feb. 13, 2019, 10:18 p.m. UTC | #1
On Thu, Jan 24, 2019 at 01:22:45PM +0000, Aisheng Dong wrote:
> Add a53 and a72 clock id, as there's still no users, we update
> IMX_LSIO_MEM_CLK base to start from 6 to allow a53 and a72 clock
> id to be continued with a35 clk.
> 
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
> v1->v2:
>  * change cpu clock to cpu cluster clock per Rob's suggestion
> ---
>  include/dt-bindings/clock/imx8-clock.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/dt-bindings/clock/imx8-clock.h b/include/dt-bindings/clock/imx8-clock.h
> index b149e63..dcce744 100644
> --- a/include/dt-bindings/clock/imx8-clock.h
> +++ b/include/dt-bindings/clock/imx8-clock.h
> @@ -14,10 +14,12 @@
>  /* CPU */
>  #define IMX_A35_CLK					1
>  #define IMX_CPU_CLUSTER_A35_CLK				1
> +#define IMX_CPU_CLUSTER_A53_CLK				2
> +#define IMX_CPU_CLUSTER_A72_CLK				3

I still don't get this. How many clock outputs does the clock controller 
have for CPUs? If 3, then this is correct. If it's the same clock 
controller bits across different SoCs, then just name it something like 
IMX_CPU_CLUSTER_CLK and reuse the same ID.

>  /* LSIO SS */
> -#define IMX_LSIO_MEM_CLK				2
> -#define IMX_LSIO_BUS_CLK				3
> +#define IMX_LSIO_MEM_CLK				6
> +#define IMX_LSIO_BUS_CLK				7

Changing numbering is not good, but I guess it's early for imx8.

>  #define IMX_LSIO_PWM0_CLK				10
>  #define IMX_LSIO_PWM1_CLK				11
>  #define IMX_LSIO_PWM2_CLK				12
> -- 
> 2.7.4
>
Dong Aisheng Feb. 20, 2019, 7:07 a.m. UTC | #2
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Thursday, February 14, 2019 6:18 AM
> On Thu, Jan 24, 2019 at 01:22:45PM +0000, Aisheng Dong wrote:
> > Add a53 and a72 clock id, as there's still no users, we update
> > IMX_LSIO_MEM_CLK base to start from 6 to allow a53 and a72 clock id to
> > be continued with a35 clk.
> >
> > Cc: Stephen Boyd <sboyd@kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: Michael Turquette <mturquette@baylibre.com>
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> > v1->v2:
> >  * change cpu clock to cpu cluster clock per Rob's suggestion
> > ---
> >  include/dt-bindings/clock/imx8-clock.h | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/dt-bindings/clock/imx8-clock.h
> > b/include/dt-bindings/clock/imx8-clock.h
> > index b149e63..dcce744 100644
> > --- a/include/dt-bindings/clock/imx8-clock.h
> > +++ b/include/dt-bindings/clock/imx8-clock.h
> > @@ -14,10 +14,12 @@
> >  /* CPU */
> >  #define IMX_A35_CLK					1
> >  #define IMX_CPU_CLUSTER_A35_CLK				1
> > +#define IMX_CPU_CLUSTER_A53_CLK				2
> > +#define IMX_CPU_CLUSTER_A72_CLK				3
> 
> I still don't get this. How many clock outputs does the clock controller have for
> CPUs? If 3, then this is correct. If it's the same clock controller bits across
> different SoCs, then just name it something like IMX_CPU_CLUSTER_CLK and
> reuse the same ID.
> 

For SCU firmware based platforms like mx8qxp/qm, the clocks are provided
by SCU firmware via SCU firmware call with unique IDs. So it's safe to use a common
Clock IDs file.

But please ignore patch 3 and 4 first because we still met a few limitations with
current approach due to the device availability may vary a bit across CPUs and
Subsystems.

We formerly planned to add all new IDs for each SS and dynamically check availability
in driver. That can be done but that may involve a lot effort and may result in more
changes In driver. Also hard to upstream device tree code due to dependency on Clock IDs.

To relief this situation, we want to move the clock definition into device tree which
can fully decouple the dependency of Clock ID definition from device tree.
And no frequent changes required in clock driver.

I will send a patch set to do it later.

Regards
Dong Aisheng

> >  /* LSIO SS */
> > -#define IMX_LSIO_MEM_CLK				2
> > -#define IMX_LSIO_BUS_CLK				3
> > +#define IMX_LSIO_MEM_CLK				6
> > +#define IMX_LSIO_BUS_CLK				7
> 
> Changing numbering is not good, but I guess it's early for imx8.
> 
> >  #define IMX_LSIO_PWM0_CLK				10
> >  #define IMX_LSIO_PWM1_CLK				11
> >  #define IMX_LSIO_PWM2_CLK				12
> > --
> > 2.7.4
> >
Stephen Boyd Feb. 20, 2019, 8:56 p.m. UTC | #3
Quoting Aisheng Dong (2019-02-19 23:07:46)
> > From: Rob Herring [mailto:robh@kernel.org]
> > Sent: Thursday, February 14, 2019 6:18 AM
> > On Thu, Jan 24, 2019 at 01:22:45PM +0000, Aisheng Dong wrote:
> > > Add a53 and a72 clock id, as there's still no users, we update
> > > IMX_LSIO_MEM_CLK base to start from 6 to allow a53 and a72 clock id to
> > > be continued with a35 clk.
> > >
> > > Cc: Stephen Boyd <sboyd@kernel.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: devicetree@vger.kernel.org
> > > Cc: Shawn Guo <shawnguo@kernel.org>
> > > Cc: Sascha Hauer <kernel@pengutronix.de>
> > > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > > Cc: Michael Turquette <mturquette@baylibre.com>
> > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > ---
> > > v1->v2:
> > >  * change cpu clock to cpu cluster clock per Rob's suggestion
> > > ---
> > >  include/dt-bindings/clock/imx8-clock.h | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/dt-bindings/clock/imx8-clock.h
> > > b/include/dt-bindings/clock/imx8-clock.h
> > > index b149e63..dcce744 100644
> > > --- a/include/dt-bindings/clock/imx8-clock.h
> > > +++ b/include/dt-bindings/clock/imx8-clock.h
> > > @@ -14,10 +14,12 @@
> > >  /* CPU */
> > >  #define IMX_A35_CLK                                        1
> > >  #define IMX_CPU_CLUSTER_A35_CLK                            1
> > > +#define IMX_CPU_CLUSTER_A53_CLK                            2
> > > +#define IMX_CPU_CLUSTER_A72_CLK                            3
> > 
> > I still don't get this. How many clock outputs does the clock controller have for
> > CPUs? If 3, then this is correct. If it's the same clock controller bits across
> > different SoCs, then just name it something like IMX_CPU_CLUSTER_CLK and
> > reuse the same ID.
> > 
> 
> For SCU firmware based platforms like mx8qxp/qm, the clocks are provided
> by SCU firmware via SCU firmware call with unique IDs. So it's safe to use a common
> Clock IDs file.
> 
> But please ignore patch 3 and 4 first because we still met a few limitations with
> current approach due to the device availability may vary a bit across CPUs and
> Subsystems.
> 
> We formerly planned to add all new IDs for each SS and dynamically check availability
> in driver. That can be done but that may involve a lot effort and may result in more
> changes In driver. Also hard to upstream device tree code due to dependency on Clock IDs.
> 
> To relief this situation, we want to move the clock definition into device tree which
> can fully decouple the dependency of Clock ID definition from device tree.
> And no frequent changes required in clock driver.
> 
> I will send a patch set to do it later.
> 

Ok. I'll take this as a signal to drop these from the clk review queue.
Thanks.
diff mbox series

Patch

diff --git a/include/dt-bindings/clock/imx8-clock.h b/include/dt-bindings/clock/imx8-clock.h
index b149e63..dcce744 100644
--- a/include/dt-bindings/clock/imx8-clock.h
+++ b/include/dt-bindings/clock/imx8-clock.h
@@ -14,10 +14,12 @@ 
 /* CPU */
 #define IMX_A35_CLK					1
 #define IMX_CPU_CLUSTER_A35_CLK				1
+#define IMX_CPU_CLUSTER_A53_CLK				2
+#define IMX_CPU_CLUSTER_A72_CLK				3
 
 /* LSIO SS */
-#define IMX_LSIO_MEM_CLK				2
-#define IMX_LSIO_BUS_CLK				3
+#define IMX_LSIO_MEM_CLK				6
+#define IMX_LSIO_BUS_CLK				7
 #define IMX_LSIO_PWM0_CLK				10
 #define IMX_LSIO_PWM1_CLK				11
 #define IMX_LSIO_PWM2_CLK				12