From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1173433AbdDXQ2a (ORCPT ); Mon, 24 Apr 2017 12:28:30 -0400 Received: from esa1.microchip.iphmx.com ([68.232.147.91]:49493 "EHLO esa1.microchip.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S973562AbdDXQ2K (ORCPT ); Mon, 24 Apr 2017 12:28:10 -0400 X-IronPort-AV: E=Sophos;i="5.37,245,1488870000"; d="scan'208";a="2043243" Subject: Re: [PATCH] clk: at91: Add sama5d2 suspend/resume To: Alexandre Belloni , Boris Brezillon , Stephen Boyd References: <20170414215618.16534-1-alexandre.belloni@free-electrons.com> CC: , From: Nicolas Ferre Organization: microchip Message-ID: Date: Mon, 24 Apr 2017 18:25:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170414215618.16534-1-alexandre.belloni@free-electrons.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 14/04/2017 à 23:56, Alexandre Belloni a écrit : > On sama5d2, VDD core maybe be cut while in suspend. This means registers > will be lost. Ensure they are saved and restored properly. I'm pretty sure we can't just restore them like you do. But the bootloader must have restored the registers already, so maybe an update of the regmap cache should be sufficient... > Signed-off-by: Alexandre Belloni > --- > drivers/clk/at91/pmc.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 108 insertions(+) > > diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c > index 526df5ba042d..5cd1f0a2ea72 100644 > --- a/drivers/clk/at91/pmc.c > +++ b/drivers/clk/at91/pmc.c > @@ -13,7 +13,9 @@ > #include > #include > #include > +#include > #include > +#include > > #include > > @@ -41,3 +43,109 @@ int of_at91_get_clk_range(struct device_node *np, const char *propname, > return 0; > } > EXPORT_SYMBOL_GPL(of_at91_get_clk_range); > + > +#ifdef CONFIG_PM > +static struct regmap *pmcreg; > + > +static struct > +{ > + u32 scsr; > + u32 pcsr0; > + u32 uckr; > + u32 mor; > + u32 mcfr; > + u32 pllar; > + u32 mckr; > + u32 usb; > + u32 imr; > + u32 pcsr1; > + u32 pcr[64]; > +} pmc_cache; > + > +static int pmc_suspend(void) > +{ > + int i; > + > + regmap_read(pmcreg, AT91_PMC_IMR, &pmc_cache.scsr); > + regmap_read(pmcreg, AT91_PMC_PCSR, &pmc_cache.pcsr0); > + regmap_read(pmcreg, AT91_CKGR_UCKR, &pmc_cache.uckr); > + regmap_read(pmcreg, AT91_CKGR_MOR, &pmc_cache.mor); > + regmap_read(pmcreg, AT91_CKGR_MCFR, &pmc_cache.mcfr); > + regmap_read(pmcreg, AT91_CKGR_PLLAR, &pmc_cache.pllar); > + regmap_read(pmcreg, AT91_PMC_MCKR, &pmc_cache.mckr); > + regmap_read(pmcreg, AT91_PMC_USB, &pmc_cache.usb); > + regmap_read(pmcreg, AT91_PMC_IMR, &pmc_cache.imr); > + regmap_read(pmcreg, AT91_PMC_PCSR1, &pmc_cache.pcsr1); > + > + for (i = 2; i < 64; i++) { > + regmap_write(pmcreg, AT91_PMC_PCR, (i & AT91_PMC_PCR_PID_MASK)); > + regmap_read(pmcreg, AT91_PMC_PCR, &pmc_cache.pcr[i]); > + } > + > + return 0; > +} > + > +static bool pmc_ready(unsigned int mask) > +{ > + unsigned int status; > + > + regmap_read(pmcreg, AT91_PMC_SR, &status); > + > + return ((status & mask) == mask) ? 1 : 0; > +} > + > +static void pmc_resume(void) > +{ > + int i; > + unsigned int mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA; ...Moreover, some of these registers needs a particular sequencing to behave properly while writing to it. The rationale behind that (if we can speak about rationale) is that you should only change *one* field at a time and for some fields wait for the proper "ready bit" to show up. A proven good sequence is available in AT91Bootstrap: > + regmap_write(pmcreg, AT91_PMC_IMR, pmc_cache.scsr); > + regmap_write(pmcreg, AT91_PMC_PCER, pmc_cache.pcsr0); > + regmap_write(pmcreg, AT91_CKGR_UCKR, pmc_cache.uckr); > + regmap_write(pmcreg, AT91_CKGR_MOR, pmc_cache.mor); > + regmap_write(pmcreg, AT91_CKGR_MCFR, pmc_cache.mcfr); for this one: > + regmap_write(pmcreg, AT91_CKGR_PLLAR, pmc_cache.pllar); https://github.com/linux4sam/at91bootstrap/blob/master/driver/pmc.c#L156 and this one at least: > + regmap_write(pmcreg, AT91_PMC_MCKR, pmc_cache.mckr); https://github.com/linux4sam/at91bootstrap/blob/master/driver/pmc.c#L168 > + regmap_write(pmcreg, AT91_PMC_USB, pmc_cache.usb); > + regmap_write(pmcreg, AT91_PMC_IMR, pmc_cache.imr); > + regmap_write(pmcreg, AT91_PMC_PCER1, pmc_cache.pcsr1); > + > + for (i = 2; i < 64; i++) { > + regmap_write(pmcreg, AT91_PMC_PCR, (i & AT91_PMC_PCR_PID_MASK)); > + regmap_write(pmcreg, AT91_PMC_PCR, pmc_cache.pcr[i]); > + } > + > + if (pmc_cache.uckr & AT91_PMC_UPLLEN) > + mask |= AT91_PMC_LOCKU; > + > + while (!pmc_ready(mask)) > + cpu_relax(); > +} > + > +static struct syscore_ops pmc_syscore_ops = { > + .suspend = pmc_suspend, > + .resume = pmc_resume, > +}; > + > +static const struct of_device_id sama5d2_pmc_dt_ids[] = { > + { .compatible = "atmel,sama5d2-pmc" }, > + { /* sentinel */ } > +}; > + > +static int __init pmc_register_ops(void) > +{ > + struct device_node *np; > + > + np = of_find_matching_node(NULL, sama5d2_pmc_dt_ids); > + > + pmcreg = syscon_node_to_regmap(np); > + if (IS_ERR(pmcreg)) > + return PTR_ERR(pmcreg); > + > + register_syscore_ops(&pmc_syscore_ops); > + > + return 0; > +} > +/* This has to happen before arch_initcall becaues of the tcb_clksrc driver */ > +postcore_initcall(pmc_register_ops); > +#endif > -- Nicolas Ferre