[PATCH 1/3] PM: wakeup: Add routine to help fetch wakeup source object.

[PATCH 1/3] PM: wakeup: Add routine to help fetch wakeup source object.

[Ran Wang]

Some user might want to go through all registered wakeup sources
and doing things accordingly. For example, SoC PM driver might need to
do HW programming to prevent powering down specific IP which wakeup
source depending on. So add this API to help walk through all registered
wakeup source objects on that list and return them one by one.

Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
Tested-by: Leonard Crestez <leonard.crestez@nxp.com>
---
Change in v8
	- Rename wakeup_source_get_next() to wakeup_sources_walk_next().
	- Add wakeup_sources_read_lock() to take over locking job of
	  wakeup_source_get_star().
	- Rename wakeup_source_get_start() to wakeup_sources_walk_start().
	- Replace wakeup_source_get_stop() with wakeup_sources_read_unlock().
	- Define macro for_each_wakeup_source(ws).

Change in v7:
	- Remove define of member *dev in wake_irq to fix conflict with commit 
	c8377adfa781 ("PM / wakeup: Show wakeup sources stats in sysfs"), user 
	will use ws->dev->parent instead.
	- Remove '#include <linux/of_device.h>' because it is not used.

Change in v6:
	- Add wakeup_source_get_star() and wakeup_source_get_stop() to aligned 
	with wakeup_sources_stats_seq_start/nex/stop.

Change in v5:
	- Update commit message, add decription of walk through all wakeup
	source objects.
	- Add SCU protection in function wakeup_source_get_next().
	- Rename wakeup_source member 'attached_dev' to 'dev' and move it up
	(before wakeirq).

Change in v4:
	- None.

Change in v3:
	- Adjust indentation of *attached_dev;.

Change in v2:
	- None.

 drivers/base/power/wakeup.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_wakeup.h   |  9 +++++++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 5817b51..8c7a5f9 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -248,6 +248,48 @@ void wakeup_source_unregister(struct wakeup_source *ws)
 EXPORT_SYMBOL_GPL(wakeup_source_unregister);
 
 /**
+ * wakeup_sources_read_lock - Lock wakeup source list for read.
+ */
+int wakeup_sources_read_lock(void)
+{
+	return srcu_read_lock(&wakeup_srcu);
+}
+EXPORT_SYMBOL_GPL(wakeup_sources_read_lock);
+
+/**
+ * wakeup_sources_read_unlock - Unlock wakeup source list.
+ */
+void wakeup_sources_read_unlock(int idx)
+{
+	srcu_read_unlock(&wakeup_srcu, idx);
+}
+EXPORT_SYMBOL_GPL(wakeup_sources_read_unlock);
+
+/**
+ * wakeup_sources_walk_start - Begin a walk on wakeup source list
+ */
+struct wakeup_source *wakeup_sources_walk_start(void)
+{
+	struct list_head *ws_head = &wakeup_sources;
+
+	return list_entry_rcu(ws_head->next, struct wakeup_source, entry);
+}
+EXPORT_SYMBOL_GPL(wakeup_sources_walk_start);
+
+/**
+ * wakeup_sources_walk_next - Get next wakeup source from the list
+ * @ws: Previous wakeup source object
+ */
+struct wakeup_source *wakeup_sources_walk_next(struct wakeup_source *ws)
+{
+	struct list_head *ws_head = &wakeup_sources;
+
+	return list_next_or_null_rcu(ws_head, &ws->entry,
+				struct wakeup_source, entry);
+}
+EXPORT_SYMBOL_GPL(wakeup_sources_walk_next);
+
+/**
  * device_wakeup_attach - Attach a wakeup source object to a device object.
  * @dev: Device to handle.
  * @ws: Wakeup source object to attach to @dev.
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
index 661efa0..aa3da66 100644
--- a/include/linux/pm_wakeup.h
+++ b/include/linux/pm_wakeup.h
@@ -63,6 +63,11 @@ struct wakeup_source {
 	bool			autosleep_enabled:1;
 };
 
+#define for_each_wakeup_source(ws) \
+	for ((ws) = wakeup_sources_walk_start();	\
+	     (ws);					\
+	     (ws) = wakeup_sources_walk_next((ws)))
+
 #ifdef CONFIG_PM_SLEEP
 
 /*
@@ -92,6 +97,10 @@ extern void wakeup_source_remove(struct wakeup_source *ws);
 extern struct wakeup_source *wakeup_source_register(struct device *dev,
 						    const char *name);
 extern void wakeup_source_unregister(struct wakeup_source *ws);
+extern int wakeup_sources_read_lock(void);
+extern void wakeup_sources_read_unlock(int idx);
+extern struct wakeup_source *wakeup_sources_walk_start(void);
+extern struct wakeup_source *wakeup_sources_walk_next(struct wakeup_source *ws);
 extern int device_wakeup_enable(struct device *dev);
 extern int device_wakeup_disable(struct device *dev);
 extern void device_set_wakeup_capable(struct device *dev, bool capable);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] Documentation: dt: binding: fsl: Add 'little-endian' and update Chassis define

[Ran Wang]

By default, QorIQ SoC's RCPM register block is Big Endian. But
there are some exceptions, such as LS1088A and LS2088A, are
Little Endian. So add this optional property to help identify
them.

Actually LS2021A and other Layerscapes won't totally follow Chassis
2.1, so separate them from powerpc SoC.

Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Change in v8:
	- None.

Change in v7:
	- None.

Change in v6:
	- None.

Change in v5:
	- Add 'Reviewed-by: Rob Herring <robh@kernel.org>' to commit message.
	- Rename property 'fsl,#rcpm-wakeup-cells' to '#fsl,rcpm-wakeup-cells'.
	please see https://lore.kernel.org/patchwork/patch/1101022/

Change in v4:
	- Adjust indectation of 'ls1021a, ls1012a, ls1043a, ls1046a'.

Change in v3:
	- None.

Change in v2:
	- None.

 Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
index e284e4e..5a33619 100644
--- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
+++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
@@ -5,7 +5,7 @@ and power management.
 
 Required properites:
   - reg : Offset and length of the register set of the RCPM block.
-  - fsl,#rcpm-wakeup-cells : The number of IPPDEXPCR register cells in the
+  - #fsl,rcpm-wakeup-cells : The number of IPPDEXPCR register cells in the
 	fsl,rcpm-wakeup property.
   - compatible : Must contain a chip-specific RCPM block compatible string
 	and (if applicable) may contain a chassis-version RCPM compatible
@@ -20,6 +20,7 @@ Required properites:
 	* "fsl,qoriq-rcpm-1.0": for chassis 1.0 rcpm
 	* "fsl,qoriq-rcpm-2.0": for chassis 2.0 rcpm
 	* "fsl,qoriq-rcpm-2.1": for chassis 2.1 rcpm
+	* "fsl,qoriq-rcpm-2.1+": for chassis 2.1+ rcpm
 
 All references to "1.0" and "2.0" refer to the QorIQ chassis version to
 which the chip complies.
@@ -27,14 +28,19 @@ Chassis Version		Example Chips
 ---------------		-------------------------------
 1.0				p4080, p5020, p5040, p2041, p3041
 2.0				t4240, b4860, b4420
-2.1				t1040, ls1021
+2.1				t1040,
+2.1+				ls1021a, ls1012a, ls1043a, ls1046a
+
+Optional properties:
+ - little-endian : RCPM register block is Little Endian. Without it RCPM
+   will be Big Endian (default case).
 
 Example:
 The RCPM node for T4240:
 	rcpm: global-utilities@e2000 {
 		compatible = "fsl,t4240-rcpm", "fsl,qoriq-rcpm-2.0";
 		reg = <0xe2000 0x1000>;
-		fsl,#rcpm-wakeup-cells = <2>;
+		#fsl,rcpm-wakeup-cells = <2>;
 	};
 
 * Freescale RCPM Wakeup Source Device Tree Bindings
@@ -44,7 +50,7 @@ can be used as a wakeup source.
 
   - fsl,rcpm-wakeup: Consists of a phandle to the rcpm node and the IPPDEXPCR
 	register cells. The number of IPPDEXPCR register cells is defined in
-	"fsl,#rcpm-wakeup-cells" in the rcpm node. The first register cell is
+	"#fsl,rcpm-wakeup-cells" in the rcpm node. The first register cell is
 	the bit mask that should be set in IPPDEXPCR0, and the second register
 	cell is for IPPDEXPCR1, and so on.
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] soc: fsl: add RCPM driver

[Ran Wang]

The NXP's QorIQ Processors based on ARM Core have RCPM module
(Run Control and Power Management), which performs system level
tasks associated with power management such as wakeup source control.

This driver depends on PM wakeup source framework which help to
collect wake information.

Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
---
Change in v8:
	- Adjust related API usage to meet wakeup.c's update in patch 1/3.
	- Add sanity checking for the case of ws->dev or ws->dev->parent
	  is null.

Change in v7:
	- Replace 'ws->dev' with 'ws->dev->parent' to get aligned with
	c8377adfa781 ("PM / wakeup: Show wakeup sources stats in sysfs")
	- Remove '+obj-y += ftm_alarm.o' since it is wrong.
	- Cosmetic work.

Change in v6:
	- Adjust related API usage to meet wakeup.c's update in patch 1/3.

Change in v5:
	- Fix v4 regression of the return value of wakeup_source_get_next()
	didn't pass to ws in while loop.
	- Rename wakeup_source member 'attached_dev' to 'dev'.
	- Rename property 'fsl,#rcpm-wakeup-cells' to '#fsl,rcpm-wakeup-cells'.
	please see https://lore.kernel.org/patchwork/patch/1101022/

Change in v4:
	- Remove extra ',' in author line of rcpm.c
	- Update usage of wakeup_source_get_next() to be less confusing to the
reader, code logic remain the same.

Change in v3:
	- Some whitespace ajdustment.

Change in v2:
	- Rebase Kconfig and Makefile update to latest mainline.

 drivers/soc/fsl/Kconfig  |   8 +++
 drivers/soc/fsl/Makefile |   1 +
 drivers/soc/fsl/rcpm.c   | 133 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+)
 create mode 100644 drivers/soc/fsl/rcpm.c

diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
index f9ad8ad..4918856 100644
--- a/drivers/soc/fsl/Kconfig
+++ b/drivers/soc/fsl/Kconfig
@@ -40,4 +40,12 @@ config DPAA2_CONSOLE
 	  /dev/dpaa2_mc_console and /dev/dpaa2_aiop_console,
 	  which can be used to dump the Management Complex and AIOP
 	  firmware logs.
+
+config FSL_RCPM
+	bool "Freescale RCPM support"
+	depends on PM_SLEEP
+	help
+	  The NXP QorIQ Processors based on ARM Core have RCPM module
+	  (Run Control and Power Management), which performs all device-level
+	  tasks associated with power management, such as wakeup source control.
 endmenu
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index 71dee8d..906f1cd 100644
--- a/drivers/soc/fsl/Makefile
+++ b/drivers/soc/fsl/Makefile
@@ -6,6 +6,7 @@
 obj-$(CONFIG_FSL_DPAA)                 += qbman/
 obj-$(CONFIG_QUICC_ENGINE)		+= qe/
 obj-$(CONFIG_CPM)			+= qe/
+obj-$(CONFIG_FSL_RCPM)			+= rcpm.o
 obj-$(CONFIG_FSL_GUTS)			+= guts.o
 obj-$(CONFIG_FSL_MC_DPIO) 		+= dpio/
 obj-$(CONFIG_DPAA2_CONSOLE)		+= dpaa2-console.o
diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
new file mode 100644
index 0000000..3ed135e
--- /dev/null
+++ b/drivers/soc/fsl/rcpm.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// rcpm.c - Freescale QorIQ RCPM driver
+//
+// Copyright 2019 NXP
+//
+// Author: Ran Wang <ran.wang_1@nxp.com>
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
+#include <linux/kernel.h>
+
+#define RCPM_WAKEUP_CELL_MAX_SIZE	7
+
+struct rcpm {
+	unsigned int	wakeup_cells;
+	void __iomem	*ippdexpcr_base;
+	bool		little_endian;
+};
+
+static int rcpm_pm_prepare(struct device *dev)
+{
+	int i, ret, idx;
+	void __iomem *base;
+	struct wakeup_source	*ws;
+	struct rcpm		*rcpm;
+	struct device_node	*np = dev->of_node;
+	u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1], tmp;
+
+	rcpm = dev_get_drvdata(dev);
+	if (!rcpm)
+		return -EINVAL;
+
+	base = rcpm->ippdexpcr_base;
+	idx = wakeup_sources_read_lock();
+
+	/* Begin with first registered wakeup source */
+	for_each_wakeup_source(ws) {
+
+		/* skip object which is not attached to device */
+		if (!ws->dev || !ws->dev->parent)
+			continue;
+
+		ret = device_property_read_u32_array(ws->dev->parent,
+				"fsl,rcpm-wakeup", value,
+				rcpm->wakeup_cells + 1);
+
+		/*  Wakeup source should refer to current rcpm device */
+		if (ret || (np->phandle != value[0])) {
+			dev_info(dev, "%s doesn't refer to this rcpm\n",
+					ws->name);
+			continue;
+		}
+
+		for (i = 0; i < rcpm->wakeup_cells; i++) {
+			/* We can only OR related bits */
+			if (value[i + 1]) {
+				if (rcpm->little_endian) {
+					tmp = ioread32(base + i * 4);
+					tmp |= value[i + 1];
+					iowrite32(tmp, base + i * 4);
+				} else {
+					tmp = ioread32be(base + i * 4);
+					tmp |= value[i + 1];
+					iowrite32be(tmp, base + i * 4);
+				}
+			}
+		}
+	}
+
+	wakeup_sources_read_unlock(idx);
+
+	return 0;
+}
+
+static const struct dev_pm_ops rcpm_pm_ops = {
+	.prepare =  rcpm_pm_prepare,
+};
+
+static int rcpm_probe(struct platform_device *pdev)
+{
+	struct device	*dev = &pdev->dev;
+	struct resource *r;
+	struct rcpm	*rcpm;
+	int ret;
+
+	rcpm = devm_kzalloc(dev, sizeof(*rcpm), GFP_KERNEL);
+	if (!rcpm)
+		return -ENOMEM;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r)
+		return -ENODEV;
+
+	rcpm->ippdexpcr_base = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(rcpm->ippdexpcr_base)) {
+		ret =  PTR_ERR(rcpm->ippdexpcr_base);
+		return ret;
+	}
+
+	rcpm->little_endian = device_property_read_bool(
+			&pdev->dev, "little-endian");
+
+	ret = device_property_read_u32(&pdev->dev,
+			"#fsl,rcpm-wakeup-cells", &rcpm->wakeup_cells);
+	if (ret)
+		return ret;
+
+	dev_set_drvdata(&pdev->dev, rcpm);
+
+	return 0;
+}
+
+static const struct of_device_id rcpm_of_match[] = {
+	{ .compatible = "fsl,qoriq-rcpm-2.1+", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rcpm_of_match);
+
+static struct platform_driver rcpm_driver = {
+	.driver = {
+		.name = "rcpm",
+		.of_match_table = rcpm_of_match,
+		.pm	= &rcpm_pm_ops,
+	},
+	.probe = rcpm_probe,
+};
+
+module_platform_driver(rcpm_driver);
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] soc: fsl: add RCPM driver

