* [PATCH v2] arm/ls1021a: Add Sleep feature for ls1021
@ 2015-09-15 9:07 Dongsheng Wang
2015-09-23 15:13 ` Shawn Guo
0 siblings, 1 reply; 3+ messages in thread
From: Dongsheng Wang @ 2015-09-15 9:07 UTC (permalink / raw)
To: shawnguo
Cc: mark.rutland, alison.wang, jason.jin, linux-arm-kernel,
linux-kernel, Wang Dongsheng, Chenhui Zhao
From: Wang Dongsheng <dongsheng.wang@freescale.com>
Add system STANDBY implement for ls1021 platform.
Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com>
Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
---
*v2*:
- Remove PSCI code. Just implement STANDBY in platform code.
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index fb689d8..d7a2d1d 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -90,6 +90,7 @@ ifeq ($(CONFIG_SUSPEND),y)
AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
+obj-$(CONFIG_SOC_LS1021A) += pm-ls1.o
endif
obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
diff --git a/arch/arm/mach-imx/pm-ls1.c b/arch/arm/mach-imx/pm-ls1.c
new file mode 100644
index 0000000..90775cf
--- /dev/null
+++ b/arch/arm/mach-imx/pm-ls1.c
@@ -0,0 +1,225 @@
+/*
+ * Support Power Management Control for LS1
+ *
+ * Copyright 2015 Freescale Semiconductor Inc.
+ *
+ * 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/io.h>
+#include <linux/kernel.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/suspend.h>
+
+#include <asm/cacheflush.h>
+#include <asm/suspend.h>
+
+#define FSL_SLEEP 0x1
+#define FSL_DEEP_SLEEP 0x2
+
+#define CCSR_SCFG_CLUSTERPMCR 0x904
+#define CCSR_SCFG_CLUSTERPMCR_WFIL2EN 0x80000000
+#define CCSR_SCFG_CLUSTERPM_ENABLE 1
+#define CCSR_SCFG_CLUSTERPM_DISABLE 0
+
+#define CCSR_RCPM_POWMGTCSR 0x130
+#define CCSR_RCPM_POWMGTCSR_LPM20_REQ 0x00100000
+#define CCSR_RCPM_IPPDEXPCR0 0x140
+#define CCSR_RCPM_IPPDEXPCR1 0x144
+#define CCSR_RCPM_IPPDEXPCR1_OCRAM1 0x10000000
+
+#define SLEEP_ARRAY_SIZE 3
+
+static u32 ippdexpcr0, ippdexpcr1;
+
+struct ls1_pm_baseaddr {
+ void __iomem *rcpm;
+ void __iomem *scfg;
+};
+
+static struct ls1_pm_baseaddr ls1_pm_base;
+
+static inline void ls1_clrsetbits_be32(void __iomem *addr, u32 clear, u32 set)
+{
+ u32 val;
+
+ val = ioread32be(addr);
+ val = (val & ~clear) | set;
+ iowrite32be(val, addr);
+}
+
+static int ls1_pm_iomap(suspend_state_t state)
+{
+ struct device_node *np;
+ void *base;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,ls1021a-scfg");
+ if (!np) {
+ pr_err("Missing SCFG node in the device tree\n");
+ return -EINVAL;
+ }
+
+ base = of_iomap(np, 0);
+ of_node_put(np);
+ if (!base) {
+ pr_err("Couldn't map SCFG registers\n");
+ return -EINVAL;
+ }
+
+ ls1_pm_base.scfg = base;
+
+ return 0;
+}
+
+static void ls1_pm_uniomap(suspend_state_t state)
+{
+ iounmap(ls1_pm_base.scfg);
+}
+
+/* set IP powerdown exception, make them work during sleep/deep sleep */
+static void ls1_set_powerdown(void)
+{
+ iowrite32be(ippdexpcr0, ls1_pm_base.rcpm + CCSR_RCPM_IPPDEXPCR0);
+ iowrite32be(ippdexpcr1, ls1_pm_base.rcpm + CCSR_RCPM_IPPDEXPCR1);
+}
+
+static void ls1_set_power_except(struct device *dev, int on)
+{
+ int ret;
+ u32 value[SLEEP_ARRAY_SIZE];
+
+ /*
+ * Get the values in the "rcpm-wakeup" property. There are three values.
+ * The first points to the RCPM node, the second is the value of
+ * the ippdexpcr0 register, and the third is the value of
+ * the ippdexpcr1 register.
+ */
+ ret = of_property_read_u32_array(dev->of_node, "rcpm-wakeup",
+ value, SLEEP_ARRAY_SIZE);
+ if (ret) {
+ dev_err(dev, "%s: Can not find the \"sleep\" property.\n",
+ __func__);
+ return;
+ }
+
+ ippdexpcr0 |= value[1];
+ ippdexpcr1 |= value[2];
+
+ pr_debug("%s: set %s as a wakeup source", __func__,
+ dev->of_node->full_name);
+}
+
+static void ls1_set_wakeup_device(struct device *dev, void *enable)
+{
+ /* set each device which can act as wakeup source */
+ if (device_may_wakeup(dev))
+ ls1_set_power_except(dev, *((int *)enable));
+}
+
+/* enable cluster to enter the PCL10 state */
+static void ls1_set_clusterpm(int enable)
+{
+ u32 val = 0;
+
+ if (enable)
+ val = CCSR_SCFG_CLUSTERPMCR_WFIL2EN;
+
+ iowrite32be(val, ls1_pm_base.scfg + CCSR_SCFG_CLUSTERPMCR);
+}
+
+static int ls1_suspend_enter(suspend_state_t state)
+{
+ int ret = 0;
+
+ ls1_set_powerdown();
+ ls1_set_clusterpm(CCSR_SCFG_CLUSTERPM_ENABLE);
+
+ switch (state) {
+ case PM_SUSPEND_STANDBY:
+ flush_cache_louis();
+ ls1_clrsetbits_be32(ls1_pm_base.rcpm + CCSR_RCPM_POWMGTCSR,
+ CCSR_RCPM_POWMGTCSR_LPM20_REQ,
+ CCSR_RCPM_POWMGTCSR_LPM20_REQ);
+ cpu_do_idle();
+ break;
+ case PM_SUSPEND_MEM:
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ ls1_set_clusterpm(CCSR_SCFG_CLUSTERPM_DISABLE);
+
+ return ret;
+}
+
+static int ls1_suspend_valid(suspend_state_t state)
+{
+ if (state == PM_SUSPEND_STANDBY)
+ return 1;
+
+ return 0;
+}
+
+static int ls1_suspend_begin(suspend_state_t state)
+{
+ int res = -EINVAL;
+
+ ippdexpcr0 = 0;
+ ippdexpcr1 = 0;
+
+ if (state == PM_SUSPEND_STANDBY)
+ res = ls1_pm_iomap(state);
+
+ if (!res)
+ dpm_for_each_dev(NULL, ls1_set_wakeup_device);
+
+ return res;
+}
+
+static void ls1_suspend_end(void)
+{
+ ls1_pm_uniomap(PM_SUSPEND_STANDBY);
+}
+
+static const struct platform_suspend_ops ls1_suspend_ops = {
+ .valid = ls1_suspend_valid,
+ .enter = ls1_suspend_enter,
+ .begin = ls1_suspend_begin,
+ .end = ls1_suspend_end,
+};
+
+static const struct of_device_id rcpm_matches[] = {
+ {
+ .compatible = "fsl,ls1021a-rcpm",
+ },
+ {}
+};
+
+static int __init ls1_pm_init(void)
+{
+ const struct of_device_id *match;
+ struct device_node *np;
+
+ np = of_find_matching_node_and_match(NULL, rcpm_matches, &match);
+ if (!np) {
+ pr_err("%s: can't find the rcpm node.\n", __func__);
+ return -EINVAL;
+ }
+
+ ls1_pm_base.rcpm = of_iomap(np, 0);
+ of_node_put(np);
+ if (!ls1_pm_base.rcpm)
+ return -ENOMEM;
+
+ suspend_set_ops(&ls1_suspend_ops);
+
+ pr_info("Freescale Power Management Control Registered\n");
+
+ return 0;
+}
+arch_initcall(ls1_pm_init);
--
2.1.0.27.g96db324
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] arm/ls1021a: Add Sleep feature for ls1021
2015-09-15 9:07 [PATCH v2] arm/ls1021a: Add Sleep feature for ls1021 Dongsheng Wang
@ 2015-09-23 15:13 ` Shawn Guo
[not found] ` <SN1PR0301MB1616B7458FB787A75DADC9369D430@SN1PR0301MB1616.namprd03.prod.outlook.com>
0 siblings, 1 reply; 3+ messages in thread
From: Shawn Guo @ 2015-09-23 15:13 UTC (permalink / raw)
To: Dongsheng Wang
Cc: mark.rutland, alison.wang, jason.jin, linux-arm-kernel,
linux-kernel, Chenhui Zhao
On Tue, Sep 15, 2015 at 05:07:10PM +0800, Dongsheng Wang wrote:
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
>
> Add system STANDBY implement for ls1021 platform.
>
> Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> ---
> *v2*:
> - Remove PSCI code. Just implement STANDBY in platform code.
Why stepping back? We are encouraged to move to PSCI, aren't we?
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index fb689d8..d7a2d1d 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -90,6 +90,7 @@ ifeq ($(CONFIG_SUSPEND),y)
> AFLAGS_suspend-imx6.o :=-Wa,-march=armv7-a
> obj-$(CONFIG_SOC_IMX6) += suspend-imx6.o
> obj-$(CONFIG_SOC_IMX53) += suspend-imx53.o
> +obj-$(CONFIG_SOC_LS1021A) += pm-ls1.o
> endif
> obj-$(CONFIG_SOC_IMX6) += pm-imx6.o
>
> diff --git a/arch/arm/mach-imx/pm-ls1.c b/arch/arm/mach-imx/pm-ls1.c
> new file mode 100644
> index 0000000..90775cf
> --- /dev/null
> +++ b/arch/arm/mach-imx/pm-ls1.c
> @@ -0,0 +1,225 @@
> +/*
> + * Support Power Management Control for LS1
> + *
> + * Copyright 2015 Freescale Semiconductor Inc.
> + *
> + * 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/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/suspend.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/suspend.h>
> +
> +#define FSL_SLEEP 0x1
> +#define FSL_DEEP_SLEEP 0x2
Unused defines.
> +
> +#define CCSR_SCFG_CLUSTERPMCR 0x904
> +#define CCSR_SCFG_CLUSTERPMCR_WFIL2EN 0x80000000
> +#define CCSR_SCFG_CLUSTERPM_ENABLE 1
> +#define CCSR_SCFG_CLUSTERPM_DISABLE 0
> +
> +#define CCSR_RCPM_POWMGTCSR 0x130
> +#define CCSR_RCPM_POWMGTCSR_LPM20_REQ 0x00100000
> +#define CCSR_RCPM_IPPDEXPCR0 0x140
> +#define CCSR_RCPM_IPPDEXPCR1 0x144
> +#define CCSR_RCPM_IPPDEXPCR1_OCRAM1 0x10000000
> +
> +#define SLEEP_ARRAY_SIZE 3
The name of the macro doesn't appealing.
> +
> +static u32 ippdexpcr0, ippdexpcr1;
It makes more sense to have a structure holds all these variables and
the following base addresses.
> +
> +struct ls1_pm_baseaddr {
> + void __iomem *rcpm;
> + void __iomem *scfg;
> +};
> +
> +static struct ls1_pm_baseaddr ls1_pm_base;
> +
> +static inline void ls1_clrsetbits_be32(void __iomem *addr, u32 clear, u32 set)
> +{
> + u32 val;
> +
> + val = ioread32be(addr);
> + val = (val & ~clear) | set;
> + iowrite32be(val, addr);
> +}
> +
> +static int ls1_pm_iomap(suspend_state_t state)
How is argument 'state' used?
> +{
> + struct device_node *np;
> + void *base;
> +
> + np = of_find_compatible_node(NULL, NULL, "fsl,ls1021a-scfg");
> + if (!np) {
> + pr_err("Missing SCFG node in the device tree\n");
> + return -EINVAL;
> + }
> +
> + base = of_iomap(np, 0);
> + of_node_put(np);
> + if (!base) {
> + pr_err("Couldn't map SCFG registers\n");
> + return -EINVAL;
> + }
> +
> + ls1_pm_base.scfg = base;
> +
> + return 0;
> +}
> +
> +static void ls1_pm_uniomap(suspend_state_t state)
> +{
> + iounmap(ls1_pm_base.scfg);
> +}
> +
> +/* set IP powerdown exception, make them work during sleep/deep sleep */
> +static void ls1_set_powerdown(void)
> +{
> + iowrite32be(ippdexpcr0, ls1_pm_base.rcpm + CCSR_RCPM_IPPDEXPCR0);
> + iowrite32be(ippdexpcr1, ls1_pm_base.rcpm + CCSR_RCPM_IPPDEXPCR1);
> +}
> +
> +static void ls1_set_power_except(struct device *dev, int on)
How argument 'on' is used?
> +{
> + int ret;
> + u32 value[SLEEP_ARRAY_SIZE];
> +
> + /*
> + * Get the values in the "rcpm-wakeup" property. There are three values.
> + * The first points to the RCPM node, the second is the value of
> + * the ippdexpcr0 register, and the third is the value of
> + * the ippdexpcr1 register.
> + */
> + ret = of_property_read_u32_array(dev->of_node, "rcpm-wakeup",
> + value, SLEEP_ARRAY_SIZE);
The property should be documented in device tree bindings. And you need
a good reason for why these register values should be defined by device
tree.
> + if (ret) {
> + dev_err(dev, "%s: Can not find the \"sleep\" property.\n",
> + __func__);
> + return;
> + }
> +
> + ippdexpcr0 |= value[1];
> + ippdexpcr1 |= value[2];
> +
> + pr_debug("%s: set %s as a wakeup source", __func__,
When you have device pointer, you should use dev_dbg() instead of
pr_debug().
> + dev->of_node->full_name);
> +}
> +
> +static void ls1_set_wakeup_device(struct device *dev, void *enable)
> +{
> + /* set each device which can act as wakeup source */
> + if (device_may_wakeup(dev))
> + ls1_set_power_except(dev, *((int *)enable));
> +}
> +
> +/* enable cluster to enter the PCL10 state */
> +static void ls1_set_clusterpm(int enable)
> +{
> + u32 val = 0;
> +
> + if (enable)
> + val = CCSR_SCFG_CLUSTERPMCR_WFIL2EN;
> +
> + iowrite32be(val, ls1_pm_base.scfg + CCSR_SCFG_CLUSTERPMCR);
> +}
> +
> +static int ls1_suspend_enter(suspend_state_t state)
> +{
> + int ret = 0;
> +
> + ls1_set_powerdown();
> + ls1_set_clusterpm(CCSR_SCFG_CLUSTERPM_ENABLE);
> +
> + switch (state) {
> + case PM_SUSPEND_STANDBY:
> + flush_cache_louis();
> + ls1_clrsetbits_be32(ls1_pm_base.rcpm + CCSR_RCPM_POWMGTCSR,
> + CCSR_RCPM_POWMGTCSR_LPM20_REQ,
> + CCSR_RCPM_POWMGTCSR_LPM20_REQ);
> + cpu_do_idle();
> + break;
> + case PM_SUSPEND_MEM:
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + ls1_set_clusterpm(CCSR_SCFG_CLUSTERPM_DISABLE);
> +
> + return ret;
> +}
> +
> +static int ls1_suspend_valid(suspend_state_t state)
> +{
> + if (state == PM_SUSPEND_STANDBY)
> + return 1;
> +
> + return 0;
> +}
> +
> +static int ls1_suspend_begin(suspend_state_t state)
> +{
> + int res = -EINVAL;
The convention of return variable is 'ret' than 'res'.
> +
> + ippdexpcr0 = 0;
> + ippdexpcr1 = 0;
> +
> + if (state == PM_SUSPEND_STANDBY)
> + res = ls1_pm_iomap(state);
> +
> + if (!res)
> + dpm_for_each_dev(NULL, ls1_set_wakeup_device);
> +
> + return res;
> +}
> +
> +static void ls1_suspend_end(void)
> +{
> + ls1_pm_uniomap(PM_SUSPEND_STANDBY);
> +}
The .begin() and .end() hooks will be called each time that system
enters/exits suspend, right? Are you sure the setup you're doing in
ls1_suspend_begin() and ls1_suspend_end() is needed each time? Or they
only need to be done in ls1_pm_init() for once?
> +
> +static const struct platform_suspend_ops ls1_suspend_ops = {
> + .valid = ls1_suspend_valid,
> + .enter = ls1_suspend_enter,
> + .begin = ls1_suspend_begin,
> + .end = ls1_suspend_end,
> +};
> +
> +static const struct of_device_id rcpm_matches[] = {
> + {
> + .compatible = "fsl,ls1021a-rcpm",
Undocumented compatible.
> + },
> + {}
> +};
> +
> +static int __init ls1_pm_init(void)
> +{
> + const struct of_device_id *match;
> + struct device_node *np;
> +
> + np = of_find_matching_node_and_match(NULL, rcpm_matches, &match);
You are simply trying to find "fsl,ls1021a-rcpm" node, so why not just
use of_find_compatible_node() like you try to find "fsl,ls1021a-scfg"
in ls1_pm_iomap().
> + if (!np) {
> + pr_err("%s: can't find the rcpm node.\n", __func__);
> + return -EINVAL;
> + }
> +
> + ls1_pm_base.rcpm = of_iomap(np, 0);
> + of_node_put(np);
> + if (!ls1_pm_base.rcpm)
> + return -ENOMEM;
Right. Why cannot iomap of scfg be done here just like rcpm?
> +
> + suspend_set_ops(&ls1_suspend_ops);
> +
> + pr_info("Freescale Power Management Control Registered\n");
> +
> + return 0;
> +}
> +arch_initcall(ls1_pm_init);
Bear it in mind that we're now in multi-platform world, where a single
kernel image will run multiple targets, LS1021A, IMX, Vybrid, and even
non-FSL SoCs. You cannot use such initcall here, which will get the
function called on non-LS1021A platform.
Shawn
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] arm/ls1021a: Add Sleep feature for ls1021
[not found] ` <SN1PR0301MB1616B7458FB787A75DADC9369D430@SN1PR0301MB1616.namprd03.prod.outlook.com>
@ 2015-09-24 11:40 ` Shawn Guo
0 siblings, 0 replies; 3+ messages in thread
From: Shawn Guo @ 2015-09-24 11:40 UTC (permalink / raw)
To: Wang Dongsheng
Cc: mark.rutland, Huan Wang, Jin Jason, linux-arm-kernel,
linux-kernel, Zhao C.H.
On Thu, Sep 24, 2015 at 09:28:31AM +0000, Wang Dongsheng wrote:
> > > Add system STANDBY implement for ls1021 platform.
> > >
> > > Signed-off-by: Chenhui Zhao <chenhui.zhao@freescale.com>
> > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > ---
> > > *v2*:
> > > - Remove PSCI code. Just implement STANDBY in platform code.
> >
> > Why stepping back? We are encouraged to move to PSCI, aren't we?
> >
>
> As Lorenzo said, PSCI v1.0 support will be in kernel v4.4. So I remove
> psci for this patch.
v4.4 is not far away, and we should probably wait instead of pushing
the code that needs to be revised quite soon.
Shawn
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-09-24 11:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-15 9:07 [PATCH v2] arm/ls1021a: Add Sleep feature for ls1021 Dongsheng Wang
2015-09-23 15:13 ` Shawn Guo
[not found] ` <SN1PR0301MB1616B7458FB787A75DADC9369D430@SN1PR0301MB1616.namprd03.prod.outlook.com>
2015-09-24 11:40 ` Shawn Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).