[PATCH V6 00/12] clk: imx8: add new clock binding for better pm support

[PATCH V6 00/12] clk: imx8: add new clock binding for better pm support

[Dong Aisheng]

This patch series is a preparation for the MX8 Architecture improvement.
As for IMX SCU based platforms like MX8QM and MX8QXP, they are comprised
of a couple of SS(Subsystems) while most of them within the same SS
can be shared. e.g. Clocks, Devices and etc.

However, current clock binding is using SW IDs for device tree to use
which can cause troubles in writing the common <soc>-ss-xx.dtsi file for
different SoCs.

This patch series aims to introduce a new binding which is more close to
hardware and platform independent and can makes us write a more general
drivers for different SCU based SoCs.

Another important thing is that on MX8, each Clock resource is associated
with a power domain. So we have to attach that clock device to the power
domain in order to make it work properly. Further more, the clock state
will be lost when its power domain is completely off during suspend/resume,
so we also introduce the clock state save&restore mechanism.

It's based on latest shanw/for-next branch.

The top commit is:
4559a11e48d5 Merge branch 'imx/defconfig' into for-next

ChangeLog:
v5->v6:
 * add scu clk unregister if add provider failed
v4->v5:
 * Address all comments from Stephen
v3->v4:
 * use clk-indices for LPCG to fetch each clks offset from dt
v2->v3:
 * change scu clk into two cells binding
 * add clk pm patches to ease the understand of the changes
v1->v2:
 * SCU clock changed to one cell clock binding inspired by arm,scpi.txt
   Documentation/devicetree/bindings/arm/arm,scpi.txt
 * Add required power domain property
 * Dropped PATCH 3&4 first, will send the updated version accordingly
   after the binding is finally determined,


Dong Aisheng (12):
  dt-bindings: firmware: imx-scu: new binding to parse clocks from
    device tree
  dt-bindings: clock: imx-lpcg: add support to parse clocks from device
    tree
  clk: imx: scu: add two cells binding support
  clk: imx: scu: bypass cpu power domains
  clk: imx: scu: allow scu clk to take device pointer
  clk: imx: scu: add runtime pm support
  clk: imx: scu: add suspend/resume support
  clk: imx: imx8qxp-lpcg: add parsing clocks from device tree
  clk: imx: lpcg: allow lpcg clk to take device pointer
  clk: imx: clk-imx8qxp-lpcg: add runtime pm support
  clk: imx: lpcg: add suspend/resume support
  clk: imx: scu: unregister clocks if add provider failed

 .../bindings/arm/freescale/fsl,scu.txt        |  12 +-
 .../bindings/clock/imx8qxp-lpcg.txt           |  36 ++-
 drivers/clk/imx/clk-imx8qxp-lpcg.c            | 139 +++++++++++
 drivers/clk/imx/clk-imx8qxp.c                 | 136 ++++++-----
 drivers/clk/imx/clk-lpcg-scu.c                |  53 +++-
 drivers/clk/imx/clk-scu.c                     | 231 +++++++++++++++++-
 drivers/clk/imx/clk-scu.h                     |  56 ++++-
 include/dt-bindings/clock/imx8-lpcg.h         |  14 ++
 include/dt-bindings/firmware/imx/rsrc.h       |  23 ++
 9 files changed, 610 insertions(+), 90 deletions(-)
 create mode 100644 include/dt-bindings/clock/imx8-lpcg.h

-- 
2.23.0

[PATCH V6 01/12] dt-bindings: firmware: imx-scu: new binding to parse clocks from device tree

[Dong Aisheng]

