On Thu, Oct 27, 2016 at 9:10 PM, Antoine Tenart antoine.tenart@free-electrons.com wrote:
Hi,
On Wed, Oct 26, 2016 at 02:38:10PM +0200, Maxime Ripard wrote:
On Wed, Oct 26, 2016 at 02:10:31PM +0200, Antoine Tenart wrote:
+#ifndef CONFIG_MACH_SUN7I
- /* switch cpuclk to losc */
- clrbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT);
+#endif
Some kind of comment here would be nice.
That's based on my experiments, switching the cpu clk to losc wasn't working (the board hanged). I agree that's not the best explanation ever...
- /* disable ldo */
- clrbits_le32(&ccm->osc24m_cfg, OSC24M_LDO_EN);
I realise I'm the one who suggested doing this, but that might turn out to be wrong. Have you tested devices that use the oscillator directly, like the PWM?
No, and good point.
I think the sun4i-timer and ARM arch timer are also clocked from OSC24M, so turning it off is really not a good idea.
ChenYu
+static void __secure sunxi_clock_leave_idle(struct sunxi_ccm_reg *ccm) +{
- /* enable ldo */
- setbits_le32(&ccm->osc24m_cfg, OSC24M_LDO_EN);
+#ifndef CONFIG_MACH_SUN7I
- /* switch cpuclk to osc24m */
- clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);+#endif
- /* enable pll1 */
- setbits_le32(&ccm->pll1_cfg, CCM_PLL1_CTRL_EN);
It might be worth waiting for the PLL to lock by polling the bit 28 before switching the mux to it.
I was doing this before, but it turned out this wasn't necessary to have the psci suspend function working. It would probably be safer though.
Looks good otherwise,
Thanks for the review!
Antoine
-- Antoine Ténart, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com