lkml.org 
[lkml]   [2015]   [Jan]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2015-01-14 22:01    [W:3.448 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site