Re: [PATCH 1/2] clk: imx7d: correct enet clock CCGR register offset

From: Stefan Agner
Date: Thu May 17 2018 - 06:25:36 EST


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@xxxxxxx>
> ---
> 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 */