[Rafael J. Wysocki]

On Tue, Oct 22, 2019 at 9:52 AM Ran Wang <ran.wang_1@nxp.com> wrote:
>
> The NXP's QorIQ Processors based on ARM Core have RCPM module
> (Run Control and Power Management), which performs system level
> tasks associated with power management such as wakeup source control.
>
> This driver depends on PM wakeup source framework which help to
> collect wake information.
>
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
> Change in v8:
>         - Adjust related API usage to meet wakeup.c's update in patch 1/3.
>         - Add sanity checking for the case of ws->dev or ws->dev->parent
>           is null.
>
> Change in v7:
>         - Replace 'ws->dev' with 'ws->dev->parent' to get aligned with
>         c8377adfa781 ("PM / wakeup: Show wakeup sources stats in sysfs")
>         - Remove '+obj-y += ftm_alarm.o' since it is wrong.
>         - Cosmetic work.
>
> Change in v6:
>         - Adjust related API usage to meet wakeup.c's update in patch 1/3.
>
> Change in v5:
>         - Fix v4 regression of the return value of wakeup_source_get_next()
>         didn't pass to ws in while loop.
>         - Rename wakeup_source member 'attached_dev' to 'dev'.
>         - Rename property 'fsl,#rcpm-wakeup-cells' to '#fsl,rcpm-wakeup-cells'.
>         please see https://lore.kernel.org/patchwork/patch/1101022/
>
> Change in v4:
>         - Remove extra ',' in author line of rcpm.c
>         - Update usage of wakeup_source_get_next() to be less confusing to the
> reader, code logic remain the same.
>
> Change in v3:
>         - Some whitespace ajdustment.
>
> Change in v2:
>         - Rebase Kconfig and Makefile update to latest mainline.
>
>  drivers/soc/fsl/Kconfig  |   8 +++
>  drivers/soc/fsl/Makefile |   1 +
>  drivers/soc/fsl/rcpm.c   | 133 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 142 insertions(+)
>  create mode 100644 drivers/soc/fsl/rcpm.c
>
> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> index f9ad8ad..4918856 100644
> --- a/drivers/soc/fsl/Kconfig
> +++ b/drivers/soc/fsl/Kconfig
> @@ -40,4 +40,12 @@ config DPAA2_CONSOLE
>           /dev/dpaa2_mc_console and /dev/dpaa2_aiop_console,
>           which can be used to dump the Management Complex and AIOP
>           firmware logs.
> +
> +config FSL_RCPM
> +       bool "Freescale RCPM support"
> +       depends on PM_SLEEP
> +       help
> +         The NXP QorIQ Processors based on ARM Core have RCPM module
> +         (Run Control and Power Management), which performs all device-level
> +         tasks associated with power management, such as wakeup source control.
>  endmenu
> diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> index 71dee8d..906f1cd 100644
> --- a/drivers/soc/fsl/Makefile
> +++ b/drivers/soc/fsl/Makefile
> @@ -6,6 +6,7 @@
>  obj-$(CONFIG_FSL_DPAA)                 += qbman/
>  obj-$(CONFIG_QUICC_ENGINE)             += qe/
>  obj-$(CONFIG_CPM)                      += qe/
> +obj-$(CONFIG_FSL_RCPM)                 += rcpm.o
>  obj-$(CONFIG_FSL_GUTS)                 += guts.o
>  obj-$(CONFIG_FSL_MC_DPIO)              += dpio/
>  obj-$(CONFIG_DPAA2_CONSOLE)            += dpaa2-console.o
> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
> new file mode 100644
> index 0000000..3ed135e
> --- /dev/null
> +++ b/drivers/soc/fsl/rcpm.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// rcpm.c - Freescale QorIQ RCPM driver
> +//
> +// Copyright 2019 NXP
> +//
> +// Author: Ran Wang <ran.wang_1@nxp.com>
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/suspend.h>
> +#include <linux/kernel.h>
> +
> +#define RCPM_WAKEUP_CELL_MAX_SIZE      7
> +
> +struct rcpm {
> +       unsigned int    wakeup_cells;
> +       void __iomem    *ippdexpcr_base;
> +       bool            little_endian;
> +};
> +

Please add a kerneldoc comment describing this routine.

> +static int rcpm_pm_prepare(struct device *dev)
> +{
> +       int i, ret, idx;
> +       void __iomem *base;
> +       struct wakeup_source    *ws;
> +       struct rcpm             *rcpm;
> +       struct device_node      *np = dev->of_node;
> +       u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1], tmp;
> +
> +       rcpm = dev_get_drvdata(dev);
> +       if (!rcpm)
> +               return -EINVAL;
> +
> +       base = rcpm->ippdexpcr_base;
> +       idx = wakeup_sources_read_lock();
> +
> +       /* Begin with first registered wakeup source */
> +       for_each_wakeup_source(ws) {
> +
> +               /* skip object which is not attached to device */
> +               if (!ws->dev || !ws->dev->parent)
> +                       continue;
> +
> +               ret = device_property_read_u32_array(ws->dev->parent,
> +                               "fsl,rcpm-wakeup", value,
> +                               rcpm->wakeup_cells + 1);
> +
> +               /*  Wakeup source should refer to current rcpm device */
> +               if (ret || (np->phandle != value[0])) {
> +                       dev_info(dev, "%s doesn't refer to this rcpm\n",
> +                                       ws->name);

IMO printing this message is not useful in general, because it looks
like you just want to skip wakeup sources that aren't associated with
rcpm devices.

Maybe use pr_debug() to print it?  Or maybe use pr_debug() to print a
message if you have found a suitable device?  Wouldn't that be more
useful?

> +                       continue;
> +               }
> +

It would be good to add a comment explaining what the code below does
here.  Or explain that in the function's kerneldoc comment.

> +               for (i = 0; i < rcpm->wakeup_cells; i++) {

It looks like 'tmp' can be defined in this block.

And I would store the value of value[i+1] in tmp upfront, that is

u32 tmp = value[i+1];

> +                       /* We can only OR related bits */
> +                       if (value[i + 1]) {

Also I would do

if (!tmp)
        continue;

to reduce the indentation level.

> +                               if (rcpm->little_endian) {
> +                                       tmp = ioread32(base + i * 4);
> +                                       tmp |= value[i + 1];
> +                                       iowrite32(tmp, base + i * 4);

So it is sufficient to do

tmp |= ioread32(base + i * 4);
iowrite32(tmp, base + i * 4);

here and analogously below.

You may as well define

void __iomem *address = base + i * 4;

and use it everywhere in this block instead of the (base + I * 4) expression.

> +                               } else {
> +                                       tmp = ioread32be(base + i * 4);
> +                                       tmp |= value[i + 1];
> +                                       iowrite32be(tmp, base + i * 4);
> +                               }
> +                       }
> +               }
> +       }
> +
> +       wakeup_sources_read_unlock(idx);
> +
> +       return 0;
> +}
> +
> +static const struct dev_pm_ops rcpm_pm_ops = {
> +       .prepare =  rcpm_pm_prepare,
> +};
> +

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] PM: wakeup: Add routine to help fetch wakeup source object.

[Rafael J. Wysocki]