There's a few limitations on the original one cell clock binding
(#clock-cells = <1>) that we have to define some SW clock IDs for device
tree to reference. This may cause troubles if we want to use common
clock IDs for multi platforms support when the clock of those platforms
are mostly the same.
e.g. Current clock IDs name are defined with SS prefix.

However the device may reside in different SS across CPUs, that means the
SS prefix may not valid anymore for a new SoC. Furthermore, the device
availability of those clocks may also vary a bit.

For such situation, we want to eliminate the using of SW Clock IDs and
change to use a more close to HW one instead.
For SCU clocks usage, only two params required: Resource id + Clock Type.
Both parameters are platform independent. So we could use two cells binding
to pass those parameters,

Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: devicetree@vger.kernel.org
Acked-by: Shawn Guo <shawnguo@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v4->v5:
 * no changes
v3->v4:
 * add some comments for various clock types
v2->v3:
 * Changed to two cells binding and register all clocks in driver
   instead of parse from device tree.
v1->v2:
 * changed to one cell binding inspired by arm,scpi.txt
   Documentation/devicetree/bindings/arm/arm,scpi.txt
   Resource ID is encoded in 'reg' property.
   Clock type is encoded in generic clock-indices property.
   Then we don't have to search all the DT nodes to fetch
   those two value to construct clocks which is relatively
   low efficiency.
 * Add required power-domain property as well.
---
 .../bindings/arm/freescale/fsl,scu.txt        | 12 ++++++----
 include/dt-bindings/firmware/imx/rsrc.h       | 23 +++++++++++++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
index e07735a8c2c7..ee061befbb8e 100644
--- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
+++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
@@ -89,7 +89,10 @@ Required properties:
 			  "fsl,imx8qm-clock"
 			  "fsl,imx8qxp-clock"
 			followed by "fsl,scu-clk"
-- #clock-cells:		Should be 1. Contains the Clock ID value.
+- #clock-cells:		Should be either
+			2: Contains the Resource and Clock ID value.
+			or
+			1: Contains the Clock ID value. (DEPRECATED)
 - clocks:		List of clock specifiers, must contain an entry for
 			each required entry in clock-names
 - clock-names:		Should include entries "xtal_32KHz", "xtal_24MHz"
@@ -195,7 +198,7 @@ firmware {
 
 		clk: clk {
 			compatible = "fsl,imx8qxp-clk", "fsl,scu-clk";
-			#clock-cells = <1>;
+			#clock-cells = <2>;
 		};
 
 		iomuxc {
@@ -245,8 +248,7 @@ serial@5a060000 {
 	...
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_lpuart0>;
-	clocks = <&clk IMX8QXP_UART0_CLK>,
-		 <&clk IMX8QXP_UART0_IPG_CLK>;
-	clock-names = "per", "ipg";
+	clocks = <&uart0_clk IMX_SC_R_UART_0 IMX_SC_PM_CLK_PER>;
+	clock-names = "ipg";
 	power-domains = <&pd IMX_SC_R_UART_0>;
 };
diff --git a/include/dt-bindings/firmware/imx/rsrc.h b/include/dt-bindings/firmware/imx/rsrc.h
index 4e61f6485097..24c153dfaa72 100644
--- a/include/dt-bindings/firmware/imx/rsrc.h
+++ b/include/dt-bindings/firmware/imx/rsrc.h
@@ -547,4 +547,27 @@
 #define IMX_SC_R_ATTESTATION		545
 #define IMX_SC_R_LAST			546
 
+/*
+ * Defines for SC PM CLK
+ */
+
+/* Normal device resource clock */
+#define IMX_SC_PM_CLK_SLV_BUS		0	/* Slave bus clock */
+#define IMX_SC_PM_CLK_MST_BUS		1	/* Master bus clock */
+#define IMX_SC_PM_CLK_PER		2	/* Peripheral clock */
+#define IMX_SC_PM_CLK_PHY		3	/* Phy clock */
+#define IMX_SC_PM_CLK_MISC		4	/* Misc clock */
+
+/* Special clock types which do not belong to above normal clock types */
+#define IMX_SC_PM_CLK_MISC0		0	/* Misc 0 clock */
+#define IMX_SC_PM_CLK_MISC1		1	/* Misc 1 clock */
+#define IMX_SC_PM_CLK_MISC2		2	/* Misc 2 clock */
+#define IMX_SC_PM_CLK_MISC3		3	/* Misc 3 clock */
+#define IMX_SC_PM_CLK_MISC4		4	/* Misc 4 clock */
+
+/* Special clock types for CPU/PLL/BYPASS only */
+#define IMX_SC_PM_CLK_CPU		2	/* CPU clock */
+#define IMX_SC_PM_CLK_PLL		4	/* PLL */
+#define IMX_SC_PM_CLK_BYPASS		4	/* Bypass clock */
+
 #endif /* __DT_BINDINGS_RSCRC_IMX_H */
-- 
2.23.0

[PATCH V6 02/12] dt-bindings: clock: imx-lpcg: add support to parse clocks from device tree

[Dong Aisheng]

MX8QM and MX8QXP LPCG Clocks are mostly the same except they may reside
in different subsystems across CPUs and also vary a bit on the availability.

Same as SCU clock, we want to move the clock definition into device tree
which can fully decouple the dependency of Clock ID definition from device
tree and make us be able to write a fully generic lpcg clock driver.

And we can also use the existence of clock nodes in device tree to address
the device and clock availability differences across different SoCs.

Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: devicetree@vger.kernel.org
Acked-by: Shawn Guo <shawnguo@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v4->v5:
 * no changes
v3->v4:
 * change bit-offset property to clock-indices
 * use constant macro to define clock indinces
 * drop hw-autogate property which is still not used by drivers
v2->v3:
 * no changes
v1->v2:
 * Update example
 * Add power domain property
---
 .../bindings/clock/imx8qxp-lpcg.txt           | 36 +++++++++++++++----
 include/dt-bindings/clock/imx8-lpcg.h         | 14 ++++++++
 2 files changed, 44 insertions(+), 6 deletions(-)
 create mode 100644 include/dt-bindings/clock/imx8-lpcg.h

diff --git a/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt b/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt
index 965cfa42e025..cad8fc4284aa 100644
--- a/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt
+++ b/Documentation/devicetree/bindings/clock/imx8qxp-lpcg.txt
@@ -11,6 +11,21 @@ enabled by these control bits, it might still not be running based
 on the base resource.
 
 Required properties:
+- compatible:		Should be one of:
+			  "fsl,imx8qxp-lpcg"
+			  "fsl,imx8qm-lpcg" followed by "fsl,imx8qxp-lpcg".
+- reg:			Address and length of the register set.
+- #clock-cells:		Should be 1. One LPCG supports multiple clocks.
+- clocks:		Input parent clocks phandle array for each clock.
+- clock-indices:	An integer array indicating the bit offset for each clock.
+			Refer to <include/dt-bindings/clock/imx8-lpcg.h> for the
+			supported LPCG clock indices.
+- clock-output-names:	Shall be the corresponding names of the outputs.
+			NOTE this property must be specified in the same order
+			as the clock-indices property.
+- power-domains:	Should contain the power domain used by this clock.
+
+Legacy binding (DEPRECATED):
 - compatible:	Should be one of:
 		  "fsl,imx8qxp-lpcg-adma",
 		  "fsl,imx8qxp-lpcg-conn",
@@ -33,10 +48,19 @@ Examples:
 
 #include <dt-bindings/clock/imx8qxp-clock.h>
 
-conn_lpcg: clock-controller@5b200000 {
-	compatible = "fsl,imx8qxp-lpcg-conn";
-	reg = <0x5b200000 0xb0000>;
+sdhc0_lpcg: clock-controller@5b200000 {
+	compatible = "fsl,imx8qxp-lpcg";
+	reg = <0x5b200000 0x10000>;
 	#clock-cells = <1>;
+	clocks = <&sdhc0_clk IMX_SC_PM_CLK_PER>,
+		 <&conn_ipg_clk>, <&conn_axi_clk>;
+	clock-indices = <IMX_LPCG_CLK_0>,
+			<IMX_LPCG_CLK_4>,
+			<IMX_LPCG_CLK_5>;
+	clock-output-names = "sdhc0_lpcg_per_clk",
+			     "sdhc0_lpcg_ipg_clk",
+			     "sdhc0_lpcg_ahb_clk";
+	power-domains = <&pd IMX_SC_R_SDHC_0>;
 };
 
 usdhc1: mmc@5b010000 {
@@ -44,8 +68,8 @@ usdhc1: mmc@5b010000 {
 	interrupt-parent = <&gic>;
 	interrupts = <GIC_SPI 232 IRQ_TYPE_LEVEL_HIGH>;
 	reg = <0x5b010000 0x10000>;
-	clocks = <&conn_lpcg IMX8QXP_CONN_LPCG_SDHC0_IPG_CLK>,
-		 <&conn_lpcg IMX8QXP_CONN_LPCG_SDHC0_PER_CLK>,
-		 <&conn_lpcg IMX8QXP_CONN_LPCG_SDHC0_HCLK>;
+	clocks = <&sdhc0_lpcg IMX_LPCG_CLK_4>,
+		 <&sdhc0_lpcg IMX_LPCG_CLK_0>,
+		 <&sdhc0_lpcg IMX_LPCG_CLK_5>;
 	clock-names = "ipg", "per", "ahb";
 };
diff --git a/include/dt-bindings/clock/imx8-lpcg.h b/include/dt-bindings/clock/imx8-lpcg.h
new file mode 100644
index 000000000000..df90aad26265
--- /dev/null
+++ b/include/dt-bindings/clock/imx8-lpcg.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2019 NXP
+ *   Dong Aisheng <aisheng.dong@nxp.com>
+ */
+
+#define IMX_LPCG_CLK_0	0
+#define IMX_LPCG_CLK_1	4
+#define IMX_LPCG_CLK_2	8
+#define IMX_LPCG_CLK_3	12
+#define IMX_LPCG_CLK_4	16
+#define IMX_LPCG_CLK_5	20
+#define IMX_LPCG_CLK_6	24
+#define IMX_LPCG_CLK_7	28
-- 
2.23.0

[PATCH V6 03/12] clk: imx: scu: add two cells binding support

[Dong Aisheng]

This patch implements the new two cells binding for SCU clocks.
The usage is as follows:
clocks = <&uart0_clk IMX_SC_R_UART_0 IMX_SC_PM_CLK_PER>

Due to each SCU clock is associated with a power domain, without power
on the domain, the SCU clock can't work. So we create platform devices
for each domain clock respectively and manually attach the required domain
before register the clock devices, then we can register clocks in the
clock platform driver accordingly.

Note because we do not have power domain info in device tree and the SCU
resource ID is the same for power domain and clock, so we use resource ID
to find power domains.

Later, we will also use this clock platform driver to support suspend/resume
and runtime pm.

Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v4->v5:
 * remove global clock_cells
 * put node after using of_find_compatible_node()
 * using ERR_PTR(ret) instead of ERR_PTR(-ENOMEM)
v4: no changes
v3: new patch
---
 drivers/clk/imx/clk-imx8qxp.c | 129 ++++++++++++++++---------------
 drivers/clk/imx/clk-scu.c     | 140 +++++++++++++++++++++++++++++++++-
 drivers/clk/imx/clk-scu.h     |  25 ++++--
 3 files changed, 227 insertions(+), 67 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
index 5e2903efc488..2ec3e0c4749d 100644
--- a/drivers/clk/imx/clk-imx8qxp.c
+++ b/drivers/clk/imx/clk-imx8qxp.c
@@ -22,9 +22,10 @@ static int imx8qxp_clk_probe(struct platform_device *pdev)
 	struct device_node *ccm_node = pdev->dev.of_node;
 	struct clk_hw_onecell_data *clk_data;
 	struct clk_hw **clks;
+	u32 clk_cells;
 	int ret, i;
 
-	ret = imx_clk_scu_init();
+	ret = imx_clk_scu_init(ccm_node);
 	if (ret)
 		return ret;
 
@@ -33,6 +34,9 @@ static int imx8qxp_clk_probe(struct platform_device *pdev)
 	if (!clk_data)
 		return -ENOMEM;
 
+	if (of_property_read_u32(ccm_node, "#clock-cells", &clk_cells))
+		return -EINVAL;
+
 	clk_data->num = IMX_SCU_CLK_END;
 	clks = clk_data->hws;
 
@@ -55,78 +59,78 @@ static int imx8qxp_clk_probe(struct platform_device *pdev)
 	clks[IMX_LSIO_BUS_CLK]		= clk_hw_register_fixed_rate(NULL, "lsio_bus_clk_root", NULL, 0, 100000000);
 
 	/* ARM core */
-	clks[IMX_A35_CLK]		= imx_clk_scu("a35_clk", IMX_SC_R_A35, IMX_SC_PM_CLK_CPU);
+	clks[IMX_A35_CLK]		= imx_clk_scu("a35_clk", IMX_SC_R_A35, IMX_SC_PM_CLK_CPU, clk_cells);
 
 	/* LSIO SS */
-	clks[IMX_LSIO_PWM0_CLK]		= imx_clk_scu("pwm0_clk", IMX_SC_R_PWM_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_PWM1_CLK]		= imx_clk_scu("pwm1_clk", IMX_SC_R_PWM_1, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_PWM2_CLK]		= imx_clk_scu("pwm2_clk", IMX_SC_R_PWM_2, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_PWM3_CLK]		= imx_clk_scu("pwm3_clk", IMX_SC_R_PWM_3, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_PWM4_CLK]		= imx_clk_scu("pwm4_clk", IMX_SC_R_PWM_4, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_PWM5_CLK]		= imx_clk_scu("pwm5_clk", IMX_SC_R_PWM_5, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_PWM6_CLK]		= imx_clk_scu("pwm6_clk", IMX_SC_R_PWM_6, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_PWM7_CLK]		= imx_clk_scu("pwm7_clk", IMX_SC_R_PWM_7, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_GPT0_CLK]		= imx_clk_scu("gpt0_clk", IMX_SC_R_GPT_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_GPT1_CLK]		= imx_clk_scu("gpt1_clk", IMX_SC_R_GPT_1, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_GPT2_CLK]		= imx_clk_scu("gpt2_clk", IMX_SC_R_GPT_2, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_GPT3_CLK]		= imx_clk_scu("gpt3_clk", IMX_SC_R_GPT_3, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_GPT4_CLK]		= imx_clk_scu("gpt4_clk", IMX_SC_R_GPT_4, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_FSPI0_CLK]	= imx_clk_scu("fspi0_clk", IMX_SC_R_FSPI_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_LSIO_FSPI1_CLK]	= imx_clk_scu("fspi1_clk", IMX_SC_R_FSPI_1, IMX_SC_PM_CLK_PER);
+	clks[IMX_LSIO_PWM0_CLK]		= imx_clk_scu("pwm0_clk", IMX_SC_R_PWM_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_PWM1_CLK]		= imx_clk_scu("pwm1_clk", IMX_SC_R_PWM_1, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_PWM2_CLK]		= imx_clk_scu("pwm2_clk", IMX_SC_R_PWM_2, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_PWM3_CLK]		= imx_clk_scu("pwm3_clk", IMX_SC_R_PWM_3, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_PWM4_CLK]		= imx_clk_scu("pwm4_clk", IMX_SC_R_PWM_4, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_PWM5_CLK]		= imx_clk_scu("pwm5_clk", IMX_SC_R_PWM_5, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_PWM6_CLK]		= imx_clk_scu("pwm6_clk", IMX_SC_R_PWM_6, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_PWM7_CLK]		= imx_clk_scu("pwm7_clk", IMX_SC_R_PWM_7, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_GPT0_CLK]		= imx_clk_scu("gpt0_clk", IMX_SC_R_GPT_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_GPT1_CLK]		= imx_clk_scu("gpt1_clk", IMX_SC_R_GPT_1, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_GPT2_CLK]		= imx_clk_scu("gpt2_clk", IMX_SC_R_GPT_2, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_GPT3_CLK]		= imx_clk_scu("gpt3_clk", IMX_SC_R_GPT_3, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_GPT4_CLK]		= imx_clk_scu("gpt4_clk", IMX_SC_R_GPT_4, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_FSPI0_CLK]	= imx_clk_scu("fspi0_clk", IMX_SC_R_FSPI_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_LSIO_FSPI1_CLK]	= imx_clk_scu("fspi1_clk", IMX_SC_R_FSPI_1, IMX_SC_PM_CLK_PER, clk_cells);
 
 	/* ADMA SS */
-	clks[IMX_ADMA_UART0_CLK]	= imx_clk_scu("uart0_clk", IMX_SC_R_UART_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_UART1_CLK]	= imx_clk_scu("uart1_clk", IMX_SC_R_UART_1, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_UART2_CLK]	= imx_clk_scu("uart2_clk", IMX_SC_R_UART_2, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_UART3_CLK]	= imx_clk_scu("uart3_clk", IMX_SC_R_UART_3, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_SPI0_CLK]		= imx_clk_scu("spi0_clk",  IMX_SC_R_SPI_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_SPI1_CLK]		= imx_clk_scu("spi1_clk",  IMX_SC_R_SPI_1, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_SPI2_CLK]		= imx_clk_scu("spi2_clk",  IMX_SC_R_SPI_2, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_SPI3_CLK]		= imx_clk_scu("spi3_clk",  IMX_SC_R_SPI_3, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_CAN0_CLK]		= imx_clk_scu("can0_clk",  IMX_SC_R_CAN_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_I2C0_CLK]		= imx_clk_scu("i2c0_clk",  IMX_SC_R_I2C_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_I2C1_CLK]		= imx_clk_scu("i2c1_clk",  IMX_SC_R_I2C_1, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_I2C2_CLK]		= imx_clk_scu("i2c2_clk",  IMX_SC_R_I2C_2, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_I2C3_CLK]		= imx_clk_scu("i2c3_clk",  IMX_SC_R_I2C_3, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_FTM0_CLK]		= imx_clk_scu("ftm0_clk",  IMX_SC_R_FTM_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_FTM1_CLK]		= imx_clk_scu("ftm1_clk",  IMX_SC_R_FTM_1, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_ADC0_CLK]		= imx_clk_scu("adc0_clk",  IMX_SC_R_ADC_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_PWM_CLK]		= imx_clk_scu("pwm_clk",   IMX_SC_R_LCD_0_PWM_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_ADMA_LCD_CLK]		= imx_clk_scu("lcd_clk",   IMX_SC_R_LCD_0, IMX_SC_PM_CLK_PER);
+	clks[IMX_ADMA_UART0_CLK]	= imx_clk_scu("uart0_clk", IMX_SC_R_UART_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_UART1_CLK]	= imx_clk_scu("uart1_clk", IMX_SC_R_UART_1, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_UART2_CLK]	= imx_clk_scu("uart2_clk", IMX_SC_R_UART_2, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_UART3_CLK]	= imx_clk_scu("uart3_clk", IMX_SC_R_UART_3, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_SPI0_CLK]		= imx_clk_scu("spi0_clk",  IMX_SC_R_SPI_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_SPI1_CLK]		= imx_clk_scu("spi1_clk",  IMX_SC_R_SPI_1, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_SPI2_CLK]		= imx_clk_scu("spi2_clk",  IMX_SC_R_SPI_2, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_SPI3_CLK]		= imx_clk_scu("spi3_clk",  IMX_SC_R_SPI_3, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_CAN0_CLK]		= imx_clk_scu("can0_clk",  IMX_SC_R_CAN_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_I2C0_CLK]		= imx_clk_scu("i2c0_clk",  IMX_SC_R_I2C_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_I2C1_CLK]		= imx_clk_scu("i2c1_clk",  IMX_SC_R_I2C_1, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_I2C2_CLK]		= imx_clk_scu("i2c2_clk",  IMX_SC_R_I2C_2, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_I2C3_CLK]		= imx_clk_scu("i2c3_clk",  IMX_SC_R_I2C_3, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_FTM0_CLK]		= imx_clk_scu("ftm0_clk",  IMX_SC_R_FTM_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_FTM1_CLK]		= imx_clk_scu("ftm1_clk",  IMX_SC_R_FTM_1, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_ADC0_CLK]		= imx_clk_scu("adc0_clk",  IMX_SC_R_ADC_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_PWM_CLK]		= imx_clk_scu("pwm_clk",   IMX_SC_R_LCD_0_PWM_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_ADMA_LCD_CLK]		= imx_clk_scu("lcd_clk",   IMX_SC_R_LCD_0, IMX_SC_PM_CLK_PER, clk_cells);
 
 	/* Connectivity */
-	clks[IMX_CONN_SDHC0_CLK]	= imx_clk_scu("sdhc0_clk", IMX_SC_R_SDHC_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_CONN_SDHC1_CLK]	= imx_clk_scu("sdhc1_clk", IMX_SC_R_SDHC_1, IMX_SC_PM_CLK_PER);
-	clks[IMX_CONN_SDHC2_CLK]	= imx_clk_scu("sdhc2_clk", IMX_SC_R_SDHC_2, IMX_SC_PM_CLK_PER);
-	clks[IMX_CONN_ENET0_ROOT_CLK]	= imx_clk_scu("enet0_clk", IMX_SC_R_ENET_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_CONN_ENET0_BYPASS_CLK]	= imx_clk_scu("enet0_bypass_clk", IMX_SC_R_ENET_0, IMX_SC_PM_CLK_BYPASS);
-	clks[IMX_CONN_ENET0_RGMII_CLK]	= imx_clk_scu("enet0_rgmii_clk", IMX_SC_R_ENET_0, IMX_SC_PM_CLK_MISC0);
-	clks[IMX_CONN_ENET1_ROOT_CLK]	= imx_clk_scu("enet1_clk", IMX_SC_R_ENET_1, IMX_SC_PM_CLK_PER);
-	clks[IMX_CONN_ENET1_BYPASS_CLK]	= imx_clk_scu("enet1_bypass_clk", IMX_SC_R_ENET_1, IMX_SC_PM_CLK_BYPASS);
-	clks[IMX_CONN_ENET1_RGMII_CLK]	= imx_clk_scu("enet1_rgmii_clk", IMX_SC_R_ENET_1, IMX_SC_PM_CLK_MISC0);
-	clks[IMX_CONN_GPMI_BCH_IO_CLK]	= imx_clk_scu("gpmi_io_clk", IMX_SC_R_NAND, IMX_SC_PM_CLK_MST_BUS);
-	clks[IMX_CONN_GPMI_BCH_CLK]	= imx_clk_scu("gpmi_bch_clk", IMX_SC_R_NAND, IMX_SC_PM_CLK_PER);
-	clks[IMX_CONN_USB2_ACLK]	= imx_clk_scu("usb3_aclk_div", IMX_SC_R_USB_2, IMX_SC_PM_CLK_PER);
-	clks[IMX_CONN_USB2_BUS_CLK]	= imx_clk_scu("usb3_bus_div", IMX_SC_R_USB_2, IMX_SC_PM_CLK_MST_BUS);
-	clks[IMX_CONN_USB2_LPM_CLK]	= imx_clk_scu("usb3_lpm_div", IMX_SC_R_USB_2, IMX_SC_PM_CLK_MISC);
+	clks[IMX_CONN_SDHC0_CLK]	= imx_clk_scu("sdhc0_clk", IMX_SC_R_SDHC_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_CONN_SDHC1_CLK]	= imx_clk_scu("sdhc1_clk", IMX_SC_R_SDHC_1, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_CONN_SDHC2_CLK]	= imx_clk_scu("sdhc2_clk", IMX_SC_R_SDHC_2, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_CONN_ENET0_ROOT_CLK]	= imx_clk_scu("enet0_clk", IMX_SC_R_ENET_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_CONN_ENET0_BYPASS_CLK]	= imx_clk_scu("enet0_bypass_clk", IMX_SC_R_ENET_0, IMX_SC_PM_CLK_BYPASS, clk_cells);
+	clks[IMX_CONN_ENET0_RGMII_CLK]	= imx_clk_scu("enet0_rgmii_clk", IMX_SC_R_ENET_0, IMX_SC_PM_CLK_MISC0, clk_cells);
+	clks[IMX_CONN_ENET1_ROOT_CLK]	= imx_clk_scu("enet1_clk", IMX_SC_R_ENET_1, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_CONN_ENET1_BYPASS_CLK]	= imx_clk_scu("enet1_bypass_clk", IMX_SC_R_ENET_1, IMX_SC_PM_CLK_BYPASS, clk_cells);
+	clks[IMX_CONN_ENET1_RGMII_CLK]	= imx_clk_scu("enet1_rgmii_clk", IMX_SC_R_ENET_1, IMX_SC_PM_CLK_MISC0, clk_cells);
+	clks[IMX_CONN_GPMI_BCH_IO_CLK]	= imx_clk_scu("gpmi_io_clk", IMX_SC_R_NAND, IMX_SC_PM_CLK_MST_BUS, clk_cells);
+	clks[IMX_CONN_GPMI_BCH_CLK]	= imx_clk_scu("gpmi_bch_clk", IMX_SC_R_NAND, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_CONN_USB2_ACLK]	= imx_clk_scu("usb3_aclk_div", IMX_SC_R_USB_2, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_CONN_USB2_BUS_CLK]	= imx_clk_scu("usb3_bus_div", IMX_SC_R_USB_2, IMX_SC_PM_CLK_MST_BUS, clk_cells);
+	clks[IMX_CONN_USB2_LPM_CLK]	= imx_clk_scu("usb3_lpm_div", IMX_SC_R_USB_2, IMX_SC_PM_CLK_MISC, clk_cells);
 
 	/* Display controller SS */
-	clks[IMX_DC0_DISP0_CLK]		= imx_clk_scu("dc0_disp0_clk", IMX_SC_R_DC_0, IMX_SC_PM_CLK_MISC0);
-	clks[IMX_DC0_DISP1_CLK]		= imx_clk_scu("dc0_disp1_clk", IMX_SC_R_DC_0, IMX_SC_PM_CLK_MISC1);
+	clks[IMX_DC0_DISP0_CLK]		= imx_clk_scu("dc0_disp0_clk", IMX_SC_R_DC_0, IMX_SC_PM_CLK_MISC0, clk_cells);
+	clks[IMX_DC0_DISP1_CLK]		= imx_clk_scu("dc0_disp1_clk", IMX_SC_R_DC_0, IMX_SC_PM_CLK_MISC1, clk_cells);
 
 	/* MIPI-LVDS SS */
-	clks[IMX_MIPI0_I2C0_CLK]	= imx_clk_scu("mipi0_i2c0_clk", IMX_SC_R_MIPI_0_I2C_0, IMX_SC_PM_CLK_MISC2);
-	clks[IMX_MIPI0_I2C1_CLK]	= imx_clk_scu("mipi0_i2c1_clk", IMX_SC_R_MIPI_0_I2C_1, IMX_SC_PM_CLK_MISC2);
+	clks[IMX_MIPI0_I2C0_CLK]	= imx_clk_scu("mipi0_i2c0_clk", IMX_SC_R_MIPI_0_I2C_0, IMX_SC_PM_CLK_MISC2, clk_cells);
+	clks[IMX_MIPI0_I2C1_CLK]	= imx_clk_scu("mipi0_i2c1_clk", IMX_SC_R_MIPI_0_I2C_1, IMX_SC_PM_CLK_MISC2, clk_cells);
 
 	/* MIPI CSI SS */
-	clks[IMX_CSI0_CORE_CLK]		= imx_clk_scu("mipi_csi0_core_clk", IMX_SC_R_CSI_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_CSI0_ESC_CLK]		= imx_clk_scu("mipi_csi0_esc_clk",  IMX_SC_R_CSI_0, IMX_SC_PM_CLK_MISC);
-	clks[IMX_CSI0_I2C0_CLK]		= imx_clk_scu("mipi_csi0_i2c0_clk", IMX_SC_R_CSI_0_I2C_0, IMX_SC_PM_CLK_PER);
-	clks[IMX_CSI0_PWM0_CLK]		= imx_clk_scu("mipi_csi0_pwm0_clk", IMX_SC_R_CSI_0_PWM_0, IMX_SC_PM_CLK_PER);
+	clks[IMX_CSI0_CORE_CLK]		= imx_clk_scu("mipi_csi0_core_clk", IMX_SC_R_CSI_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_CSI0_ESC_CLK]		= imx_clk_scu("mipi_csi0_esc_clk",  IMX_SC_R_CSI_0, IMX_SC_PM_CLK_MISC, clk_cells);
+	clks[IMX_CSI0_I2C0_CLK]		= imx_clk_scu("mipi_csi0_i2c0_clk", IMX_SC_R_CSI_0_I2C_0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_CSI0_PWM0_CLK]		= imx_clk_scu("mipi_csi0_pwm0_clk", IMX_SC_R_CSI_0_PWM_0, IMX_SC_PM_CLK_PER, clk_cells);
 
 	/* GPU SS */
-	clks[IMX_GPU0_CORE_CLK]		= imx_clk_scu("gpu_core0_clk",	 IMX_SC_R_GPU_0_PID0, IMX_SC_PM_CLK_PER);
-	clks[IMX_GPU0_SHADER_CLK]	= imx_clk_scu("gpu_shader0_clk", IMX_SC_R_GPU_0_PID0, IMX_SC_PM_CLK_MISC);
+	clks[IMX_GPU0_CORE_CLK]		= imx_clk_scu("gpu_core0_clk",	 IMX_SC_R_GPU_0_PID0, IMX_SC_PM_CLK_PER, clk_cells);
+	clks[IMX_GPU0_SHADER_CLK]	= imx_clk_scu("gpu_shader0_clk", IMX_SC_R_GPU_0_PID0, IMX_SC_PM_CLK_MISC, clk_cells);
 
 	for (i = 0; i < clk_data->num; i++) {
 		if (IS_ERR(clks[i]))
@@ -134,7 +138,12 @@ static int imx8qxp_clk_probe(struct platform_device *pdev)
 				i, PTR_ERR(clks[i]));
 	}
 
-	return of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data);
+	if (clk_cells == 2)
+		ret = of_clk_add_hw_provider(ccm_node, imx_scu_of_clk_src_get, imx_scu_clks);
+	else
+		ret = of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data);
+
+	return ret;
 }
 
 static const struct of_device_id imx8qxp_match[] = {
diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index b8b2072742a5..4fadff14d8b2 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -8,6 +8,9 @@
 #include <linux/arm-smccc.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
 #include <linux/slab.h>
 
 #include "clk-scu.h"
@@ -16,6 +19,20 @@
 #define IMX_SIP_SET_CPUFREQ		0x00
 
 static struct imx_sc_ipc *ccm_ipc_handle;
+struct device_node *pd_np;
+
+struct imx_scu_clk_node {
+	const char *name;
+	u32 rsrc;
+	u8 clk_type;
+	const char * const *parents;
+	int num_parents;
+
+	struct clk_hw *hw;
+	struct list_head node;
+};
+
+struct list_head imx_scu_clks[IMX_SC_R_LAST];
 
 /*
  * struct clk_scu - Description of one SCU clock
@@ -128,9 +145,32 @@ static inline struct clk_scu *to_clk_scu(struct clk_hw *hw)
 	return container_of(hw, struct clk_scu, hw);
 }
 
-int imx_clk_scu_init(void)
+int imx_clk_scu_init(struct device_node *np)
 {
-	return imx_scu_get_handle(&ccm_ipc_handle);
+	struct platform_device *pd_dev;
+	u32 clk_cells;
+	int ret, i;
+
+	ret = imx_scu_get_handle(&ccm_ipc_handle);
+	if (ret)
+		return ret;
+
+	if (of_property_read_u32(np, "#clock-cells", &clk_cells))
+		return -EINVAL;
+
+	if (clk_cells == 2) {
+		for (i = 0; i < IMX_SC_R_LAST; i++)
+			INIT_LIST_HEAD(&imx_scu_clks[i]);
+
+		pd_np = of_find_compatible_node(NULL, NULL, "fsl,scu-pd");
+		pd_dev = of_find_device_by_node(pd_np);
+		if (!pd_dev || !device_is_bound(&pd_dev->dev)) {
+			of_node_put(pd_np);
+			return -EPROBE_DEFER;
+		}
+	}
+
+	return 0;
 }
 
 /*
@@ -387,3 +427,99 @@ struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 
 	return hw;
 }
+
+struct clk_hw *imx_scu_of_clk_src_get(struct of_phandle_args *clkspec,
+				      void *data)
+{
+	unsigned int rsrc = clkspec->args[0];
+	unsigned int idx = clkspec->args[1];
+	struct list_head *scu_clks = data;
+	struct imx_scu_clk_node *clk;
+
+	list_for_each_entry(clk, &scu_clks[rsrc], node) {
+		if (clk->clk_type == idx)
+			return clk->hw;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+static int imx_clk_scu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct imx_scu_clk_node *clk = dev_get_platdata(dev);
+	struct clk_hw *hw;
+
+	hw = __imx_clk_scu(clk->name, clk->parents, clk->num_parents,
+			   clk->rsrc, clk->clk_type);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	clk->hw = hw;
+	list_add_tail(&clk->node, &imx_scu_clks[clk->rsrc]);
+
+	dev_dbg(dev, "register SCU clock rsrc:%d type:%d\n", clk->rsrc,
+		clk->clk_type);
+
+	return 0;
+}
+
+static struct platform_driver imx_clk_scu_driver = {
+	.driver = {
+		.name = "imx-scu-clk",
+		.suppress_bind_attrs = true,
+	},
+	.probe = imx_clk_scu_probe,
+};
+builtin_platform_driver(imx_clk_scu_driver);
+
+static int imx_clk_scu_attach_pd(struct device *dev, u32 rsrc_id)
+{
+	struct of_phandle_args genpdspec = {
+		.np = pd_np,
+		.args_count = 1,
+		.args[0] = rsrc_id,
+	};
+
+	return of_genpd_add_device(&genpdspec, dev);
+}
+
+struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
+				     const char * const *parents,
+				     int num_parents, u32 rsrc_id, u8 clk_type)
+{
+	struct imx_scu_clk_node clk = {
+		.name = name,
+		.rsrc = rsrc_id,
+		.clk_type = clk_type,
+		.parents = parents,
+		.num_parents = num_parents,
+	};
+	struct platform_device *pdev;
+	int ret;
+
+	pdev = platform_device_alloc(name, PLATFORM_DEVID_NONE);
+	if (!pdev) {
+		pr_err("%s: failed to allocate scu clk dev rsrc %d type %d\n",
+		       name, rsrc_id, clk_type);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ret = platform_device_add_data(pdev, &clk, sizeof(clk));
+	if (ret) {
+		platform_device_put(pdev);
+		return ERR_PTR(ret);
+	}
+
+	pdev->driver_override = "imx-scu-clk";
+
+	ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id);
+	if (ret)
+		pr_warn("%s: failed to attached the power domain %d\n",
+			name, ret);
+
+	platform_device_add(pdev);
+
+	/* For API backwards compatiblilty, simply return NULL for success */
+	return NULL;
+}
diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h
index 2bcfaf06a458..a512f81ed801 100644
--- a/drivers/clk/imx/clk-scu.h
+++ b/drivers/clk/imx/clk-scu.h
@@ -8,22 +8,37 @@
 #define __IMX_CLK_SCU_H
 
 #include <linux/firmware/imx/sci.h>
+#include <linux/of.h>
 
-int imx_clk_scu_init(void);
+extern struct list_head imx_scu_clks[];
+
+int imx_clk_scu_init(struct device_node *np);
+struct clk_hw *imx_scu_of_clk_src_get(struct of_phandle_args *clkspec,
+				      void *data);
+struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
+				     const char * const *parents,
+				     int num_parents, u32 rsrc_id, u8 clk_type);
 
 struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 			     int num_parents, u32 rsrc_id, u8 clk_type);
 
 static inline struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id,
