From 4b0556a441dd37e598887215bc89b49a6ef525b3 Mon Sep 17 00:00:00 2001 From: Shawn Lin Date: Wed, 21 Mar 2018 10:39:19 +0800 Subject: [PATCH 1/5] clk: rockchip: Fix wrong parent for SDMMC phase clock for rk3228 commit c420c1e4db22 ("clk: rockchip: Prevent calculating mmc phase if clock rate is zero") catches one gremlin again for clk-rk3228.c that the parent of SDMMC phase clock should be sclk_sdmmc0, but not sclk_sdmmc. However, the naming of the sdmmc clocks varies in the manual with the card clock having the 0 while the hclk is named without appended 0. So standardize one one format to prevent confusion, as there also is only one (non-sdio) mmc controller on the soc. Signed-off-by: Shawn Lin Signed-off-by: Heiko Stuebner --- drivers/clk/rockchip/clk-rk3228.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/rockchip/clk-rk3228.c b/drivers/clk/rockchip/clk-rk3228.c index 11e7f2d1c054..7af48184b022 100644 --- a/drivers/clk/rockchip/clk-rk3228.c +++ b/drivers/clk/rockchip/clk-rk3228.c @@ -387,7 +387,7 @@ static struct rockchip_clk_branch rk3228_clk_branches[] __initdata = { RK2928_CLKSEL_CON(23), 5, 2, MFLAGS, 0, 6, DFLAGS, RK2928_CLKGATE_CON(2), 15, GFLAGS), - COMPOSITE(SCLK_SDMMC, "sclk_sdmmc0", mux_mmc_src_p, 0, + COMPOSITE(SCLK_SDMMC, "sclk_sdmmc", mux_mmc_src_p, 0, RK2928_CLKSEL_CON(11), 8, 2, MFLAGS, 0, 8, DFLAGS, RK2928_CLKGATE_CON(2), 11, GFLAGS), From ce84eca927af24ca27897ba5fee4fbeed443d5fc Mon Sep 17 00:00:00 2001 From: Shawn Lin Date: Wed, 21 Mar 2018 10:39:18 +0800 Subject: [PATCH 2/5] clk: rockchip: Fix wrong parents for MMC phase clock for rk3328 commit c420c1e4db22 ("clk: rockchip: Prevent calculating mmc phase if clock rate is zero") catches some gremlins for clk-rk3328.c that the parents of MMC phase clock should be clk_{sdmmc, sdio, emmc}, but not sclk_{sdmmc, sdio, emmc}. Signed-off-by: Shawn Lin Signed-off-by: Heiko Stuebner --- drivers/clk/rockchip/clk-rk3328.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/clk/rockchip/clk-rk3328.c b/drivers/clk/rockchip/clk-rk3328.c index f680b421b6d5..252366a5231f 100644 --- a/drivers/clk/rockchip/clk-rk3328.c +++ b/drivers/clk/rockchip/clk-rk3328.c @@ -810,24 +810,24 @@ static struct rockchip_clk_branch rk3328_clk_branches[] __initdata = { GATE(0, "pclk_phy_niu", "pclk_phy_pre", 0, RK3328_CLKGATE_CON(15), 15, GFLAGS), /* PD_MMC */ - MMC(SCLK_SDMMC_DRV, "sdmmc_drv", "sclk_sdmmc", + MMC(SCLK_SDMMC_DRV, "sdmmc_drv", "clk_sdmmc", RK3328_SDMMC_CON0, 1), - MMC(SCLK_SDMMC_SAMPLE, "sdmmc_sample", "sclk_sdmmc", + MMC(SCLK_SDMMC_SAMPLE, "sdmmc_sample", "clk_sdmmc", RK3328_SDMMC_CON1, 1), - MMC(SCLK_SDIO_DRV, "sdio_drv", "sclk_sdio", + MMC(SCLK_SDIO_DRV, "sdio_drv", "clk_sdio", RK3328_SDIO_CON0, 1), - MMC(SCLK_SDIO_SAMPLE, "sdio_sample", "sclk_sdio", + MMC(SCLK_SDIO_SAMPLE, "sdio_sample", "clk_sdio", RK3328_SDIO_CON1, 1), - MMC(SCLK_EMMC_DRV, "emmc_drv", "sclk_emmc", + MMC(SCLK_EMMC_DRV, "emmc_drv", "clk_emmc", RK3328_EMMC_CON0, 1), - MMC(SCLK_EMMC_SAMPLE, "emmc_sample", "sclk_emmc", + MMC(SCLK_EMMC_SAMPLE, "emmc_sample", "clk_emmc", RK3328_EMMC_CON1, 1), - MMC(SCLK_SDMMC_EXT_DRV, "sdmmc_ext_drv", "sclk_sdmmc_ext", + MMC(SCLK_SDMMC_EXT_DRV, "sdmmc_ext_drv", "clk_sdmmc_ext", RK3328_SDMMC_EXT_CON0, 1), - MMC(SCLK_SDMMC_EXT_SAMPLE, "sdmmc_ext_sample", "sclk_sdmmc_ext", + MMC(SCLK_SDMMC_EXT_SAMPLE, "sdmmc_ext_sample", "clk_sdmmc_ext", RK3328_SDMMC_EXT_CON1, 1), }; From 570fda972b3178f9a981d1b7ac18e05245a52eab Mon Sep 17 00:00:00 2001 From: Shawn Lin Date: Wed, 21 Mar 2018 10:39:20 +0800 Subject: [PATCH 3/5] clk: rockchip: Correct the behaviour of restoring cached phase We can't restore every phase, for instance the invalid phase and the phase for coming rate which is out of the scope of boards' ability. And this patch also corrects the error path to return invalid pointer to clk if clk_notifier_register failed introduced by the same offending commit. Fixes: 60cf09e45fbc ("clk: rockchip: Restore the clock phase after the rate was changed") Reported-by: wlq Signed-off-by: Shawn Lin Tested-by: wlq Signed-off-by: Heiko Stuebner --- drivers/clk/rockchip/clk-mmc-phase.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c index dc4c227732bd..b7b9f2e7d64b 100644 --- a/drivers/clk/rockchip/clk-mmc-phase.c +++ b/drivers/clk/rockchip/clk-mmc-phase.c @@ -170,18 +170,30 @@ static int rockchip_mmc_clk_rate_notify(struct notifier_block *nb, unsigned long event, void *data) { struct rockchip_mmc_clock *mmc_clock = to_rockchip_mmc_clock(nb); + struct clk_notifier_data *ndata = data; /* * rockchip_mmc_clk is mostly used by mmc controllers to sample * the intput data, which expects the fixed phase after the tuning * process. However if the clock rate is changed, the phase is stale * and may break the data sampling. So here we try to restore the phase - * for that case. + * for that case, except that + * (1) cached_phase is invaild since we inevitably cached it when the + * clock provider be reparented from orphan to its real parent in the + * first place. Otherwise we may mess up the initialization of MMC cards + * since we only set the default sample phase and drive phase later on. + * (2) the new coming rate is higher than the older one since mmc driver + * set the max-frequency to match the boards' ability but we can't go + * over the heads of that, otherwise the tests smoke out the issue. */ + if (ndata->old_rate <= ndata->new_rate) + return NOTIFY_DONE; + if (event == PRE_RATE_CHANGE) mmc_clock->cached_phase = rockchip_mmc_get_phase(&mmc_clock->hw); - else if (event == POST_RATE_CHANGE) + else if (mmc_clock->cached_phase != -EINVAL && + event == POST_RATE_CHANGE) rockchip_mmc_set_phase(&mmc_clock->hw, mmc_clock->cached_phase); return NOTIFY_DONE; From 0d92d1802ced45dab0cbb1d130ace7410bcaec99 Mon Sep 17 00:00:00 2001 From: Shawn Lin Date: Wed, 21 Mar 2018 10:39:20 +0800 Subject: [PATCH 4/5] clk: rockchip: Fix error return in phase clock registration The newly added clock notifier may return an error code but so far the error output in the function would only return an error pointer from registering the clock. So when the clock notifier fails the clock would be unregistered but the return would still be the clock pointer which could then not be dereferenced correctly. So fix the error handling to prevent that. Fixes: 60cf09e45fbc ("clk: rockchip: Restore the clock phase after the rate was changed") Signed-off-by: Shawn Lin Signed-off-by: Heiko Stuebner --- drivers/clk/rockchip/clk-mmc-phase.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/clk/rockchip/clk-mmc-phase.c b/drivers/clk/rockchip/clk-mmc-phase.c index b7b9f2e7d64b..026a26bb702d 100644 --- a/drivers/clk/rockchip/clk-mmc-phase.c +++ b/drivers/clk/rockchip/clk-mmc-phase.c @@ -223,8 +223,10 @@ struct clk *rockchip_clk_register_mmc(const char *name, mmc_clock->shift = shift; clk = clk_register(NULL, &mmc_clock->hw); - if (IS_ERR(clk)) + if (IS_ERR(clk)) { + ret = PTR_ERR(clk); goto err_register; + } mmc_clock->clk_rate_change_nb.notifier_call = &rockchip_mmc_clk_rate_notify; @@ -237,5 +239,5 @@ struct clk *rockchip_clk_register_mmc(const char *name, clk_unregister(clk); err_register: kfree(mmc_clock); - return clk; + return ERR_PTR(ret); } From 9dc486fdf6cc0d7f635954810ab119c5db2cbb60 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Tue, 20 Mar 2018 10:06:28 +0800 Subject: [PATCH 5/5] clk: rockchip: assign correct id for pclk_ddr and hclk_sd in rk3399 Since hclk_sd and pclk_ddr source clock from CPLL or GPLL, and these two PLL may change their frequency. If we do not assign right id to pclk_ddr and hclk_sd, they will alway use default cur register value, and may get the frequency exceed their signed off frequency. So assign correct Id for them, then we can assign frequency for them in dts. Signed-off-by: Lin Huang Reviewed-by: Douglas Anderson Reviewed-by: Shawn Lin Signed-off-by: Heiko Stuebner --- drivers/clk/rockchip/clk-rk3399.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c index 3e57c6eef93d..bca10d618f0a 100644 --- a/drivers/clk/rockchip/clk-rk3399.c +++ b/drivers/clk/rockchip/clk-rk3399.c @@ -671,7 +671,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = { RK3399_CLKGATE_CON(9), 7, GFLAGS, &rk3399_uart3_fracmux), - COMPOSITE(0, "pclk_ddr", mux_pll_src_cpll_gpll_p, CLK_IGNORE_UNUSED, + COMPOSITE(PCLK_DDR, "pclk_ddr", mux_pll_src_cpll_gpll_p, CLK_IGNORE_UNUSED, RK3399_CLKSEL_CON(6), 15, 1, MFLAGS, 8, 5, DFLAGS, RK3399_CLKGATE_CON(3), 4, GFLAGS), @@ -887,7 +887,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = { RK3399_CLKGATE_CON(31), 8, GFLAGS), /* sdio & sdmmc */ - COMPOSITE(0, "hclk_sd", mux_pll_src_cpll_gpll_p, 0, + COMPOSITE(HCLK_SD, "hclk_sd", mux_pll_src_cpll_gpll_p, 0, RK3399_CLKSEL_CON(13), 15, 1, MFLAGS, 8, 5, DFLAGS, RK3399_CLKGATE_CON(12), 13, GFLAGS), GATE(HCLK_SDMMC, "hclk_sdmmc", "hclk_sd", 0,