On Tue, Oct 22, 2019 at 9:51 AM Ran Wang <ran.wang_1@nxp.com> wrote:
>
> Some user might want to go through all registered wakeup sources
> and doing things accordingly. For example, SoC PM driver might need to
> do HW programming to prevent powering down specific IP which wakeup
> source depending on. So add this API to help walk through all registered
> wakeup source objects on that list and return them one by one.
>
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> Tested-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
> Change in v8
>         - Rename wakeup_source_get_next() to wakeup_sources_walk_next().
>         - Add wakeup_sources_read_lock() to take over locking job of
>           wakeup_source_get_star().
>         - Rename wakeup_source_get_start() to wakeup_sources_walk_start().
>         - Replace wakeup_source_get_stop() with wakeup_sources_read_unlock().
>         - Define macro for_each_wakeup_source(ws).
>
> Change in v7:
>         - Remove define of member *dev in wake_irq to fix conflict with commit
>         c8377adfa781 ("PM / wakeup: Show wakeup sources stats in sysfs"), user
>         will use ws->dev->parent instead.
>         - Remove '#include <linux/of_device.h>' because it is not used.
>
> Change in v6:
>         - Add wakeup_source_get_star() and wakeup_source_get_stop() to aligned
>         with wakeup_sources_stats_seq_start/nex/stop.
>
> Change in v5:
>         - Update commit message, add decription of walk through all wakeup
>         source objects.
>         - Add SCU protection in function wakeup_source_get_next().
>         - Rename wakeup_source member 'attached_dev' to 'dev' and move it up
>         (before wakeirq).
>
> Change in v4:
>         - None.
>
> Change in v3:
>         - Adjust indentation of *attached_dev;.
>
> Change in v2:
>         - None.
>
>  drivers/base/power/wakeup.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pm_wakeup.h   |  9 +++++++++
>  2 files changed, 51 insertions(+)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 5817b51..8c7a5f9 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -248,6 +248,48 @@ void wakeup_source_unregister(struct wakeup_source *ws)
>  EXPORT_SYMBOL_GPL(wakeup_source_unregister);
>
>  /**
> + * wakeup_sources_read_lock - Lock wakeup source list for read.

Please document the return value.

> + */
> +int wakeup_sources_read_lock(void)
> +{
> +       return srcu_read_lock(&wakeup_srcu);
> +}
> +EXPORT_SYMBOL_GPL(wakeup_sources_read_lock);
> +
> +/**
> + * wakeup_sources_read_unlock - Unlock wakeup source list.

Please document the argument.

> + */
> +void wakeup_sources_read_unlock(int idx)
> +{
> +       srcu_read_unlock(&wakeup_srcu, idx);
> +}
> +EXPORT_SYMBOL_GPL(wakeup_sources_read_unlock);
> +
> +/**
> + * wakeup_sources_walk_start - Begin a walk on wakeup source list

Please document the return value and add a note that the wakeup
sources list needs to be locked for reading for this to be safe.

> + */
> +struct wakeup_source *wakeup_sources_walk_start(void)
> +{
> +       struct list_head *ws_head = &wakeup_sources;
> +
> +       return list_entry_rcu(ws_head->next, struct wakeup_source, entry);
> +}
> +EXPORT_SYMBOL_GPL(wakeup_sources_walk_start);
> +
> +/**
> + * wakeup_sources_walk_next - Get next wakeup source from the list
> + * @ws: Previous wakeup source object

Please add a note that the wakeup sources list needs to be locked for
reading for this to be safe.

> + */
> +struct wakeup_source *wakeup_sources_walk_next(struct wakeup_source *ws)
> +{
> +       struct list_head *ws_head = &wakeup_sources;
> +
> +       return list_next_or_null_rcu(ws_head, &ws->entry,
> +                               struct wakeup_source, entry);
> +}
> +EXPORT_SYMBOL_GPL(wakeup_sources_walk_next);
> +
> +/**
>   * device_wakeup_attach - Attach a wakeup source object to a device object.
>   * @dev: Device to handle.
>   * @ws: Wakeup source object to attach to @dev.
> diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
> index 661efa0..aa3da66 100644
> --- a/include/linux/pm_wakeup.h
> +++ b/include/linux/pm_wakeup.h
> @@ -63,6 +63,11 @@ struct wakeup_source {
>         bool                    autosleep_enabled:1;
>  };
>
> +#define for_each_wakeup_source(ws) \
> +       for ((ws) = wakeup_sources_walk_start();        \
> +            (ws);                                      \
> +            (ws) = wakeup_sources_walk_next((ws)))
> +
>  #ifdef CONFIG_PM_SLEEP
>
>  /*
> @@ -92,6 +97,10 @@ extern void wakeup_source_remove(struct wakeup_source *ws);
>  extern struct wakeup_source *wakeup_source_register(struct device *dev,
>                                                     const char *name);
>  extern void wakeup_source_unregister(struct wakeup_source *ws);
> +extern int wakeup_sources_read_lock(void);
> +extern void wakeup_sources_read_unlock(int idx);
> +extern struct wakeup_source *wakeup_sources_walk_start(void);
> +extern struct wakeup_source *wakeup_sources_walk_next(struct wakeup_source *ws);
>  extern int device_wakeup_enable(struct device *dev);
>  extern int device_wakeup_disable(struct device *dev);
>  extern void device_set_wakeup_capable(struct device *dev, bool capable);
> --
> 2.7.4
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] soc: fsl: add RCPM driver

[Ran Wang]

The NXP's QorIQ Processors based on ARM Core have RCPM module
(Run Control and Power Management), which performs all device-level
tasks associated with power management such as wakeup source control.

This driver depends on PM wakeup source framework which help to
collect wake information.

Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
---
 drivers/soc/fsl/Kconfig  |    8 +++
 drivers/soc/fsl/Makefile |    1 +
 drivers/soc/fsl/rcpm.c   |  124 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+), 0 deletions(-)
 create mode 100644 drivers/soc/fsl/rcpm.c

diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
index 3b85e18..a25e05b 100644
--- a/drivers/soc/fsl/Kconfig
+++ b/drivers/soc/fsl/Kconfig
@@ -50,4 +50,12 @@ config FSL_SLEEP_FSM
 if ARM || ARM64
 source "drivers/soc/fsl/Kconfig.arm"
 endif
+
+config FSL_RCPM
+	bool "Freescale RCPM support"
+	depends on PM_SLEEP
+	help
+	  The NXP's QorIQ Processors based on ARM Core have RCPM module
+	  (Run Control and Power Management), which performs all device-level
+	  tasks associated with power management, such as wakeup source control.
 endmenu
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index db7b09b..aab9f9b 100644
--- a/drivers/soc/fsl/Makefile
+++ b/drivers/soc/fsl/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_FSL_MC_DPIO) 		+= dpio/
 obj-$(CONFIG_FSL_LS2_CONSOLE)		+= ls2-console/
 obj-$(CONFIG_LS_SOC_DRIVERS)		+= layerscape/
 obj-$(CONFIG_FSL_SLEEP_FSM)	+= sleep_fsm.o
+obj-$(CONFIG_FSL_RCPM)		+= rcpm.o
diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
new file mode 100644
index 0000000..b817319
--- /dev/null
+++ b/drivers/soc/fsl/rcpm.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// rcpm.c - Freescale QorIQ RCPM driver
+//
+// Copyright 2019 NXP
+//
+// Author: Ran Wang <ran.wang_1@nxp.com>,
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
+#include <linux/kernel.h>
+
+#define RCPM_WAKEUP_CELL_MAX_SIZE	7
+
+struct rcpm {
+	unsigned int wakeup_cells;
+	void __iomem *ippdexpcr_base;
+	bool	little_endian;
+};
+
+static int rcpm_pm_prepare(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct wakeup_source *ws;
+	struct rcpm *rcpm;
+	u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1], tmp;
+	int i, ret;
+
+	rcpm = dev_get_drvdata(dev);
+	if (!rcpm)
+		return -EINVAL;
+
+	/* Begin with first registered wakeup source */
+	ws = wakeup_source_get_next(NULL);
+	while (ws) {
+		ret = device_property_read_u32_array(ws->attached_dev,
+				"fsl,rcpm-wakeup", value, rcpm->wakeup_cells + 1);
+
+		/*  Wakeup source should refer to current rcpm device */
+		if (ret || (np->phandle != value[0])) {
+			dev_info(dev, "%s doesn't refer to this rcpm\n",
+					ws->name);
+			ws = wakeup_source_get_next(ws);
+			continue;
+		}
+
+		for (i = 0; i < rcpm->wakeup_cells; i++) {
+			/* We can only OR related bits */
+			if (value[i + 1]) {
+				if (rcpm->little_endian) {
+					tmp = ioread32(rcpm->ippdexpcr_base + i * 4);
+					tmp |= value[i + 1];
+					iowrite32(tmp, rcpm->ippdexpcr_base + i * 4);
+				} else {
+					tmp = ioread32be(rcpm->ippdexpcr_base + i * 4);
+					tmp |= value[i + 1];
+					iowrite32be(tmp, rcpm->ippdexpcr_base + i * 4);
+				}
+			}
+		}
+		ws = wakeup_source_get_next(ws);
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops rcpm_pm_ops = {
+	.prepare =  rcpm_pm_prepare,
+};
+
+static int rcpm_probe(struct platform_device *pdev)
+{
+	struct device	*dev = &pdev->dev;
+	struct resource *r;
+	struct rcpm		*rcpm;
+	int ret;
+
+	rcpm = devm_kzalloc(dev, sizeof(*rcpm), GFP_KERNEL);
+	if (!rcpm)
+		return -ENOMEM;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r)
+		return -ENODEV;
+
+	rcpm->ippdexpcr_base = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(rcpm->ippdexpcr_base)) {
+		ret =  PTR_ERR(rcpm->ippdexpcr_base);
+		return ret;
+	}
+
+	rcpm->little_endian = device_property_read_bool(
+			&pdev->dev, "little-endian");
+
+	ret = device_property_read_u32(&pdev->dev,
+			"fsl,#rcpm-wakeup-cells", &rcpm->wakeup_cells);
+	if (ret)
+		return ret;
+
+	dev_set_drvdata(&pdev->dev, rcpm);
+
+	return 0;
+}
+
+static const struct of_device_id rcpm_of_match[] = {
+	{ .compatible = "fsl,qoriq-rcpm-2.1+", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rcpm_of_match);
+
+static struct platform_driver rcpm_driver = {
+	.driver = {
+		.name = "rcpm",
+		.of_match_table = rcpm_of_match,
+		.pm	= &rcpm_pm_ops,
+	},
+	.probe = rcpm_probe,
+};
+
+module_platform_driver(rcpm_driver);
-- 
1.7.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] soc: fsl: add RCPM driver

[Ran Wang]

Hi Scott,

On 2018/9/8 18:16, Scott Wood wrote:
> 
> On Fri, 2018-08-31 at 11:52 +0800, Ran Wang wrote:
> > The NXP's QorIQ Processors based on ARM Core have RCPM module (Run
> > Control and Power Management), which performs all device-level tasks
> > associated with power management such as wakeup source control.
> >
> > This driver depends on FSL platform PM driver framework which help to
> > isolate user and PM service provider (such as RCPM driver).
> >
> > Signed-off-by: Chenhui Zhao <chenhui.zhao@nxp.com>
> > Signed-off-by: Ying Zhang <ying.zhang22455@nxp.com>
> > Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> > ---
> >  drivers/soc/fsl/Kconfig   |    6 ++
> >  drivers/soc/fsl/Makefile  |    1 +
> >  drivers/soc/fsl/ls-rcpm.c |  153
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 160 insertions(+), 0 deletions(-)  create mode
> > 100644 drivers/soc/fsl/ls-rcpm.c
> 
> Is there a reason why this is LS-specific, or could it be used with PPC RCPM
> blocks?

They have different SW arch design on low power operation: PPC RCPM
driver has taken care of most things of suspend enter & exit. And LS RCPM driver
will only handle wakeup source configure and left rest work to system firmware
to do. So you might be aware that LS RCPM will only get call whenever plat_pm
driver API is called rather than system suspend begin where.

> 
> > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig index
> > 6517412..882330d 100644
> > --- a/drivers/soc/fsl/Kconfig
> > +++ b/drivers/soc/fsl/Kconfig
> > @@ -30,3 +30,9 @@ config FSL_PLAT_PM
> >  	  have to know the implement details of wakeup function it require.
> >  	  Besides, it is also easy for service side to upgrade its logic
> > when
> >  	  design changed and remain user side unchanged.
> > +
> > +config LS_RCPM
> > +	bool "Freescale RCPM support"
> > +	depends on (FSL_PLAT_PM)
> 
> Why is this parenthesized?

