[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140312080814.GE4706@localhost.localdomain>
Date: Wed, 12 Mar 2014 16:08:14 +0800
From: Chenhui Zhao <chenhui.zhao@...escale.com>
To: Scott Wood <scottwood@...escale.com>
CC: <linuxppc-dev@...ts.ozlabs.org>, <linux-kernel@...r.kernel.org>,
<leoli@...escale.com>, <Jason.Jin@...escale.com>
Subject: Re: [PATCH 6/9] powerpc/85xx: support sleep feature on QorIQ SoCs
with RCPM
On Tue, Mar 11, 2014 at 07:00:27PM -0500, Scott Wood wrote:
> On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
> > In sleep mode, the clocks of e500 cores and unused IP blocks is
> > turned off. The IP blocks which are allowed to wake up the processor
> > are still running.
> >
> > The sleep mode is equal to the Standby state in Linux. Use the
> > command to enter sleep mode:
> > echo standby > /sys/power/state
> >
> > Signed-off-by: Chenhui Zhao <chenhui.zhao@...escale.com>
> > ---
> > arch/powerpc/Kconfig | 4 +-
> > arch/powerpc/platforms/85xx/Makefile | 3 +
> > arch/powerpc/platforms/85xx/qoriq_pm.c | 78 ++++++++++++++++++++++++++++++++
> > 3 files changed, 83 insertions(+), 2 deletions(-)
> > create mode 100644 arch/powerpc/platforms/85xx/qoriq_pm.c
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index 05f6323..e1d6510 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -222,7 +222,7 @@ config ARCH_HIBERNATION_POSSIBLE
> > config ARCH_SUSPEND_POSSIBLE
> > def_bool y
> > depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \
> > - (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \
> > + FSL_SOC_BOOKE || PPC_86xx || PPC_PSERIES \
> > || 44x || 40x
> >
> > config PPC_DCR_NATIVE
> > @@ -709,7 +709,7 @@ config FSL_PCI
> > config FSL_PMC
> > bool
> > default y
> > - depends on SUSPEND && (PPC_85xx || PPC_86xx)
> > + depends on SUSPEND && (PPC_85xx && !PPC_E500MC || PPC_86xx)
>
> Don't mix && and || without parentheses.
>
> Maybe convert this into being selected (similar to FSL_RCPM), rather
> than default y?
Yes, will do.
>
> > diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> > index 25cebe7..7fae817 100644
> > --- a/arch/powerpc/platforms/85xx/Makefile
> > +++ b/arch/powerpc/platforms/85xx/Makefile
> > @@ -2,6 +2,9 @@
> > # Makefile for the PowerPC 85xx linux kernel.
> > #
> > obj-$(CONFIG_SMP) += smp.o
> > +ifeq ($(CONFIG_FSL_CORENET_RCPM), y)
> > +obj-$(CONFIG_SUSPEND) += qoriq_pm.o
> > +endif
>
> There should probably be a kconfig symbol for this.
OK.
>
> > diff --git a/arch/powerpc/platforms/85xx/qoriq_pm.c b/arch/powerpc/platforms/85xx/qoriq_pm.c
> > new file mode 100644
> > index 0000000..915b13b
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/85xx/qoriq_pm.c
> > @@ -0,0 +1,78 @@
> > +/*
> > + * Support Power Management feature
> > + *
> > + * Copyright 2014 Freescale Semiconductor Inc.
> > + *
> > + * Author: Chenhui Zhao <chenhui.zhao@...escale.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the
> > + * Free Software Foundation; either version 2 of the License, or (at your
> > + * option) any later version.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/suspend.h>
> > +#include <linux/of_platform.h>
> > +
> > +#include <sysdev/fsl_soc.h>
> > +
> > +#define FSL_SLEEP 0x1
> > +#define FSL_DEEP_SLEEP 0x2
>
> FSL_DEEP_SLEEP is unused.
Will be used in the last patch.
[PATCH 9/9] powerpc/pm: support deep sleep feature on T1040
>
> > +
> > +/* specify the sleep state of the present platform */
> > +int sleep_pm_state;
> > +/* supported sleep modes by the present platform */
> > +static unsigned int sleep_modes;
>
> Why is one signed and the other unsigned?
Undesigned. Will change them all to unsigned.
>
> > +
> > +static int qoriq_suspend_enter(suspend_state_t state)
> > +{
> > + int ret = 0;
> > +
> > + switch (state) {
> > + case PM_SUSPEND_STANDBY:
> > +
> > + if (cur_cpu_spec->cpu_flush_caches)
> > + cur_cpu_spec->cpu_flush_caches();
> > +
> > + ret = qoriq_pm_ops->plat_enter_state(sleep_pm_state);
> > +
> > + break;
> > +
> > + default:
> > + ret = -EINVAL;
> > +
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int qoriq_suspend_valid(suspend_state_t state)
> > +{
> > + if (state == PM_SUSPEND_STANDBY && (sleep_modes & FSL_SLEEP))
> > + return 1;
> > +
> > + return 0;
> > +}
> > +
> > +static const struct platform_suspend_ops qoriq_suspend_ops = {
> > + .valid = qoriq_suspend_valid,
> > + .enter = qoriq_suspend_enter,
> > +};
> > +
> > +static int __init qoriq_suspend_init(void)
> > +{
> > + struct device_node *np;
> > +
> > + sleep_modes = FSL_SLEEP;
> > + sleep_pm_state = PLAT_PM_SLEEP;
> > +
> > + np = of_find_compatible_node(NULL, NULL, "fsl,qoriq-rcpm-2.0");
> > + if (np)
> > + sleep_pm_state = PLAT_PM_LPM20;
> > +
> > + suspend_set_ops(&qoriq_suspend_ops);
> > +
> > + return 0;
> > +}
> > +arch_initcall(qoriq_suspend_init);
>
> Why is this not a platform driver? If fsl_pmc can do it...
>
> -Scott
It can be, but what advantage of being a platform driver.
-Chenhui
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists