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@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx> >> 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 -- Marek Szyprowski, PhD Samsung R&D Institute Poland