Because we'd like to decouple RCPM driver and its user.
Benefit is that will allow user doesn't have to know who will serve it for some PM
features (such as wake up source control), and provide some kind of flexibility when
either RCMP or user driver evolve in the future. So I add a plat_pm driver to prevent
wake up IP knowing any information of RCPM.

Regards,
Ran

> 
> -Scott

[PATCH 3/3] soc: fsl: add RCPM driver

[Ran Wang]

Hi Leo,

On 2018/9/7 4:51, Yang Li wrote:
> 
> On Fri, Sep 7, 2018 at 4:51 AM Ran Wang <ran.wang_1@nxp.com> wrote:
> >
> > Hi Leo,
> >
> > On September 05, 2018 at 11:22 Yang Li wrote:
> > > -----Original Message-----
> > > From: Li Yang <leoyang.li@nxp.com>
> > > Sent: Wednesday, September 05, 2018 11:22
> > > To: dongsheng.wang at hxt-semitech.com
> > > Cc: Ran Wang <ran.wang_1@nxp.com>; Rob Herring
> <robh+dt@kernel.org>;
> > > Mark Rutland <mark.rutland@arm.com>; open list:OPEN FIRMWARE AND
> > > FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>;
> > > linuxppc- dev <linuxppc-dev@lists.ozlabs.org>; lkml
> > > <linux-kernel@vger.kernel.org>; moderated list:ARM/FREESCALE IMX /
> > > MXC ARM ARCHITECTURE <linux-arm- kernel@lists.infradead.org>
> > > Subject: Re: [PATCH 3/3] soc: fsl: add RCPM driver
> > >
> > > On Tue, Sep 4, 2018 at 9:58 PM Wang, Dongsheng
> <dongsheng.wang@hxt-
> > > semitech.com> wrote:
> > > >
> > > > Please change your comments style.
> > >
> > > Although this doesn't get into the Linux kernel coding style
> > > documentation yet, Linus seems changed his mind to prefer // than /*
> > > */ comment style now.
> > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fl
> > >
> kml .org%2Flkml%2F2017%2F11%2F25%2F133&amp;data=02%7C01%7Cran.w
> ang_
> > >
> 1%40nxp.com%7Cc0d88e6690384e02b95108d612dec235%7C686ea1d3bc2b4c
> > >
> 6fa92cd99c5c301635%7C0%7C0%7C636717145285126200&amp;sdata=JIoCZp
> > >
> WhRyW76EqgSflfTDA1f0gMQGKa%2FcbvSc5CO%2Fw%3D&amp;reserved=0
> > > So the
> > > // style should be acceptable for now.
> > >
> > > >
> > > > On 2018/8/31 11:56, Ran Wang wrote:
> > > > > The NXP's QorIQ Processors based on ARM Core have RCPM module
> > > > > (Run Control and Power Management), which performs all
> > > > > device-level tasks associated with power management such as
> wakeup source control.
> > > > >
> > > > > This driver depends on FSL platform PM driver framework which
> > > > > help to isolate user and PM service provider (such as RCPM driver).
> > > > >
> > > > > Signed-off-by: Chenhui Zhao <chenhui.zhao@nxp.com>
> > > > > Signed-off-by: Ying Zhang <ying.zhang22455@nxp.com>
> > > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> > > > > ---
> > > > >  drivers/soc/fsl/Kconfig   |    6 ++
> > > > >  drivers/soc/fsl/Makefile  |    1 +
> > > > >  drivers/soc/fsl/ls-rcpm.c |  153
> > > > > +++++++++++++++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 160 insertions(+), 0 deletions(-)  create mode
> > > > > 100644 drivers/soc/fsl/ls-rcpm.c
> > > > >
> > > > > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> > > > > index 6517412..882330d 100644
> > > > > --- a/drivers/soc/fsl/Kconfig
> > > > > +++ b/drivers/soc/fsl/Kconfig
> > > > > @@ -30,3 +30,9 @@ config FSL_PLAT_PM
> > > > >         have to know the implement details of wakeup function it
> require.
> > > > >         Besides, it is also easy for service side to upgrade its logic when
> > > > >         design changed and remain user side unchanged.
> > > > > +
> > > > > +config LS_RCPM
> > > > > +     bool "Freescale RCPM support"
> > > > > +     depends on (FSL_PLAT_PM)
> > > > > +     help
> > > > > +       This feature is to enable specified wakeup source for system
> sleep.
> > > > > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> > > > > index 8f9db23..43ff71a 100644
> > > > > --- a/drivers/soc/fsl/Makefile
> > > > > +++ b/drivers/soc/fsl/Makefile
> > > > > @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)            += qe/
> > > > >  obj-$(CONFIG_CPM)                    += qe/
> > > > >  obj-$(CONFIG_FSL_GUTS)                       += guts.o
> > > > >  obj-$(CONFIG_FSL_PLAT_PM)    += plat_pm.o
> > > > > +obj-$(CONFIG_LS_RCPM)                += ls-rcpm.o
> > >
> > > Probably use "_" instead of "-" for alignment.
> >
> > OK, will update in next version
> >
> > > > > diff --git a/drivers/soc/fsl/ls-rcpm.c
> > > > > b/drivers/soc/fsl/ls-rcpm.c new file mode 100644 index
> > > > > 0000000..b0feb88
> > > > > --- /dev/null
> > > > > +++ b/drivers/soc/fsl/ls-rcpm.c
> > > > > @@ -0,0 +1,153 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0 // // plat_pm.c - Freescale
> > > > > +Layerscape RCPM driver
> > >
> > > The file name here is not the same as the real file name.
> >
> > Got it, will correct it.
> >
> > > > > +//
> > > > > +// Copyright 2018 NXP
> > > > > +//
> > > > > +// Author: Ran Wang <ran.wang_1@nxp.com>,
> > >
> > > Where do you need the comma in the end?
> >
> > My bad, will remove comma in next version.
> >
> > > > > +
> > > > > +#include <linux/init.h>
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/platform_device.h> #include
> > > > > +<linux/of_address.h> #include <linux/slab.h> #include
> > > > > +<soc/fsl/plat_pm.h>
> > > > > +
> > > > > +#define MAX_COMPATIBLE_NUM   10
> > > > > +
> > > > > +struct rcpm_t {
> > > > > +     struct device *dev;
> > > > > +     void __iomem *ippdexpcr_addr;
> > > > > +     bool big_endian;        /* Big/Little endian of RCPM module */
> > > > > +};
> > > > > +
> > > > > +// rcpm_handle - Configure RCPM reg according to wake up source
> > > > > +request // @user_dev: pointer to user's device struct // @flag:
> > > > > +to
> > > > > +enable(true) or disable(false) wakeup source // @handle_priv:
> > > > > +pointer to struct rcpm_t instance // // Return 0 on success
> > > > > +other negative errno
> > >
> > > Although Linus preferred this // comment style.  I'm not sure if
> > > this will be handled correctly by the kernel-doc compiler.
> > >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw
> > > w
> > > w.kernel.org%2Fdoc%2Fhtml%2Fv4.18%2Fdoc-guide%2Fkernel-
> > >
> doc.html&amp;data=02%7C01%7Cran.wang_1%40nxp.com%7Cc0d88e669038
> > >
> 4e02b95108d612dec235%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> > > %7C636717145285126200&amp;sdata=H7GkUNOLVG%2FCcZESzhtHBeHC
> bO9
> > > %2FK4k9EdH30Cxq2%2BM%3D&amp;reserved=0
> >
> > So, do you think I need to change all comment style back to '/* ... */' ?
> > Actually I feel a little bit confused here.
> 
> I think Linus's comment about // comment style applies to normal code
> comment.  But kernel-doc comment is a special kind of code comment that
> needs to meet certain requirements.  People can use the scripts/kernel-doc
> tool to generate readable API documents from the source code.  It looks like
> you wanted to make the function description aligned with the kernel-doc
> format, but kernel-doc specifically requires to use the /* */ style(at least for
> now).

OK, will change style back to /* */.

Regards,
Ran

> Regards,
> Leo

[PATCH 3/3] soc: fsl: add RCPM driver

[Scott Wood]

On Fri, 2018-08-31 at 11:52 +0800, Ran Wang wrote:
> The NXP's QorIQ Processors based on ARM Core have RCPM module (Run
> Control and Power Management), which performs all device-level
> tasks associated with power management such as wakeup source control.
> 
> This driver depends on FSL platform PM driver framework which help to
> isolate user and PM service provider (such as RCPM driver).
> 
> Signed-off-by: Chenhui Zhao <chenhui.zhao@nxp.com>
> Signed-off-by: Ying Zhang <ying.zhang22455@nxp.com>
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
>  drivers/soc/fsl/Kconfig   |    6 ++
>  drivers/soc/fsl/Makefile  |    1 +
>  drivers/soc/fsl/ls-rcpm.c |  153
> +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 160 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/soc/fsl/ls-rcpm.c

Is there a reason why this is LS-specific, or could it be used with PPC RCPM
blocks?

> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> index 6517412..882330d 100644
> --- a/drivers/soc/fsl/Kconfig
> +++ b/drivers/soc/fsl/Kconfig
> @@ -30,3 +30,9 @@ config FSL_PLAT_PM
>  	  have to know the implement details of wakeup function it require.
>  	  Besides, it is also easy for service side to upgrade its logic
> when
>  	  design changed and remain user side unchanged.
> +
> +config LS_RCPM
> +	bool "Freescale RCPM support"
> +	depends on (FSL_PLAT_PM)

Why is this parenthesized?

-Scott

[PATCH 3/3] soc: fsl: add RCPM driver

[Li Yang]