-					 u8 clk_type)
+					 u8 clk_type, u8 clk_cells)
 {
-	return __imx_clk_scu(name, NULL, 0, rsrc_id, clk_type);
+	if (clk_cells == 2)
+		return imx_clk_scu_alloc_dev(name, NULL, 0, rsrc_id, clk_type);
+	else
+		return __imx_clk_scu(name, NULL, 0, rsrc_id, clk_type);
 }
 
 static inline struct clk_hw *imx_clk_scu2(const char *name, const char * const *parents,
-					  int num_parents, u32 rsrc_id, u8 clk_type)
+					  int num_parents, u32 rsrc_id, u8 clk_type,
+					  u8 clk_cells)
 {
-	return __imx_clk_scu(name, parents, num_parents, rsrc_id, clk_type);
+	if (clk_cells == 2)
+		return imx_clk_scu_alloc_dev(name, parents, num_parents, rsrc_id, clk_type);
+	else
+		return __imx_clk_scu(name, parents, num_parents, rsrc_id, clk_type);
 }
 
 struct clk_hw *imx_clk_lpcg_scu(const char *name, const char *parent_name,
-- 
2.23.0

[PATCH V6 04/12] clk: imx: scu: bypass cpu power domains

[Dong Aisheng]

Bypass cpu power domains which are owned by ATF.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v5:
 * remove unnecessary ()
v4: no changes
v3: new patch
---
 drivers/clk/imx/clk-scu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index 4fadff14d8b2..4c1980d5901d 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -481,6 +481,10 @@ static int imx_clk_scu_attach_pd(struct device *dev, u32 rsrc_id)
 		.args[0] = rsrc_id,
 	};
 
+	if (rsrc_id == IMX_SC_R_A35 || rsrc_id == IMX_SC_R_A53 ||
+	    rsrc_id == IMX_SC_R_A72)
+		return 0;
+
 	return of_genpd_add_device(&genpdspec, dev);
 }
 
-- 
2.23.0

[PATCH V6 05/12] clk: imx: scu: allow scu clk to take device pointer

[Dong Aisheng]

Used to support runtime pm.

Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v4->v5:
 * no changes
v3->v4:
 * add the missing dev poninter when call __imx_clk_scu in probe
v3: new patch
---
 drivers/clk/imx/clk-scu.c | 9 +++++----
 drivers/clk/imx/clk-scu.h | 9 +++++----
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index 4c1980d5901d..d474753712f7 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -384,8 +384,9 @@ static const struct clk_ops clk_scu_cpu_ops = {
 	.unprepare = clk_scu_unprepare,
 };
 
-struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
-			     int num_parents, u32 rsrc_id, u8 clk_type)
+struct clk_hw *__imx_clk_scu(struct device *dev, const char *name,
+			     const char * const *parents, int num_parents,
+			     u32 rsrc_id, u8 clk_type)
 {
 	struct clk_init_data init;
 	struct clk_scu *clk;
@@ -419,7 +420,7 @@ struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
 	clk->hw.init = &init;
 
 	hw = &clk->hw;
-	ret = clk_hw_register(NULL, hw);
+	ret = clk_hw_register(dev, hw);
 	if (ret) {
 		kfree(clk);
 		hw = ERR_PTR(ret);
@@ -450,7 +451,7 @@ static int imx_clk_scu_probe(struct platform_device *pdev)
 	struct imx_scu_clk_node *clk = dev_get_platdata(dev);
 	struct clk_hw *hw;
 
-	hw = __imx_clk_scu(clk->name, clk->parents, clk->num_parents,
+	hw = __imx_clk_scu(NULL, clk->name, clk->parents, clk->num_parents,
 			   clk->rsrc, clk->clk_type);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h
index a512f81ed801..11eb3781ab6b 100644
--- a/drivers/clk/imx/clk-scu.h
+++ b/drivers/clk/imx/clk-scu.h
@@ -19,8 +19,9 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
 				     const char * const *parents,
 				     int num_parents, u32 rsrc_id, u8 clk_type);
 
-struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
-			     int num_parents, u32 rsrc_id, u8 clk_type);
+struct clk_hw *__imx_clk_scu(struct device *dev, const char *name,
+			     const char * const *parents, int num_parents,
+			     u32 rsrc_id, u8 clk_type);
 
 static inline struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id,
 					 u8 clk_type, u8 clk_cells)
@@ -28,7 +29,7 @@ static inline struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id,
 	if (clk_cells == 2)
 		return imx_clk_scu_alloc_dev(name, NULL, 0, rsrc_id, clk_type);
 	else
