Message ID | 1458111489-23774-4-git-send-email-wenyou.yang@atmel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 16/03/2016 at 14:58:07 +0800, Wenyou Yang wrote : > The fast startup signal is used as wake up sources for ULP1 mode. > As soon as a fast startup signal is asserted, the embedded 12 MHz > RC oscillator restarts automatically. > > This patch is to configure the fast startup signals, which signal > is enabled to trigger the PMC to wake up the system from ULP1 mode > should be configured via the DT. > > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> I would actually avoid doing that from the PMC driver and do that configuration from the aic5 driver. It has all the information you need, it knows what kind of level or edge is needed to wake up and what are the wakeup interrupts to enable. This will allow you to stop introducing a new binding. Also, this will avoid discrepancies between what is configured in the DT and what the user really wants (for exemple differences between the edge direction configured for the PIOBu in userspace versus what is in the device tree or wakeonlan activation/deactivation). You can get the PMC syscon from irq-atmel-aic5.c and then use a table to map the hwirq to the offset in PMC_FSMR. Use it in aic5_set_type to set the polarity and then in aic5_suspend to enable the wakeup. Maybe we could even go further and avoid ulp1 if no ulp1 compatbile wakeup sources are defined but there are ulp0 wakeup sources.
Hi Alexandre, > -----Original Message----- > From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com] > Sent: 2016?3?18? 1:15 > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > Cc: Ferre, Nicolas <Nicolas.FERRE@atmel.com>; Jean-Christophe Plagniol- > Villard <plagnioj@jcrosoft.com>; Russell King <linux@arm.linux.org.uk>; linux- > kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-clk@vger.kernel.org; Rob Herring > <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark Brown > <broonie@kernel.org>; Ian Campbell <ijc+devicetree@hellion.org.uk>; Kumar > Gala <galak@codeaurora.org> > Subject: Re: [PATCH v5 3/5] ARM: at91: pm: configure PMC fast startup signals > > On 16/03/2016 at 14:58:07 +0800, Wenyou Yang wrote : > > The fast startup signal is used as wake up sources for ULP1 mode. > > As soon as a fast startup signal is asserted, the embedded 12 MHz RC > > oscillator restarts automatically. > > > > This patch is to configure the fast startup signals, which signal is > > enabled to trigger the PMC to wake up the system from ULP1 mode should > > be configured via the DT. > > > > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> > > I would actually avoid doing that from the PMC driver and do that configuration > from the aic5 driver. It has all the information you need, it knows what kind of level > or edge is needed to wake up and what are the wakeup interrupts to enable. This > will allow you to stop introducing a new binding. Also, this will avoid discrepancies > between what is configured in the DT and what the user really wants (for exemple > differences between the edge direction configured for the PIOBu in userspace > versus what is in the device tree or wakeonlan activation/deactivation). Thank you for your feedback. But some wake-ups such as WKUP pin, ACC_CE, RXLP_MCE, don't have the corresponding interrupt number. Moreover, I think, the ULP1 is very different form the ULP0, it is not woken up by the interrupt. It is fallen sleep and woken up by the some mechanism in the PMC. Maybe I was wrong. I still think the aic5 driver should be devoted on the AIC5's behaviors. > > You can get the PMC syscon from irq-atmel-aic5.c and then use a table to map > the hwirq to the offset in PMC_FSMR. Use it in aic5_set_type to set the polarity > and then in aic5_suspend to enable the wakeup. > > Maybe we could even go further and avoid ulp1 if no ulp1 compatbile wakeup > sources are defined but there are ulp0 wakeup sources. > > > -- > Alexandre Belloni, Free Electrons > Embedded Linux, Kernel and Android engineering http://free-electrons.com Best Regards, Wenyou Yang
On 21/03/2016 at 02:24:32 +0000, Yang, Wenyou wrote : > Hi Alexandre, > > > -----Original Message----- > > From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com] > > Sent: 2016?3?18? 1:15 > > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > > Cc: Ferre, Nicolas <Nicolas.FERRE@atmel.com>; Jean-Christophe Plagniol- > > Villard <plagnioj@jcrosoft.com>; Russell King <linux@arm.linux.org.uk>; linux- > > kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > > kernel@lists.infradead.org; linux-clk@vger.kernel.org; Rob Herring > > <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark Brown > > <broonie@kernel.org>; Ian Campbell <ijc+devicetree@hellion.org.uk>; Kumar > > Gala <galak@codeaurora.org> > > Subject: Re: [PATCH v5 3/5] ARM: at91: pm: configure PMC fast startup signals > > > > On 16/03/2016 at 14:58:07 +0800, Wenyou Yang wrote : > > > The fast startup signal is used as wake up sources for ULP1 mode. > > > As soon as a fast startup signal is asserted, the embedded 12 MHz RC > > > oscillator restarts automatically. > > > > > > This patch is to configure the fast startup signals, which signal is > > > enabled to trigger the PMC to wake up the system from ULP1 mode should > > > be configured via the DT. > > > > > > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> > > > > I would actually avoid doing that from the PMC driver and do that configuration > > from the aic5 driver. It has all the information you need, it knows what kind of level > > or edge is needed to wake up and what are the wakeup interrupts to enable. This > > will allow you to stop introducing a new binding. Also, this will avoid discrepancies > > between what is configured in the DT and what the user really wants (for exemple > > differences between the edge direction configured for the PIOBu in userspace > > versus what is in the device tree or wakeonlan activation/deactivation). > > Thank you for your feedback. > > But some wake-ups such as WKUP pin, ACC_CE, RXLP_MCE, don't have the corresponding The WKUP pin can be configured from the shdwc driver, ACC_CE from the ACC driver, RXLP_MCE, from the RXLP driver because you will need drivers for those at some point anyway. > interrupt number. Moreover, I think, the ULP1 is very different form the ULP0, it is not woken > up by the interrupt. It is fallen sleep and woken up by the some mechanism in the PMC. > Well, we don't really care about the mechanism. We only care about how the user is able to configure the wakeup sources. With your patch set, what happens when no ULP1 sources are defined but there are ULP0 sources? What happens when there are both ULP1 and ULP0 sources? What would be good is to use ULP1 when only ULP1 sources are set up and ULP0 in the other cases. This will greatly help the user. Also, what I'm suggesting actually allows to change the ULP1 sources at runtime from devices that are actually used which is quite better than setting them up statically from DT.
Hi Alexandre, > -----Original Message----- > From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com] > Sent: 2016?3?24? 19:25 > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > Cc: Ferre, Nicolas <Nicolas.FERRE@atmel.com>; Jean-Christophe Plagniol- > Villard <plagnioj@jcrosoft.com>; Russell King <linux@arm.linux.org.uk>; linux- > kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-clk@vger.kernel.org; Rob Herring > <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark Brown > <broonie@kernel.org>; Ian Campbell <ijc+devicetree@hellion.org.uk>; Kumar > Gala <galak@codeaurora.org> > Subject: Re: [PATCH v5 3/5] ARM: at91: pm: configure PMC fast startup signals > > On 21/03/2016 at 02:24:32 +0000, Yang, Wenyou wrote : > > Hi Alexandre, > > > > > -----Original Message----- > > > From: Alexandre Belloni > > > [mailto:alexandre.belloni@free-electrons.com] > > > Sent: 2016?3?18? 1:15 > > > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > > > Cc: Ferre, Nicolas <Nicolas.FERRE@atmel.com>; Jean-Christophe > > > Plagniol- Villard <plagnioj@jcrosoft.com>; Russell King > > > <linux@arm.linux.org.uk>; linux- kernel@vger.kernel.org; > > > devicetree@vger.kernel.org; linux-arm- kernel@lists.infradead.org; > > > linux-clk@vger.kernel.org; Rob Herring <robh+dt@kernel.org>; Pawel > > > Moll <pawel.moll@arm.com>; Mark Brown <broonie@kernel.org>; Ian > > > Campbell <ijc+devicetree@hellion.org.uk>; Kumar Gala > > > <galak@codeaurora.org> > > > Subject: Re: [PATCH v5 3/5] ARM: at91: pm: configure PMC fast > > > startup signals > > > > > > On 16/03/2016 at 14:58:07 +0800, Wenyou Yang wrote : > > > > The fast startup signal is used as wake up sources for ULP1 mode. > > > > As soon as a fast startup signal is asserted, the embedded 12 MHz > > > > RC oscillator restarts automatically. > > > > > > > > This patch is to configure the fast startup signals, which signal > > > > is enabled to trigger the PMC to wake up the system from ULP1 mode > > > > should be configured via the DT. > > > > > > > > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> > > > > > > I would actually avoid doing that from the PMC driver and do that > > > configuration from the aic5 driver. It has all the information you > > > need, it knows what kind of level or edge is needed to wake up and > > > what are the wakeup interrupts to enable. This will allow you to > > > stop introducing a new binding. Also, this will avoid discrepancies > > > between what is configured in the DT and what the user really wants > > > (for exemple differences between the edge direction configured for the PIOBu > in userspace versus what is in the device tree or wakeonlan > activation/deactivation). > > > > Thank you for your feedback. > > > > But some wake-ups such as WKUP pin, ACC_CE, RXLP_MCE, don't have the > > corresponding > > The WKUP pin can be configured from the shdwc driver, ACC_CE from the ACC > driver, RXLP_MCE, from the RXLP driver because you will need drivers for those > at some point anyway. > > > interrupt number. Moreover, I think, the ULP1 is very different form > > the ULP0, it is not woken up by the interrupt. It is fallen sleep and woken up by > the some mechanism in the PMC. > > > > Well, we don't really care about the mechanism. We only care about how the user > is able to configure the wakeup sources. Understand your concerns. > > With your patch set, what happens when no ULP1 sources are defined but there > are ULP0 sources? What happens when there are both ULP1 and ULP0 sources? I think there is only one mode, either ULP1 or ULP0, in a runtime system. The ULP1 sources are used to wake up the ULP1, and the ULP0 sources wake up the ULP0, they don't mutually effect each other. If system is in the ULP1 mode, and there are no ULP1 sources defined, the system will fail to be waken up, even though there are ULP0 sources defined. The ULP0 source can't wake up the ULP1. > > What would be good is to use ULP1 when only ULP1 sources are set up and > ULP0 in the other cases. This will greatly help the user. Also, what I'm suggesting > actually allows to change the ULP1 sources at runtime from devices that are > actually used which is quite better than setting them up statically from DT. > In the ULP1 mode, all clocks are disabled. It can get lower consumption and quicker wake up (the processor restarts in less than 10us) than ULP0 mode. The number of wake up sources is limited as well. It is very different from ULP0's Hi Nicolas, what is your opinions? Best Regards, Wenyou Yang
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c index 5c2db34..29895f1 100644 --- a/arch/arm/mach-at91/pm.c +++ b/arch/arm/mach-at91/pm.c @@ -24,6 +24,8 @@ #include <linux/platform_device.h> #include <linux/io.h> #include <linux/clk/at91_pmc.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> #include <asm/irq.h> #include <linux/atomic.h> @@ -461,6 +463,68 @@ static void __init at91_pm_init(void (*pm_idle)(void)) pr_info("AT91: PM not supported, due to no SRAM allocated\n"); } +static int __init at91_pmc_fast_startup_init(void) +{ + struct device_node *np, *cnp; + struct regmap *regmap; + u32 input, input_mask; + u32 mode = 0, polarity = 0; + + np = of_find_compatible_node(NULL, NULL, + "atmel,sama5d2-pmc-fast-startup"); + if (!np) + return -ENODEV; + + regmap = syscon_node_to_regmap(of_get_parent(np)); + if (IS_ERR(regmap)) { + pr_info("AT91: failed to find PMC fast startup node\n"); + return PTR_ERR(regmap); + } + + for_each_child_of_node(np, cnp) { + if (of_property_read_u32(cnp, "reg", &input)) { + pr_warn("AT91: reg property is missing for %s\n", + cnp->full_name); + continue; + } + + input_mask = 1 << input; + if (!(input_mask & AT91_PMC_FS_INPUT_MASK)) { + pr_warn("AT91: wake-up input %d out of range\n", input); + continue; + } + mode |= input_mask; + + if (of_property_read_bool(cnp, "atmel,wakeup-active-high")) + polarity |= input_mask; + } + + if (of_property_read_bool(np, "atmel,wakeup-rtc-timer")) + mode |= AT91_PMC_RTCAL; + + if (of_property_read_bool(np, "atmel,wakeup-usb-resume")) + mode |= AT91_PMC_USBAL; + + if (of_property_read_bool(np, "atmel,wakeup-sdmmc-cd")) + mode |= AT91_PMC_SDMMC_CD; + + if (of_property_read_bool(np, "atmel,wakeup-rxlp-match")) + mode |= AT91_PMC_RXLP_MCE; + + if (of_property_read_bool(np, "atmel,wakeup-acc-comparison")) + mode |= AT91_PMC_ACC_CE; + + pr_debug("AT91: mode = 0x%x, ploarity = 0%x\n", mode, polarity); + + regmap_write(regmap, AT91_PMC_FSMR, mode); + + regmap_write(regmap, AT91_PMC_FSPR, polarity); + + of_node_put(np); + + return 0; +} + void __init at91rm9200_pm_init(void) { at91_dt_ramc(); @@ -509,4 +573,6 @@ void __init sama5_pm_init(void) if (readl(pmc + AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION) at91_pm_data.ulp_mode = ULP1_MODE; + + at91_pmc_fast_startup_init(); } diff --git a/include/linux/clk/at91_pmc.h b/include/linux/clk/at91_pmc.h index 4afd891..4b942c5 100644 --- a/include/linux/clk/at91_pmc.h +++ b/include/linux/clk/at91_pmc.h @@ -168,6 +168,20 @@ #define AT91_PMC_WPVS (0x1 << 0) /* Write Protect Violation Status */ #define AT91_PMC_WPVSRC (0xffff << 8) /* Write Protect Violation Source */ +#define AT91_PMC_FSMR 0x70 /* Fast Startup Mode Register */ +#define AT91_PMC_FSTT(n) (0x1 << n) +#define AT91_PMC_RTCAL BIT(17) /* RTC Alarm Enable */ +#define AT91_PMC_USBAL BIT(18) /* USB Resume Enable */ +#define AT91_PMC_SDMMC_CD BIT(19) /* SDMMC Card Detect Enable */ +#define AT91_PMC_LPM BIT(20) /* Low-power Mode */ +#define AT91_PMC_RXLP_MCE BIT(24) /* Backup UART Receive Enable */ +#define AT91_PMC_ACC_CE BIT(25) /* ACC Enable */ + +#define AT91_PMC_FSPR 0x74 /* Fast Startup Polarity Reg */ +#define AT91_PMC_FSTP(n) (0x1 << n) + +#define AT91_PMC_FS_INPUT_MASK 0x7ff + #define AT91_PMC_VERSION 0xfc #define AT91_PMC_PCER1 0x100 /* Peripheral Clock Enable Register 1 [SAMA5 only]*/
The fast startup signal is used as wake up sources for ULP1 mode. As soon as a fast startup signal is asserted, the embedded 12 MHz RC oscillator restarts automatically. This patch is to configure the fast startup signals, which signal is enabled to trigger the PMC to wake up the system from ULP1 mode should be configured via the DT. Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> --- Changes in v5: - to improve the scalability, rework the DT expression part, use the child nodes to describe the wake-up input and its active level. Changes in v4: None Changes in v3: - use 0 and 1, not string, to define the trigger active polarity. Changes in v2: - shorten the pmc-fast-startup property's name. - use the value property, instead of bool property for high or low triggered. arch/arm/mach-at91/pm.c | 66 ++++++++++++++++++++++++++++++++++++++++++ include/linux/clk/at91_pmc.h | 14 +++++++++ 2 files changed, 80 insertions(+)