On Fri, Sep 7, 2018 at 4:51 AM Ran Wang <ran.wang_1@nxp.com> wrote:
>
> Hi Leo,
>
> On September 05, 2018 at 11:22 Yang Li wrote:
> > -----Original Message-----
> > From: Li Yang <leoyang.li@nxp.com>
> > Sent: Wednesday, September 05, 2018 11:22
> > To: dongsheng.wang at hxt-semitech.com
> > Cc: Ran Wang <ran.wang_1@nxp.com>; Rob Herring <robh+dt@kernel.org>;
> > Mark Rutland <mark.rutland@arm.com>; open list:OPEN FIRMWARE AND
> > FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; linuxppc-
> > dev <linuxppc-dev@lists.ozlabs.org>; lkml <linux-kernel@vger.kernel.org>;
> > moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE <linux-arm-
> > kernel at lists.infradead.org>
> > Subject: Re: [PATCH 3/3] soc: fsl: add RCPM driver
> >
> > On Tue, Sep 4, 2018 at 9:58 PM Wang, Dongsheng <dongsheng.wang@hxt-
> > semitech.com> wrote:
> > >
> > > Please change your comments style.
> >
> > Although this doesn't get into the Linux kernel coding style documentation
> > yet, Linus seems changed his mind to prefer // than /*
> > */ comment style now.
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml
> > .org%2Flkml%2F2017%2F11%2F25%2F133&amp;data=02%7C01%7Cran.wang_
> > 1%40nxp.com%7Cc0d88e6690384e02b95108d612dec235%7C686ea1d3bc2b4c
> > 6fa92cd99c5c301635%7C0%7C0%7C636717145285126200&amp;sdata=JIoCZp
> > WhRyW76EqgSflfTDA1f0gMQGKa%2FcbvSc5CO%2Fw%3D&amp;reserved=0
> > So the
> > // style should be acceptable for now.
> >
> > >
> > > On 2018/8/31 11:56, Ran Wang wrote:
> > > > The NXP's QorIQ Processors based on ARM Core have RCPM module (Run
> > > > Control and Power Management), which performs all device-level tasks
> > > > associated with power management such as wakeup source control.
> > > >
> > > > This driver depends on FSL platform PM driver framework which help
> > > > to isolate user and PM service provider (such as RCPM driver).
> > > >
> > > > Signed-off-by: Chenhui Zhao <chenhui.zhao@nxp.com>
> > > > Signed-off-by: Ying Zhang <ying.zhang22455@nxp.com>
> > > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> > > > ---
> > > >  drivers/soc/fsl/Kconfig   |    6 ++
> > > >  drivers/soc/fsl/Makefile  |    1 +
> > > >  drivers/soc/fsl/ls-rcpm.c |  153
> > > > +++++++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 160 insertions(+), 0 deletions(-)  create mode
> > > > 100644 drivers/soc/fsl/ls-rcpm.c
> > > >
> > > > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig index
> > > > 6517412..882330d 100644
> > > > --- a/drivers/soc/fsl/Kconfig
> > > > +++ b/drivers/soc/fsl/Kconfig
> > > > @@ -30,3 +30,9 @@ config FSL_PLAT_PM
> > > >         have to know the implement details of wakeup function it require.
> > > >         Besides, it is also easy for service side to upgrade its logic when
> > > >         design changed and remain user side unchanged.
> > > > +
> > > > +config LS_RCPM
> > > > +     bool "Freescale RCPM support"
> > > > +     depends on (FSL_PLAT_PM)
> > > > +     help
> > > > +       This feature is to enable specified wakeup source for system sleep.
> > > > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> > > > index 8f9db23..43ff71a 100644
> > > > --- a/drivers/soc/fsl/Makefile
> > > > +++ b/drivers/soc/fsl/Makefile
> > > > @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)            += qe/
> > > >  obj-$(CONFIG_CPM)                    += qe/
> > > >  obj-$(CONFIG_FSL_GUTS)                       += guts.o
> > > >  obj-$(CONFIG_FSL_PLAT_PM)    += plat_pm.o
> > > > +obj-$(CONFIG_LS_RCPM)                += ls-rcpm.o
> >
> > Probably use "_" instead of "-" for alignment.
>
> OK, will update in next version
>
> > > > diff --git a/drivers/soc/fsl/ls-rcpm.c b/drivers/soc/fsl/ls-rcpm.c
> > > > new file mode 100644 index 0000000..b0feb88
> > > > --- /dev/null
> > > > +++ b/drivers/soc/fsl/ls-rcpm.c
> > > > @@ -0,0 +1,153 @@
> > > > +// SPDX-License-Identifier: GPL-2.0 // // plat_pm.c - Freescale
> > > > +Layerscape RCPM driver
> >
> > The file name here is not the same as the real file name.
>
> Got it, will correct it.
>
> > > > +//
> > > > +// Copyright 2018 NXP
> > > > +//
> > > > +// Author: Ran Wang <ran.wang_1@nxp.com>,
> >
> > Where do you need the comma in the end?
>
> My bad, will remove comma in next version.
>
> > > > +
> > > > +#include <linux/init.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/platform_device.h>
> > > > +#include <linux/of_address.h>
> > > > +#include <linux/slab.h>
> > > > +#include <soc/fsl/plat_pm.h>
> > > > +
> > > > +#define MAX_COMPATIBLE_NUM   10
> > > > +
> > > > +struct rcpm_t {
> > > > +     struct device *dev;
> > > > +     void __iomem *ippdexpcr_addr;
> > > > +     bool big_endian;        /* Big/Little endian of RCPM module */
> > > > +};
> > > > +
> > > > +// rcpm_handle - Configure RCPM reg according to wake up source
> > > > +request // @user_dev: pointer to user's device struct // @flag: to
> > > > +enable(true) or disable(false) wakeup source // @handle_priv:
> > > > +pointer to struct rcpm_t instance // // Return 0 on success other
> > > > +negative errno
> >
> > Although Linus preferred this // comment style.  I'm not sure if this will be
> > handled correctly by the kernel-doc compiler.
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> > w.kernel.org%2Fdoc%2Fhtml%2Fv4.18%2Fdoc-guide%2Fkernel-
> > doc.html&amp;data=02%7C01%7Cran.wang_1%40nxp.com%7Cc0d88e669038
> > 4e02b95108d612dec235%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> > %7C636717145285126200&amp;sdata=H7GkUNOLVG%2FCcZESzhtHBeHCbO9
> > %2FK4k9EdH30Cxq2%2BM%3D&amp;reserved=0
>
> So, do you think I need to change all comment style back to '/* ... */' ?
> Actually I feel a little bit confused here.

I think Linus's comment about // comment style applies to normal code
comment.  But kernel-doc comment is a special kind of code comment
that needs to meet certain requirements.  People can use the
scripts/kernel-doc tool to generate readable API documents from the
source code.  It looks like you wanted to make the function
description aligned with the kernel-doc format, but kernel-doc
specifically requires to use the /* */ style(at least for now).

Regards,
Leo

[PATCH 3/3] soc: fsl: add RCPM driver

[Ran Wang]

Hi Leo,

On September 05, 2018 at 11:22 Yang Li wrote:
> -----Original Message-----
> From: Li Yang <leoyang.li@nxp.com>
> Sent: Wednesday, September 05, 2018 11:22
> To: dongsheng.wang at hxt-semitech.com
> Cc: Ran Wang <ran.wang_1@nxp.com>; Rob Herring <robh+dt@kernel.org>;
> Mark Rutland <mark.rutland@arm.com>; open list:OPEN FIRMWARE AND
> FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; linuxppc-
> dev <linuxppc-dev@lists.ozlabs.org>; lkml <linux-kernel@vger.kernel.org>;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE <linux-arm-
> kernel at lists.infradead.org>
> Subject: Re: [PATCH 3/3] soc: fsl: add RCPM driver
> 
> On Tue, Sep 4, 2018 at 9:58 PM Wang, Dongsheng <dongsheng.wang@hxt-
> semitech.com> wrote:
> >
> > Please change your comments style.
> 
> Although this doesn't get into the Linux kernel coding style documentation
> yet, Linus seems changed his mind to prefer // than /*
> */ comment style now.
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml
> .org%2Flkml%2F2017%2F11%2F25%2F133&amp;data=02%7C01%7Cran.wang_
> 1%40nxp.com%7Cc0d88e6690384e02b95108d612dec235%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C636717145285126200&amp;sdata=JIoCZp
> WhRyW76EqgSflfTDA1f0gMQGKa%2FcbvSc5CO%2Fw%3D&amp;reserved=0
> So the
> // style should be acceptable for now.
> 
> >
> > On 2018/8/31 11:56, Ran Wang wrote:
> > > The NXP's QorIQ Processors based on ARM Core have RCPM module (Run
> > > Control and Power Management), which performs all device-level tasks
> > > associated with power management such as wakeup source control.
> > >
> > > This driver depends on FSL platform PM driver framework which help
> > > to isolate user and PM service provider (such as RCPM driver).
> > >
> > > Signed-off-by: Chenhui Zhao <chenhui.zhao@nxp.com>
> > > Signed-off-by: Ying Zhang <ying.zhang22455@nxp.com>
> > > Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> > > ---
> > >  drivers/soc/fsl/Kconfig   |    6 ++
> > >  drivers/soc/fsl/Makefile  |    1 +
> > >  drivers/soc/fsl/ls-rcpm.c |  153
> > > +++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 160 insertions(+), 0 deletions(-)  create mode
> > > 100644 drivers/soc/fsl/ls-rcpm.c
> > >
> > > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig index
> > > 6517412..882330d 100644
> > > --- a/drivers/soc/fsl/Kconfig
> > > +++ b/drivers/soc/fsl/Kconfig
> > > @@ -30,3 +30,9 @@ config FSL_PLAT_PM
> > >         have to know the implement details of wakeup function it require.
> > >         Besides, it is also easy for service side to upgrade its logic when
> > >         design changed and remain user side unchanged.
> > > +
> > > +config LS_RCPM
> > > +     bool "Freescale RCPM support"
> > > +     depends on (FSL_PLAT_PM)
> > > +     help
> > > +       This feature is to enable specified wakeup source for system sleep.
> > > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> > > index 8f9db23..43ff71a 100644
> > > --- a/drivers/soc/fsl/Makefile
> > > +++ b/drivers/soc/fsl/Makefile
> > > @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)            += qe/
> > >  obj-$(CONFIG_CPM)                    += qe/
> > >  obj-$(CONFIG_FSL_GUTS)                       += guts.o
> > >  obj-$(CONFIG_FSL_PLAT_PM)    += plat_pm.o
> > > +obj-$(CONFIG_LS_RCPM)                += ls-rcpm.o
> 
> Probably use "_" instead of "-" for alignment.

OK, will update in next version

> > > diff --git a/drivers/soc/fsl/ls-rcpm.c b/drivers/soc/fsl/ls-rcpm.c
> > > new file mode 100644 index 0000000..b0feb88
> > > --- /dev/null
> > > +++ b/drivers/soc/fsl/ls-rcpm.c
> > > @@ -0,0 +1,153 @@
> > > +// SPDX-License-Identifier: GPL-2.0 // // plat_pm.c - Freescale
> > > +Layerscape RCPM driver
> 
> The file name here is not the same as the real file name.

Got it, will correct it.

> > > +//
> > > +// Copyright 2018 NXP
> > > +//
> > > +// Author: Ran Wang <ran.wang_1@nxp.com>,
> 
> Where do you need the comma in the end?

My bad, will remove comma in next version.

> > > +
> > > +#include <linux/init.h>
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/slab.h>
> > > +#include <soc/fsl/plat_pm.h>
> > > +
> > > +#define MAX_COMPATIBLE_NUM   10
> > > +
> > > +struct rcpm_t {
> > > +     struct device *dev;
> > > +     void __iomem *ippdexpcr_addr;
> > > +     bool big_endian;        /* Big/Little endian of RCPM module */
> > > +};
> > > +
> > > +// rcpm_handle - Configure RCPM reg according to wake up source
> > > +request // @user_dev: pointer to user's device struct // @flag: to
> > > +enable(true) or disable(false) wakeup source // @handle_priv:
> > > +pointer to struct rcpm_t instance // // Return 0 on success other
> > > +negative errno
> 
> Although Linus preferred this // comment style.  I'm not sure if this will be
> handled correctly by the kernel-doc compiler.
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> w.kernel.org%2Fdoc%2Fhtml%2Fv4.18%2Fdoc-guide%2Fkernel-
> doc.html&amp;data=02%7C01%7Cran.wang_1%40nxp.com%7Cc0d88e669038
> 4e02b95108d612dec235%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636717145285126200&amp;sdata=H7GkUNOLVG%2FCcZESzhtHBeHCbO9
> %2FK4k9EdH30Cxq2%2BM%3D&amp;reserved=0

So, do you think I need to change all comment style back to '/* ... */' ?
Actually I feel a little bit confused here.

Regards,
Ran