-		return __imx_clk_scu(name, NULL, 0, rsrc_id, clk_type);
+		return __imx_clk_scu(NULL, name, NULL, 0, rsrc_id, clk_type);
 }
 
 static inline struct clk_hw *imx_clk_scu2(const char *name, const char * const *parents,
@@ -38,7 +39,7 @@ static inline struct clk_hw *imx_clk_scu2(const char *name, const char * const *
 	if (clk_cells == 2)
 		return imx_clk_scu_alloc_dev(name, parents, num_parents, rsrc_id, clk_type);
 	else
-		return __imx_clk_scu(name, parents, num_parents, rsrc_id, clk_type);
+		return __imx_clk_scu(NULL, name, parents, num_parents, rsrc_id, clk_type);
 }
 
 struct clk_hw *imx_clk_lpcg_scu(const char *name, const char *parent_name,
-- 
2.23.0

[PATCH V6 06/12] clk: imx: scu: add runtime pm support

[Dong Aisheng]

Add runtime pm support

Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v4->v5: no changes
v3: new patch
---
 drivers/clk/imx/clk-scu.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index d474753712f7..7c7a839a3569 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -11,6 +11,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
 #include "clk-scu.h"
@@ -450,15 +451,32 @@ static int imx_clk_scu_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct imx_scu_clk_node *clk = dev_get_platdata(dev);
 	struct clk_hw *hw;
+	int ret;
+
+	pm_runtime_set_suspended(dev);
+	pm_runtime_set_autosuspend_delay(dev, 50);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_enable(dev);
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret) {
+		pm_runtime_disable(dev);
+		return ret;
+	}
 
-	hw = __imx_clk_scu(NULL, clk->name, clk->parents, clk->num_parents,
+	hw = __imx_clk_scu(dev, clk->name, clk->parents, clk->num_parents,
 			   clk->rsrc, clk->clk_type);
-	if (IS_ERR(hw))
+	if (IS_ERR(hw)) {
+		pm_runtime_disable(dev);
 		return PTR_ERR(hw);
+	}
 
 	clk->hw = hw;
 	list_add_tail(&clk->node, &imx_scu_clks[clk->rsrc]);
 
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
+
 	dev_dbg(dev, "register SCU clock rsrc:%d type:%d\n", clk->rsrc,
 		clk->clk_type);
 
-- 
2.23.0

[PATCH V6 07/12] clk: imx: scu: add suspend/resume support

[Dong Aisheng]

Clock state will be lost when its power domain is completely off
during system suspend/resume. So we save and restore the state
accordingly in suspend/resume callback.

Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v5:
 * make suspend/resume function static
v4: no changes
v3: new patch
---
 drivers/clk/imx/clk-scu.c | 49 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index 7c7a839a3569..7b910922aecf 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -45,6 +45,10 @@ struct clk_scu {
 	struct clk_hw hw;
 	u16 rsrc_id;
 	u8 clk_type;
+
+	/* for state save&restore */
+	bool is_enabled;
+	u32 rate;
 };
 
 /*
@@ -427,6 +431,9 @@ struct clk_hw *__imx_clk_scu(struct device *dev, const char *name,
 		hw = ERR_PTR(ret);
 	}
 
+	if (dev)
+		dev_set_drvdata(dev, clk);
+
 	return hw;
 }
 
@@ -483,10 +490,52 @@ static int imx_clk_scu_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int __maybe_unused imx_clk_scu_suspend(struct device *dev)
+{
+	struct clk_scu *clk = dev_get_drvdata(dev);
+
+	clk->rate = clk_hw_get_rate(&clk->hw);
+	clk->is_enabled = clk_hw_is_enabled(&clk->hw);
+
+	if (clk->rate)
+		dev_dbg(dev, "save rate %d\n", clk->rate);
+
+	if (clk->is_enabled)
+		dev_dbg(dev, "save enabled state\n");
+
+	return 0;
+}
+
+static int __maybe_unused imx_clk_scu_resume(struct device *dev)
+{
+	struct clk_scu *clk = dev_get_drvdata(dev);
+	int ret = 0;
+
+	if (clk->rate) {
+		ret = clk_scu_set_rate(&clk->hw, clk->rate, 0);
+		dev_dbg(dev, "restore rate %d %s\n", clk->rate,
+			!ret ? "success" : "failed");
+	}
+
+	if (clk->is_enabled) {
+		ret = clk_scu_prepare(&clk->hw);
+		dev_dbg(dev, "restore enabled state %s\n",
+			!ret ? "success" : "failed");
+	}
+
+	return ret;
+}
+
+static const struct dev_pm_ops imx_clk_scu_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_clk_scu_suspend,
+				      imx_clk_scu_resume)
+};
+
 static struct platform_driver imx_clk_scu_driver = {
 	.driver = {
 		.name = "imx-scu-clk",
 		.suppress_bind_attrs = true,
+		.pm = &imx_clk_scu_pm_ops,
 	},
 	.probe = imx_clk_scu_probe,
 };
-- 
2.23.0

[PATCH V6 08/12] clk: imx: imx8qxp-lpcg: add parsing clocks from device tree

[Dong Aisheng]

One LPCG controller supports up to 8 clock outputs while each of them
is fixed to 4 bits.  It supports only gating function with fixed bits.
So we can use the clk-indices to fetch the corresponding clock idx from
device tree. With this way, we can write a generic LPCG clock drivers.

This patch add that support to parse clocks from device tree.

Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
Changelog:
v4->v5:
 * add error check for devm_of_clk_add_hw_provider
 * add more commit message
 * a few minor updates
v3->v4:
 * remove hw_autogate which is not still used by driver
 * use clock-indices to indicate LPCG clock bit offset
v1->v3: no changes
---
 drivers/clk/imx/clk-imx8qxp-lpcg.c | 124 +++++++++++++++++++++++++++++
 drivers/clk/imx/clk-lpcg-scu.c     |   8 ++
 drivers/clk/imx/clk-scu.h          |   1 +
 3 files changed, 133 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
index 04c8ee35e14c..1881d7f3ccec 100644
--- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
+++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
@@ -9,6 +9,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -157,6 +158,122 @@ static const struct imx8qxp_ss_lpcg imx8qxp_ss_lsio = {
 	.num_max = IMX_LSIO_LPCG_CLK_END,
 };
 
+#define IMX_LPCG_MAX_CLKS	8
+
+static struct clk_hw *imx_lpcg_of_clk_src_get(struct of_phandle_args *clkspec,
+					      void *data)
+{
+	struct clk_hw_onecell_data *hw_data = data;
+	unsigned int idx = clkspec->args[0] / 4;
+
+	if (idx >= hw_data->num) {
+		pr_err("%s: invalid index %u\n", __func__, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return hw_data->hws[idx];
+}
+
+static int imx_lpcg_parse_clks_from_dt(struct platform_device *pdev,
+				       struct device_node *np)
+{
+	const char *output_names[IMX_LPCG_MAX_CLKS];
+	const char *parent_names[IMX_LPCG_MAX_CLKS];
+	unsigned int bit_offset[IMX_LPCG_MAX_CLKS];
+	struct clk_hw_onecell_data *clk_data;
+	struct clk_hw **clk_hws;
+	struct resource *res;
+	void __iomem *base;
+	int count;
+	int idx;
+	int ret;
+	int i;
+
+	if (!of_device_is_compatible(np, "fsl,imx8qxp-lpcg"))
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	count = of_property_count_u32_elems(np, "clock-indices");
+	if (count < 0) {
+		dev_err(&pdev->dev, "failed to count clocks\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * A trick here is that we set the num of clks to the MAX instead
+	 * of the count from clock-indices because one LPCG supports up to
+	 * 8 clock outputs which each of them is fixed to 4 bits. Then we can
+	 * easily get the clock by clk-indices (bit-offset) / 4.
+	 * And the cost is very limited few pointers.
+	 */
+
+	clk_data = devm_kzalloc(&pdev->dev, struct_size(clk_data, hws,
+				IMX_LPCG_MAX_CLKS), GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	clk_data->num = IMX_LPCG_MAX_CLKS;
+	clk_hws = clk_data->hws;
+
+	ret = of_property_read_u32_array(np, "clock-indices", bit_offset,
+					 count);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to read clock-indices\n");
+		return -EINVAL;
+	}
+
+	ret = of_clk_parent_fill(np, parent_names, count);
+	if (ret != count) {
+		dev_err(&pdev->dev, "failed to get clock parent names\n");
+		return count;
+	}
+
+	ret = of_property_read_string_array(np, "clock-output-names",
+					    output_names, count);
+	if (ret != count) {
+		dev_err(&pdev->dev, "failed to read clock-output-names\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < count; i++) {
+		idx = bit_offset[i] / 4;
+		if (idx > IMX_LPCG_MAX_CLKS) {
+			dev_warn(&pdev->dev, "invalid bit offset of clock %d\n",
+				 i);
+			ret = -EINVAL;
+			goto unreg;
+		}
+
+		clk_hws[idx] = imx_clk_lpcg_scu(output_names[i],
+						parent_names[i], 0, base,
+						bit_offset[i], false);
+		if (IS_ERR(clk_hws[idx])) {
+			dev_warn(&pdev->dev, "failed to register clock %d\n",
+				 idx);
+			ret = PTR_ERR(clk_hws[idx]);
+			goto unreg;
+		}
+	}
+
+	ret = devm_of_clk_add_hw_provider(&pdev->dev, imx_lpcg_of_clk_src_get,
+					  clk_data);
+	if (!ret)
+		return 0;
+
+unreg:
+	while (--i >= 0) {
+		idx = bit_offset[i] / 4;
+		if (clk_hws[idx])
+			imx_clk_lpcg_scu_unregister(clk_hws[idx]);
+	}
+
+	return ret;
+}
+
 static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -167,8 +284,14 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct clk_hw **clks;
 	void __iomem *base;
+	int ret;
 	int i;
 
+	/* try new binding to parse clocks from device tree first */
+	ret = imx_lpcg_parse_clks_from_dt(pdev, np);
+	if (!ret)
+		return 0;
+
 	ss_lpcg = of_device_get_match_data(dev);
 	if (!ss_lpcg)
 		return -ENODEV;
@@ -219,6 +342,7 @@ static const struct of_device_id imx8qxp_lpcg_match[] = {
 	{ .compatible = "fsl,imx8qxp-lpcg-adma", &imx8qxp_ss_adma, },
 	{ .compatible = "fsl,imx8qxp-lpcg-conn", &imx8qxp_ss_conn, },
 	{ .compatible = "fsl,imx8qxp-lpcg-lsio", &imx8qxp_ss_lsio, },
+	{ .compatible = "fsl,imx8qxp-lpcg", NULL },
 	{ /* sentinel */ }
 };
 
diff --git a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c
index a73a799fb777..897a4c3c02e2 100644
--- a/drivers/clk/imx/clk-lpcg-scu.c
+++ b/drivers/clk/imx/clk-lpcg-scu.c
@@ -114,3 +114,11 @@ struct clk_hw *imx_clk_lpcg_scu(const char *name, const char *parent_name,
 
 	return hw;
 }
+
+void imx_clk_lpcg_scu_unregister(struct clk_hw *hw)
+{
+	struct clk_lpcg_scu *clk = to_clk_lpcg_scu(hw);
+
+	clk_hw_unregister(&clk->hw);
+	kfree(clk);
+}
diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h
index 11eb3781ab6b..2db226a2401f 100644
--- a/drivers/clk/imx/clk-scu.h
+++ b/drivers/clk/imx/clk-scu.h
@@ -45,4 +45,5 @@ static inline struct clk_hw *imx_clk_scu2(const char *name, const char * const *
 struct clk_hw *imx_clk_lpcg_scu(const char *name, const char *parent_name,
 				unsigned long flags, void __iomem *reg,
 				u8 bit_idx, bool hw_gate);
+void imx_clk_lpcg_scu_unregister(struct clk_hw *hw);
 #endif
-- 
2.23.0

[PATCH V6 09/12] clk: imx: lpcg: allow lpcg clk to take device pointer

[Dong Aisheng]

Used to support runtime pm.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v4->v5: no changes
v3: new patch
---
 drivers/clk/imx/clk-lpcg-scu.c |  8 ++++----
 drivers/clk/imx/clk-scu.h      | 24 ++++++++++++++++++++----
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c
index 897a4c3c02e2..2242e5047a0e 100644
--- a/drivers/clk/imx/clk-lpcg-scu.c
+++ b/drivers/clk/imx/clk-lpcg-scu.c
@@ -80,9 +80,9 @@ static const struct clk_ops clk_lpcg_scu_ops = {
 	.disable = clk_lpcg_scu_disable,
 };
 
-struct clk_hw *imx_clk_lpcg_scu(const char *name, const char *parent_name,
-				unsigned long flags, void __iomem *reg,
-				u8 bit_idx, bool hw_gate)
+struct clk_hw *__imx_clk_lpcg_scu(struct device *dev, const char *name,
+				  const char *parent_name, unsigned long flags,
+				  void __iomem *reg, u8 bit_idx, bool hw_gate)
 {
 	struct clk_lpcg_scu *clk;
 	struct clk_init_data init;
@@ -106,7 +106,7 @@ struct clk_hw *imx_clk_lpcg_scu(const char *name, const char *parent_name,
 	clk->hw.init = &init;
 
 	hw = &clk->hw;
-	ret = clk_hw_register(NULL, hw);
+	ret = clk_hw_register(dev, hw);
 	if (ret) {
 		kfree(clk);
 		hw = ERR_PTR(ret);
diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h
index 2db226a2401f..6b29713a71d2 100644
--- a/drivers/clk/imx/clk-scu.h
+++ b/drivers/clk/imx/clk-scu.h
@@ -23,6 +23,11 @@ struct clk_hw *__imx_clk_scu(struct device *dev, const char *name,
 			     const char * const *parents, int num_parents,
 			     u32 rsrc_id, u8 clk_type);
 
+struct clk_hw *__imx_clk_lpcg_scu(struct device *dev, const char *name,
+				  const char *parent_name, unsigned long flags,
+				  void __iomem *reg, u8 bit_idx, bool hw_gate);
+void imx_clk_lpcg_scu_unregister(struct clk_hw *hw);
+
 static inline struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id,
 					 u8 clk_type, u8 clk_cells)
 {
@@ -42,8 +47,19 @@ static inline struct clk_hw *imx_clk_scu2(const char *name, const char * const *
 		return __imx_clk_scu(NULL, name, parents, num_parents, rsrc_id, clk_type);
 }
 
-struct clk_hw *imx_clk_lpcg_scu(const char *name, const char *parent_name,
-				unsigned long flags, void __iomem *reg,
-				u8 bit_idx, bool hw_gate);
-void imx_clk_lpcg_scu_unregister(struct clk_hw *hw);
+static inline struct clk_hw *imx_clk_lpcg_scu_dev(struct device *dev, const char *name,
+						  const char *parent_name, unsigned long flags,
+						  void __iomem *reg, u8 bit_idx, bool hw_gate)
+{
+	return __imx_clk_lpcg_scu(dev, name, parent_name, flags, reg,
+				  bit_idx, hw_gate);
+}
+
+static inline struct clk_hw *imx_clk_lpcg_scu(const char *name, const char *parent_name,
+					      unsigned long flags, void __iomem *reg,
+					      u8 bit_idx, bool hw_gate)
+{
+	return __imx_clk_lpcg_scu(NULL, name, parent_name, flags, reg,
+				  bit_idx, hw_gate);
+}
 #endif
-- 
2.23.0

[PATCH V6 10/12] clk: imx: clk-imx8qxp-lpcg: add runtime pm support

[Dong Aisheng]

add runtime pm support

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v4->v5:
 * minor err check fix
v3->v4:
 * disable rpm when error out
v3: new patch
---
 drivers/clk/imx/clk-imx8qxp-lpcg.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
index 1881d7f3ccec..3385de76f6c7 100644
--- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
+++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
@@ -12,6 +12,7 @@
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
 #include "clk-scu.h"
@@ -239,6 +240,12 @@ static int imx_lpcg_parse_clks_from_dt(struct platform_device *pdev,
 		return -EINVAL;
 	}
 
+	pm_runtime_get_noresume(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
 	for (i = 0; i < count; i++) {
 		idx = bit_offset[i] / 4;
 		if (idx > IMX_LPCG_MAX_CLKS) {
@@ -248,9 +255,9 @@ static int imx_lpcg_parse_clks_from_dt(struct platform_device *pdev,
 			goto unreg;
 		}
 
-		clk_hws[idx] = imx_clk_lpcg_scu(output_names[i],
-						parent_names[i], 0, base,
-						bit_offset[i], false);
+		clk_hws[idx] = imx_clk_lpcg_scu_dev(&pdev->dev, output_names[i],
+						    parent_names[i], 0, base,
+						    bit_offset[i], false);
 		if (IS_ERR(clk_hws[idx])) {
 			dev_warn(&pdev->dev, "failed to register clock %d\n",
 				 idx);
@@ -261,8 +268,13 @@ static int imx_lpcg_parse_clks_from_dt(struct platform_device *pdev,
 
 	ret = devm_of_clk_add_hw_provider(&pdev->dev, imx_lpcg_of_clk_src_get,
 					  clk_data);
-	if (!ret)
-		return 0;
+	if (ret)
+		goto unreg;
+
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
+
+	return 0;
 
 unreg:
 	while (--i >= 0) {
@@ -271,6 +283,8 @@ static int imx_lpcg_parse_clks_from_dt(struct platform_device *pdev,
 			imx_clk_lpcg_scu_unregister(clk_hws[idx]);
 	}
 
+	pm_runtime_disable(&pdev->dev);
+
 	return ret;
 }
 
-- 
2.23.0

[PATCH V6 11/12] clk: imx: lpcg: add suspend/resume support

[Dong Aisheng]

LPCG clock state may be lost when it's power domain is completely
off during system suspend/resume and we need save and restore the
state properly.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
ChangeLog:
v5:
 * make suspend/resume function static
 * improve code comments
v4: no changes
v3: new patch
---
 drivers/clk/imx/clk-imx8qxp-lpcg.c |  1 +
 drivers/clk/imx/clk-lpcg-scu.c     | 37 ++++++++++++++++++++++++++++++
 drivers/clk/imx/clk-scu.h          |  1 +
 3 files changed, 39 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c
index 3385de76f6c7..4d4a3a65bcec 100644
--- a/drivers/clk/imx/clk-imx8qxp-lpcg.c
+++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c
@@ -364,6 +364,7 @@ static struct platform_driver imx8qxp_lpcg_clk_driver = {
 	.driver = {
 		.name = "imx8qxp-lpcg-clk",
 		.of_match_table = imx8qxp_lpcg_match,
+		.pm = &imx_clk_lpcg_scu_pm_ops,
 		.suppress_bind_attrs = true,
 	},
 	.probe = imx8qxp_lpcg_clk_probe,
diff --git a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c
index 2242e5047a0e..5bd8af371059 100644
--- a/drivers/clk/imx/clk-lpcg-scu.c
+++ b/drivers/clk/imx/clk-lpcg-scu.c
@@ -33,6 +33,9 @@ struct clk_lpcg_scu {
 	void __iomem *reg;
 	u8 bit_idx;
 	bool hw_gate;
+
+	/* for state save&restore */
+	u32 state;
 };
 
 #define to_clk_lpcg_scu(_hw) container_of(_hw, struct clk_lpcg_scu, hw)
@@ -112,6 +115,9 @@ struct clk_hw *__imx_clk_lpcg_scu(struct device *dev, const char *name,
 		hw = ERR_PTR(ret);
 	}
 
+	if (dev)
+		dev_set_drvdata(dev, clk);
+
 	return hw;
 }
 
@@ -122,3 +128,34 @@ void imx_clk_lpcg_scu_unregister(struct clk_hw *hw)
 	clk_hw_unregister(&clk->hw);
 	kfree(clk);
 }
+
+static int __maybe_unused imx_clk_lpcg_scu_suspend(struct device *dev)
+{
+	struct clk_lpcg_scu *clk = dev_get_drvdata(dev);
+
+	clk->state = readl_relaxed(clk->reg);
+	dev_dbg(dev, "save lpcg state 0x%x\n", clk->state);
+
+	return 0;
+}
+
+static int __maybe_unused imx_clk_lpcg_scu_resume(struct device *dev)
+{
+	struct clk_lpcg_scu *clk = dev_get_drvdata(dev);
+
+	/*
+	 * FIXME: Sometimes writes don't work unless the CPU issues
+	 * them twice
+	 */
+
+	writel(clk->state, clk->reg);
+	writel(clk->state, clk->reg);
+	dev_dbg(dev, "restore lpcg state 0x%x\n", clk->state);
+
+	return 0;
+}
+
+const struct dev_pm_ops imx_clk_lpcg_scu_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_clk_lpcg_scu_suspend,
+				      imx_clk_lpcg_scu_resume)
+};
diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h
index 6b29713a71d2..b1dfdaf0734e 100644
--- a/drivers/clk/imx/clk-scu.h
+++ b/drivers/clk/imx/clk-scu.h
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 
 extern struct list_head imx_scu_clks[];
+extern const struct dev_pm_ops imx_clk_lpcg_scu_pm_ops;
 
 int imx_clk_scu_init(struct device_node *np);
 struct clk_hw *imx_scu_of_clk_src_get(struct of_phandle_args *clkspec,
-- 
2.23.0

[PATCH V6 12/12] clk: imx: scu: unregister clocks if add provider failed

[Dong Aisheng]

Unregister clocks if add provider failed

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

---
ChangeLog:
v6: new patch
---
 drivers/clk/imx/clk-imx8qxp.c | 11 +++++++++--
 drivers/clk/imx/clk-scu.c     | 13 +++++++++++++
 drivers/clk/imx/clk-scu.h     |  2 ++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
index 2ec3e0c4749d..e615214495c0 100644
--- a/drivers/clk/imx/clk-imx8qxp.c
+++ b/drivers/clk/imx/clk-imx8qxp.c
@@ -138,10 +138,17 @@ static int imx8qxp_clk_probe(struct platform_device *pdev)
 				i, PTR_ERR(clks[i]));
 	}
 
-	if (clk_cells == 2)
+	if (clk_cells == 2) {
 		ret = of_clk_add_hw_provider(ccm_node, imx_scu_of_clk_src_get, imx_scu_clks);
-	else
+		if (ret)
+			imx_clk_scu_unregister();
+	} else {
+		/*
+		 * NOTE: we did not unregister clocks for the legacy way cause
+		 * it will be removed later.
+		 */
 		ret = of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data);
+	}
 
 	return ret;
 }
diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index 7b910922aecf..1933cf90ae30 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -595,3 +595,16 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
 	/* For API backwards compatiblilty, simply return NULL for success */
 	return NULL;
 }
+
+void imx_clk_scu_unregister(void)
+{
+	struct imx_scu_clk_node *clk;
+	int i;
+
+	for (i = 0; i < IMX_SC_R_LAST; i++) {
+		list_for_each_entry(clk, &imx_scu_clks[i], node) {
+			clk_hw_unregister(clk->hw);
+			kfree(clk);
+		}
+	}
+}
diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h
index b1dfdaf0734e..e8352164923e 100644
--- a/drivers/clk/imx/clk-scu.h
+++ b/drivers/clk/imx/clk-scu.h
@@ -24,6 +24,8 @@ struct clk_hw *__imx_clk_scu(struct device *dev, const char *name,
 			     const char * const *parents, int num_parents,
 			     u32 rsrc_id, u8 clk_type);
 
+void imx_clk_scu_unregister(void);
+
 struct clk_hw *__imx_clk_lpcg_scu(struct device *dev, const char *name,
 				  const char *parent_name, unsigned long flags,
 				  void __iomem *reg, u8 bit_idx, bool hw_gate);
-- 
2.23.0

RE: [PATCH V6 00/12] clk: imx8: add new clock binding for better pm support

[Aisheng Dong]

Hi Stephen,

> From: Dong Aisheng <aisheng.dong@nxp.com>
> Sent: Sunday, March 15, 2020 9:44 PM
> 
> This patch series is a preparation for the MX8 Architecture improvement.
> As for IMX SCU based platforms like MX8QM and MX8QXP, they are comprised
> of a couple of SS(Subsystems) while most of them within the same SS can be
> shared. e.g. Clocks, Devices and etc.
> 
> However, current clock binding is using SW IDs for device tree to use which can
> cause troubles in writing the common <soc>-ss-xx.dtsi file for different SoCs.
> 
> This patch series aims to introduce a new binding which is more close to
> hardware and platform independent and can makes us write a more general
> drivers for different SCU based SoCs.
> 
> Another important thing is that on MX8, each Clock resource is associated with
> a power domain. So we have to attach that clock device to the power domain in
> order to make it work properly. Further more, the clock state will be lost when
> its power domain is completely off during suspend/resume, so we also introduce
> the clock state save&restore mechanism.
> 
> It's based on latest shanw/for-next branch.
> 
> The top commit is:
> 4559a11e48d5 Merge branch 'imx/defconfig' into for-next
> 

I carefully went through all your review comments on V4 [1] and i'm sure all of them have been addressed.
And for the V5-Resend version [2] , we didn't receive your comment.
It seems Shawn is also waiting for your feedback for the next step.
Could you help check V6?

For V6, only a minor update that added scu clk unregister if met error. As the original code does not support it,
so i added an extra patch to handle it.

[1] https://lore.kernel.org/linux-clk/AM0PR04MB421181E4376070636898906F80A10@AM0PR04MB4211.eurprd04.prod.outlook.com/t/
[2] https://patchwork.kernel.org/cover/11248249/

Regards
Aisheng

> ChangeLog:
> v5->v6:
>  * add scu clk unregister if add provider failed
> v4->v5:
>  * Address all comments from Stephen
> v3->v4:
>  * use clk-indices for LPCG to fetch each clks offset from dt
> v2->v3:
>  * change scu clk into two cells binding
>  * add clk pm patches to ease the understand of the changes
> v1->v2:
>  * SCU clock changed to one cell clock binding inspired by arm,scpi.txt
>    Documentation/devicetree/bindings/arm/arm,scpi.txt
>  * Add required power domain property
>  * Dropped PATCH 3&4 first, will send the updated version accordingly
>    after the binding is finally determined,
> 
> 
> Dong Aisheng (12):
>   dt-bindings: firmware: imx-scu: new binding to parse clocks from
>     device tree
>   dt-bindings: clock: imx-lpcg: add support to parse clocks from device
>     tree
>   clk: imx: scu: add two cells binding support
>   clk: imx: scu: bypass cpu power domains
>   clk: imx: scu: allow scu clk to take device pointer
>   clk: imx: scu: add runtime pm support
>   clk: imx: scu: add suspend/resume support
>   clk: imx: imx8qxp-lpcg: add parsing clocks from device tree
>   clk: imx: lpcg: allow lpcg clk to take device pointer
>   clk: imx: clk-imx8qxp-lpcg: add runtime pm support
>   clk: imx: lpcg: add suspend/resume support
>   clk: imx: scu: unregister clocks if add provider failed
> 
>  .../bindings/arm/freescale/fsl,scu.txt        |  12 +-
>  .../bindings/clock/imx8qxp-lpcg.txt           |  36 ++-
>  drivers/clk/imx/clk-imx8qxp-lpcg.c            | 139 +++++++++++
>  drivers/clk/imx/clk-imx8qxp.c                 | 136 ++++++-----
>  drivers/clk/imx/clk-lpcg-scu.c                |  53 +++-
>  drivers/clk/imx/clk-scu.c                     | 231 +++++++++++++++++-
>  drivers/clk/imx/clk-scu.h                     |  56 ++++-
>  include/dt-bindings/clock/imx8-lpcg.h         |  14 ++
>  include/dt-bindings/firmware/imx/rsrc.h       |  23 ++
>  9 files changed, 610 insertions(+), 90 deletions(-)  create mode 100644
> include/dt-bindings/clock/imx8-lpcg.h
> 
> --
> 2.23.0

RE: [PATCH V6 00/12] clk: imx8: add new clock binding for better pm support

[Aisheng Dong]

Ping

> From: Aisheng Dong <aisheng.dong@nxp.com>
> Sent: Sunday, March 15, 2020 10:11 PM
> 
> Hi Stephen,
> 
> > From: Dong Aisheng <aisheng.dong@nxp.com>
> > Sent: Sunday, March 15, 2020 9:44 PM
> >
> > This patch series is a preparation for the MX8 Architecture improvement.
> > As for IMX SCU based platforms like MX8QM and MX8QXP, they are
> > comprised of a couple of SS(Subsystems) while most of them within the
> > same SS can be shared. e.g. Clocks, Devices and etc.
> >
> > However, current clock binding is using SW IDs for device tree to use
> > which can cause troubles in writing the common <soc>-ss-xx.dtsi file for
> different SoCs.
> >
> > This patch series aims to introduce a new binding which is more close
> > to hardware and platform independent and can makes us write a more
> > general drivers for different SCU based SoCs.
> >
> > Another important thing is that on MX8, each Clock resource is
> > associated with a power domain. So we have to attach that clock device
> > to the power domain in order to make it work properly. Further more,
> > the clock state will be lost when its power domain is completely off
> > during suspend/resume, so we also introduce the clock state save&restore
> mechanism.
> >
> > It's based on latest shanw/for-next branch.
> >
> > The top commit is:
> > 4559a11e48d5 Merge branch 'imx/defconfig' into for-next
> >
> 
> I carefully went through all your review comments on V4 [1] and i'm sure all of
> them have been addressed.
> And for the V5-Resend version [2] , we didn't receive your comment.
> It seems Shawn is also waiting for your feedback for the next step.
> Could you help check V6?
> 
> For V6, only a minor update that added scu clk unregister if met error. As the
> original code does not support it, so i added an extra patch to handle it.
> 
> [1]
> https://lore.kernel.org/linux-clk/AM0PR04MB421181E4376070636898906F80
> A10@AM0PR04MB4211.eurprd04.prod.outlook.com/t/
> [2] https://patchwork.kernel.org/cover/11248249/
> 
> Regards
> Aisheng
> 
> > ChangeLog:
> > v5->v6:
> >  * add scu clk unregister if add provider failed
> > v4->v5:
> >  * Address all comments from Stephen
> > v3->v4:
> >  * use clk-indices for LPCG to fetch each clks offset from dt
> > v2->v3:
> >  * change scu clk into two cells binding
> >  * add clk pm patches to ease the understand of the changes
> > v1->v2:
> >  * SCU clock changed to one cell clock binding inspired by arm,scpi.txt
> >    Documentation/devicetree/bindings/arm/arm,scpi.txt
> >  * Add required power domain property
> >  * Dropped PATCH 3&4 first, will send the updated version accordingly
> >    after the binding is finally determined,
> >
> >
> > Dong Aisheng (12):
> >   dt-bindings: firmware: imx-scu: new binding to parse clocks from
> >     device tree
> >   dt-bindings: clock: imx-lpcg: add support to parse clocks from device
> >     tree
> >   clk: imx: scu: add two cells binding support
> >   clk: imx: scu: bypass cpu power domains
> >   clk: imx: scu: allow scu clk to take device pointer
> >   clk: imx: scu: add runtime pm support
> >   clk: imx: scu: add suspend/resume support
> >   clk: imx: imx8qxp-lpcg: add parsing clocks from device tree
> >   clk: imx: lpcg: allow lpcg clk to take device pointer
> >   clk: imx: clk-imx8qxp-lpcg: add runtime pm support
> >   clk: imx: lpcg: add suspend/resume support
> >   clk: imx: scu: unregister clocks if add provider failed
> >
> >  .../bindings/arm/freescale/fsl,scu.txt        |  12 +-
> >  .../bindings/clock/imx8qxp-lpcg.txt           |  36 ++-
> >  drivers/clk/imx/clk-imx8qxp-lpcg.c            | 139 +++++++++++
> >  drivers/clk/imx/clk-imx8qxp.c                 | 136 ++++++-----
> >  drivers/clk/imx/clk-lpcg-scu.c                |  53 +++-
> >  drivers/clk/imx/clk-scu.c                     | 231
> +++++++++++++++++-
> >  drivers/clk/imx/clk-scu.h                     |  56 ++++-
> >  include/dt-bindings/clock/imx8-lpcg.h         |  14 ++
> >  include/dt-bindings/firmware/imx/rsrc.h       |  23 ++
> >  9 files changed, 610 insertions(+), 90 deletions(-)  create mode
> > 100644 include/dt-bindings/clock/imx8-lpcg.h
> >
> > --
> > 2.23.0

RE: [PATCH V6 00/12] clk: imx8: add new clock binding for better pm support

[Aisheng Dong]

Ping... This patchset is pending for quite a long time without comments. It also blocked a lot continued upstreaming work.
Any suggestions how to proceed?

Regards
Aisheng

> From: Aisheng Dong <aisheng.dong@nxp.com>
> Sent: Thursday, March 26, 2020 11:15 AM
> 
> Ping
> 
> > From: Aisheng Dong <aisheng.dong@nxp.com>
> > Sent: Sunday, March 15, 2020 10:11 PM
> >
> > Hi Stephen,
> >
> > > From: Dong Aisheng <aisheng.dong@nxp.com>
> > > Sent: Sunday, March 15, 2020 9:44 PM
> > >
> > > This patch series is a preparation for the MX8 Architecture improvement.
> > > As for IMX SCU based platforms like MX8QM and MX8QXP, they are
> > > comprised of a couple of SS(Subsystems) while most of them within
> > > the same SS can be shared. e.g. Clocks, Devices and etc.
> > >
> > > However, current clock binding is using SW IDs for device tree to
> > > use which can cause troubles in writing the common <soc>-ss-xx.dtsi
> > > file for
> > different SoCs.
> > >
> > > This patch series aims to introduce a new binding which is more
> > > close to hardware and platform independent and can makes us write a
> > > more general drivers for different SCU based SoCs.
> > >
> > > Another important thing is that on MX8, each Clock resource is
> > > associated with a power domain. So we have to attach that clock
> > > device to the power domain in order to make it work properly.
> > > Further more, the clock state will be lost when its power domain is
> > > completely off during suspend/resume, so we also introduce the clock
> > > state save&restore
> > mechanism.
> > >
> > > It's based on latest shanw/for-next branch.
> > >
> > > The top commit is:
> > > 4559a11e48d5 Merge branch 'imx/defconfig' into for-next
> > >
> >
> > I carefully went through all your review comments on V4 [1] and i'm
> > sure all of them have been addressed.
> > And for the V5-Resend version [2] , we didn't receive your comment.
> > It seems Shawn is also waiting for your feedback for the next step.
> > Could you help check V6?
> >
> > For V6, only a minor update that added scu clk unregister if met
> > error. As the original code does not support it, so i added an extra patch to
> handle it.
> >
> > [1]
> >
> https://lore.kernel.org/linux-clk/AM0PR04MB421181E4376070636898906F80
> > A10@AM0PR04MB4211.eurprd04.prod.outlook.com/t/
> > [2] https://patchwork.kernel.org/cover/11248249/
> >
> > Regards
> > Aisheng
> >
> > > ChangeLog:
> > > v5->v6:
> > >  * add scu clk unregister if add provider failed
> > > v4->v5:
> > >  * Address all comments from Stephen
> > > v3->v4:
> > >  * use clk-indices for LPCG to fetch each clks offset from dt
> > > v2->v3:
> > >  * change scu clk into two cells binding
> > >  * add clk pm patches to ease the understand of the changes
> > > v1->v2:
> > >  * SCU clock changed to one cell clock binding inspired by arm,scpi.txt
> > >    Documentation/devicetree/bindings/arm/arm,scpi.txt
> > >  * Add required power domain property
> > >  * Dropped PATCH 3&4 first, will send the updated version accordingly
> > >    after the binding is finally determined,
> > >
> > >
> > > Dong Aisheng (12):
> > >   dt-bindings: firmware: imx-scu: new binding to parse clocks from
> > >     device tree
> > >   dt-bindings: clock: imx-lpcg: add support to parse clocks from device
> > >     tree
> > >   clk: imx: scu: add two cells binding support
> > >   clk: imx: scu: bypass cpu power domains
> > >   clk: imx: scu: allow scu clk to take device pointer
> > >   clk: imx: scu: add runtime pm support
> > >   clk: imx: scu: add suspend/resume support
> > >   clk: imx: imx8qxp-lpcg: add parsing clocks from device tree
> > >   clk: imx: lpcg: allow lpcg clk to take device pointer
> > >   clk: imx: clk-imx8qxp-lpcg: add runtime pm support
> > >   clk: imx: lpcg: add suspend/resume support
> > >   clk: imx: scu: unregister clocks if add provider failed
> > >
> > >  .../bindings/arm/freescale/fsl,scu.txt        |  12 +-
> > >  .../bindings/clock/imx8qxp-lpcg.txt           |  36 ++-
> > >  drivers/clk/imx/clk-imx8qxp-lpcg.c            | 139 +++++++++++
> > >  drivers/clk/imx/clk-imx8qxp.c                 | 136 ++++++-----
> > >  drivers/clk/imx/clk-lpcg-scu.c                |  53 +++-
> > >  drivers/clk/imx/clk-scu.c                     | 231
> > +++++++++++++++++-
> > >  drivers/clk/imx/clk-scu.h                     |  56 ++++-
> > >  include/dt-bindings/clock/imx8-lpcg.h         |  14 ++
> > >  include/dt-bindings/firmware/imx/rsrc.h       |  23 ++
> > >  9 files changed, 610 insertions(+), 90 deletions(-)  create mode
> > > 100644 include/dt-bindings/clock/imx8-lpcg.h
> > >
> > > --
> > > 2.23.0

RE: [PATCH V6 00/12] clk: imx8: add new clock binding for better pm support

[Aisheng Dong]

Gently Ping..
This patchset is without comments for a few months.
Could you help this?

> From: Aisheng Dong <aisheng.dong@nxp.com>
> Sent: Tuesday, April 7, 2020 10:23 AM
> 
> Ping... This patchset is pending for quite a long time without comments. It also
> blocked a lot continued upstreaming work.
> Any suggestions how to proceed?
> 
> Regards
> Aisheng
> 
> > From: Aisheng Dong <aisheng.dong@nxp.com>
> > Sent: Thursday, March 26, 2020 11:15 AM
> >
> > Ping
> >
> > > From: Aisheng Dong <aisheng.dong@nxp.com>
> > > Sent: Sunday, March 15, 2020 10:11 PM
> > >
> > > Hi Stephen,
> > >
> > > > From: Dong Aisheng <aisheng.dong@nxp.com>
> > > > Sent: Sunday, March 15, 2020 9:44 PM
> > > >
> > > > This patch series is a preparation for the MX8 Architecture improvement.
> > > > As for IMX SCU based platforms like MX8QM and MX8QXP, they are
> > > > comprised of a couple of SS(Subsystems) while most of them within
> > > > the same SS can be shared. e.g. Clocks, Devices and etc.
> > > >
> > > > However, current clock binding is using SW IDs for device tree to
> > > > use which can cause troubles in writing the common
> > > > <soc>-ss-xx.dtsi file for
> > > different SoCs.
> > > >
> > > > This patch series aims to introduce a new binding which is more
> > > > close to hardware and platform independent and can makes us write
> > > > a more general drivers for different SCU based SoCs.
> > > >
> > > > Another important thing is that on MX8, each Clock resource is
> > > > associated with a power domain. So we have to attach that clock
> > > > device to the power domain in order to make it work properly.
> > > > Further more, the clock state will be lost when its power domain
> > > > is completely off during suspend/resume, so we also introduce the
> > > > clock state save&restore
> > > mechanism.
> > > >
> > > > It's based on latest shanw/for-next branch.
> > > >
> > > > The top commit is:
> > > > 4559a11e48d5 Merge branch 'imx/defconfig' into for-next
> > > >
> > >
> > > I carefully went through all your review comments on V4 [1] and i'm
> > > sure all of them have been addressed.
> > > And for the V5-Resend version [2] , we didn't receive your comment.
> > > It seems Shawn is also waiting for your feedback for the next step.
> > > Could you help check V6?
> > >
> > > For V6, only a minor update that added scu clk unregister if met
> > > error. As the original code does not support it, so i added an extra
> > > patch to
> > handle it.
> > >
> > > [1]
> > >
> >
> https://lore.kernel.org/linux-clk/AM0PR04MB421181E4376070636898906F80
> > > A10@AM0PR04MB4211.eurprd04.prod.outlook.com/t/
> > > [2] https://patchwork.kernel.org/cover/11248249/
> > >
> > > Regards
> > > Aisheng
> > >
> > > > ChangeLog:
> > > > v5->v6:
> > > >  * add scu clk unregister if add provider failed
> > > > v4->v5:
> > > >  * Address all comments from Stephen
> > > > v3->v4:
> > > >  * use clk-indices for LPCG to fetch each clks offset from dt
> > > > v2->v3:
> > > >  * change scu clk into two cells binding
> > > >  * add clk pm patches to ease the understand of the changes
> > > > v1->v2:
> > > >  * SCU clock changed to one cell clock binding inspired by arm,scpi.txt
> > > >    Documentation/devicetree/bindings/arm/arm,scpi.txt
> > > >  * Add required power domain property
> > > >  * Dropped PATCH 3&4 first, will send the updated version accordingly
> > > >    after the binding is finally determined,
> > > >
> > > >
> > > > Dong Aisheng (12):
> > > >   dt-bindings: firmware: imx-scu: new binding to parse clocks from
> > > >     device tree
> > > >   dt-bindings: clock: imx-lpcg: add support to parse clocks from device
> > > >     tree
> > > >   clk: imx: scu: add two cells binding support
> > > >   clk: imx: scu: bypass cpu power domains
> > > >   clk: imx: scu: allow scu clk to take device pointer
> > > >   clk: imx: scu: add runtime pm support
> > > >   clk: imx: scu: add suspend/resume support
> > > >   clk: imx: imx8qxp-lpcg: add parsing clocks from device tree
> > > >   clk: imx: lpcg: allow lpcg clk to take device pointer
> > > >   clk: imx: clk-imx8qxp-lpcg: add runtime pm support
> > > >   clk: imx: lpcg: add suspend/resume support
> > > >   clk: imx: scu: unregister clocks if add provider failed
> > > >
> > > >  .../bindings/arm/freescale/fsl,scu.txt        |  12 +-
> > > >  .../bindings/clock/imx8qxp-lpcg.txt           |  36 ++-
> > > >  drivers/clk/imx/clk-imx8qxp-lpcg.c            | 139 +++++++++++
> > > >  drivers/clk/imx/clk-imx8qxp.c                 | 136 ++++++-----
> > > >  drivers/clk/imx/clk-lpcg-scu.c                |  53 +++-
> > > >  drivers/clk/imx/clk-scu.c                     | 231
> > > +++++++++++++++++-
> > > >  drivers/clk/imx/clk-scu.h                     |  56 ++++-
> > > >  include/dt-bindings/clock/imx8-lpcg.h         |  14 ++
> > > >  include/dt-bindings/firmware/imx/rsrc.h       |  23 ++
> > > >  9 files changed, 610 insertions(+), 90 deletions(-)  create mode
> > > > 100644 include/dt-bindings/clock/imx8-lpcg.h
> > > >
> > > > --
> > > > 2.23.0

Re: [PATCH V6 04/12] clk: imx: scu: bypass cpu power domains

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:48)
> Bypass cpu power domains which are owned by ATF.
> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH V6 07/12] clk: imx: scu: add suspend/resume support

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:51)
> Clock state will be lost when its power domain is completely off
> during system suspend/resume. So we save and restore the state
> accordingly in suspend/resume callback.
> 
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH V6 10/12] clk: imx: clk-imx8qxp-lpcg: add runtime pm support

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:54)
> add runtime pm support
> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH V6 11/12] clk: imx: lpcg: add suspend/resume support

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:55)
> LPCG clock state may be lost when it's power domain is completely
> off during system suspend/resume and we need save and restore the
> state properly.
> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

