Messages in this thread | | | Date | Wed, 14 Jan 2015 21:21:39 +0100 | From | Boris Brezillon <> | Subject | Re: [PATCH 1/6] ARM: at91: pm: rework cpu detection |
| |
Hi Jean-Christophe,
On Wed, 14 Jan 2015 20:14:12 +0100 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> On 22:23 Mon 12 Jan , Alexandre Belloni wrote: > > Store SoC differences in a struct to remove cpu_is_* usage. > > > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > > --- > > arch/arm/mach-at91/pm.c | 54 ++++++++++++++++++++++++++++++------------------- > > 1 file changed, 33 insertions(+), 21 deletions(-) > > > > diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c > > index 9b15169a1c62..79aa793d1f00 100644 > > --- a/arch/arm/mach-at91/pm.c > > +++ b/arch/arm/mach-at91/pm.c > > @@ -17,6 +17,7 @@ > > #include <linux/interrupt.h> > > #include <linux/sysfs.h> > > #include <linux/module.h> > > +#include <linux/of.h> > > #include <linux/platform_device.h> > > #include <linux/io.h> > > #include <linux/clk/at91_pmc.h> > > @@ -32,6 +33,11 @@ > > #include "generic.h" > > #include "pm.h" > > > > +static struct { > > + unsigned long uhp_udp_mask; > > + int memctrl; > > +} at91_pm_data; > > + > > static void (*at91_pm_standby)(void); > > > > static int at91_pm_valid_state(suspend_state_t state) > > @@ -71,17 +77,9 @@ static int at91_pm_verify_clocks(void) > > scsr = at91_pmc_read(AT91_PMC_SCSR); > > > > /* USB must not be using PLLB */ > > - if (cpu_is_at91rm9200()) { > > - if ((scsr & (AT91RM9200_PMC_UHP | AT91RM9200_PMC_UDP)) != 0) { > > - pr_err("AT91: PM - Suspend-to-RAM with USB still active\n"); > > - return 0; > > - } > > - } else if (cpu_is_at91sam9260() || cpu_is_at91sam9261() || cpu_is_at91sam9263() > > - || cpu_is_at91sam9g20() || cpu_is_at91sam9g10()) { > > - if ((scsr & (AT91SAM926x_PMC_UHP | AT91SAM926x_PMC_UDP)) != 0) { > > - pr_err("AT91: PM - Suspend-to-RAM with USB still active\n"); > > - return 0; > > - } > > + if ((scsr & at91_pm_data.uhp_udp_mask) != 0) { > > + pr_err("AT91: PM - Suspend-to-RAM with USB still active\n"); > > + return 0; > > } > > > > /* PCK0..PCK3 must be disabled, or configured to use clk32k */ > > @@ -149,18 +147,13 @@ static int at91_pm_enter(suspend_state_t state) > > * turning off the main oscillator; reverse on wakeup. > > */ > > if (slow_clock) { > > - int memctrl = AT91_MEMCTRL_SDRAMC; > > - > > - if (cpu_is_at91rm9200()) > > - memctrl = AT91_MEMCTRL_MC; > > - else if (cpu_is_at91sam9g45()) > > - memctrl = AT91_MEMCTRL_DDRSDR; > > #ifdef CONFIG_AT91_SLOW_CLOCK > > /* copy slow_clock handler to SRAM, and call it */ > > memcpy(slow_clock, at91_slow_clock, at91_slow_clock_sz); > > #endif > > slow_clock(at91_pmc_base, at91_ramc_base[0], > > - at91_ramc_base[1], memctrl); > > + at91_ramc_base[1], > > + at91_pm_data.memctrl); > > break; > > } else { > > pr_info("AT91: PM - no slow clock mode enabled ...\n"); > > @@ -237,10 +230,29 @@ static int __init at91_pm_init(void) > > > > pr_info("AT91: Power Management%s\n", (slow_clock ? " (with slow clock mode)" : "")); > > > > - /* AT91RM9200 SDRAM low-power mode cannot be used with self-refresh. */ > > - if (cpu_is_at91rm9200()) > > + at91_pm_data.memctrl = AT91_MEMCTRL_SDRAMC; > > + > > + if (of_machine_is_compatible("atmel,at91rm9200")) { > > + /* > > + * AT91RM9200 SDRAM low-power mode cannot be used with > > + * self-refresh. > > + */ > > at91_ramc_write(0, AT91RM9200_SDRAMC_LPR, 0); > > - > > + > > + at91_pm_data.uhp_udp_mask = AT91RM9200_PMC_UHP | > > + AT91RM9200_PMC_UDP; > > + at91_pm_data.memctrl = AT91_MEMCTRL_MC; > > + } else if (of_machine_is_compatible("atmel,at91sam9260") || > > + of_machine_is_compatible("atmel,at91sam9g20") || > > + of_machine_is_compatible("atmel,at91sam9261") || > > + of_machine_is_compatible("atmel,at91sam9g10") || > > + of_machine_is_compatible("atmel,at91sam9263")) { > nack here > > you switch for runtime information from the SOC register store in ram to DT > > DT is great but I do prefer to rely on the SoC register here as the whole was > also to check that the is correct
Well, cpu_is_xxx macros are defined in a mach specific header, and we're currently trying to enable multi platform support.
> > wihich is way slower
Hmm, this SoC detection has been move from the suspend path (where, as you stated, speed is a real concern) to the pm init function (which is only called once at startup), and I doubt the boot time penalty will even be noticeable...
Best Regards,
Boris
-- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
| |