> > > +static int rcpm_handle(struct device *user_dev, bool flag, void
> > > +*handle_priv) {
> > > +     struct rcpm_t *rcpm;
> > > +     bool big_endian;
> > > +     const char  *dev_compatible_array[MAX_COMPATIBLE_NUM];
> > > +     void __iomem *ippdexpcr_addr;
> > > +     u32 ippdexpcr;
> > > +     u32 set_bit;
> > > +     int ret, num, i;
> > > +
> > > +     rcpm = handle_priv;
> > > +     big_endian = rcpm->big_endian;
> > > +     ippdexpcr_addr = rcpm->ippdexpcr_addr;
> > > +
> > > +     num = device_property_read_string_array(user_dev, "compatible",
> > > +                     dev_compatible_array, MAX_COMPATIBLE_NUM);
> > > +     if (num < 0)
> > > +             return num;
> > > +
> > > +     for (i = 0; i < num; i++) {
> > > +             if (!device_property_present(rcpm->dev,
> > > +                                     dev_compatible_array[i]))
> > > +                     continue;
> > > +             else {
> > Remove this else.
> > > +                     ret = device_property_read_u32(rcpm->dev,
> > > +                                     dev_compatible_array[i], &set_bit);
> > > +                     if (ret)
> > > +                             return ret;
> > > +
> > > +                     if (!device_property_present(rcpm->dev,
> > > +
> > > + dev_compatible_array[i]))
> > This has been checked. Continue ? or return ENODEV?
> > > +                             return -ENODEV;
> > > +                     else {
> > Remove this else.
> > > +                             ret = device_property_read_u32(rcpm->dev,
> > > +                                             dev_compatible_array[i], &set_bit);
> > > +                             if (ret)
> > > +                                     return ret;
> > > +
> > > +                             if (big_endian)
> > > +                                     ippdexpcr = ioread32be(ippdexpcr_addr);
> > > +                             else
> > > +                                     ippdexpcr =
> > > + ioread32(ippdexpcr_addr);
> > > +
> > > +                             if (flag)
> > > +                                     ippdexpcr |= set_bit;
> > > +                             else
> > > +                                     ippdexpcr &= ~set_bit;
> > > +
> > > +                             if (big_endian) {
> > > +                                     iowrite32be(ippdexpcr, ippdexpcr_addr);
> > > +                                     ippdexpcr = ioread32be(ippdexpcr_addr);
> > > +                             } else
> > if (x) {
> > ....
> > ....
> > }  else {
> >
> > }
> > > +                                     iowrite32(ippdexpcr,
> > > + ippdexpcr_addr);
> > > +
> > > +                             return 0;
> > > +                     }
> > > +             }
> > > +     }
> > > +
> > > +     return -ENODEV;
> > > +}
> > > +
> > > +static int ls_rcpm_probe(struct platform_device *pdev) {
> > > +     struct resource *r;
> > > +     struct rcpm_t *rcpm;
> > > +
> > > +     r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > +     if (!r)
> > > +             return -ENODEV;
> > > +
> > > +     rcpm = kmalloc(sizeof(*rcpm), GFP_KERNEL);
> > kzalloc is better.
> > > +     if (!rcpm)
> > > +             return -ENOMEM;
> > > +
> > > +     rcpm->big_endian = device_property_read_bool(&pdev->dev,
> > > + "big-endian");
> > > +
> > > +     rcpm->ippdexpcr_addr = devm_ioremap_resource(&pdev->dev, r);
> > > +     if (IS_ERR(rcpm->ippdexpcr_addr))
> > > +             return PTR_ERR(rcpm->ippdexpcr_addr);
> > > +
> > > +     rcpm->dev = &pdev->dev;
> > > +     platform_set_drvdata(pdev, rcpm);
> > > +
> > > +     return register_fsl_platform_wakeup_source(rcpm_handle, rcpm);
> > > +}
> > > +
> > > +static int ls_rcpm_remove(struct platform_device *pdev) {
> > > +     struct rcpm_t   *rcpm;
> > Not need a table.
> >
> > Cheers,
> > -Dongsheng
> >
> > > +
> > > +     rcpm = platform_get_drvdata(pdev);
> > > +     deregister_fsl_platform_wakeup_source(rcpm);
> > > +     kfree(rcpm);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static const struct of_device_id ls_rcpm_of_match[] = {
> > > +     { .compatible = "fsl,qoriq-rcpm-2.1", },
> > > +     {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ls_rcpm_of_match);
> > > +
> > > +static struct platform_driver ls_rcpm_driver = {
> > > +     .driver = {
> > > +             .name = "ls-rcpm",
> > > +             .of_match_table = ls_rcpm_of_match,
> > > +     },
> > > +     .probe = ls_rcpm_probe,
> > > +     .remove = ls_rcpm_remove,
> > > +};
> > > +
> > > +static int __init ls_rcpm_init(void) {
> > > +     return platform_driver_register(&ls_rcpm_driver);
> > > +}
> > > +subsys_initcall(ls_rcpm_init);
> > > +
> > > +static void __exit ls_rcpm_exit(void) {
> > > +     platform_driver_unregister(&ls_rcpm_driver);
> > > +}
> > > +module_exit(ls_rcpm_exit);
> >
> >

[PATCH 3/3] soc: fsl: add RCPM driver

[Ran Wang]

Hi Dongsheng,

On 2018/9/5 10:58, Dongsheng Wang wrote:
> 
> Please change your comments style.
> 
> On 2018/8/31 11:56, Ran Wang wrote:
> > The NXP's QorIQ Processors based on ARM Core have RCPM module (Run
> > Control and Power Management), which performs all device-level tasks
> > associated with power management such as wakeup source control.
> >
> > This driver depends on FSL platform PM driver framework which help to
> > isolate user and PM service provider (such as RCPM driver).
> >
> > Signed-off-by: Chenhui Zhao <chenhui.zhao@nxp.com>
> > Signed-off-by: Ying Zhang <ying.zhang22455@nxp.com>
> > Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> > ---
> >  drivers/soc/fsl/Kconfig   |    6 ++
> >  drivers/soc/fsl/Makefile  |    1 +
> >  drivers/soc/fsl/ls-rcpm.c |  153
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 160 insertions(+), 0 deletions(-)  create mode
> > 100644 drivers/soc/fsl/ls-rcpm.c
> >
> > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig index
> > 6517412..882330d 100644
> > --- a/drivers/soc/fsl/Kconfig
> > +++ b/drivers/soc/fsl/Kconfig
> > @@ -30,3 +30,9 @@ config FSL_PLAT_PM
> >  	  have to know the implement details of wakeup function it require.
> >  	  Besides, it is also easy for service side to upgrade its logic when
> >  	  design changed and remain user side unchanged.
> > +
> > +config LS_RCPM
> > +	bool "Freescale RCPM support"
> > +	depends on (FSL_PLAT_PM)
> > +	help
> > +	  This feature is to enable specified wakeup source for system sleep.
> > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile index
> > 8f9db23..43ff71a 100644
> > --- a/drivers/soc/fsl/Makefile
> > +++ b/drivers/soc/fsl/Makefile
> > @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)		+= qe/
> >  obj-$(CONFIG_CPM)			+= qe/
> >  obj-$(CONFIG_FSL_GUTS)			+= guts.o
> >  obj-$(CONFIG_FSL_PLAT_PM)	+= plat_pm.o
> > +obj-$(CONFIG_LS_RCPM)		+= ls-rcpm.o
> > diff --git a/drivers/soc/fsl/ls-rcpm.c b/drivers/soc/fsl/ls-rcpm.c new
> > file mode 100644 index 0000000..b0feb88
> > --- /dev/null
> > +++ b/drivers/soc/fsl/ls-rcpm.c
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// plat_pm.c - Freescale Layerscape RCPM driver // // Copyright 2018
> > +NXP // // Author: Ran Wang <ran.wang_1@nxp.com>,
> > +
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
> > +#include <soc/fsl/plat_pm.h>
> > +
> > +#define MAX_COMPATIBLE_NUM	10
> > +
> > +struct rcpm_t {
> > +	struct device *dev;
> > +	void __iomem *ippdexpcr_addr;
> > +	bool big_endian;	/* Big/Little endian of RCPM module */
> > +};
> > +
> > +// rcpm_handle - Configure RCPM reg according to wake up source
> > +request // @user_dev: pointer to user's device struct // @flag: to
> > +enable(true) or disable(false) wakeup source // @handle_priv: pointer
> > +to struct rcpm_t instance // // Return 0 on success other negative
> > +errno static int rcpm_handle(struct device *user_dev, bool flag, void
> > +*handle_priv) {
> > +	struct rcpm_t *rcpm;
> > +	bool big_endian;
> > +	const char  *dev_compatible_array[MAX_COMPATIBLE_NUM];
> > +	void __iomem *ippdexpcr_addr;
> > +	u32 ippdexpcr;
> > +	u32 set_bit;
> > +	int ret, num, i;
> > +
> > +	rcpm = handle_priv;
> > +	big_endian = rcpm->big_endian;
> > +	ippdexpcr_addr = rcpm->ippdexpcr_addr;
> > +
> > +	num = device_property_read_string_array(user_dev, "compatible",
> > +			dev_compatible_array, MAX_COMPATIBLE_NUM);
> > +	if (num < 0)
> > +		return num;
> > +
> > +	for (i = 0; i < num; i++) {
> > +		if (!device_property_present(rcpm->dev,
> > +					dev_compatible_array[i]))
> > +			continue;
> > +		else {
> Remove this else.

Got it! Will update in next version.

> > +			ret = device_property_read_u32(rcpm->dev,
> > +					dev_compatible_array[i], &set_bit);
> > +			if (ret)
> > +				return ret;
> > +
> > +			if (!device_property_present(rcpm->dev,
> > +						dev_compatible_array[i]))
> This has been checked. Continue ? or return ENODEV?

Yes, this checking is not necessary, will remove in next version

> > +				return -ENODEV;
> > +			else {
> Remove this else.

OK

> > +				ret = device_property_read_u32(rcpm->dev,
> > +						dev_compatible_array[i],
> &set_bit);
> > +				if (ret)
> > +					return ret;
> > +
> > +				if (big_endian)
> > +					ippdexpcr =
> ioread32be(ippdexpcr_addr);
> > +				else
> > +					ippdexpcr =
> ioread32(ippdexpcr_addr);
> > +
> > +				if (flag)
> > +					ippdexpcr |= set_bit;
> > +				else
> > +					ippdexpcr &= ~set_bit;
> > +
> > +				if (big_endian) {
> > +					iowrite32be(ippdexpcr,
> ippdexpcr_addr);
> > +					ippdexpcr =
> ioread32be(ippdexpcr_addr);
> > +				} else
> if (x) {
> ....
> ....
> }  else {
> 
> }

Got it!

> > +					iowrite32(ippdexpcr, ippdexpcr_addr);
> > +
> > +				return 0;
> > +			}
> > +		}
> > +	}
> > +
> > +	return -ENODEV;
> > +}
> > +
> > +static int ls_rcpm_probe(struct platform_device *pdev) {
> > +	struct resource *r;
> > +	struct rcpm_t *rcpm;
> > +
> > +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!r)
> > +		return -ENODEV;
> > +
> > +	rcpm = kmalloc(sizeof(*rcpm), GFP_KERNEL);
> kzalloc is better.

OK

