| Message ID | 20190128230700.7325-17-stuart.menefy@mathembedded.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Resuscitate Exynos 5260 support | expand |
Hi Stuart On 2019-01-29 00:06, Stuart Menefy wrote: > Add the CLK_IGNORE_UNUSED flag to a number of clocks, to fix problems > seen with suspend/resume: > - AUD UART: if the UART isn't configured (in s3c24xx_serial_set_termios) > then the baudclk isn't set up, and so s3c24xx_serial_resume doesn't > enable the clock prior to accessing the registers, causing an external > abort. > - FIMD1: no idea, but suspend doesn't work if this is off > - TZPCn: on a secure part suspend doesn't work if the TrustZone clocks > are disabled. > > Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com> To force given state of the certain clocks during system suspend/resume, please use .suspend_regs property in samsung_cmu_info. For a good example, please check top_suspend_regs in clk-exynos5433.c Also some really top clocks might be needed to be marked as CRITICAL to avoid disabling them if they are needed by the core SoC logic, which is not yet modeled by the drivers. > --- > drivers/clk/samsung/clk-exynos5260.c | 27 ++++++++++++++------------- > 1 file changed, 14 insertions(+), 13 deletions(-) > > diff --git a/drivers/clk/samsung/clk-exynos5260.c b/drivers/clk/samsung/clk-exynos5260.c > index 9a0024866a36..9635c462f030 100644 > --- a/drivers/clk/samsung/clk-exynos5260.c > +++ b/drivers/clk/samsung/clk-exynos5260.c > @@ -119,7 +119,8 @@ static const struct samsung_gate_clock aud_gate_clks[] __initconst = { > GATE(AUD_SCLK_PCM, "sclk_aud_pcm", "dout_sclk_aud_pcm", > EN_SCLK_AUD, 1, CLK_SET_RATE_PARENT, 0), > GATE(AUD_SCLK_AUD_UART, "sclk_aud_uart", "dout_sclk_aud_uart", > - EN_SCLK_AUD, 2, CLK_SET_RATE_PARENT, 0), > + EN_SCLK_AUD, 2, > + CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0), > > GATE(AUD_CLK_SRAMC, "clk_sramc", "dout_aclk_aud_131", EN_IP_AUD, > 0, 0, 0), > @@ -298,7 +299,7 @@ static const struct samsung_gate_clock disp_gate_clks[] __initconst = { > GATE(DISP_CLK_DSIM1, "clk_dsim1", "mout_aclk_disp_222_user", > EN_IP_DISP, 6, 0, 0), > GATE(DISP_CLK_FIMD1, "clk_fimd1", "mout_aclk_disp_222_user", > - EN_IP_DISP, 7, 0, 0), > + EN_IP_DISP, 7, CLK_IGNORE_UNUSED, 0), > GATE(DISP_CLK_HDMI, "clk_hdmi", "mout_aclk_disp_222_user", > EN_IP_DISP, 8, 0, 0), > GATE(DISP_CLK_HDMIPHY, "clk_hdmiphy", "mout_aclk_disp_222_user", > @@ -1314,27 +1315,27 @@ static const struct samsung_gate_clock peri_gate_clks[] __initconst = { > EN_IP_PERI_SECURE_TOP_RTC, 5, 0, 0), > > GATE(PERI_CLK_TZPC0, "clk_tzpc0", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 10, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 10, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC1, "clk_tzpc1", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 11, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 11, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC2, "clk_tzpc2", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 12, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 12, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC3, "clk_tzpc3", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 13, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 13, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC4, "clk_tzpc4", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 14, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 14, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC5, "clk_tzpc5", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 15, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 15, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC6, "clk_tzpc6", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 16, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 16, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC7, "clk_tzpc7", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 17, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 17, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC8, "clk_tzpc8", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 18, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 18, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC9, "clk_tzpc9", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 19, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 19, CLK_IGNORE_UNUSED, 0), > GATE(PERI_CLK_TZPC10, "clk_tzpc10", "dout_aclk_peri_66", > - EN_IP_PERI_SECURE_TZPC, 20, 0, 0), > + EN_IP_PERI_SECURE_TZPC, 20, CLK_IGNORE_UNUSED, 0), > }; > > static const struct samsung_cmu_info peri_cmu __initconst = { Best regards
Hi Marek Thanks for looking at the patches. I'm just getting version 2 ready, so I wanted to double check a couple of things. On Tue, 29 Jan 2019 at 08:50, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > On 2019-01-29 00:06, Stuart Menefy wrote: > > Add the CLK_IGNORE_UNUSED flag to a number of clocks, to fix problems > > seen with suspend/resume: > > - AUD UART: if the UART isn't configured (in s3c24xx_serial_set_termios) > > then the baudclk isn't set up, and so s3c24xx_serial_resume doesn't > > enable the clock prior to accessing the registers, causing an external > > abort. > > - FIMD1: no idea, but suspend doesn't work if this is off > > - TZPCn: on a secure part suspend doesn't work if the TrustZone clocks > > are disabled. > > > > Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com> > > To force given state of the certain clocks during system suspend/resume, > please use .suspend_regs property in samsung_cmu_info. For a good > example, please check top_suspend_regs in clk-exynos5433.c I've been able to remove the UART clock by making a change to the driver, and the FIMD clock has been moved to suspend_regs. However I've left the TZPCn clocks as CLK_IGNORE_UNUSED, that's what the 5433 does, and I assume the TrustZone clocks need to be running all the time, not just at suspend? > Also some really top clocks might be needed to be marked as CRITICAL to > avoid disabling them if they are needed by the core SoC logic, which is > not yet modeled by the drivers. Looking at the 5433, the only clocks which are marked as CRITICAL are either - clocks which are inputs to other CMU blocks - bus clocks Looking at the 5260, none of the clocks which fall into those categories can be gated, so at least on that basis I'm proposing not marking any clocks as CRITICAL. Does that sound reasonable? Thanks Stuart
Hi Stuart, On 2019-02-06 22:43, Stuart Menefy wrote: > Thanks for looking at the patches. I'm just getting version 2 ready, > so I wanted to double check a couple of things. > > On Tue, 29 Jan 2019 at 08:50, Marek Szyprowski <m.szyprowski@samsung.com> wrote: >> On 2019-01-29 00:06, Stuart Menefy wrote: >>> Add the CLK_IGNORE_UNUSED flag to a number of clocks, to fix problems >>> seen with suspend/resume: >>> - AUD UART: if the UART isn't configured (in s3c24xx_serial_set_termios) >>> then the baudclk isn't set up, and so s3c24xx_serial_resume doesn't >>> enable the clock prior to accessing the registers, causing an external >>> abort. >>> - FIMD1: no idea, but suspend doesn't work if this is off >>> - TZPCn: on a secure part suspend doesn't work if the TrustZone clocks >>> are disabled. >>> >>> Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com> >> To force given state of the certain clocks during system suspend/resume, >> please use .suspend_regs property in samsung_cmu_info. For a good >> example, please check top_suspend_regs in clk-exynos5433.c > I've been able to remove the UART clock by making a change to the > driver, and the FIMD clock has been moved to suspend_regs. However > I've left the TZPCn clocks as CLK_IGNORE_UNUSED, that's what the 5433 > does, and I assume the TrustZone clocks need to be running all the time, > not just at suspend? I've didn't observe that they are needed during normal system operation, but they are definitely needed for suspend/resume cycle. I would prefer to add them to suspend list just in case as we know they have to be enabled. 5433 should be fixed the same way. >> Also some really top clocks might be needed to be marked as CRITICAL to >> avoid disabling them if they are needed by the core SoC logic, which is >> not yet modeled by the drivers. > Looking at the 5433, the only clocks which are marked as CRITICAL are either > - clocks which are inputs to other CMU blocks > - bus clocks > Looking at the 5260, none of the clocks which fall into those > categories can be gated, so at least on that basis I'm proposing not > marking any clocks as CRITICAL. > > Does that sound reasonable? Yes. One can always add additional clocks to critical/suspend lists later when there will be such need. Best regards
diff --git a/drivers/clk/samsung/clk-exynos5260.c b/drivers/clk/samsung/clk-exynos5260.c index 9a0024866a36..9635c462f030 100644 --- a/drivers/clk/samsung/clk-exynos5260.c +++ b/drivers/clk/samsung/clk-exynos5260.c @@ -119,7 +119,8 @@ static const struct samsung_gate_clock aud_gate_clks[] __initconst = { GATE(AUD_SCLK_PCM, "sclk_aud_pcm", "dout_sclk_aud_pcm", EN_SCLK_AUD, 1, CLK_SET_RATE_PARENT, 0), GATE(AUD_SCLK_AUD_UART, "sclk_aud_uart", "dout_sclk_aud_uart", - EN_SCLK_AUD, 2, CLK_SET_RATE_PARENT, 0), + EN_SCLK_AUD, 2, + CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0), GATE(AUD_CLK_SRAMC, "clk_sramc", "dout_aclk_aud_131", EN_IP_AUD, 0, 0, 0), @@ -298,7 +299,7 @@ static const struct samsung_gate_clock disp_gate_clks[] __initconst = { GATE(DISP_CLK_DSIM1, "clk_dsim1", "mout_aclk_disp_222_user", EN_IP_DISP, 6, 0, 0), GATE(DISP_CLK_FIMD1, "clk_fimd1", "mout_aclk_disp_222_user", - EN_IP_DISP, 7, 0, 0), + EN_IP_DISP, 7, CLK_IGNORE_UNUSED, 0), GATE(DISP_CLK_HDMI, "clk_hdmi", "mout_aclk_disp_222_user", EN_IP_DISP, 8, 0, 0), GATE(DISP_CLK_HDMIPHY, "clk_hdmiphy", "mout_aclk_disp_222_user", @@ -1314,27 +1315,27 @@ static const struct samsung_gate_clock peri_gate_clks[] __initconst = { EN_IP_PERI_SECURE_TOP_RTC, 5, 0, 0), GATE(PERI_CLK_TZPC0, "clk_tzpc0", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 10, 0, 0), + EN_IP_PERI_SECURE_TZPC, 10, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC1, "clk_tzpc1", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 11, 0, 0), + EN_IP_PERI_SECURE_TZPC, 11, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC2, "clk_tzpc2", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 12, 0, 0), + EN_IP_PERI_SECURE_TZPC, 12, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC3, "clk_tzpc3", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 13, 0, 0), + EN_IP_PERI_SECURE_TZPC, 13, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC4, "clk_tzpc4", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 14, 0, 0), + EN_IP_PERI_SECURE_TZPC, 14, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC5, "clk_tzpc5", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 15, 0, 0), + EN_IP_PERI_SECURE_TZPC, 15, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC6, "clk_tzpc6", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 16, 0, 0), + EN_IP_PERI_SECURE_TZPC, 16, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC7, "clk_tzpc7", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 17, 0, 0), + EN_IP_PERI_SECURE_TZPC, 17, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC8, "clk_tzpc8", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 18, 0, 0), + EN_IP_PERI_SECURE_TZPC, 18, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC9, "clk_tzpc9", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 19, 0, 0), + EN_IP_PERI_SECURE_TZPC, 19, CLK_IGNORE_UNUSED, 0), GATE(PERI_CLK_TZPC10, "clk_tzpc10", "dout_aclk_peri_66", - EN_IP_PERI_SECURE_TZPC, 20, 0, 0), + EN_IP_PERI_SECURE_TZPC, 20, CLK_IGNORE_UNUSED, 0), }; static const struct samsung_cmu_info peri_cmu __initconst = {
Add the CLK_IGNORE_UNUSED flag to a number of clocks, to fix problems seen with suspend/resume: - AUD UART: if the UART isn't configured (in s3c24xx_serial_set_termios) then the baudclk isn't set up, and so s3c24xx_serial_resume doesn't enable the clock prior to accessing the registers, causing an external abort. - FIMD1: no idea, but suspend doesn't work if this is off - TZPCn: on a secure part suspend doesn't work if the TrustZone clocks are disabled. Signed-off-by: Stuart Menefy <stuart.menefy@mathembedded.com> --- drivers/clk/samsung/clk-exynos5260.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-)