> diff --git a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c
> index 2242e5047a0e..5bd8af371059 100644
> --- a/drivers/clk/imx/clk-lpcg-scu.c
> +++ b/drivers/clk/imx/clk-lpcg-scu.c
> @@ -122,3 +128,34 @@ void imx_clk_lpcg_scu_unregister(struct clk_hw *hw)
>         clk_hw_unregister(&clk->hw);
>         kfree(clk);
>  }
> +
> +static int __maybe_unused imx_clk_lpcg_scu_suspend(struct device *dev)
> +{
> +       struct clk_lpcg_scu *clk = dev_get_drvdata(dev);
> +
> +       clk->state = readl_relaxed(clk->reg);
> +       dev_dbg(dev, "save lpcg state 0x%x\n", clk->state);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused imx_clk_lpcg_scu_resume(struct device *dev)
> +{
> +       struct clk_lpcg_scu *clk = dev_get_drvdata(dev);
> +
> +       /*
> +        * FIXME: Sometimes writes don't work unless the CPU issues

Is this ever going to be resolved? Maybe the FIXME can be dropped and we
can accept that writes don't work?

> +        * them twice
> +        */
> +
> +       writel(clk->state, clk->reg);
> +       writel(clk->state, clk->reg);
> +       dev_dbg(dev, "restore lpcg state 0x%x\n", clk->state);

Re: [PATCH V6 12/12] clk: imx: scu: unregister clocks if add provider failed

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:56)
> Unregister clocks if add provider failed
> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> 
> ---

Why isn't this squashed in to where it's needed?

> ChangeLog:
> v6: new patch
> ---
>  drivers/clk/imx/clk-imx8qxp.c | 11 +++++++++--
>  drivers/clk/imx/clk-scu.c     | 13 +++++++++++++
>  drivers/clk/imx/clk-scu.h     |  2 ++
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
> index 2ec3e0c4749d..e615214495c0 100644
> --- a/drivers/clk/imx/clk-imx8qxp.c
> +++ b/drivers/clk/imx/clk-imx8qxp.c
> @@ -138,10 +138,17 @@ static int imx8qxp_clk_probe(struct platform_device *pdev)
>                                 i, PTR_ERR(clks[i]));
>         }
>  
> -       if (clk_cells == 2)
> +       if (clk_cells == 2) {
>                 ret = of_clk_add_hw_provider(ccm_node, imx_scu_of_clk_src_get, imx_scu_clks);
> -       else
> +               if (ret)
> +                       imx_clk_scu_unregister();
> +       } else {
> +               /*
> +                * NOTE: we did not unregister clocks for the legacy way cause
> +                * it will be removed later.

I got confused what 'it' was. I think it's the legacy way entirely.
Maybe say "legacy binding code path doesn't unregister here because..."

> +                */
>                 ret = of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data);
> +       }
>  
>         return ret;
>  }