> > +	if (!rcpm)
> > +		return -ENOMEM;
> > +
> > +	rcpm->big_endian = device_property_read_bool(&pdev->dev,
> > +"big-endian");
> > +
> > +	rcpm->ippdexpcr_addr = devm_ioremap_resource(&pdev->dev, r);
> > +	if (IS_ERR(rcpm->ippdexpcr_addr))
> > +		return PTR_ERR(rcpm->ippdexpcr_addr);
> > +
> > +	rcpm->dev = &pdev->dev;
> > +	platform_set_drvdata(pdev, rcpm);
> > +
> > +	return register_fsl_platform_wakeup_source(rcpm_handle, rcpm); }
> > +
> > +static int ls_rcpm_remove(struct platform_device *pdev) {
> > +	struct rcpm_t	*rcpm;
> Not need a table.

OK, thanks for your careful review.

Regards,
Ran
 

> Cheers,
> -Dongsheng
> 
> > +
> > +	rcpm = platform_get_drvdata(pdev);
> > +	deregister_fsl_platform_wakeup_source(rcpm);
> > +	kfree(rcpm);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id ls_rcpm_of_match[] = {
> > +	{ .compatible = "fsl,qoriq-rcpm-2.1", },
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(of, ls_rcpm_of_match);
> > +
> > +static struct platform_driver ls_rcpm_driver = {
> > +	.driver = {
> > +		.name = "ls-rcpm",
> > +		.of_match_table = ls_rcpm_of_match,
> > +	},
> > +	.probe = ls_rcpm_probe,
> > +	.remove = ls_rcpm_remove,
> > +};
> > +
> > +static int __init ls_rcpm_init(void)
> > +{
> > +	return platform_driver_register(&ls_rcpm_driver);
> > +}
> > +subsys_initcall(ls_rcpm_init);
> > +
> > +static void __exit ls_rcpm_exit(void) {
> > +	platform_driver_unregister(&ls_rcpm_driver);
> > +}
> > +module_exit(ls_rcpm_exit);
> 

[PATCH 3/3] soc: fsl: add RCPM driver

[Li Yang]

On Tue, Sep 4, 2018 at 9:58 PM Wang, Dongsheng
<dongsheng.wang@hxt-semitech.com> wrote:
>
> Please change your comments style.

Although this doesn't get into the Linux kernel coding style
documentation yet, Linus seems changed his mind to prefer // than /*
*/ comment style now.  https://lkml.org/lkml/2017/11/25/133   So the
// style should be acceptable for now.

>
> On 2018/8/31 11:56, Ran Wang wrote:
> > The NXP's QorIQ Processors based on ARM Core have RCPM module (Run
> > Control and Power Management), which performs all device-level
> > tasks associated with power management such as wakeup source control.
> >
> > This driver depends on FSL platform PM driver framework which help to
> > isolate user and PM service provider (such as RCPM driver).
> >
> > Signed-off-by: Chenhui Zhao <chenhui.zhao@nxp.com>
> > Signed-off-by: Ying Zhang <ying.zhang22455@nxp.com>
> > Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> > ---
> >  drivers/soc/fsl/Kconfig   |    6 ++
> >  drivers/soc/fsl/Makefile  |    1 +
> >  drivers/soc/fsl/ls-rcpm.c |  153 +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 160 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/soc/fsl/ls-rcpm.c
> >
> > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> > index 6517412..882330d 100644
> > --- a/drivers/soc/fsl/Kconfig
> > +++ b/drivers/soc/fsl/Kconfig
> > @@ -30,3 +30,9 @@ config FSL_PLAT_PM
> >         have to know the implement details of wakeup function it require.
> >         Besides, it is also easy for service side to upgrade its logic when
> >         design changed and remain user side unchanged.
> > +
> > +config LS_RCPM
> > +     bool "Freescale RCPM support"
> > +     depends on (FSL_PLAT_PM)
> > +     help
> > +       This feature is to enable specified wakeup source for system sleep.
> > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> > index 8f9db23..43ff71a 100644
> > --- a/drivers/soc/fsl/Makefile
> > +++ b/drivers/soc/fsl/Makefile
> > @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)            += qe/
> >  obj-$(CONFIG_CPM)                    += qe/
> >  obj-$(CONFIG_FSL_GUTS)                       += guts.o
> >  obj-$(CONFIG_FSL_PLAT_PM)    += plat_pm.o
> > +obj-$(CONFIG_LS_RCPM)                += ls-rcpm.o

Probably use "_" instead of "-" for alignment.

> > diff --git a/drivers/soc/fsl/ls-rcpm.c b/drivers/soc/fsl/ls-rcpm.c
> > new file mode 100644
> > index 0000000..b0feb88
> > --- /dev/null
> > +++ b/drivers/soc/fsl/ls-rcpm.c
> > @@ -0,0 +1,153 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// plat_pm.c - Freescale Layerscape RCPM driver

The file name here is not the same as the real file name.

> > +//
> > +// Copyright 2018 NXP
> > +//
> > +// Author: Ran Wang <ran.wang_1@nxp.com>,

Where do you need the comma in the end?

> > +
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
> > +#include <soc/fsl/plat_pm.h>
> > +
> > +#define MAX_COMPATIBLE_NUM   10
> > +
> > +struct rcpm_t {
> > +     struct device *dev;
> > +     void __iomem *ippdexpcr_addr;
> > +     bool big_endian;        /* Big/Little endian of RCPM module */
> > +};
> > +
> > +// rcpm_handle - Configure RCPM reg according to wake up source request
> > +// @user_dev: pointer to user's device struct
> > +// @flag: to enable(true) or disable(false) wakeup source
> > +// @handle_priv: pointer to struct rcpm_t instance
> > +//
> > +// Return 0 on success other negative errno

Although Linus preferred this // comment style.  I'm not sure if this
will be handled correctly by the kernel-doc compiler.
https://www.kernel.org/doc/html/v4.18/doc-guide/kernel-doc.html

> > +static int rcpm_handle(struct device *user_dev, bool flag, void *handle_priv)
> > +{
> > +     struct rcpm_t *rcpm;
> > +     bool big_endian;
> > +     const char  *dev_compatible_array[MAX_COMPATIBLE_NUM];
> > +     void __iomem *ippdexpcr_addr;
> > +     u32 ippdexpcr;
> > +     u32 set_bit;
> > +     int ret, num, i;
> > +
> > +     rcpm = handle_priv;
> > +     big_endian = rcpm->big_endian;
> > +     ippdexpcr_addr = rcpm->ippdexpcr_addr;
> > +
> > +     num = device_property_read_string_array(user_dev, "compatible",
> > +                     dev_compatible_array, MAX_COMPATIBLE_NUM);
> > +     if (num < 0)
> > +             return num;
> > +
> > +     for (i = 0; i < num; i++) {
> > +             if (!device_property_present(rcpm->dev,
> > +                                     dev_compatible_array[i]))
> > +                     continue;
> > +             else {
> Remove this else.
> > +                     ret = device_property_read_u32(rcpm->dev,
> > +                                     dev_compatible_array[i], &set_bit);
> > +                     if (ret)
> > +                             return ret;
> > +
> > +                     if (!device_property_present(rcpm->dev,
> > +                                             dev_compatible_array[i]))
> This has been checked. Continue ? or return ENODEV?
> > +                             return -ENODEV;
> > +                     else {
> Remove this else.
> > +                             ret = device_property_read_u32(rcpm->dev,
> > +                                             dev_compatible_array[i], &set_bit);
> > +                             if (ret)
> > +                                     return ret;
> > +
> > +                             if (big_endian)
> > +                                     ippdexpcr = ioread32be(ippdexpcr_addr);
> > +                             else
> > +                                     ippdexpcr = ioread32(ippdexpcr_addr);
> > +
> > +                             if (flag)
> > +                                     ippdexpcr |= set_bit;
> > +                             else
> > +                                     ippdexpcr &= ~set_bit;
> > +
> > +                             if (big_endian) {
> > +                                     iowrite32be(ippdexpcr, ippdexpcr_addr);
> > +                                     ippdexpcr = ioread32be(ippdexpcr_addr);
> > +                             } else
> if (x) {
> ....
> ....
> }  else {
>
> }
> > +                                     iowrite32(ippdexpcr, ippdexpcr_addr);
> > +
> > +                             return 0;
> > +                     }
> > +             }
> > +     }
> > +
> > +     return -ENODEV;
> > +}
> > +
> > +static int ls_rcpm_probe(struct platform_device *pdev)
> > +{
> > +     struct resource *r;
> > +     struct rcpm_t *rcpm;
> > +
> > +     r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +     if (!r)
> > +             return -ENODEV;
> > +
> > +     rcpm = kmalloc(sizeof(*rcpm), GFP_KERNEL);
> kzalloc is better.
> > +     if (!rcpm)
> > +             return -ENOMEM;
> > +
> > +     rcpm->big_endian = device_property_read_bool(&pdev->dev, "big-endian");
> > +
> > +     rcpm->ippdexpcr_addr = devm_ioremap_resource(&pdev->dev, r);
> > +     if (IS_ERR(rcpm->ippdexpcr_addr))
> > +             return PTR_ERR(rcpm->ippdexpcr_addr);
> > +
> > +     rcpm->dev = &pdev->dev;
> > +     platform_set_drvdata(pdev, rcpm);
> > +
> > +     return register_fsl_platform_wakeup_source(rcpm_handle, rcpm);
> > +}
> > +
> > +static int ls_rcpm_remove(struct platform_device *pdev)
> > +{
> > +     struct rcpm_t   *rcpm;
> Not need a table.
>
> Cheers,
> -Dongsheng
>
> > +
> > +     rcpm = platform_get_drvdata(pdev);
> > +     deregister_fsl_platform_wakeup_source(rcpm);
> > +     kfree(rcpm);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct of_device_id ls_rcpm_of_match[] = {
> > +     { .compatible = "fsl,qoriq-rcpm-2.1", },
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(of, ls_rcpm_of_match);
> > +
> > +static struct platform_driver ls_rcpm_driver = {
> > +     .driver = {
> > +             .name = "ls-rcpm",
> > +             .of_match_table = ls_rcpm_of_match,
> > +     },
> > +     .probe = ls_rcpm_probe,
> > +     .remove = ls_rcpm_remove,
> > +};
> > +
> > +static int __init ls_rcpm_init(void)
> > +{
> > +     return platform_driver_register(&ls_rcpm_driver);
> > +}
> > +subsys_initcall(ls_rcpm_init);
> > +
> > +static void __exit ls_rcpm_exit(void)
> > +{
> > +     platform_driver_unregister(&ls_rcpm_driver);
> > +}
> > +module_exit(ls_rcpm_exit);
>
>

[PATCH 3/3] soc: fsl: add RCPM driver

[Wang, Dongsheng]

Please change your comments style.

On 2018/8/31 11:56, Ran Wang wrote:
> The NXP's QorIQ Processors based on ARM Core have RCPM module (Run
> Control and Power Management), which performs all device-level
> tasks associated with power management such as wakeup source control.
>
> This driver depends on FSL platform PM driver framework which help to
> isolate user and PM service provider (such as RCPM driver).
>
> Signed-off-by: Chenhui Zhao <chenhui.zhao@nxp.com>
> Signed-off-by: Ying Zhang <ying.zhang22455@nxp.com>
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
>  drivers/soc/fsl/Kconfig   |    6 ++
>  drivers/soc/fsl/Makefile  |    1 +
>  drivers/soc/fsl/ls-rcpm.c |  153 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 160 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/soc/fsl/ls-rcpm.c
>
> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> index 6517412..882330d 100644
> --- a/drivers/soc/fsl/Kconfig
> +++ b/drivers/soc/fsl/Kconfig
> @@ -30,3 +30,9 @@ config FSL_PLAT_PM
>  	  have to know the implement details of wakeup function it require.
>  	  Besides, it is also easy for service side to upgrade its logic when
>  	  design changed and remain user side unchanged.
> +
> +config LS_RCPM
> +	bool "Freescale RCPM support"
> +	depends on (FSL_PLAT_PM)
> +	help
> +	  This feature is to enable specified wakeup source for system sleep.
> diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> index 8f9db23..43ff71a 100644
> --- a/drivers/soc/fsl/Makefile
> +++ b/drivers/soc/fsl/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)		+= qe/
>  obj-$(CONFIG_CPM)			+= qe/
>  obj-$(CONFIG_FSL_GUTS)			+= guts.o
>  obj-$(CONFIG_FSL_PLAT_PM)	+= plat_pm.o
> +obj-$(CONFIG_LS_RCPM)		+= ls-rcpm.o
> diff --git a/drivers/soc/fsl/ls-rcpm.c b/drivers/soc/fsl/ls-rcpm.c
> new file mode 100644
> index 0000000..b0feb88
> --- /dev/null
> +++ b/drivers/soc/fsl/ls-rcpm.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// plat_pm.c - Freescale Layerscape RCPM driver
> +//
> +// Copyright 2018 NXP
> +//
> +// Author: Ran Wang <ran.wang_1@nxp.com>,
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <soc/fsl/plat_pm.h>
> +
> +#define MAX_COMPATIBLE_NUM	10
> +
> +struct rcpm_t {
> +	struct device *dev;
> +	void __iomem *ippdexpcr_addr;
> +	bool big_endian;	/* Big/Little endian of RCPM module */
> +};
> +
> +// rcpm_handle - Configure RCPM reg according to wake up source request
> +// @user_dev: pointer to user's device struct
> +// @flag: to enable(true) or disable(false) wakeup source
> +// @handle_priv: pointer to struct rcpm_t instance
> +//
> +// Return 0 on success other negative errno
> +static int rcpm_handle(struct device *user_dev, bool flag, void *handle_priv)
> +{
> +	struct rcpm_t *rcpm;
> +	bool big_endian;
> +	const char  *dev_compatible_array[MAX_COMPATIBLE_NUM];
> +	void __iomem *ippdexpcr_addr;
> +	u32 ippdexpcr;
> +	u32 set_bit;
> +	int ret, num, i;
> +
> +	rcpm = handle_priv;
> +	big_endian = rcpm->big_endian;
> +	ippdexpcr_addr = rcpm->ippdexpcr_addr;
> +
> +	num = device_property_read_string_array(user_dev, "compatible",
> +			dev_compatible_array, MAX_COMPATIBLE_NUM);
> +	if (num < 0)
> +		return num;
> +
> +	for (i = 0; i < num; i++) {
> +		if (!device_property_present(rcpm->dev,
> +					dev_compatible_array[i]))
> +			continue;
> +		else {
Remove this else.
> +			ret = device_property_read_u32(rcpm->dev,
> +					dev_compatible_array[i], &set_bit);
> +			if (ret)
> +				return ret;
> +
> +			if (!device_property_present(rcpm->dev,
> +						dev_compatible_array[i]))
This has been checked. Continue ? or return ENODEV?
> +				return -ENODEV;
> +			else {
Remove this else.
> +				ret = device_property_read_u32(rcpm->dev,
> +						dev_compatible_array[i], &set_bit);
> +				if (ret)
> +					return ret;
> +
> +				if (big_endian)
> +					ippdexpcr = ioread32be(ippdexpcr_addr);
> +				else
> +					ippdexpcr = ioread32(ippdexpcr_addr);
> +
> +				if (flag)
> +					ippdexpcr |= set_bit;
> +				else
> +					ippdexpcr &= ~set_bit;
> +
> +				if (big_endian) {
> +					iowrite32be(ippdexpcr, ippdexpcr_addr);
> +					ippdexpcr = ioread32be(ippdexpcr_addr);
> +				} else
if (x) {
....
....
}  else {

}
> +					iowrite32(ippdexpcr, ippdexpcr_addr);
> +
> +				return 0;
> +			}
> +		}
> +	}
> +
> +	return -ENODEV;
> +}
> +
> +static int ls_rcpm_probe(struct platform_device *pdev)
> +{
> +	struct resource *r;
> +	struct rcpm_t *rcpm;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!r)
> +		return -ENODEV;
> +
> +	rcpm = kmalloc(sizeof(*rcpm), GFP_KERNEL);
kzalloc is better.
> +	if (!rcpm)
> +		return -ENOMEM;
> +
> +	rcpm->big_endian = device_property_read_bool(&pdev->dev, "big-endian");
> +
> +	rcpm->ippdexpcr_addr = devm_ioremap_resource(&pdev->dev, r);
> +	if (IS_ERR(rcpm->ippdexpcr_addr))
> +		return PTR_ERR(rcpm->ippdexpcr_addr);
> +
> +	rcpm->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, rcpm);
> +
> +	return register_fsl_platform_wakeup_source(rcpm_handle, rcpm);
> +}
> +
> +static int ls_rcpm_remove(struct platform_device *pdev)
> +{
> +	struct rcpm_t	*rcpm;
Not need a table.

Cheers,
-Dongsheng

> +
> +	rcpm = platform_get_drvdata(pdev);
> +	deregister_fsl_platform_wakeup_source(rcpm);
> +	kfree(rcpm);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ls_rcpm_of_match[] = {
> +	{ .compatible = "fsl,qoriq-rcpm-2.1", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ls_rcpm_of_match);
> +
> +static struct platform_driver ls_rcpm_driver = {
> +	.driver = {
> +		.name = "ls-rcpm",
> +		.of_match_table = ls_rcpm_of_match,
> +	},
> +	.probe = ls_rcpm_probe,
> +	.remove = ls_rcpm_remove,
> +};
> +
> +static int __init ls_rcpm_init(void)
> +{
> +	return platform_driver_register(&ls_rcpm_driver);
> +}
> +subsys_initcall(ls_rcpm_init);
> +
> +static void __exit ls_rcpm_exit(void)
> +{
> +	platform_driver_unregister(&ls_rcpm_driver);
> +}
> +module_exit(ls_rcpm_exit);

[PATCH 3/3] soc: fsl: add RCPM driver

[Ran Wang]

The NXP's QorIQ Processors based on ARM Core have RCPM module (Run
Control and Power Management), which performs all device-level
tasks associated with power management such as wakeup source control.

This driver depends on FSL platform PM driver framework which help to
isolate user and PM service provider (such as RCPM driver).

Signed-off-by: Chenhui Zhao <chenhui.zhao@nxp.com>
Signed-off-by: Ying Zhang <ying.zhang22455@nxp.com>
Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
---
 drivers/soc/fsl/Kconfig   |    6 ++
 drivers/soc/fsl/Makefile  |    1 +
 drivers/soc/fsl/ls-rcpm.c |  153 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 160 insertions(+), 0 deletions(-)
 create mode 100644 drivers/soc/fsl/ls-rcpm.c

diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
index 6517412..882330d 100644
--- a/drivers/soc/fsl/Kconfig
+++ b/drivers/soc/fsl/Kconfig
@@ -30,3 +30,9 @@ config FSL_PLAT_PM
 	  have to know the implement details of wakeup function it require.
 	  Besides, it is also easy for service side to upgrade its logic when
 	  design changed and remain user side unchanged.
+
+config LS_RCPM
+	bool "Freescale RCPM support"
+	depends on (FSL_PLAT_PM)
+	help
+	  This feature is to enable specified wakeup source for system sleep.
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index 8f9db23..43ff71a 100644
--- a/drivers/soc/fsl/Makefile
+++ b/drivers/soc/fsl/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)		+= qe/
 obj-$(CONFIG_CPM)			+= qe/
 obj-$(CONFIG_FSL_GUTS)			+= guts.o
 obj-$(CONFIG_FSL_PLAT_PM)	+= plat_pm.o
