On Mon, Jan 29, 2018 at 04:51:35PM +0100, Simon Horman wrote: > On Mon, Jan 29, 2018 at 04:19:29PM +0100, Simon Horman wrote: > > On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote: > > > Hi Simon, > > > > > > On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@xxxxxxxxxxxx> wrote: > > > > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote: > > > >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@xxxxxxxxxxxx> wrote: > > > >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote: > > > >> >> On Wed, Jan 3, 2018 at 1:18 PM, Simon Horman <horms+renesas@xxxxxxxxxxxx> wrote: > > > >> >> > From: Takeshi Kihara <takeshi.kihara.df@xxxxxxxxxxx> > > > >> >> > This patch adds Z2 clock divider support for R-Car Gen3 SoC. > > > >> >> > > > > >> >> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@xxxxxxxxxxx> > > > >> >> > Signed-off-by: Simon Horman <horms+renesas@xxxxxxxxxxxx> > > > >> > > > >> >> As the CPG/MSSR driver now has suspend/resume support, do we need > > > >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically > > > >> >> by cpufreq during system resume, for both the primary and the secondary > > > >> >> CPU cores? > > > >> > > > > >> > I am a bit unsure. > > > >> > > > > >> > When using the A57 cores, which is the default case, the Z clk is queried > > > >> > by CPUFreq on resume. It appears that on my system its already set to the > > > >> > correct value but I assume if it was not then it would be reset. However, > > > >> > this does not cover Z2 clk. So perhaps to be safe we need to register > > > >> > notifiers and make sure they they play nicely with CPUFreq? > > > >> > > > >> Of course the CPU is special: unlike many other devices, it must be running > > > >> when the kernel is reentered upon system resume. > > > >> It may be running using a different frequency setting, though. > > > >> However, following "opp-suspend", the system will always suspend with the > > > >> Z clock running at 1.5GHz, which is the default? > > > >> So Z is probably OK. > > > >> > > > >> It's more interesting to check what happens when the little cores are > > > >> enabled as well (unfortunately that requires different firmware). > > > >> 1. Does cpufreq handle them correctly when they are onlined again during > > > >> system resume? > > > > > > > > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS > > > > using the instructions at > > > > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support > > > > > > > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary > > > > z2 0 0 1198080000 0 0 > > > > .pll1 1 1 3194880000 0 0 > > > > .pll0 0 0 2995200000 0 0 > > > > z 0 0 1497600000 0 0 > > > > > > You know it's PLL2, not PLL1, you want to look at? ;-) > > > > Sorry, I will double check but I had looked at PLL2 earlier: > > I just messed things up when preparing things to post in email. > > I checked, PLL2 is constant at 2396160000 across the test described > in my earlier email. I have now been able to reproduce an equivalent result on an M3W ES1.0 / Salvator-X. > > > Thanks for checking, looks all good! > > > > Ok, so do you think we can merge this series with > > the off-by-one problem fixed?