Re: [PATCH V6 09/12] clk: imx: lpcg: allow lpcg clk to take device pointer

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:53)
> Used to support runtime pm.
> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH V6 03/12] clk: imx: scu: add two cells binding support

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:47)
> This patch implements the new two cells binding for SCU clocks.
> The usage is as follows:
> clocks = <&uart0_clk IMX_SC_R_UART_0 IMX_SC_PM_CLK_PER>
> 
> Due to each SCU clock is associated with a power domain, without power
> on the domain, the SCU clock can't work. So we create platform devices
> for each domain clock respectively and manually attach the required domain
> before register the clock devices, then we can register clocks in the
> clock platform driver accordingly.

That's odd. See below.

> 
> Note because we do not have power domain info in device tree and the SCU
> resource ID is the same for power domain and clock, so we use resource ID
> to find power domains.
> 
> Later, we will also use this clock platform driver to support suspend/resume
> and runtime pm.
> 
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
[...]
> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> index b8b2072742a5..4fadff14d8b2 100644
> --- a/drivers/clk/imx/clk-scu.c
> +++ b/drivers/clk/imx/clk-scu.c
> @@ -8,6 +8,9 @@
>  #include <linux/arm-smccc.h>
>  #include <linux/clk-provider.h>
>  #include <linux/err.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/slab.h>
>  
>  #include "clk-scu.h"
> @@ -16,6 +19,20 @@
>  #define IMX_SIP_SET_CPUFREQ            0x00
>  
>  static struct imx_sc_ipc *ccm_ipc_handle;
> +struct device_node *pd_np;
> +
> +struct imx_scu_clk_node {
> +       const char *name;
> +       u32 rsrc;
> +       u8 clk_type;
> +       const char * const *parents;
> +       int num_parents;
> +
> +       struct clk_hw *hw;
> +       struct list_head node;
> +};
> +
> +struct list_head imx_scu_clks[IMX_SC_R_LAST];
>  
>  /*
>   * struct clk_scu - Description of one SCU clock
> @@ -128,9 +145,32 @@ static inline struct clk_scu *to_clk_scu(struct clk_hw *hw)
>         return container_of(hw, struct clk_scu, hw);
>  }
>  
> -int imx_clk_scu_init(void)
> +int imx_clk_scu_init(struct device_node *np)
>  {
> -       return imx_scu_get_handle(&ccm_ipc_handle);
> +       struct platform_device *pd_dev;
> +       u32 clk_cells;
> +       int ret, i;
> +
> +       ret = imx_scu_get_handle(&ccm_ipc_handle);
> +       if (ret)
> +               return ret;
> +
> +       if (of_property_read_u32(np, "#clock-cells", &clk_cells))

Why wouldn't there be #clock-cells in the node?

> +               return -EINVAL;
> +
> +       if (clk_cells == 2) {
> +               for (i = 0; i < IMX_SC_R_LAST; i++)
> +                       INIT_LIST_HEAD(&imx_scu_clks[i]);
> +
> +               pd_np = of_find_compatible_node(NULL, NULL, "fsl,scu-pd");
> +               pd_dev = of_find_device_by_node(pd_np);
> +               if (!pd_dev || !device_is_bound(&pd_dev->dev)) {

What is device_is_bound() check for? Add a comment?

> +                       of_node_put(pd_np);
> +                       return -EPROBE_DEFER;
> +               }
> +       }
> +
> +       return 0;
>  }
>  
>  /*
> @@ -387,3 +427,99 @@ struct clk_hw *__imx_clk_scu(const char *name, const char * const *parents,
>  
>         return hw;
>  }
> +
> +struct clk_hw *imx_scu_of_clk_src_get(struct of_phandle_args *clkspec,
> +                                     void *data)
> +{
> +       unsigned int rsrc = clkspec->args[0];
> +       unsigned int idx = clkspec->args[1];
> +       struct list_head *scu_clks = data;
> +       struct imx_scu_clk_node *clk;
> +
> +       list_for_each_entry(clk, &scu_clks[rsrc], node) {
> +               if (clk->clk_type == idx)
> +                       return clk->hw;
> +       }
> +
> +       return ERR_PTR(-ENODEV);
> +}
> +
> +static int imx_clk_scu_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct imx_scu_clk_node *clk = dev_get_platdata(dev);
> +       struct clk_hw *hw;
> +
> +       hw = __imx_clk_scu(clk->name, clk->parents, clk->num_parents,
> +                          clk->rsrc, clk->clk_type);
> +       if (IS_ERR(hw))
> +               return PTR_ERR(hw);
> +
> +       clk->hw = hw;
> +       list_add_tail(&clk->node, &imx_scu_clks[clk->rsrc]);
> +
> +       dev_dbg(dev, "register SCU clock rsrc:%d type:%d\n", clk->rsrc,
> +               clk->clk_type);
> +
> +       return 0;
> +}
> +
> +static struct platform_driver imx_clk_scu_driver = {
> +       .driver = {
> +               .name = "imx-scu-clk",
> +               .suppress_bind_attrs = true,
> +       },
> +       .probe = imx_clk_scu_probe,
> +};
> +builtin_platform_driver(imx_clk_scu_driver);
> +
> +static int imx_clk_scu_attach_pd(struct device *dev, u32 rsrc_id)
> +{
> +       struct of_phandle_args genpdspec = {
> +               .np = pd_np,
> +               .args_count = 1,
> +               .args[0] = rsrc_id,
> +       };
> +
> +       return of_genpd_add_device(&genpdspec, dev);
> +}
> +
> +struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
> +                                    const char * const *parents,
> +                                    int num_parents, u32 rsrc_id, u8 clk_type)
> +{
> +       struct imx_scu_clk_node clk = {
> +               .name = name,
> +               .rsrc = rsrc_id,
> +               .clk_type = clk_type,
> +               .parents = parents,
> +               .num_parents = num_parents,
> +       };
> +       struct platform_device *pdev;
> +       int ret;
> +
> +       pdev = platform_device_alloc(name, PLATFORM_DEVID_NONE);
> +       if (!pdev) {
> +               pr_err("%s: failed to allocate scu clk dev rsrc %d type %d\n",
> +                      name, rsrc_id, clk_type);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       ret = platform_device_add_data(pdev, &clk, sizeof(clk));
> +       if (ret) {
> +               platform_device_put(pdev);
> +               return ERR_PTR(ret);
> +       }
> +
> +       pdev->driver_override = "imx-scu-clk";
> +
> +       ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id);

Why do we have to allocate a device for each power domain? Is this
because we don't have support for one device being in multiple power
domains? That is supported now as far as I recall, by basically making
dummy platform devices like this. So maybe this code isn't necessary and
we can have one platform device for the clk controller and then have it
control certain power domains manually from runtime PM callbacks? It's
possible the runtime PM callbacks are too simple for this case and we
need to tell clk providers what clk is having runtime PM enabled for it.
Maybe we can adjust the core clk framework to introduce a callback for
the clk that is runtime PM enabling and put the logic there about what
to do?

Re: [PATCH V6 05/12] clk: imx: scu: allow scu clk to take device pointer

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:49)
> Used to support runtime pm.
> 
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH V6 06/12] clk: imx: scu: add runtime pm support

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:50)
> Add runtime pm support
> 
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH V6 08/12] clk: imx: imx8qxp-lpcg: add parsing clocks from device tree

[Stephen Boyd]

Quoting Dong Aisheng (2020-03-15 06:43:52)
> One LPCG controller supports up to 8 clock outputs while each of them
> is fixed to 4 bits.  It supports only gating function with fixed bits.
> So we can use the clk-indices to fetch the corresponding clock idx from
> device tree. With this way, we can write a generic LPCG clock drivers.
> 
> This patch add that support to parse clocks from device tree.
> 
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

RE: [PATCH V6 03/12] clk: imx: scu: add two cells binding support

[Aisheng Dong]

Hi Stephen,

Thanks for the review.

> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Tuesday, May 5, 2020 1:08 PM
> 
> Quoting Dong Aisheng (2020-03-15 06:43:47)
> > This patch implements the new two cells binding for SCU clocks.
> > The usage is as follows:
> > clocks = <&uart0_clk IMX_SC_R_UART_0 IMX_SC_PM_CLK_PER>
> >
> > Due to each SCU clock is associated with a power domain, without power
> > on the domain, the SCU clock can't work. So we create platform devices
> > for each domain clock respectively and manually attach the required
> > domain before register the clock devices, then we can register clocks
> > in the clock platform driver accordingly.
> 
> That's odd. See below.
> 
> >
> > Note because we do not have power domain info in device tree and the
> > SCU resource ID is the same for power domain and clock, so we use
> > resource ID to find power domains.
> >
> > Later, we will also use this clock platform driver to support
> > suspend/resume and runtime pm.
> >
> > Cc: Stephen Boyd <sboyd@kernel.org>
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: Michael Turquette <mturquette@baylibre.com>
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> [...]
> > diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> > index b8b2072742a5..4fadff14d8b2 100644
> > --- a/drivers/clk/imx/clk-scu.c
> > +++ b/drivers/clk/imx/clk-scu.c
> > @@ -8,6 +8,9 @@
> >  #include <linux/arm-smccc.h>
> >  #include <linux/clk-provider.h>
> >  #include <linux/err.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
> >  #include <linux/slab.h>
> >
> >  #include "clk-scu.h"
> > @@ -16,6 +19,20 @@
> >  #define IMX_SIP_SET_CPUFREQ            0x00
> >
> >  static struct imx_sc_ipc *ccm_ipc_handle;
> > +struct device_node *pd_np;
> > +
> > +struct imx_scu_clk_node {
> > +       const char *name;
> > +       u32 rsrc;
> > +       u8 clk_type;
> > +       const char * const *parents;
> > +       int num_parents;
> > +
> > +       struct clk_hw *hw;
> > +       struct list_head node;
> > +};
> > +
> > +struct list_head imx_scu_clks[IMX_SC_R_LAST];
> >
> >  /*
> >   * struct clk_scu - Description of one SCU clock @@ -128,9 +145,32 @@
> > static inline struct clk_scu *to_clk_scu(struct clk_hw *hw)
> >         return container_of(hw, struct clk_scu, hw);  }
> >
> > -int imx_clk_scu_init(void)
> > +int imx_clk_scu_init(struct device_node *np)
> >  {
> > -       return imx_scu_get_handle(&ccm_ipc_handle);
> > +       struct platform_device *pd_dev;
> > +       u32 clk_cells;
> > +       int ret, i;
> > +
> > +       ret = imx_scu_get_handle(&ccm_ipc_handle);
> > +       if (ret)
> > +               return ret;
> > +
> > +       if (of_property_read_u32(np, "#clock-cells", &clk_cells))
> 
> Why wouldn't there be #clock-cells in the node?

Okay, will remove the check.

> 
> > +               return -EINVAL;
> > +
> > +       if (clk_cells == 2) {
> > +               for (i = 0; i < IMX_SC_R_LAST; i++)
> > +                       INIT_LIST_HEAD(&imx_scu_clks[i]);
> > +
> > +               pd_np = of_find_compatible_node(NULL, NULL,
> "fsl,scu-pd");
> > +               pd_dev = of_find_device_by_node(pd_np);
> > +               if (!pd_dev || !device_is_bound(&pd_dev->dev)) {
> 
> What is device_is_bound() check for? Add a comment?

Yes, I can add a comment in the code.
It is because SCU clock driver depends on SCU power domain to be ready first.

> 
> > +                       of_node_put(pd_np);
> > +                       return -EPROBE_DEFER;
> > +               }
> > +       }
> > +
> > +       return 0;
> >  }
> >
> >  /*
> > @@ -387,3 +427,99 @@ struct clk_hw *__imx_clk_scu(const char *name,
> > const char * const *parents,
> >
> >         return hw;
> >  }
> > +
> > +struct clk_hw *imx_scu_of_clk_src_get(struct of_phandle_args *clkspec,
> > +                                     void *data) {
> > +       unsigned int rsrc = clkspec->args[0];
> > +       unsigned int idx = clkspec->args[1];
> > +       struct list_head *scu_clks = data;
> > +       struct imx_scu_clk_node *clk;
> > +
> > +       list_for_each_entry(clk, &scu_clks[rsrc], node) {
> > +               if (clk->clk_type == idx)
> > +                       return clk->hw;
> > +       }
> > +
> > +       return ERR_PTR(-ENODEV);
> > +}
> > +
> > +static int imx_clk_scu_probe(struct platform_device *pdev) {
> > +       struct device *dev = &pdev->dev;
> > +       struct imx_scu_clk_node *clk = dev_get_platdata(dev);
> > +       struct clk_hw *hw;
> > +
> > +       hw = __imx_clk_scu(clk->name, clk->parents, clk->num_parents,
> > +                          clk->rsrc, clk->clk_type);
> > +       if (IS_ERR(hw))
> > +               return PTR_ERR(hw);
> > +
> > +       clk->hw = hw;
> > +       list_add_tail(&clk->node, &imx_scu_clks[clk->rsrc]);
> > +
> > +       dev_dbg(dev, "register SCU clock rsrc:%d type:%d\n", clk->rsrc,
> > +               clk->clk_type);
> > +
> > +       return 0;
> > +}
> > +
> > +static struct platform_driver imx_clk_scu_driver = {
> > +       .driver = {
> > +               .name = "imx-scu-clk",
> > +               .suppress_bind_attrs = true,
> > +       },
> > +       .probe = imx_clk_scu_probe,
> > +};
> > +builtin_platform_driver(imx_clk_scu_driver);
> > +
> > +static int imx_clk_scu_attach_pd(struct device *dev, u32 rsrc_id) {
> > +       struct of_phandle_args genpdspec = {
> > +               .np = pd_np,
> > +               .args_count = 1,
> > +               .args[0] = rsrc_id,
> > +       };
> > +
> > +       return of_genpd_add_device(&genpdspec, dev); }
> > +
> > +struct clk_hw *imx_clk_scu_alloc_dev(const char *name,
> > +                                    const char * const *parents,
> > +                                    int num_parents, u32 rsrc_id, u8
> > +clk_type) {
> > +       struct imx_scu_clk_node clk = {
> > +               .name = name,
> > +               .rsrc = rsrc_id,
> > +               .clk_type = clk_type,
> > +               .parents = parents,
> > +               .num_parents = num_parents,
> > +       };
> > +       struct platform_device *pdev;
> > +       int ret;
> > +
> > +       pdev = platform_device_alloc(name, PLATFORM_DEVID_NONE);
> > +       if (!pdev) {
> > +               pr_err("%s: failed to allocate scu clk dev rsrc %d
> type %d\n",
> > +                      name, rsrc_id, clk_type);
> > +               return ERR_PTR(-ENOMEM);
> > +       }
> > +
> > +       ret = platform_device_add_data(pdev, &clk, sizeof(clk));
> > +       if (ret) {
> > +               platform_device_put(pdev);
> > +               return ERR_PTR(ret);
> > +       }
> > +
> > +       pdev->driver_override = "imx-scu-clk";
> > +
> > +       ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id);
> 
> Why do we have to allocate a device for each power domain? 

This is mainly for each clock runtime pm and suspend/resume support as they're
independent with each other.

> Is this because we
> don't have support for one device being in multiple power domains? That is
> supported now as far as I recall, by basically making dummy platform devices
> like this. 

I know kernel supports multi power domains, but I didn't realize it could be used
for our case.

> So maybe this code isn't necessary and we can have one platform
> device for the clk controller and then have it control certain power domains
> manually from runtime PM callbacks? It's possible the runtime PM callbacks are
> too simple for this case and we need to tell clk providers what clk is having
> runtime PM enabled for it.

To make sure I understand correctly, do you mean we use only one general clk controller
Runtime pm callback to handle all clocks runtime pm status manually?
If doing that, how do we handle different clocks pm requirements with only one device runtime
pm status (clock controller)?
e.g.
One Clock Provider
Consumer A -> Clock A -> Clock Provider resumed -> Clock A resumed
Consumer B -> Clock B (Since Clock Provided is already resumed, no chance to run callback to resume Clock B now).
(Note: assume all clocks need runtime pm enabled for i.MX case)

Or you mean we simply resume all clocks? but that seems lose the granularity
and possibly have no chance to enter runtime suspend anymore once there was one clock in use.

Not sure if I missed something. Please help clarify a bit more.

Right now, I'm a bit afraid this may make things a bit complicated as we have ~150 clocks
and ~150 power domains. Putting them all under one clock controller node in DT may scare people.
And even we did not create platform devices for each clock in the clock driver, using multi-pd
will still result in creating dummy platform devices for each clock automatically by power domain
framework. That means we didn't save any platform devices.

> Maybe we can adjust the core clk framework to introduce a callback for the clk
> that is runtime PM enabling and put the logic there about what to do?

That may help. Since we still only have one device for runtime pm state management,
Still not understand how to do it as it may mix the usage with the runtime pm framework.

Regards
Aisheng

RE: [PATCH V6 12/12] clk: imx: scu: unregister clocks if add provider failed

[Aisheng Dong]

> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Tuesday, May 5, 2020 12:56 PM
> 
> Quoting Dong Aisheng (2020-03-15 06:43:56)
> > Unregister clocks if add provider failed
> >
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> >
> > ---
> 
> Why isn't this squashed in to where it's needed?

There were two reasons:
1. The original code also has the issue, so I thought maybe It could be
an extra fix patch.
2. It saved some rebase conflicts.

But anyway, if you'd like to see it was squashed, I can do it in next version.

> 
> > ChangeLog:
> > v6: new patch
> > ---
> >  drivers/clk/imx/clk-imx8qxp.c | 11 +++++++++--
> >  drivers/clk/imx/clk-scu.c     | 13 +++++++++++++
> >  drivers/clk/imx/clk-scu.h     |  2 ++
> >  3 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx8qxp.c
> > b/drivers/clk/imx/clk-imx8qxp.c index 2ec3e0c4749d..e615214495c0
> > 100644
> > --- a/drivers/clk/imx/clk-imx8qxp.c
> > +++ b/drivers/clk/imx/clk-imx8qxp.c
> > @@ -138,10 +138,17 @@ static int imx8qxp_clk_probe(struct
> platform_device *pdev)
> >                                 i, PTR_ERR(clks[i]));
> >         }
> >
> > -       if (clk_cells == 2)
> > +       if (clk_cells == 2) {
> >                 ret = of_clk_add_hw_provider(ccm_node,
> imx_scu_of_clk_src_get, imx_scu_clks);
> > -       else
> > +               if (ret)
> > +                       imx_clk_scu_unregister();
> > +       } else {
> > +               /*
> > +                * NOTE: we did not unregister clocks for the legacy way
> cause
> > +                * it will be removed later.
> 
> I got confused what 'it' was. I think it's the legacy way entirely.
> Maybe say "legacy binding code path doesn't unregister here because..."

Thanks for the suggestion.
I will change to use your version 😊

Regards
Aisheng

> 
> > +                */
> >                 ret = of_clk_add_hw_provider(ccm_node,
> > of_clk_hw_onecell_get, clk_data);
> > +       }
> >
> >         return ret;
> >  }