+obj-$(CONFIG_LS_RCPM)		+= ls-rcpm.o
diff --git a/drivers/soc/fsl/ls-rcpm.c b/drivers/soc/fsl/ls-rcpm.c
new file mode 100644
index 0000000..b0feb88
--- /dev/null
+++ b/drivers/soc/fsl/ls-rcpm.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// plat_pm.c - Freescale Layerscape RCPM driver
+//
+// Copyright 2018 NXP
+//
+// Author: Ran Wang <ran.wang_1@nxp.com>,
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <soc/fsl/plat_pm.h>
+
+#define MAX_COMPATIBLE_NUM	10
+
+struct rcpm_t {
+	struct device *dev;
+	void __iomem *ippdexpcr_addr;
+	bool big_endian;	/* Big/Little endian of RCPM module */
+};
+
+// rcpm_handle - Configure RCPM reg according to wake up source request
+// @user_dev: pointer to user's device struct
+// @flag: to enable(true) or disable(false) wakeup source
+// @handle_priv: pointer to struct rcpm_t instance
+//
+// Return 0 on success other negative errno
+static int rcpm_handle(struct device *user_dev, bool flag, void *handle_priv)
+{
+	struct rcpm_t *rcpm;
+	bool big_endian;
+	const char  *dev_compatible_array[MAX_COMPATIBLE_NUM];
+	void __iomem *ippdexpcr_addr;
+	u32 ippdexpcr;
+	u32 set_bit;
+	int ret, num, i;
+
+	rcpm = handle_priv;
+	big_endian = rcpm->big_endian;
+	ippdexpcr_addr = rcpm->ippdexpcr_addr;
+
+	num = device_property_read_string_array(user_dev, "compatible",
+			dev_compatible_array, MAX_COMPATIBLE_NUM);
+	if (num < 0)
+		return num;
+
+	for (i = 0; i < num; i++) {
+		if (!device_property_present(rcpm->dev,
+					dev_compatible_array[i]))
+			continue;
+		else {
+			ret = device_property_read_u32(rcpm->dev,
+					dev_compatible_array[i], &set_bit);
+			if (ret)
+				return ret;
+
+			if (!device_property_present(rcpm->dev,
+						dev_compatible_array[i]))
+				return -ENODEV;
+			else {
+				ret = device_property_read_u32(rcpm->dev,
+						dev_compatible_array[i], &set_bit);
+				if (ret)
+					return ret;
+
+				if (big_endian)
+					ippdexpcr = ioread32be(ippdexpcr_addr);
+				else
+					ippdexpcr = ioread32(ippdexpcr_addr);
+
+				if (flag)
+					ippdexpcr |= set_bit;
+				else
+					ippdexpcr &= ~set_bit;
+
+				if (big_endian) {
+					iowrite32be(ippdexpcr, ippdexpcr_addr);
+					ippdexpcr = ioread32be(ippdexpcr_addr);
+				} else
+					iowrite32(ippdexpcr, ippdexpcr_addr);
+
+				return 0;
+			}
+		}
+	}
+
+	return -ENODEV;
+}
+
+static int ls_rcpm_probe(struct platform_device *pdev)
+{
+	struct resource *r;
+	struct rcpm_t *rcpm;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r)
+		return -ENODEV;
+
+	rcpm = kmalloc(sizeof(*rcpm), GFP_KERNEL);
+	if (!rcpm)
+		return -ENOMEM;
+
+	rcpm->big_endian = device_property_read_bool(&pdev->dev, "big-endian");
+
+	rcpm->ippdexpcr_addr = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(rcpm->ippdexpcr_addr))
+		return PTR_ERR(rcpm->ippdexpcr_addr);
+
+	rcpm->dev = &pdev->dev;
+	platform_set_drvdata(pdev, rcpm);
+
+	return register_fsl_platform_wakeup_source(rcpm_handle, rcpm);
+}
+
+static int ls_rcpm_remove(struct platform_device *pdev)
+{
+	struct rcpm_t	*rcpm;
+
+	rcpm = platform_get_drvdata(pdev);
+	deregister_fsl_platform_wakeup_source(rcpm);
+	kfree(rcpm);
+
+	return 0;
+}
+
+static const struct of_device_id ls_rcpm_of_match[] = {
+	{ .compatible = "fsl,qoriq-rcpm-2.1", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ls_rcpm_of_match);
+
+static struct platform_driver ls_rcpm_driver = {
+	.driver = {
+		.name = "ls-rcpm",
+		.of_match_table = ls_rcpm_of_match,
+	},
+	.probe = ls_rcpm_probe,
+	.remove = ls_rcpm_remove,
+};
+
+static int __init ls_rcpm_init(void)
+{
+	return platform_driver_register(&ls_rcpm_driver);
+}
+subsys_initcall(ls_rcpm_init);
+
+static void __exit ls_rcpm_exit(void)
+{
+	platform_driver_unregister(&ls_rcpm_driver);
+}
+module_exit(ls_rcpm_exit);
-- 
1.7.1