From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755446AbcHCDay (ORCPT ); Tue, 2 Aug 2016 23:30:54 -0400 Received: from mail-vk0-f65.google.com ([209.85.213.65]:33387 "EHLO mail-vk0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751860AbcHCDaV (ORCPT ); Tue, 2 Aug 2016 23:30:21 -0400 MIME-Version: 1.0 In-Reply-To: <579F4D12.4000004@arm.com> References: <1470044943-3814-1-git-send-email-chenhui.zhao@nxp.com> <1470044943-3814-2-git-send-email-chenhui.zhao@nxp.com> <579F4D12.4000004@arm.com> From: Chenhui Zhao Date: Wed, 3 Aug 2016 11:28:18 +0800 Message-ID: Subject: Re: [PATCH 2/2] soc: nxp: Add a RCPM driver To: Marc Zyngier Cc: Chenhui Zhao , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, jason.jin@nxp.com, Mark Rutland Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Aug 1, 2016 at 9:22 PM, Marc Zyngier wrote: > > On 01/08/16 10:49, Chenhui Zhao wrote: > > The NXP's QorIQ Processors based on ARM Core have a RCPM module > > (Run Control and Power Management), which performs all device-level > > tasks associated with power management. > > > > This patch mainly implements the wakeup sources configuration before > > entering LPM20, a low power state of device-level. The devices can be > > waked up by specified sources, such as Flextimer, GPIO and so on. > > > > Signed-off-by: Chenhui Zhao > > --- > > drivers/soc/fsl/Makefile | 1 + > > drivers/soc/fsl/pm/Makefile | 1 + > > drivers/soc/fsl/pm/ls-rcpm.c | 144 +++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 146 insertions(+) > > create mode 100644 drivers/soc/fsl/pm/Makefile > > create mode 100644 drivers/soc/fsl/pm/ls-rcpm.c > > > > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile > > index 203307f..b5adbaf 100644 > > --- a/drivers/soc/fsl/Makefile > > +++ b/drivers/soc/fsl/Makefile > > @@ -4,3 +4,4 @@ > > > > obj-$(CONFIG_QUICC_ENGINE) += qe/ > > obj-$(CONFIG_CPM) += qe/ > > +obj-$(CONFIG_SUSPEND) += pm/ > > diff --git a/drivers/soc/fsl/pm/Makefile b/drivers/soc/fsl/pm/Makefile > > new file mode 100644 > > index 0000000..e275d63 > > --- /dev/null > > +++ b/drivers/soc/fsl/pm/Makefile > > @@ -0,0 +1 @@ > > +obj-$(CONFIG_ARCH_LAYERSCAPE) += ls-rcpm.o > > diff --git a/drivers/soc/fsl/pm/ls-rcpm.c b/drivers/soc/fsl/pm/ls-rcpm.c > > new file mode 100644 > > index 0000000..c5f2b97 > > --- /dev/null > > +++ b/drivers/soc/fsl/pm/ls-rcpm.c > > @@ -0,0 +1,144 @@ > > +/* > > + * Run Control and Power Management (RCPM) driver > > + * > > + * Copyright 2016 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. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + */ > > +#define pr_fmt(fmt) "RCPM: %s: " fmt, __func__ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +/* So far there are not more than two registers */ > > +#define RCPM_IPPDEXPCR0 0x140 > > +#define RCPM_IPPDEXPCR1 0x144 > > +#define RCPM_IPPDEXPCR(x) (RCPM_IPPDEXPCR0 + 4 * x) > > +#define RCPM_WAKEUP_CELL_MAX_SIZE 2 > > + > > +/* it reprents the number of the registers RCPM_IPPDEXPCR */ > > +static unsigned int rcpm_wakeup_cells; > > +static void __iomem *rcpm_reg_base; > > +static u32 ippdexpcr[RCPM_WAKEUP_CELL_MAX_SIZE]; > > + > > +static inline void rcpm_reg_write(u32 offset, u32 value) > > +{ > > + iowrite32be(value, rcpm_reg_base + offset); > > +} > > + > > +static inline u32 rcpm_reg_read(u32 offset) > > +{ > > + return ioread32be(rcpm_reg_base + offset); > > +} > > + > > +static void rcpm_wakeup_fixup(struct device *dev, void *data) > > +{ > > + struct device_node *node = dev ? dev->of_node : NULL; > > + u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1]; > > + int ret; > > + int i; > > + > > + if (!dev || !node || !device_may_wakeup(dev)) > > + return; > > + > > + /* > > + * Get the values in the "rcpm-wakeup" property. > > + * Refer to Documentation/devicetree/bindings/soc/fsl/rcpm.txt > > + */ > > + ret = of_property_read_u32_array(node, "rcpm-wakeup", > > + value, rcpm_wakeup_cells + 1); > > + if (ret) > > + return; > > + > > + pr_debug("wakeup source: the device %s\n", node->full_name); > > This looks absolutely dreadful. You are parsing every node for each > device, and apply a set-in-stone configuration. > > The normal way to handle this is by making this device an interrupt > controller, and honour the irq_set_wake API. This has already been done > on other FSL/NXP hardware (see the iMX GPC stuff). > > Thanks, > > M. The RCPM IP block is really not an interrupt controller, instead it is related to power management. The code in this patch is to set the bits in the IPPDEXPCR register. Setting these bits can make the corresponding IP block alive during the period of sleep, so that the IP blocks working as wakeup sources can wake the device. The "rcpm-wakeup" property records the bit mask which should be set when the IP block is working as wakeup source. The bit definition may be different for each QorIQ SoC. The operation is specific to QorIQ platform, so put the code in drivers/soc/fsl/pm/. Thanks, Chenhui From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chenhui Zhao Subject: Re: [PATCH 2/2] soc: nxp: Add a RCPM driver Date: Wed, 3 Aug 2016 11:28:18 +0800 Message-ID: References: <1470044943-3814-1-git-send-email-chenhui.zhao@nxp.com> <1470044943-3814-2-git-send-email-chenhui.zhao@nxp.com> <579F4D12.4000004@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <579F4D12.4000004@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Marc Zyngier Cc: Mark Rutland , devicetree@vger.kernel.org, Chenhui Zhao , jason.jin@nxp.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org On Mon, Aug 1, 2016 at 9:22 PM, Marc Zyngier wrote: > > On 01/08/16 10:49, Chenhui Zhao wrote: > > The NXP's QorIQ Processors based on ARM Core have a RCPM module > > (Run Control and Power Management), which performs all device-level > > tasks associated with power management. > > > > This patch mainly implements the wakeup sources configuration before > > entering LPM20, a low power state of device-level. The devices can be > > waked up by specified sources, such as Flextimer, GPIO and so on. > > > > Signed-off-by: Chenhui Zhao > > --- > > drivers/soc/fsl/Makefile | 1 + > > drivers/soc/fsl/pm/Makefile | 1 + > > drivers/soc/fsl/pm/ls-rcpm.c | 144 +++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 146 insertions(+) > > create mode 100644 drivers/soc/fsl/pm/Makefile > > create mode 100644 drivers/soc/fsl/pm/ls-rcpm.c > > > > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile > > index 203307f..b5adbaf 100644 > > --- a/drivers/soc/fsl/Makefile > > +++ b/drivers/soc/fsl/Makefile > > @@ -4,3 +4,4 @@ > > > > obj-$(CONFIG_QUICC_ENGINE) += qe/ > > obj-$(CONFIG_CPM) += qe/ > > +obj-$(CONFIG_SUSPEND) += pm/ > > diff --git a/drivers/soc/fsl/pm/Makefile b/drivers/soc/fsl/pm/Makefile > > new file mode 100644 > > index 0000000..e275d63 > > --- /dev/null > > +++ b/drivers/soc/fsl/pm/Makefile > > @@ -0,0 +1 @@ > > +obj-$(CONFIG_ARCH_LAYERSCAPE) += ls-rcpm.o > > diff --git a/drivers/soc/fsl/pm/ls-rcpm.c b/drivers/soc/fsl/pm/ls-rcpm.c > > new file mode 100644 > > index 0000000..c5f2b97 > > --- /dev/null > > +++ b/drivers/soc/fsl/pm/ls-rcpm.c > > @@ -0,0 +1,144 @@ > > +/* > > + * Run Control and Power Management (RCPM) driver > > + * > > + * Copyright 2016 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. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + */ > > +#define pr_fmt(fmt) "RCPM: %s: " fmt, __func__ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +/* So far there are not more than two registers */ > > +#define RCPM_IPPDEXPCR0 0x140 > > +#define RCPM_IPPDEXPCR1 0x144 > > +#define RCPM_IPPDEXPCR(x) (RCPM_IPPDEXPCR0 + 4 * x) > > +#define RCPM_WAKEUP_CELL_MAX_SIZE 2 > > + > > +/* it reprents the number of the registers RCPM_IPPDEXPCR */ > > +static unsigned int rcpm_wakeup_cells; > > +static void __iomem *rcpm_reg_base; > > +static u32 ippdexpcr[RCPM_WAKEUP_CELL_MAX_SIZE]; > > + > > +static inline void rcpm_reg_write(u32 offset, u32 value) > > +{ > > + iowrite32be(value, rcpm_reg_base + offset); > > +} > > + > > +static inline u32 rcpm_reg_read(u32 offset) > > +{ > > + return ioread32be(rcpm_reg_base + offset); > > +} > > + > > +static void rcpm_wakeup_fixup(struct device *dev, void *data) > > +{ > > + struct device_node *node = dev ? dev->of_node : NULL; > > + u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1]; > > + int ret; > > + int i; > > + > > + if (!dev || !node || !device_may_wakeup(dev)) > > + return; > > + > > + /* > > + * Get the values in the "rcpm-wakeup" property. > > + * Refer to Documentation/devicetree/bindings/soc/fsl/rcpm.txt > > + */ > > + ret = of_property_read_u32_array(node, "rcpm-wakeup", > > + value, rcpm_wakeup_cells + 1); > > + if (ret) > > + return; > > + > > + pr_debug("wakeup source: the device %s\n", node->full_name); > > This looks absolutely dreadful. You are parsing every node for each > device, and apply a set-in-stone configuration. > > The normal way to handle this is by making this device an interrupt > controller, and honour the irq_set_wake API. This has already been done > on other FSL/NXP hardware (see the iMX GPC stuff). > > Thanks, > > M. The RCPM IP block is really not an interrupt controller, instead it is related to power management. The code in this patch is to set the bits in the IPPDEXPCR register. Setting these bits can make the corresponding IP block alive during the period of sleep, so that the IP blocks working as wakeup sources can wake the device. The "rcpm-wakeup" property records the bit mask which should be set when the IP block is working as wakeup source. The bit definition may be different for each QorIQ SoC. The operation is specific to QorIQ platform, so put the code in drivers/soc/fsl/pm/. Thanks, Chenhui From mboxrd@z Thu Jan 1 00:00:00 1970 From: z.chenhui@gmail.com (Chenhui Zhao) Date: Wed, 3 Aug 2016 11:28:18 +0800 Subject: [PATCH 2/2] soc: nxp: Add a RCPM driver In-Reply-To: <579F4D12.4000004@arm.com> References: <1470044943-3814-1-git-send-email-chenhui.zhao@nxp.com> <1470044943-3814-2-git-send-email-chenhui.zhao@nxp.com> <579F4D12.4000004@arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Aug 1, 2016 at 9:22 PM, Marc Zyngier wrote: > > On 01/08/16 10:49, Chenhui Zhao wrote: > > The NXP's QorIQ Processors based on ARM Core have a RCPM module > > (Run Control and Power Management), which performs all device-level > > tasks associated with power management. > > > > This patch mainly implements the wakeup sources configuration before > > entering LPM20, a low power state of device-level. The devices can be > > waked up by specified sources, such as Flextimer, GPIO and so on. > > > > Signed-off-by: Chenhui Zhao > > --- > > drivers/soc/fsl/Makefile | 1 + > > drivers/soc/fsl/pm/Makefile | 1 + > > drivers/soc/fsl/pm/ls-rcpm.c | 144 +++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 146 insertions(+) > > create mode 100644 drivers/soc/fsl/pm/Makefile > > create mode 100644 drivers/soc/fsl/pm/ls-rcpm.c > > > > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile > > index 203307f..b5adbaf 100644 > > --- a/drivers/soc/fsl/Makefile > > +++ b/drivers/soc/fsl/Makefile > > @@ -4,3 +4,4 @@ > > > > obj-$(CONFIG_QUICC_ENGINE) += qe/ > > obj-$(CONFIG_CPM) += qe/ > > +obj-$(CONFIG_SUSPEND) += pm/ > > diff --git a/drivers/soc/fsl/pm/Makefile b/drivers/soc/fsl/pm/Makefile > > new file mode 100644 > > index 0000000..e275d63 > > --- /dev/null > > +++ b/drivers/soc/fsl/pm/Makefile > > @@ -0,0 +1 @@ > > +obj-$(CONFIG_ARCH_LAYERSCAPE) += ls-rcpm.o > > diff --git a/drivers/soc/fsl/pm/ls-rcpm.c b/drivers/soc/fsl/pm/ls-rcpm.c > > new file mode 100644 > > index 0000000..c5f2b97 > > --- /dev/null > > +++ b/drivers/soc/fsl/pm/ls-rcpm.c > > @@ -0,0 +1,144 @@ > > +/* > > + * Run Control and Power Management (RCPM) driver > > + * > > + * Copyright 2016 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. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + */ > > +#define pr_fmt(fmt) "RCPM: %s: " fmt, __func__ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +/* So far there are not more than two registers */ > > +#define RCPM_IPPDEXPCR0 0x140 > > +#define RCPM_IPPDEXPCR1 0x144 > > +#define RCPM_IPPDEXPCR(x) (RCPM_IPPDEXPCR0 + 4 * x) > > +#define RCPM_WAKEUP_CELL_MAX_SIZE 2 > > + > > +/* it reprents the number of the registers RCPM_IPPDEXPCR */ > > +static unsigned int rcpm_wakeup_cells; > > +static void __iomem *rcpm_reg_base; > > +static u32 ippdexpcr[RCPM_WAKEUP_CELL_MAX_SIZE]; > > + > > +static inline void rcpm_reg_write(u32 offset, u32 value) > > +{ > > + iowrite32be(value, rcpm_reg_base + offset); > > +} > > + > > +static inline u32 rcpm_reg_read(u32 offset) > > +{ > > + return ioread32be(rcpm_reg_base + offset); > > +} > > + > > +static void rcpm_wakeup_fixup(struct device *dev, void *data) > > +{ > > + struct device_node *node = dev ? dev->of_node : NULL; > > + u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1]; > > + int ret; > > + int i; > > + > > + if (!dev || !node || !device_may_wakeup(dev)) > > + return; > > + > > + /* > > + * Get the values in the "rcpm-wakeup" property. > > + * Refer to Documentation/devicetree/bindings/soc/fsl/rcpm.txt > > + */ > > + ret = of_property_read_u32_array(node, "rcpm-wakeup", > > + value, rcpm_wakeup_cells + 1); > > + if (ret) > > + return; > > + > > + pr_debug("wakeup source: the device %s\n", node->full_name); > > This looks absolutely dreadful. You are parsing every node for each > device, and apply a set-in-stone configuration. > > The normal way to handle this is by making this device an interrupt > controller, and honour the irq_set_wake API. This has already been done > on other FSL/NXP hardware (see the iMX GPC stuff). > > Thanks, > > M. The RCPM IP block is really not an interrupt controller, instead it is related to power management. The code in this patch is to set the bits in the IPPDEXPCR register. Setting these bits can make the corresponding IP block alive during the period of sleep, so that the IP blocks working as wakeup sources can wake the device. The "rcpm-wakeup" property records the bit mask which should be set when the IP block is working as wakeup source. The bit definition may be different for each QorIQ SoC. The operation is specific to QorIQ platform, so put the code in drivers/soc/fsl/pm/. Thanks, Chenhui