RE: [PATCH V6 03/12] clk: imx: scu: add two cells binding support

[Aisheng Dong]

> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Friday, June 19, 2020 12:05 AM
> 
> Quoting Aisheng Dong (2020-06-01 07:28:57)
> > > From: Stephen Boyd <sboyd@kernel.org>
> > > Sent: Wednesday, May 27, 2020 4:34 PM
> > >
> > > Quoting Aisheng Dong (2020-05-05 06:47:02)
> > > > Hi Stephen,
> > > >
> > > > Thanks for the review.
> > > >
> > > > > From: Stephen Boyd <sboyd@kernel.org>
> > > > > Sent: Tuesday, May 5, 2020 1:08 PM
> > > > >
> > > > > Quoting Dong Aisheng (2020-03-15 06:43:47)
> > > > > > This patch implements the new two cells binding for SCU clocks.
> > > > > > The usage is as follows:
> > > > > > clocks = <&uart0_clk IMX_SC_R_UART_0 IMX_SC_PM_CLK_PER>
> > > > > >
> > > > > > Due to each SCU clock is associated with a power domain,
> > > > > > without power on the domain, the SCU clock can't work. So we
> > > > > > create platform devices for each domain clock respectively and
> > > > > > manually attach the required domain before register the clock
> > > > > > devices, then we can register clocks in the clock platform driver
> accordingly.
> > > > >
> > > > > That's odd. See below.
> > > > >
> > > > > >
> > > > > > Note because we do not have power domain info in device tree
> > > > > > and the SCU resource ID is the same for power domain and
> > > > > > clock, so we use resource ID to find power domains.
> > > > > >
> > > > > > Later, we will also use this clock platform driver to support
> > > > > > suspend/resume and runtime pm.
> > > > > >
> > > > > > Cc: Stephen Boyd <sboyd@kernel.org>
> > > > > > Cc: Shawn Guo <shawnguo@kernel.org>
> > > > > > Cc: Sascha Hauer <kernel@pengutronix.de>
> > > > > > Cc: Michael Turquette <mturquette@baylibre.com>
> > > > > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > > > > ---
> > > > > [...]
> > > > > > diff --git a/drivers/clk/imx/clk-scu.c
> > > > > > b/drivers/clk/imx/clk-scu.c index b8b2072742a5..4fadff14d8b2
> > > > > > 100644
> > > > > > --- a/drivers/clk/imx/clk-scu.c
> > > > > > +++ b/drivers/clk/imx/clk-scu.c
> > > >
> > > > >
> > > > > > +               return -EINVAL;
> > > > > > +
> > > > > > +       if (clk_cells == 2) {
> > > > > > +               for (i = 0; i < IMX_SC_R_LAST; i++)
> > > > > > +                       INIT_LIST_HEAD(&imx_scu_clks[i]);
> > > > > > +
> > > > > > +               pd_np = of_find_compatible_node(NULL, NULL,
> > > > > "fsl,scu-pd");
> > > > > > +               pd_dev = of_find_device_by_node(pd_np);
> > > > > > +               if (!pd_dev || !device_is_bound(&pd_dev->dev))
> > > > > > + {
> > > > >
> > > > > What is device_is_bound() check for? Add a comment?
> > > >
> > > > Yes, I can add a comment in the code.
> > > > It is because SCU clock driver depends on SCU power domain to be ready
> first.
> > >
> > > Does EPROBE_DEFER not kick in automatically in that case?
> > >
> >
> > No, there're no power domains under scu clock node in dts.
> > So can't use PROBE_DEFER automatically.
> 
> Why isn't the scu clock node using power domains?
> 

We have tried this in several ways before but finally didn't go that way
due to various reasons.

e.g.
Method 1:
conn-scu-clock-controller {
        compatible = "fsl,imx8qxp-clk", "fsl,scu-clk";
        #address-cells = <1>;
        #size-cells = <0>;

        uart0_clk: clock-scu@57 {
        reg = <57>; 
                #clock-cells = <1>;
                clock-indices = <IMX_SC_PM_CLK_PER>;
                clock-output-names = "uart0_clk";
                power-domains = <&pd IMX_SC_R_UART_0>;
        };
        ...
}

Method 2:
#define IMX_SCU_CLK_ID(rsrc, type)      (type << 16 | rsrc)
scu_clk: scu-clock-controller {
        compatible = "fsl,imx8qxp-scu-clk", "fsl,scu-clk";
        #clock-cells = <1>;
        clock-indices = <IMX_SCU_CLK_ID(IMX_SC_R_ENET_0, IMX_SC_PM_CLK_PER)>,
                        <IMX_SCU_CLK_ID(IMX_SC_R_ENET_0, IMX_SC_PM_CLK_BYPASS)>,
                        <IMX_SCU_CLK_ID(IMX_SC_R_ENET_0, IMX_SC_PM_CLK_MISC0)>,
                        <IMX_SCU_CLK_ID(IMX_SC_R_UART_0, IMX_SC_PM_CLK_PER)>,
                        ...

        clock-output-names = "enet0_clk",
                             "enet0_bypass_clk",
                             "enet0_rgmii_clk",
                             "uart0_clk",
                             ...

        power-domains = <&pd IMX_SC_R_ENET_0>,
                        <&pd IMX_SC_R_ENET_0>,
                        <&pd IMX_SC_R_ENET_0>,
                        <&pd IMX_SC_R_UART_0>,
                        ...
};

See details here:
https://spinics.net/lists/devicetree/msg298642.html

> >
> > > > > > +
> > > > > > +       pdev->driver_override = "imx-scu-clk";
> > > > > > +
> > > > > > +       ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id);
> > > > >
> > > > > Why do we have to allocate a device for each power domain?
> > > >
> > > > This is mainly for each clock runtime pm and suspend/resume
> > > > support as they're independent with each other.
> > > >
> > > > > Is this because we
> > > > > don't have support for one device being in multiple power domains?
> > > > > That is supported now as far as I recall, by basically making
> > > > > dummy platform devices like this.
> > > >
> > > > I know kernel supports multi power domains, but I didn't realize
> > > > it could be used for our case.
> > > >
> > > > > So maybe this code isn't necessary and we can have one platform
> > > > > device for the clk controller and then have it control certain
> > > > > power domains manually from runtime PM callbacks? It's possible
> > > > > the runtime PM callbacks are too simple for this case and we
> > > > > need to tell clk providers what clk is having runtime PM enabled for it.
> > > >
> > > > To make sure I understand correctly, do you mean we use only one
> > > > general clk controller
> > >
> > > Typically I see HW designers make one hardware block for a certain
> > > group of clks, like general purpose IO hardware, multimedia hardware, DSP
> hardware, etc.
> > > That isn't always true though. Sometimes I see the hardware
> > > designers decide to stamp down the same hard macro for a clk in
> > > every IP block they develop in the SoC. It gets interesting when
> > > they have to integrate with a third party core like dwc3. Usually
> > > they look to see if they can add some vendor specific register to the IP block
> or make a wrapper around the IP block with the SoC glue bits.
> > >
> > > This usually happens when the clk hard macro library is used by all
> > > the hardware designers. Instead of consolidating that library into a
> > > hardware IP block that gets delivered for the particular SoC they
> > > just spread the hardware block around the system and hope that software
> will figure it out.
> > >
> > > I suspect hardware blocks are given certain addresses on the MMIO
> > > bus because ARM's AMBA/AHB spec mandated that a "device" starts at a 1K
> aligned address.
> > > This probably forced SoC hardware engineers to split their hardware
> > > blocks up into different pieces that they could then plug into the
> > > bus and easily route CPU addresses to different slaves on the bus.
> > >
> > > What's the situation here? I thought that this was a firmware
> > > interface that logically combines all the hardware block clk
> > > controllers. The firmware interface doesn't do everything though
> > > because it seems we still have to power on power domains to use various
> clks?
> >
> > Yes, it is.
> > For MX8QM/QXP, the whole SoC is comprised of a few HW Subsystems.
> > Those HW Subsystems are independent with each other as they have
> > separate power/clock Controllers.
> > Within each subsystem, there's a DSC (distributed slave controller)
> > acting as a standard interface to System Controller (SCU) to provide
> power/clock/reset and some other misc control functions.
> 
> If it's a few then there are maybe 5 SCUs in the SoC?
> 

There's only one SCU in the SoC running on a dedicated M4 core.
SCU communicated with each Subsystem via standard DSC interface
to provide centralized clock/power/reset/pinctrl/resource management/misc services.

> >
> > >
> > > > Runtime pm callback to handle all clocks runtime pm status manually?
> > > > If doing that, how do we handle different clocks pm requirements
> > > > with only one device runtime pm status (clock controller)?
> > > > e.g.
> > > > One Clock Provider
> > > > Consumer A -> Clock A -> Clock Provider resumed -> Clock A resumed
> > > > Consumer B -> Clock B (Since Clock Provided is already resumed, no
> > > > chance to
> > > run callback to resume Clock B now).
> > > > (Note: assume all clocks need runtime pm enabled for i.MX case)
> > > >
> > > > Or you mean we simply resume all clocks? but that seems lose the
> > > > granularity and possibly have no chance to enter runtime suspend
> > > > anymore
> > > once there was one clock in use.
> > > >
> > > > Not sure if I missed something. Please help clarify a bit more.
> > > >
> > > > Right now, I'm a bit afraid this may make things a bit complicated
> > > > as we have ~150 clocks and ~150 power domains. Putting them all
> > > > under one
> > > clock controller node in DT may scare people.
> > > > And even we did not create platform devices for each clock in the
> > > > clock driver, using multi-pd will still result in creating dummy
> > > > platform devices for each clock automatically by power domain
> framework.
> > > That means we didn't save any platform devices.
> > >
> > > What software entity is providing the power domains? I wonder if
> > > things are going about all wrong and the power domains are provided
> > > by the same piece of code that's managing the clks?
> >
> > There's a separate power domain driver based on SCU protocol for only power
> control.
> > This power control is a lit complicated as it supports multi low power states.
> > When power is completely gated, the HW state will be lost and need restore
> by SW.
> > Some of those features are still not upstreamed.
> >
> > > If so, why can't we control the power domains directly from the clk
> > > code without having to go through runtime PM layer or genpd?
> >
> > It's probably hard to do that because:
> > 1. power domain supports multiple low power states for different purpose.
> > Now genpd seems a better place to handle it 2. Not all power domains
> > associated with a clock.
> >
> > > Or to flip it the other way, why do we need clk control for
> > > enable/disable in that case?
> > > Can we just expose rate control for the clks and then let genpd
> > > handle turning clks on an off if they're associated with that power
> > > domain?
> >
> > I wonder this way might also lose the granularity control capability.
> > For a simple device with only one clock, we might be able to do it at most
> times.
> > But for some complicated devices with multiple clocks and which clocks
> > to use depends on the real user cases, simply enable all clocks when PD is UP
> seems not very suitable.
> > For MX8, it's a complicated systems and some device resources may have
> > multi clocks, especially for Audio, Video IO, Display related cases.
> 
> Do they have multiple clks and multiple power domains for one device, like
> Audio or Video? Is it 1:1 between some number of clks and some power
> domain?
> 

For simple devices with only one clock, it might be 1:1.
But for complicated devices with multiple clocks, it's usually not 1:1.
e.g.
AUDIO_PLL_0     PLL     User programmable PLL
AUDIO_PLL_0     MISC0   Audio PLL div 0
AUDIO_PLL_0     MISC1   Audio rec 0

DC_0    MISC0   Display 0
DC_0    MISC1   Display 1

MIPI_0  BYPASS  Bypass
MIPI_0  SLV_BUS DSI rx escape
MIPI_0  MST_BUS DSI tx escape
MIPI_0  PHY     DPHY PLL ref
MIPI_0  PER     DPI - pixel

> >
> > >
> > > >
> > > > > Maybe we can adjust the core clk framework to introduce a
> > > > > callback for the clk that is runtime PM enabling and put the
> > > > > logic there about what to
> > > do?
> > > >
> > > > That may help. Since we still only have one device for runtime pm
> > > > state management, Still not understand how to do it as it may mix
> > > > the usage
> > > with the runtime pm framework.
> > > >
> > >
> > > Right. I'm thinking that we may need a clk_op that is called when a
> > > clk is runtime PM enabled so the device driver that provides that
> > > clk can decide what genpds to manually enable. This would only be
> > > used in the case where the clks are provided by a clk controller
> > > that exists in multiple power domains. I'm not super clear on
> > > multi-pd and how it interacts with runtime PM but I assume that it
> > > doesn't actually turn on any genpd when the consumer device is runtime
> PM active. Instead the consumer driver has to figure out what to power on by
> itself.
> > > The callback would do this similarly from the CCF so that the manual
> > > steps could be done again.
> >
> > After more thinking, I guess we probably don't need a clk_op if we
> > register the correct struct device (abstract clk provider) to clk register API. E.g.
> clk_hw_register(dev, hw).
> > That abstract struct dev could be the virtual power domain device
> > created and returned by PD framework when calling multi PD APIs. This
> > can make us be able to fully reuse all the current rumtime PM support
> > in CCF. Nothing else special need to care. (synchronization work with
> > rpm/genpd framework)
> 
> Whatever 'dev' is used to register the clk is presumed to be the device that is the
> clk provider. If 'dev' isn't the one that's matched to the device node in DT the
> CCF will become confused about what parent clks are supposed to be. That's
> one problem but it may not be a problem here because this is just one big DT
> node?
> 
> >
> > See: multi pd API will create a virtual PD device to be associated with that PD
> for later RPM usages.
> > struct device *dev_pm_domain_attach_by_name(struct device *dev,
> >                                             const char *name) {
> >         if (dev->pm_domain)
> >                 return ERR_PTR(-EEXIST);
> >
> >         return genpd_dev_pm_attach_by_name(dev, name); }
> > EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_name);
> >
> > However, I'm still a bit feeling this probably could work for a clock controller
> simply with <10 PDs.
> > But not very suitable for MX8QM/QXP as it has 150+ PDs. Defining them
> > all will make DT looks terrible and drivers slightly a bit complicated
> > to fetch those PDs efficiently from DT for each clock, also make the future
> maintain work a bit harder when adding new clocks.
> > Not sure whether it's worth to do that as compared with current
> > approach, it really does not Improve too much as multi pd also need create
> virtual devices if the virtual devices are the only concern.
> > In contrast, it might increase, probably unnecessary complexities.
> 
> Either way there are going to be 150+ devices for the 150+ power domains in
> the system.  Why are there so many power domains? That seems like the
> problem. I doubt there are 150 devices in the SoC, or even 75 of them.
> 

