devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset
@ 2018-05-17  8:40 Anson Huang
  2018-05-17  8:40 ` [PATCH 2/2] ARM: dts: imx7: correct enet clock settings Anson Huang
  2018-05-17 11:22 ` [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset Stefan Agner
  0 siblings, 2 replies; 4+ messages in thread
From: Anson Huang @ 2018-05-17  8:40 UTC (permalink / raw)
  To: shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland,
	mturquette, sboyd, stefan, adriana.reus, rui.silva
  Cc: Linux-imx, linux-arm-kernel, devicetree, linux-kernel, linux-clk

Correct enet clock CCGR register offset.

CCGR6: IMX7D_ENET_AXI_ROOT_CLK (enet1 and enet2 bus clocks)
CCGR112: IMX7D_ENET1_TIME_ROOT_CLK, IMX7D_ENET1_IPG_ROOT_CLK
CCGR113: IMX7D_ENET2_TIME_ROOT_CLK, IMX7D_ENET2_IPG_ROOT_CLK

IMX7D_ENET_PHY_REF_ROOT_DIV supplies clock for PHY,
no gate after this clock, its parent clock root has gate.
IMX7D_ENET1_REF_ROOT_DIV/IMX7D_ENET2_REF_ROOT_DIV supplies clocks
for enet IPG_CLK_RMII, no gate after the clock, its parent
clock root has gate.

IMX7D_PLL_ENET_MAIN_125M_CLK (anatop pll) supplies clock for
enet RGMII tx_clk.

Based on Andy Duan's patch from the NXP kernel tree.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/clk/imx/clk-imx7d.c             | 11 ++++++-----
 include/dt-bindings/clock/imx7d-clock.h |  4 +++-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 975a20d..485ab49 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -26,6 +26,8 @@ static u32 share_count_sai1;
 static u32 share_count_sai2;
 static u32 share_count_sai3;
 static u32 share_count_nand;
+static u32 share_count_enet1;
+static u32 share_count_enet2;
 
 static const struct clk_div_table test_div_table[] = {
 	{ .val = 3, .div = 1, },
@@ -805,6 +807,10 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	clks[IMX7D_MIPI_DSI_ROOT_CLK] = imx_clk_gate4("mipi_dsi_root_clk", "mipi_dsi_post_div", base + 0x4650, 0);
 	clks[IMX7D_MIPI_CSI_ROOT_CLK] = imx_clk_gate4("mipi_csi_root_clk", "mipi_csi_post_div", base + 0x4640, 0);
 	clks[IMX7D_MIPI_DPHY_ROOT_CLK] = imx_clk_gate4("mipi_dphy_root_clk", "mipi_dphy_post_div", base + 0x4660, 0);
+	clks[IMX7D_ENET1_IPG_ROOT_CLK] = imx_clk_gate2_shared2("enet1_ipg_root_clk", "enet_axi_post_div", base + 0x4700, 0, &share_count_enet1);
+	clks[IMX7D_ENET1_TIME_ROOT_CLK] = imx_clk_gate2_shared2("enet1_time_root_clk", "enet1_time_post_div", base + 0x4700, 0, &share_count_enet1);
+	clks[IMX7D_ENET2_IPG_ROOT_CLK] = imx_clk_gate2_shared2("enet2_ipg_root_clk", "enet_axi_post_div", base + 0x4710, 0, &share_count_enet2);
+	clks[IMX7D_ENET2_TIME_ROOT_CLK] = imx_clk_gate2_shared2("enet2_time_root_clk", "enet2_time_post_div", base + 0x4710, 0, &share_count_enet2);
 	clks[IMX7D_SAI1_ROOT_CLK] = imx_clk_gate2_shared2("sai1_root_clk", "sai1_post_div", base + 0x48c0, 0, &share_count_sai1);
 	clks[IMX7D_SAI1_IPG_CLK]  = imx_clk_gate2_shared2("sai1_ipg_clk",  "ipg_root_clk",  base + 0x48c0, 0, &share_count_sai1);
 	clks[IMX7D_SAI2_ROOT_CLK] = imx_clk_gate2_shared2("sai2_root_clk", "sai2_post_div", base + 0x48d0, 0, &share_count_sai2);
@@ -812,11 +818,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	clks[IMX7D_SAI3_ROOT_CLK] = imx_clk_gate2_shared2("sai3_root_clk", "sai3_post_div", base + 0x48e0, 0, &share_count_sai3);
 	clks[IMX7D_SAI3_IPG_CLK]  = imx_clk_gate2_shared2("sai3_ipg_clk",  "ipg_root_clk",  base + 0x48e0, 0, &share_count_sai3);
 	clks[IMX7D_SPDIF_ROOT_CLK] = imx_clk_gate4("spdif_root_clk", "spdif_post_div", base + 0x44d0, 0);
-	clks[IMX7D_ENET1_REF_ROOT_CLK] = imx_clk_gate4("enet1_ref_root_clk", "enet1_ref_post_div", base + 0x44e0, 0);
-	clks[IMX7D_ENET1_TIME_ROOT_CLK] = imx_clk_gate4("enet1_time_root_clk", "enet1_time_post_div", base + 0x44f0, 0);
-	clks[IMX7D_ENET2_REF_ROOT_CLK] = imx_clk_gate4("enet2_ref_root_clk", "enet2_ref_post_div", base + 0x4500, 0);
-	clks[IMX7D_ENET2_TIME_ROOT_CLK] = imx_clk_gate4("enet2_time_root_clk", "enet2_time_post_div", base + 0x4510, 0);
-	clks[IMX7D_ENET_PHY_REF_ROOT_CLK] = imx_clk_gate4("enet_phy_ref_root_clk", "enet_phy_ref_post_div", base + 0x4520, 0);
 	clks[IMX7D_EIM_ROOT_CLK] = imx_clk_gate4("eim_root_clk", "eim_post_div", base + 0x4160, 0);
 	clks[IMX7D_NAND_RAWNAND_CLK] = imx_clk_gate2_shared2("nand_rawnand_clk", "nand_root_clk", base + 0x4140, 0, &share_count_nand);
 	clks[IMX7D_NAND_USDHC_BUS_RAWNAND_CLK] = imx_clk_gate2_shared2("nand_usdhc_rawnand_clk", "nand_usdhc_root_clk", base + 0x4140, 0, &share_count_nand);
diff --git a/include/dt-bindings/clock/imx7d-clock.h b/include/dt-bindings/clock/imx7d-clock.h
index b2325d3e2..fef0647 100644
--- a/include/dt-bindings/clock/imx7d-clock.h
+++ b/include/dt-bindings/clock/imx7d-clock.h
@@ -455,5 +455,7 @@
 #define IMX7D_SNVS_CLK			442
 #define IMX7D_CAAM_CLK			443
 #define IMX7D_KPP_ROOT_CLK		444
-#define IMX7D_CLK_END			445
+#define IMX7D_ENET1_IPG_ROOT_CLK        445
+#define IMX7D_ENET2_IPG_ROOT_CLK        446
+#define IMX7D_CLK_END			447
 #endif /* __DT_BINDINGS_CLOCK_IMX7D_H */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] ARM: dts: imx7: correct enet clock settings
  2018-05-17  8:40 [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset Anson Huang
@ 2018-05-17  8:40 ` Anson Huang
  2018-05-17 11:22 ` [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset Stefan Agner
  1 sibling, 0 replies; 4+ messages in thread
From: Anson Huang @ 2018-05-17  8:40 UTC (permalink / raw)
  To: shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland,
	mturquette, sboyd, stefan, adriana.reus, rui.silva
  Cc: Linux-imx, linux-arm-kernel, devicetree, linux-kernel, linux-clk

This patch corrects ENET "ipg" and "enet_out"
clock settings according to clock driver's changes.

Based on Andy Duan's patch from the NXP kernel tree.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/boot/dts/imx7d.dtsi | 4 ++--
 arch/arm/boot/dts/imx7s.dtsi | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 200714e..92022ea 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -120,11 +120,11 @@
 			<GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
 			<GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
 			<GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&clks IMX7D_ENET_AXI_ROOT_CLK>,
+		clocks = <&clks IMX7D_ENET2_IPG_ROOT_CLK>,
 			<&clks IMX7D_ENET_AXI_ROOT_CLK>,
 			<&clks IMX7D_ENET2_TIME_ROOT_CLK>,
 			<&clks IMX7D_PLL_ENET_MAIN_125M_CLK>,
-			<&clks IMX7D_ENET_PHY_REF_ROOT_CLK>;
+			<&clks IMX7D_ENET_PHY_REF_ROOT_DIV>;
 		clock-names = "ipg", "ahb", "ptp",
 			"enet_clk_ref", "enet_out";
 		fsl,num-tx-queues=<3>;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 4d42335..cf4ba53 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -1091,11 +1091,11 @@
 					<GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
-				clocks = <&clks IMX7D_ENET_AXI_ROOT_CLK>,
+				clocks = <&clks IMX7D_ENET1_IPG_ROOT_CLK>,
 					<&clks IMX7D_ENET_AXI_ROOT_CLK>,
 					<&clks IMX7D_ENET1_TIME_ROOT_CLK>,
 					<&clks IMX7D_PLL_ENET_MAIN_125M_CLK>,
-					<&clks IMX7D_ENET_PHY_REF_ROOT_CLK>;
+					<&clks IMX7D_ENET_PHY_REF_ROOT_DIV>;
 				clock-names = "ipg", "ahb", "ptp",
 					"enet_clk_ref", "enet_out";
 				fsl,num-tx-queues=<3>;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset
  2018-05-17  8:40 [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset Anson Huang
  2018-05-17  8:40 ` [PATCH 2/2] ARM: dts: imx7: correct enet clock settings Anson Huang
@ 2018-05-17 11:22 ` Stefan Agner
  2018-05-18  1:06   ` Anson Huang
  1 sibling, 1 reply; 4+ messages in thread
From: Stefan Agner @ 2018-05-17 11:22 UTC (permalink / raw)
  To: Anson Huang
  Cc: shawnguo, kernel, fabio.estevam, robh+dt, mark.rutland,
	mturquette, sboyd, adriana.reus, rui.silva, Linux-imx,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk

On 17.05.2018 10:40, Anson Huang wrote:
> Correct enet clock CCGR register offset.
> 
> CCGR6: IMX7D_ENET_AXI_ROOT_CLK (enet1 and enet2 bus clocks)
> CCGR112: IMX7D_ENET1_TIME_ROOT_CLK, IMX7D_ENET1_IPG_ROOT_CLK
> CCGR113: IMX7D_ENET2_TIME_ROOT_CLK, IMX7D_ENET2_IPG_ROOT_CLK
> 
> IMX7D_ENET_PHY_REF_ROOT_DIV supplies clock for PHY,
> no gate after this clock, its parent clock root has gate.
> IMX7D_ENET1_REF_ROOT_DIV/IMX7D_ENET2_REF_ROOT_DIV supplies clocks
> for enet IPG_CLK_RMII, no gate after the clock, its parent
> clock root has gate.
> 
> IMX7D_PLL_ENET_MAIN_125M_CLK (anatop pll) supplies clock for
> enet RGMII tx_clk.

As far as I can tell there are two changes here in one patch:

1. The non existing IMX7D_ENET_PHY_REF_ROOT_CLK gate is removed

2. Shared clock gate for the enet time/ipg clock is taken into account.


I would rather prefer to have separate patches. The device tree change
also does two things, so this would lead to 4 patches total.

We can avoid the device tree change for the PHY clk and even maintain
backward compatibility for that part by renaming
IMX7D_ENET_PHY_REF_ROOT_DIV to IMX7D_ENET_PHY_REF_ROOT_CLK.


So this would end up with the following first patch to address the
PHY_ROOT clock issue:

--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -738,7 +738,7 @@ static void __init imx7d_clocks_init(struct
device_node *ccm_node)
        clks[IMX7D_ENET1_TIME_ROOT_DIV] =
imx_clk_divider2("enet1_time_post_div", "enet1_time_pre_div", base +
0xa780, 0, 6);
        clks[IMX7D_ENET2_REF_ROOT_DIV] =
imx_clk_divider2("enet2_ref_post_div", "enet2_ref_pre_div", base +
0xa800, 0, 6);
        clks[IMX7D_ENET2_TIME_ROOT_DIV] =
imx_clk_divider2("enet2_time_post_div", "enet2_time_pre_div", base +
0xa880, 0, 6);
-       clks[IMX7D_ENET_PHY_REF_ROOT_DIV] =
imx_clk_divider2("enet_phy_ref_post_div", "enet_phy_ref_pre_div", base +
0xa900, 0, 6);
+       clks[IMX7D_ENET_PHY_REF_ROOT_CLK] =
imx_clk_divider2("enet_phy_ref_root_clk", "enet_phy_ref_pre_div", base +
0xa900, 0, 6);
        clks[IMX7D_EIM_ROOT_DIV] = imx_clk_divider2("eim_post_div",
"eim_pre_div", base + 0xa980, 0, 6);
        clks[IMX7D_NAND_ROOT_CLK] = imx_clk_divider2("nand_root_clk",
"nand_pre_div", base + 0xaa00, 0, 6);
        clks[IMX7D_QSPI_ROOT_DIV] = imx_clk_divider2("qspi_post_div",
"qspi_pre_div", base + 0xaa80, 0, 6);
@@ -816,7 +816,6 @@ static void __init imx7d_clocks_init(struct
device_node *ccm_node)
        clks[IMX7D_ENET1_TIME_ROOT_CLK] =
imx_clk_gate4("enet1_time_root_clk", "enet1_time_post_div", base +
0x44f0, 0);
        clks[IMX7D_ENET2_REF_ROOT_CLK] =
imx_clk_gate4("enet2_ref_root_clk", "enet2_ref_post_div", base + 0x4500,
0);
        clks[IMX7D_ENET2_TIME_ROOT_CLK] =
imx_clk_gate4("enet2_time_root_clk", "enet2_time_post_div", base +
0x4510, 0);
-       clks[IMX7D_ENET_PHY_REF_ROOT_CLK] =
imx_clk_gate4("enet_phy_ref_root_clk", "enet_phy_ref_post_div", base +
0x4520, 0);
        clks[IMX7D_EIM_ROOT_CLK] = imx_clk_gate4("eim_root_clk",
"eim_post_div", base + 0x4160, 0);
        clks[IMX7D_NAND_RAWNAND_CLK] =
imx_clk_gate2_shared2("nand_rawnand_clk", "nand_root_clk", base +
0x4140, 0, &share_count_nand);
        clks[IMX7D_NAND_USDHC_BUS_RAWNAND_CLK] =
imx_clk_gate2_shared2("nand_usdhc_rawnand_clk", "nand_usdhc_root_clk",
base + 0x4140, 0, &share_count_nand);


A second patch would then fix the clock gate issue and the third the
unavoidable device tree change for the ipg clock.

--
Stefan


> 
> Based on Andy Duan's patch from the NXP kernel tree.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/clk/imx/clk-imx7d.c             | 11 ++++++-----
>  include/dt-bindings/clock/imx7d-clock.h |  4 +++-
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
> index 975a20d..485ab49 100644
> --- a/drivers/clk/imx/clk-imx7d.c
> +++ b/drivers/clk/imx/clk-imx7d.c
> @@ -26,6 +26,8 @@ static u32 share_count_sai1;
>  static u32 share_count_sai2;
>  static u32 share_count_sai3;
>  static u32 share_count_nand;
> +static u32 share_count_enet1;
> +static u32 share_count_enet2;
>  
>  static const struct clk_div_table test_div_table[] = {
>  	{ .val = 3, .div = 1, },
> @@ -805,6 +807,10 @@ static void __init imx7d_clocks_init(struct
> device_node *ccm_node)
>  	clks[IMX7D_MIPI_DSI_ROOT_CLK] = imx_clk_gate4("mipi_dsi_root_clk",
> "mipi_dsi_post_div", base + 0x4650, 0);
>  	clks[IMX7D_MIPI_CSI_ROOT_CLK] = imx_clk_gate4("mipi_csi_root_clk",
> "mipi_csi_post_div", base + 0x4640, 0);
>  	clks[IMX7D_MIPI_DPHY_ROOT_CLK] = imx_clk_gate4("mipi_dphy_root_clk",
> "mipi_dphy_post_div", base + 0x4660, 0);
> +	clks[IMX7D_ENET1_IPG_ROOT_CLK] =
> imx_clk_gate2_shared2("enet1_ipg_root_clk", "enet_axi_post_div", base
> + 0x4700, 0, &share_count_enet1);
> +	clks[IMX7D_ENET1_TIME_ROOT_CLK] =
> imx_clk_gate2_shared2("enet1_time_root_clk", "enet1_time_post_div",
> base + 0x4700, 0, &share_count_enet1);
> +	clks[IMX7D_ENET2_IPG_ROOT_CLK] =
> imx_clk_gate2_shared2("enet2_ipg_root_clk", "enet_axi_post_div", base
> + 0x4710, 0, &share_count_enet2);
> +	clks[IMX7D_ENET2_TIME_ROOT_CLK] =
> imx_clk_gate2_shared2("enet2_time_root_clk", "enet2_time_post_div",
> base + 0x4710, 0, &share_count_enet2);
>  	clks[IMX7D_SAI1_ROOT_CLK] = imx_clk_gate2_shared2("sai1_root_clk",
> "sai1_post_div", base + 0x48c0, 0, &share_count_sai1);
>  	clks[IMX7D_SAI1_IPG_CLK]  = imx_clk_gate2_shared2("sai1_ipg_clk", 
> "ipg_root_clk",  base + 0x48c0, 0, &share_count_sai1);
>  	clks[IMX7D_SAI2_ROOT_CLK] = imx_clk_gate2_shared2("sai2_root_clk",
> "sai2_post_div", base + 0x48d0, 0, &share_count_sai2);
> @@ -812,11 +818,6 @@ static void __init imx7d_clocks_init(struct
> device_node *ccm_node)
>  	clks[IMX7D_SAI3_ROOT_CLK] = imx_clk_gate2_shared2("sai3_root_clk",
> "sai3_post_div", base + 0x48e0, 0, &share_count_sai3);
>  	clks[IMX7D_SAI3_IPG_CLK]  = imx_clk_gate2_shared2("sai3_ipg_clk", 
> "ipg_root_clk",  base + 0x48e0, 0, &share_count_sai3);
>  	clks[IMX7D_SPDIF_ROOT_CLK] = imx_clk_gate4("spdif_root_clk",
> "spdif_post_div", base + 0x44d0, 0);
> -	clks[IMX7D_ENET1_REF_ROOT_CLK] = imx_clk_gate4("enet1_ref_root_clk",
> "enet1_ref_post_div", base + 0x44e0, 0);
> -	clks[IMX7D_ENET1_TIME_ROOT_CLK] =
> imx_clk_gate4("enet1_time_root_clk", "enet1_time_post_div", base +
> 0x44f0, 0);
> -	clks[IMX7D_ENET2_REF_ROOT_CLK] = imx_clk_gate4("enet2_ref_root_clk",
> "enet2_ref_post_div", base + 0x4500, 0);
> -	clks[IMX7D_ENET2_TIME_ROOT_CLK] =
> imx_clk_gate4("enet2_time_root_clk", "enet2_time_post_div", base +
> 0x4510, 0);
> -	clks[IMX7D_ENET_PHY_REF_ROOT_CLK] =
> imx_clk_gate4("enet_phy_ref_root_clk", "enet_phy_ref_post_div", base +
> 0x4520, 0);
>  	clks[IMX7D_EIM_ROOT_CLK] = imx_clk_gate4("eim_root_clk",
> "eim_post_div", base + 0x4160, 0);
>  	clks[IMX7D_NAND_RAWNAND_CLK] =
> imx_clk_gate2_shared2("nand_rawnand_clk", "nand_root_clk", base +
> 0x4140, 0, &share_count_nand);
>  	clks[IMX7D_NAND_USDHC_BUS_RAWNAND_CLK] =
> imx_clk_gate2_shared2("nand_usdhc_rawnand_clk", "nand_usdhc_root_clk",
> base + 0x4140, 0, &share_count_nand);
> diff --git a/include/dt-bindings/clock/imx7d-clock.h
> b/include/dt-bindings/clock/imx7d-clock.h
> index b2325d3e2..fef0647 100644
> --- a/include/dt-bindings/clock/imx7d-clock.h
> +++ b/include/dt-bindings/clock/imx7d-clock.h
> @@ -455,5 +455,7 @@
>  #define IMX7D_SNVS_CLK			442
>  #define IMX7D_CAAM_CLK			443
>  #define IMX7D_KPP_ROOT_CLK		444
> -#define IMX7D_CLK_END			445
> +#define IMX7D_ENET1_IPG_ROOT_CLK        445
> +#define IMX7D_ENET2_IPG_ROOT_CLK        446
> +#define IMX7D_CLK_END			447
>  #endif /* __DT_BINDINGS_CLOCK_IMX7D_H */

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset
  2018-05-17 11:22 ` [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset Stefan Agner
@ 2018-05-18  1:06   ` Anson Huang
  0 siblings, 0 replies; 4+ messages in thread
From: Anson Huang @ 2018-05-18  1:06 UTC (permalink / raw)
  To: Stefan Agner
  Cc: shawnguo, kernel, Fabio Estevam, robh+dt, mark.rutland,
	mturquette, sboyd, Adriana Reus, rui.silva, dl-linux-imx,
	linux-arm-kernel, devicetree, linux-kernel, linux-clk

Hi, Stefan

Anson Huang
Best Regards!


> -----Original Message-----
> From: Stefan Agner [mailto:stefan@agner.ch]
> Sent: Thursday, May 17, 2018 7:22 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: shawnguo@kernel.org; kernel@pengutronix.de; Fabio Estevam
> <fabio.estevam@nxp.com>; robh+dt@kernel.org; mark.rutland@arm.com;
> mturquette@baylibre.com; sboyd@kernel.org; Adriana Reus
> <adriana.reus@nxp.com>; rui.silva@linaro.org; dl-linux-imx
> <linux-imx@nxp.com>; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-clk@vger.kernel.org
> Subject: Re: [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset
> 
> On 17.05.2018 10:40, Anson Huang wrote:
> > Correct enet clock CCGR register offset.
> >
> > CCGR6: IMX7D_ENET_AXI_ROOT_CLK (enet1 and enet2 bus clocks)
> > CCGR112: IMX7D_ENET1_TIME_ROOT_CLK, IMX7D_ENET1_IPG_ROOT_CLK
> > CCGR113: IMX7D_ENET2_TIME_ROOT_CLK, IMX7D_ENET2_IPG_ROOT_CLK
> >
> > IMX7D_ENET_PHY_REF_ROOT_DIV supplies clock for PHY, no gate after this
> > clock, its parent clock root has gate.
> > IMX7D_ENET1_REF_ROOT_DIV/IMX7D_ENET2_REF_ROOT_DIV supplies
> clocks for
> > enet IPG_CLK_RMII, no gate after the clock, its parent clock root has
> > gate.
> >
> > IMX7D_PLL_ENET_MAIN_125M_CLK (anatop pll) supplies clock for enet
> > RGMII tx_clk.
> 
> As far as I can tell there are two changes here in one patch:
> 
> 1. The non existing IMX7D_ENET_PHY_REF_ROOT_CLK gate is removed
> 
> 2. Shared clock gate for the enet time/ipg clock is taken into account.
> 
> 
> I would rather prefer to have separate patches. The device tree change also
> does two things, so this would lead to 4 patches total.
> 
> We can avoid the device tree change for the PHY clk and even maintain
> backward compatibility for that part by renaming
> IMX7D_ENET_PHY_REF_ROOT_DIV to IMX7D_ENET_PHY_REF_ROOT_CLK.
> 
> 
> So this would end up with the following first patch to address the PHY_ROOT
> clock issue:
> 
> --- a/drivers/clk/imx/clk-imx7d.c
> +++ b/drivers/clk/imx/clk-imx7d.c
> @@ -738,7 +738,7 @@ static void __init imx7d_clocks_init(struct device_node
> *ccm_node)
>         clks[IMX7D_ENET1_TIME_ROOT_DIV] =
> imx_clk_divider2("enet1_time_post_div", "enet1_time_pre_div", base +
> 0xa780, 0, 6);
>         clks[IMX7D_ENET2_REF_ROOT_DIV] =
> imx_clk_divider2("enet2_ref_post_div", "enet2_ref_pre_div", base + 0xa800, 0,
> 6);
>         clks[IMX7D_ENET2_TIME_ROOT_DIV] =
> imx_clk_divider2("enet2_time_post_div", "enet2_time_pre_div", base +
> 0xa880, 0, 6);
> -       clks[IMX7D_ENET_PHY_REF_ROOT_DIV] =
> imx_clk_divider2("enet_phy_ref_post_div", "enet_phy_ref_pre_div", base +
> 0xa900, 0, 6);
> +       clks[IMX7D_ENET_PHY_REF_ROOT_CLK] =
> imx_clk_divider2("enet_phy_ref_root_clk", "enet_phy_ref_pre_div", base +
> 0xa900, 0, 6);
>         clks[IMX7D_EIM_ROOT_DIV] = imx_clk_divider2("eim_post_div",
> "eim_pre_div", base + 0xa980, 0, 6);
>         clks[IMX7D_NAND_ROOT_CLK] = imx_clk_divider2("nand_root_clk",
> "nand_pre_div", base + 0xaa00, 0, 6);
>         clks[IMX7D_QSPI_ROOT_DIV] = imx_clk_divider2("qspi_post_div",
> "qspi_pre_div", base + 0xaa80, 0, 6); @@ -816,7 +816,6 @@ static void __init
> imx7d_clocks_init(struct device_node *ccm_node)
>         clks[IMX7D_ENET1_TIME_ROOT_CLK] =
> imx_clk_gate4("enet1_time_root_clk", "enet1_time_post_div", base + 0x44f0,
> 0);
>         clks[IMX7D_ENET2_REF_ROOT_CLK] =
> imx_clk_gate4("enet2_ref_root_clk", "enet2_ref_post_div", base + 0x4500, 0);
>         clks[IMX7D_ENET2_TIME_ROOT_CLK] =
> imx_clk_gate4("enet2_time_root_clk", "enet2_time_post_div", base + 0x4510,
> 0);
> -       clks[IMX7D_ENET_PHY_REF_ROOT_CLK] =
> imx_clk_gate4("enet_phy_ref_root_clk", "enet_phy_ref_post_div", base +
> 0x4520, 0);
>         clks[IMX7D_EIM_ROOT_CLK] = imx_clk_gate4("eim_root_clk",
> "eim_post_div", base + 0x4160, 0);
>         clks[IMX7D_NAND_RAWNAND_CLK] =
> imx_clk_gate2_shared2("nand_rawnand_clk", "nand_root_clk", base + 0x4140,
> 0, &share_count_nand);
>         clks[IMX7D_NAND_USDHC_BUS_RAWNAND_CLK] =
> imx_clk_gate2_shared2("nand_usdhc_rawnand_clk", "nand_usdhc_root_clk",
> base + 0x4140, 0, &share_count_nand);
> 
> 
> A second patch would then fix the clock gate issue and the third the
> unavoidable device tree change for the ipg clock.
> 
> --
> Stefan
 
I follow your suggestion and re-structure the patch set to 3 patches, please help review it, thanks!

Anson.


> 
> 
> >
> > Based on Andy Duan's patch from the NXP kernel tree.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/clk/imx/clk-imx7d.c             | 11 ++++++-----
> >  include/dt-bindings/clock/imx7d-clock.h |  4 +++-
> >  2 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
> > index 975a20d..485ab49 100644
> > --- a/drivers/clk/imx/clk-imx7d.c
> > +++ b/drivers/clk/imx/clk-imx7d.c
> > @@ -26,6 +26,8 @@ static u32 share_count_sai1;  static u32
> > share_count_sai2;  static u32 share_count_sai3;  static u32
> > share_count_nand;
> > +static u32 share_count_enet1;
> > +static u32 share_count_enet2;
> >
> >  static const struct clk_div_table test_div_table[] = {
> >  	{ .val = 3, .div = 1, },
> > @@ -805,6 +807,10 @@ static void __init imx7d_clocks_init(struct
> > device_node *ccm_node)
> >  	clks[IMX7D_MIPI_DSI_ROOT_CLK] = imx_clk_gate4("mipi_dsi_root_clk",
> > "mipi_dsi_post_div", base + 0x4650, 0);
> >  	clks[IMX7D_MIPI_CSI_ROOT_CLK] = imx_clk_gate4("mipi_csi_root_clk",
> > "mipi_csi_post_div", base + 0x4640, 0);
> >  	clks[IMX7D_MIPI_DPHY_ROOT_CLK] =
> imx_clk_gate4("mipi_dphy_root_clk",
> > "mipi_dphy_post_div", base + 0x4660, 0);
> > +	clks[IMX7D_ENET1_IPG_ROOT_CLK] =
> > imx_clk_gate2_shared2("enet1_ipg_root_clk", "enet_axi_post_div", base
> > + 0x4700, 0, &share_count_enet1);
> > +	clks[IMX7D_ENET1_TIME_ROOT_CLK] =
> > imx_clk_gate2_shared2("enet1_time_root_clk", "enet1_time_post_div",
> > base + 0x4700, 0, &share_count_enet1);
> > +	clks[IMX7D_ENET2_IPG_ROOT_CLK] =
> > imx_clk_gate2_shared2("enet2_ipg_root_clk", "enet_axi_post_div", base
> > + 0x4710, 0, &share_count_enet2);
> > +	clks[IMX7D_ENET2_TIME_ROOT_CLK] =
> > imx_clk_gate2_shared2("enet2_time_root_clk", "enet2_time_post_div",
> > base + 0x4710, 0, &share_count_enet2);
> >  	clks[IMX7D_SAI1_ROOT_CLK] = imx_clk_gate2_shared2("sai1_root_clk",
> > "sai1_post_div", base + 0x48c0, 0, &share_count_sai1);
> >  	clks[IMX7D_SAI1_IPG_CLK]  = imx_clk_gate2_shared2("sai1_ipg_clk",
> > "ipg_root_clk",  base + 0x48c0, 0, &share_count_sai1);
> >  	clks[IMX7D_SAI2_ROOT_CLK] = imx_clk_gate2_shared2("sai2_root_clk",
> > "sai2_post_div", base + 0x48d0, 0, &share_count_sai2); @@ -812,11
> > +818,6 @@ static void __init imx7d_clocks_init(struct device_node
> > *ccm_node)
> >  	clks[IMX7D_SAI3_ROOT_CLK] = imx_clk_gate2_shared2("sai3_root_clk",
> > "sai3_post_div", base + 0x48e0, 0, &share_count_sai3);
> >  	clks[IMX7D_SAI3_IPG_CLK]  = imx_clk_gate2_shared2("sai3_ipg_clk",
> > "ipg_root_clk",  base + 0x48e0, 0, &share_count_sai3);
> >  	clks[IMX7D_SPDIF_ROOT_CLK] = imx_clk_gate4("spdif_root_clk",
> > "spdif_post_div", base + 0x44d0, 0);
> > -	clks[IMX7D_ENET1_REF_ROOT_CLK] =
> imx_clk_gate4("enet1_ref_root_clk",
> > "enet1_ref_post_div", base + 0x44e0, 0);
> > -	clks[IMX7D_ENET1_TIME_ROOT_CLK] =
> > imx_clk_gate4("enet1_time_root_clk", "enet1_time_post_div", base +
> > 0x44f0, 0);
> > -	clks[IMX7D_ENET2_REF_ROOT_CLK] =
> imx_clk_gate4("enet2_ref_root_clk",
> > "enet2_ref_post_div", base + 0x4500, 0);
> > -	clks[IMX7D_ENET2_TIME_ROOT_CLK] =
> > imx_clk_gate4("enet2_time_root_clk", "enet2_time_post_div", base +
> > 0x4510, 0);
> > -	clks[IMX7D_ENET_PHY_REF_ROOT_CLK] =
> > imx_clk_gate4("enet_phy_ref_root_clk", "enet_phy_ref_post_div", base +
> > 0x4520, 0);
> >  	clks[IMX7D_EIM_ROOT_CLK] = imx_clk_gate4("eim_root_clk",
> > "eim_post_div", base + 0x4160, 0);
> >  	clks[IMX7D_NAND_RAWNAND_CLK] =
> > imx_clk_gate2_shared2("nand_rawnand_clk", "nand_root_clk", base +
> > 0x4140, 0, &share_count_nand);
> >  	clks[IMX7D_NAND_USDHC_BUS_RAWNAND_CLK] =
> > imx_clk_gate2_shared2("nand_usdhc_rawnand_clk",
> "nand_usdhc_root_clk",
> > base + 0x4140, 0, &share_count_nand); diff --git
> > a/include/dt-bindings/clock/imx7d-clock.h
> > b/include/dt-bindings/clock/imx7d-clock.h
> > index b2325d3e2..fef0647 100644
> > --- a/include/dt-bindings/clock/imx7d-clock.h
> > +++ b/include/dt-bindings/clock/imx7d-clock.h
> > @@ -455,5 +455,7 @@
> >  #define IMX7D_SNVS_CLK			442
> >  #define IMX7D_CAAM_CLK			443
> >  #define IMX7D_KPP_ROOT_CLK		444
> > -#define IMX7D_CLK_END			445
> > +#define IMX7D_ENET1_IPG_ROOT_CLK        445
> > +#define IMX7D_ENET2_IPG_ROOT_CLK        446
> > +#define IMX7D_CLK_END			447
> >  #endif /* __DT_BINDINGS_CLOCK_IMX7D_H */

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-05-18  1:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-17  8:40 [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset Anson Huang
2018-05-17  8:40 ` [PATCH 2/2] ARM: dts: imx7: correct enet clock settings Anson Huang
2018-05-17 11:22 ` [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset Stefan Agner
2018-05-18  1:06   ` Anson Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).