| Message ID | 1505468516-3324-1-git-send-email-m.szyprowski@samsung.com |
|---|---|
| State | Accepted |
| Commit | 5dcbeca615ef12047a5f4097b91030cbf995b1d2 |
| Headers | show |
| Series | [v2] clk: samsung: exynos4: Enable VPLL and EPLL clocks for suspend/resume cycle | expand |
Hi Marek, On 2017년 09월 15일 18:41, Marek Szyprowski wrote: > Commit 6edfa11cb396 ("clk: samsung: Add enable/disable operation for > PLL36XX clocks") added enable/disable operations to PLL clocks. Prior that > VPLL and EPPL clocks were always enabled because the enable bit was never > touched. Those clocks have to be enabled during suspend/resume cycle, > because otherwise board fails to enter sleep mode. This patch enables them > unconditionally before entering system suspend state. System restore > function will set them to the previous state saved in the register cache > done before that unconditional enable. > > Fixes: 6edfa11cb396 ("clk: samsung: Add enable/disable operation for PLL36XX clocks") > CC: stable@vger.kernel.org # v4.13 > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > Changelog: > v2: > - wait until PLL is properly locked > --- > drivers/clk/samsung/clk-exynos4.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c > index e40b77583c47..d8d3cb67b402 100644 > --- a/drivers/clk/samsung/clk-exynos4.c > +++ b/drivers/clk/samsung/clk-exynos4.c > @@ -294,6 +294,18 @@ enum exynos4_plls { > #define PLL_ENABLED (1 << 31) > #define PLL_LOCKED (1 << 29) > > +static void exynos4_clk_enable_pll(u32 reg) > +{ > + u32 pll_con = readl(reg_base + reg); > + pll_con |= PLL_ENABLED; > + writel(pll_con, reg_base + reg); > + > + while (!(pll_con & PLL_LOCKED)) { > + cpu_relax(); > + pll_con = readl(reg_base + reg); > + } > +} > + > static void exynos4_clk_wait_for_pll(u32 reg) > { > u32 pll_con; > @@ -315,6 +327,9 @@ static int exynos4_clk_suspend(void) > samsung_clk_save(reg_base, exynos4_save_pll, > ARRAY_SIZE(exynos4_clk_pll_regs)); > > + exynos4_clk_enable_pll(EPLL_CON0); > + exynos4_clk_enable_pll(VPLL_CON0); > + To support runtime pm for Exynos5433 on your patch[1], You get the clock from device-tree and enable the clock on the suspend function (exynos5433_cmu_suspend) as following: [1] [PATCH v9 3/5] clk: samsung: exynos5433: Add support for runtime PM - https://patchwork.kernel.org/patch/9911753/ ------------ static int exynos5433_cmu_suspend(struct device *dev) { struct exynos5433_cmu_data *data = dev_get_drvdata(dev); int i; samsung_clk_save(data->ctx->reg_base, data->clk_save, data->nr_clk_save); for (i = 0; i < data->nr_pclks; i++) clk_prepare_enable(data->pclks[i]); samsung_clk_restore(data->ctx->reg_base, data->clk_suspend, data->nr_clk_suspend); return 0; } ------------ IMHO, I think that you better to get the PLL clock from device-tree instead of adding the duplicate code for enabling PLL(EPLL_CON0/VPLL_CON0). But, Maybe many -EPROBE_DEFER happen on device driver if using the 'platform_driver' method instead of CLK_OF_DECLARE macro. I'm not sure what is more important on below: - Remove the duplicate code - Prevent the -EPROBE_DEFER Anyway, I will follow Sylwester's opinion. > if (exynos4_soc == EXYNOS4210) { > samsung_clk_save(reg_base, exynos4_save_soc, > ARRAY_SIZE(exynos4210_clk_save)); > -- Best Regards, Chanwoo Choi Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017년 09월 15일 19:10, Chanwoo Choi wrote: > Hi Marek, > > On 2017년 09월 15일 18:41, Marek Szyprowski wrote: >> Commit 6edfa11cb396 ("clk: samsung: Add enable/disable operation for >> PLL36XX clocks") added enable/disable operations to PLL clocks. Prior that >> VPLL and EPPL clocks were always enabled because the enable bit was never >> touched. Those clocks have to be enabled during suspend/resume cycle, >> because otherwise board fails to enter sleep mode. This patch enables them >> unconditionally before entering system suspend state. System restore >> function will set them to the previous state saved in the register cache >> done before that unconditional enable. >> >> Fixes: 6edfa11cb396 ("clk: samsung: Add enable/disable operation for PLL36XX clocks") >> CC: stable@vger.kernel.org # v4.13 >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> Changelog: >> v2: >> - wait until PLL is properly locked >> --- >> drivers/clk/samsung/clk-exynos4.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c >> index e40b77583c47..d8d3cb67b402 100644 >> --- a/drivers/clk/samsung/clk-exynos4.c >> +++ b/drivers/clk/samsung/clk-exynos4.c >> @@ -294,6 +294,18 @@ enum exynos4_plls { >> #define PLL_ENABLED (1 << 31) >> #define PLL_LOCKED (1 << 29) >> >> +static void exynos4_clk_enable_pll(u32 reg) >> +{ >> + u32 pll_con = readl(reg_base + reg); >> + pll_con |= PLL_ENABLED; >> + writel(pll_con, reg_base + reg); >> + >> + while (!(pll_con & PLL_LOCKED)) { >> + cpu_relax(); >> + pll_con = readl(reg_base + reg); >> + } >> +} >> + >> static void exynos4_clk_wait_for_pll(u32 reg) >> { >> u32 pll_con; >> @@ -315,6 +327,9 @@ static int exynos4_clk_suspend(void) >> samsung_clk_save(reg_base, exynos4_save_pll, >> ARRAY_SIZE(exynos4_clk_pll_regs)); >> >> + exynos4_clk_enable_pll(EPLL_CON0); >> + exynos4_clk_enable_pll(VPLL_CON0); >> + > > To support runtime pm for Exynos5433 on your patch[1], > You get the clock from device-tree and enable the clock > on the suspend function (exynos5433_cmu_suspend) as following: > > [1] [PATCH v9 3/5] clk: samsung: exynos5433: Add support for runtime PM > - https://patchwork.kernel.org/patch/9911753/ > ------------ > static int exynos5433_cmu_suspend(struct device *dev) > { > struct exynos5433_cmu_data *data = dev_get_drvdata(dev); > int i; > > samsung_clk_save(data->ctx->reg_base, data->clk_save, > data->nr_clk_save); > > for (i = 0; i < data->nr_pclks; i++) > clk_prepare_enable(data->pclks[i]); > > samsung_clk_restore(data->ctx->reg_base, data->clk_suspend, > data->nr_clk_suspend); > > return 0; > } > ------------ > > IMHO, I think that you better to get the PLL clock from device-tree > instead of adding the duplicate code for enabling PLL(EPLL_CON0/VPLL_CON0). > But, Maybe many -EPROBE_DEFER happen on device driver > if using the 'platform_driver' method instead of CLK_OF_DECLARE macro. > > I'm not sure what is more important on below: > - Remove the duplicate code > - Prevent the -EPROBE_DEFER > > Anyway, I will follow Sylwester's opinion. Ah, it is my mistake. the clk-exynos4.c have to use the CLK_OF_DECLARE macro in order to support the timer using CLK_MCT. Please ignore my comment. Feel free to add my reviewed-by tag Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com> ditto. -- Best Regards, Chanwoo Choi Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 15, 2017 at 11:41 AM, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > Commit 6edfa11cb396 ("clk: samsung: Add enable/disable operation for > PLL36XX clocks") added enable/disable operations to PLL clocks. Prior that > VPLL and EPPL clocks were always enabled because the enable bit was never > touched. Those clocks have to be enabled during suspend/resume cycle, > because otherwise board fails to enter sleep mode. This patch enables them > unconditionally before entering system suspend state. System restore > function will set them to the previous state saved in the register cache > done before that unconditional enable. > > Fixes: 6edfa11cb396 ("clk: samsung: Add enable/disable operation for PLL36XX clocks") > CC: stable@vger.kernel.org # v4.13 > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > Changelog: > v2: > - wait until PLL is properly locked > --- > drivers/clk/samsung/clk-exynos4.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c > index e40b77583c47..d8d3cb67b402 100644 > --- a/drivers/clk/samsung/clk-exynos4.c > +++ b/drivers/clk/samsung/clk-exynos4.c > @@ -294,6 +294,18 @@ enum exynos4_plls { > #define PLL_ENABLED (1 << 31) > #define PLL_LOCKED (1 << 29) > > +static void exynos4_clk_enable_pll(u32 reg) > +{ > + u32 pll_con = readl(reg_base + reg); > + pll_con |= PLL_ENABLED; > + writel(pll_con, reg_base + reg); > + > + while (!(pll_con & PLL_LOCKED)) { > + cpu_relax(); > + pll_con = readl(reg_base + reg); > + } Looks good: Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> As a follow up for the fix (separate patch) we could consider adding timeouts to both busy-loops (waiting on PLL_LOCKED). Also the busy loop itself could be factored out (less code duplicate). However both should not be part of this fix. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/15/2017 11:41 AM, Marek Szyprowski wrote: > Commit 6edfa11cb396 ("clk: samsung: Add enable/disable operation for > PLL36XX clocks") added enable/disable operations to PLL clocks. Prior that > VPLL and EPPL clocks were always enabled because the enable bit was never > touched. Those clocks have to be enabled during suspend/resume cycle, > because otherwise board fails to enter sleep mode. This patch enables them > unconditionally before entering system suspend state. System restore > function will set them to the previous state saved in the register cache > done before that unconditional enable. > > Fixes: 6edfa11cb396 ("clk: samsung: Add enable/disable operation for PLL36XX clocks") > CC:stable@vger.kernel.org # v4.13 > Signed-off-by: Marek Szyprowski<m.szyprowski@samsung.com> Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Thanks, can you please resend so the patchwork catches the patch and with clk maintainers in Cc? -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c index e40b77583c47..d8d3cb67b402 100644 --- a/drivers/clk/samsung/clk-exynos4.c +++ b/drivers/clk/samsung/clk-exynos4.c @@ -294,6 +294,18 @@ enum exynos4_plls { #define PLL_ENABLED (1 << 31) #define PLL_LOCKED (1 << 29) +static void exynos4_clk_enable_pll(u32 reg) +{ + u32 pll_con = readl(reg_base + reg); + pll_con |= PLL_ENABLED; + writel(pll_con, reg_base + reg); + + while (!(pll_con & PLL_LOCKED)) { + cpu_relax(); + pll_con = readl(reg_base + reg); + } +} + static void exynos4_clk_wait_for_pll(u32 reg) { u32 pll_con; @@ -315,6 +327,9 @@ static int exynos4_clk_suspend(void) samsung_clk_save(reg_base, exynos4_save_pll, ARRAY_SIZE(exynos4_clk_pll_regs)); + exynos4_clk_enable_pll(EPLL_CON0); + exynos4_clk_enable_pll(VPLL_CON0); + if (exynos4_soc == EXYNOS4210) { samsung_clk_save(reg_base, exynos4_save_soc, ARRAY_SIZE(exynos4210_clk_save));
Commit 6edfa11cb396 ("clk: samsung: Add enable/disable operation for PLL36XX clocks") added enable/disable operations to PLL clocks. Prior that VPLL and EPPL clocks were always enabled because the enable bit was never touched. Those clocks have to be enabled during suspend/resume cycle, because otherwise board fails to enter sleep mode. This patch enables them unconditionally before entering system suspend state. System restore function will set them to the previous state saved in the register cache done before that unconditional enable. Fixes: 6edfa11cb396 ("clk: samsung: Add enable/disable operation for PLL36XX clocks") CC: stable@vger.kernel.org # v4.13 Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- Changelog: v2: - wait until PLL is properly locked --- drivers/clk/samsung/clk-exynos4.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html