It's abstract power domain concept introduced by SCU firmware that each device
Is associated with a power domain.
In real HW, many devices within the same Subsystem might share one of the same HW
power domain. But SCU firmware hides those internal details completely to users.
So users are unaware of it.

Additionally, power domain are also associated with device resource management by
xRDC (Resource Domain Controller) component for security reasons.

Without enable the resource power by SCU firmware, one SW execution environment 
(e.g. OS/ATF/OPTEE/VM) can't access those resource like registers, clocks and etc.

> >
> > How would you suggest?
> > Do you think if we should continue to investigate this Or if we can
> > make the function work first as this improvement work does not affect users?
> > Currently as this patch is a very fundamental change which blocks a lot other
> drivers upstream work.
> >
> 
> If you want to go ahead with this patch I guess I will stop caring.

Thanks a lot

> Just be aware
> that needing to make virtual devices to fit this into the kernel looks wrong. I'd
> like to see one device node for the firmware interface of clks, and one struct
> device for said firmware interface that attaches to one struct driver for that
> firmware interface.
> 

So far, I'm still no sure if we can finally archive that due to complexities of
various requirements in MX8. Personally i feel a bit risky and trying it definitely need
lot more time to ensure it can meet all of our feature requirements on MX8.
But I did understand your concern. Anyway, I will continue to investigate.
However, if you do not mind too much, I'd treat it as a continued improvement work
in order to not block the whole MX8 upstream work too long.
(dt binding part of users won't change)

The current approach has been verified working well in NXP official released 5.4 kernel and
can meet various product requirements. Probably could be a good start point for the reset 
drivers upstreaming work first.

> Connecting that to genpd for proper power domain management is another
> issue. If the core clk framework needs to be multi-device power domain aware
> so that various clks can turn on along with the power domain that's needed to
> operate them then we'll need to add that in the core framework.
> 

Yes. I guess the problem for MX8 is how to co-work with the exist power domain driver.
For MX8, there're also more issues because MX8 power domain supports multiple lower power
States. And we need a genpd governer to help choose a state. That's the future work we may do
In the next step.
If we power up clock power in CCF, we need think how to do It well by co-exist with genpd framework.

> I'm amazed that the firmware interface doesn't handle the requirement that
> genpds need to be powered on to control the clks. I'd think the firmware would
> be happy to turn on power domains so that it can go poke clk registers.

I guess one reason may be SCU support multiple low power states.
It wants user to choose which one to enter.
/*
 * Defines for SC PM Power Mode
 */
#define IMX_SC_PM_PW_MODE_OFF   0       /* Power off */
#define IMX_SC_PM_PW_MODE_STBY  1       /* Power in standby */
#define IMX_SC_PM_PW_MODE_LP    2       /* Power in low-power */
#define IMX_SC_PM_PW_MODE_ON    3       /* Power on */

Regards
Aisheng