[PATCH RFC 0/5] Dove PMU support

[PATCH RFC 0/5] Dove PMU support

[Russell King - ARM Linux]

The following series of patches add better PMU support for Dove.  This
has been developed on the Cubox, and tested in non-DT and DT modes.

This also improves the interrupt handling over the existing code: the
existing code ends up calling the interrupt handlers twice for every
interrupt raised, because the interrupt clear-down is done at the
wrong point - we need to clear down the interrupt in the device first,
then clear it down in the controller.

The problem this gives is that it can be racy (see comments in the
driver) so we're careful about how we do that to minimise the window.

I've included all patches here - the initial set are targetted towards
adding DT support, with the final adding the non-DT support.  There is
a call to the initialisation function missing for DT mode - I'd like
the mvebu people to comment on how that should be handled, as it needs
to be done pretty early.

Also included are two PM domain changes: the first I've discussed with
Rafael who seems happy with it.  The second is necessary because we
have no way to know if a generic PM domain is associated with a device
or whether something else making use of the PM domain is installed in
the dev->pm_domain pointer, so this allows that decision to be made by
core PM code.

This is more a "this is where I'm at" with this stuff than a real
submission, nevertheless comments on how to get it ready for submission
would be welcome.  I'd like to get this off my plate ASAP.

 arch/arm/Kconfig                     |   1 +
 arch/arm/boot/dts/dove.dtsi          |   7 +
 arch/arm/mach-dove/Makefile          |   1 +
 arch/arm/mach-dove/common.c          |   2 +
 arch/arm/mach-dove/common.h          |   1 +
 arch/arm/mach-dove/include/mach/pm.h |  17 --
 arch/arm/mach-dove/irq.c             |  87 ------
 arch/arm/mach-dove/pmu.c             | 531 +++++++++++++++++++++++++++++++++++
 drivers/base/power/domain.c          |   8 +-
 9 files changed, 547 insertions(+), 108 deletions(-)

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH RFC 0/5] Dove PMU support

[Russell King - ARM Linux]

The following series of patches add better PMU support for Dove.  This
has been developed on the Cubox, and tested in non-DT and DT modes.

This also improves the interrupt handling over the existing code: the
existing code ends up calling the interrupt handlers twice for every
interrupt raised, because the interrupt clear-down is done at the
wrong point - we need to clear down the interrupt in the device first,
then clear it down in the controller.

The problem this gives is that it can be racy (see comments in the
driver) so we're careful about how we do that to minimise the window.

I've included all patches here - the initial set are targetted towards
adding DT support, with the final adding the non-DT support.  There is
a call to the initialisation function missing for DT mode - I'd like
the mvebu people to comment on how that should be handled, as it needs
to be done pretty early.

Also included are two PM domain changes: the first I've discussed with
Rafael who seems happy with it.  The second is necessary because we
have no way to know if a generic PM domain is associated with a device
or whether something else making use of the PM domain is installed in
the dev->pm_domain pointer, so this allows that decision to be made by
core PM code.

This is more a "this is where I'm at" with this stuff than a real
submission, nevertheless comments on how to get it ready for submission
would be welcome.  I'd like to get this off my plate ASAP.

 arch/arm/Kconfig                     |   1 +
 arch/arm/boot/dts/dove.dtsi          |   7 +
 arch/arm/mach-dove/Makefile          |   1 +
 arch/arm/mach-dove/common.c          |   2 +
 arch/arm/mach-dove/common.h          |   1 +
 arch/arm/mach-dove/include/mach/pm.h |  17 --
 arch/arm/mach-dove/irq.c             |  87 ------
 arch/arm/mach-dove/pmu.c             | 531 +++++++++++++++++++++++++++++++++++
 drivers/base/power/domain.c          |   8 +-
 9 files changed, 547 insertions(+), 108 deletions(-)

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 1/5] pm: domains: quieten down generic pm domains

[Russell King]

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 drivers/base/power/domain.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index bfb8955c406c..ea91ea0e137b 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -42,7 +42,7 @@
 	struct gpd_timing_data *__td = &dev_gpd_data(dev)->td;			\
 	if (!__retval && __elapsed > __td->field) {				\
 		__td->field = __elapsed;					\
-		dev_warn(dev, name " latency exceeded, new value %lld ns\n",	\
+		dev_dbg(dev, name " latency exceeded, new value %lld ns\n",	\
 			__elapsed);						\
 		genpd->max_off_time_changed = true;				\
 		__td->constraint_changed = true;				\
@@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
 			genpd->max_off_time_changed = true;
 			genpd_recalc_cpu_exit_latency(genpd);
 			if (genpd->name)
-				pr_warning("%s: Power-on latency exceeded, "
+				pr_debug("%s: Power-on latency exceeded, "
 					"new value %lld ns\n", genpd->name,
 					elapsed_ns);
 		}
@@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
 			genpd->power_off_latency_ns = elapsed_ns;
 			genpd->max_off_time_changed = true;
 			if (genpd->name)
-				pr_warning("%s: Power-off latency exceeded, "
+				pr_debug("%s: Power-off latency exceeded, "
 					"new value %lld ns\n", genpd->name,
 					elapsed_ns);
 		}
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] pm: domains: quieten down generic pm domains

[Russell King]

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/base/power/domain.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index bfb8955c406c..ea91ea0e137b 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -42,7 +42,7 @@
 	struct gpd_timing_data *__td = &dev_gpd_data(dev)->td;			\
 	if (!__retval && __elapsed > __td->field) {				\
 		__td->field = __elapsed;					\
-		dev_warn(dev, name " latency exceeded, new value %lld ns\n",	\
+		dev_dbg(dev, name " latency exceeded, new value %lld ns\n",	\
 			__elapsed);						\
 		genpd->max_off_time_changed = true;				\
 		__td->constraint_changed = true;				\
@@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
 			genpd->max_off_time_changed = true;
 			genpd_recalc_cpu_exit_latency(genpd);
 			if (genpd->name)
-				pr_warning("%s: Power-on latency exceeded, "
+				pr_debug("%s: Power-on latency exceeded, "
 					"new value %lld ns\n", genpd->name,
 					elapsed_ns);
 		}
@@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
 			genpd->power_off_latency_ns = elapsed_ns;
 			genpd->max_off_time_changed = true;
 			if (genpd->name)
-				pr_warning("%s: Power-off latency exceeded, "
+				pr_debug("%s: Power-off latency exceeded, "
 					"new value %lld ns\n", genpd->name,
 					elapsed_ns);
 		}
-- 
1.8.3.1

[PATCH 2/5] pm: domains: avoid potential oops in pm_genpd_remove_device()

[Russell King]

pm_genpd_remove_device() should only be called with valid and present
pm domain.  There are circumstances where we may end up with something
that isn't a generic PM domain in dev->pm_domain (eg, vga_switcheroo
stuff.)

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index ea91ea0e137b..9d8faecc060c 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1531,7 +1531,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 
 	dev_dbg(dev, "%s()\n", __func__);
 
-	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev)
+	if (!pm_genpd_present(genpd) || IS_ERR_OR_NULL(dev)
 	    ||  IS_ERR_OR_NULL(dev->pm_domain)
 	    ||  pd_to_genpd(dev->pm_domain) != genpd)
 		return -EINVAL;
-- 
1.8.3.1

[PATCH 2/5] pm: domains: avoid potential oops in pm_genpd_remove_device()

[Russell King]

pm_genpd_remove_device() should only be called with valid and present
pm domain.  There are circumstances where we may end up with something
that isn't a generic PM domain in dev->pm_domain (eg, vga_switcheroo
stuff.)

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/base/power/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index ea91ea0e137b..9d8faecc060c 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1531,7 +1531,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 
 	dev_dbg(dev, "%s()\n", __func__);
 
-	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev)
+	if (!pm_genpd_present(genpd) || IS_ERR_OR_NULL(dev)
 	    ||  IS_ERR_OR_NULL(dev->pm_domain)
 	    ||  pd_to_genpd(dev->pm_domain) != genpd)
 		return -EINVAL;
-- 
1.8.3.1

[PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King]

The PMU device contains an interrupt controller, power control and
resets.  The interrupt controller is a little sub-standard in that
there is no race free way to clear down pending interrupts, so we try
to avoid problems by reducing the window as much as possible, and
clearing as infrequently as possible.

The interrupt support is implemented using an IRQ domain, and the
parent interrupt referenced in the standard DT way.

The power domains and reset support is closely related - there is a
defined sequence for powering down a domain which is tightly coupled
with asserting the reset.  Hence, it makes sense to group these two
together.

This patch adds the core PMU driver: power domains must be defined in
the DT file in order to make use of them.  The reset controller can
be referenced in the standard way for reset controllers.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/Kconfig                     |   1 +
 arch/arm/mach-dove/Makefile          |   1 +
 arch/arm/mach-dove/common.c          |   2 +
 arch/arm/mach-dove/common.h          |   1 +
 arch/arm/mach-dove/include/mach/pm.h |  17 --
 arch/arm/mach-dove/irq.c             |  87 -------
 arch/arm/mach-dove/pmu.c             | 457 +++++++++++++++++++++++++++++++++++
 7 files changed, 462 insertions(+), 104 deletions(-)
 create mode 100644 arch/arm/mach-dove/pmu.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 15949459611f..cec3ff2dfad4 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -534,6 +534,7 @@ config ARCH_DOVE
 	select PINCTRL
 	select PINCTRL_DOVE
 	select PLAT_ORION_LEGACY
+	select PM_GENERIC_DOMAINS if PM
 	select USB_ARCH_HAS_EHCI
 	help
 	  Support for the Marvell Dove SoC 88AP510
diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile
index cbc5c0618788..8e59c57dfa3c 100644
--- a/arch/arm/mach-dove/Makefile
+++ b/arch/arm/mach-dove/Makefile
@@ -1,4 +1,5 @@
 obj-y				+= common.o
+obj-$(CONFIG_PM_GENERIC_DOMAINS)+= pmu.o
 obj-$(CONFIG_DOVE_LEGACY)	+= irq.o mpp.o
 obj-$(CONFIG_PCI)		+= pcie.o
 obj-$(CONFIG_MACH_DOVE_DB)	+= dove-db-setup.o
diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c
index 0d1a89298ece..195871c87819 100644
--- a/arch/arm/mach-dove/common.c
+++ b/arch/arm/mach-dove/common.c
@@ -377,6 +377,8 @@ void __init dove_setup_cpu_wins(void)
 
 void __init dove_init(void)
 {
+	dove_init_pmu();
+
 	pr_info("Dove 88AP510 SoC, TCLK = %d MHz.\n",
 		(dove_tclk + 499999) / 1000000);
 
diff --git a/arch/arm/mach-dove/common.h b/arch/arm/mach-dove/common.h
index 1d725224d146..261e0e995daa 100644
--- a/arch/arm/mach-dove/common.h
+++ b/arch/arm/mach-dove/common.h
@@ -45,5 +45,6 @@ void dove_i2c_init(void);
 void dove_sdio0_init(void);
 void dove_sdio1_init(void);
 void dove_restart(enum reboot_mode, const char *);
+int dove_init_pmu(void);
 
 #endif
diff --git a/arch/arm/mach-dove/include/mach/pm.h b/arch/arm/mach-dove/include/mach/pm.h
index b47f75038686..625a89c15c1f 100644
--- a/arch/arm/mach-dove/include/mach/pm.h
+++ b/arch/arm/mach-dove/include/mach/pm.h
@@ -51,22 +51,5 @@
 #define  CLOCK_GATING_GIGA_PHY_MASK	(1 << CLOCK_GATING_BIT_GIGA_PHY)
 
 #define PMU_INTERRUPT_CAUSE	(DOVE_PMU_VIRT_BASE + 0x50)
-#define PMU_INTERRUPT_MASK	(DOVE_PMU_VIRT_BASE + 0x54)
-
-static inline int pmu_to_irq(int pin)
-{
-	if (pin < NR_PMU_IRQS)
-		return pin + IRQ_DOVE_PMU_START;
-
-	return -EINVAL;
-}
-
-static inline int irq_to_pmu(int irq)
-{
-	if (IRQ_DOVE_PMU_START <= irq && irq < NR_IRQS)
-		return irq - IRQ_DOVE_PMU_START;
-
-	return -EINVAL;
-}
 
 #endif
diff --git a/arch/arm/mach-dove/irq.c b/arch/arm/mach-dove/irq.c
index bc4344aa1009..ca14d45a699b 100644
--- a/arch/arm/mach-dove/irq.c
+++ b/arch/arm/mach-dove/irq.c
@@ -7,86 +7,14 @@
  * License version 2.  This program is licensed "as is" without any
  * warranty of any kind, whether express or implied.
  */
-
-#include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/irq.h>
-#include <linux/gpio.h>
 #include <linux/io.h>
-#include <asm/mach/arch.h>
 #include <plat/irq.h>
-#include <asm/mach/irq.h>
-#include <mach/pm.h>
 #include <mach/bridge-regs.h>
 #include <plat/orion-gpio.h>
 #include "common.h"
 
-static void pmu_irq_mask(struct irq_data *d)
-{
-	int pin = irq_to_pmu(d->irq);
-	u32 u;
-
-	u = readl(PMU_INTERRUPT_MASK);
-	u &= ~(1 << (pin & 31));
-	writel(u, PMU_INTERRUPT_MASK);
-}
-
-static void pmu_irq_unmask(struct irq_data *d)
-{
-	int pin = irq_to_pmu(d->irq);
-	u32 u;
-
-	u = readl(PMU_INTERRUPT_MASK);
-	u |= 1 << (pin & 31);
-	writel(u, PMU_INTERRUPT_MASK);
-}
-
-static void pmu_irq_ack(struct irq_data *d)
-{
-	int pin = irq_to_pmu(d->irq);
-	u32 u;
-
-	/*
-	 * The PMU mask register is not RW0C: it is RW.  This means that
-	 * the bits take whatever value is written to them; if you write
-	 * a '1', you will set the interrupt.
-	 *
-	 * Unfortunately this means there is NO race free way to clear
-	 * these interrupts.
-	 *
-	 * So, let's structure the code so that the window is as small as
-	 * possible.
-	 */
-	u = ~(1 << (pin & 31));
-	u &= readl_relaxed(PMU_INTERRUPT_CAUSE);
-	writel_relaxed(u, PMU_INTERRUPT_CAUSE);
-}
-
-static struct irq_chip pmu_irq_chip = {
-	.name		= "pmu_irq",
-	.irq_mask	= pmu_irq_mask,
-	.irq_unmask	= pmu_irq_unmask,
-	.irq_ack	= pmu_irq_ack,
-};
-
-static void pmu_irq_handler(unsigned int irq, struct irq_desc *desc)
-{
-	unsigned long cause = readl(PMU_INTERRUPT_CAUSE);
-
-	cause &= readl(PMU_INTERRUPT_MASK);
-	if (cause == 0) {
-		do_bad_IRQ(irq, desc);
-		return;
-	}
-
-	for (irq = 0; irq < NR_PMU_IRQS; irq++) {
-		if (!(cause & (1 << irq)))
-			continue;
-		irq = pmu_to_irq(irq);
-		generic_handle_irq(irq);
-	}
-}
-
 static int __initdata gpio0_irqs[4] = {
 	IRQ_DOVE_GPIO_0_7,
 	IRQ_DOVE_GPIO_8_15,
@@ -110,8 +38,6 @@ static int __initdata gpio2_irqs[4] = {
 
 void __init dove_init_irq(void)
 {
-	int i;
-
 	orion_irq_init(0, IRQ_VIRT_BASE + IRQ_MASK_LOW_OFF);
 	orion_irq_init(32, IRQ_VIRT_BASE + IRQ_MASK_HIGH_OFF);
 
@@ -126,17 +52,4 @@ void __init dove_init_irq(void)
 
 	orion_gpio_init(NULL, 64, 8, DOVE_GPIO2_VIRT_BASE, 0,
 			IRQ_DOVE_GPIO_START + 64, gpio2_irqs);
-
-	/*
-	 * Mask and clear PMU interrupts
-	 */
-	writel(0, PMU_INTERRUPT_MASK);
-	writel(0, PMU_INTERRUPT_CAUSE);
-
-	for (i = IRQ_DOVE_PMU_START; i < NR_IRQS; i++) {
-		irq_set_chip_and_handler(i, &pmu_irq_chip, handle_level_irq);
-		irq_set_status_flags(i, IRQ_LEVEL);
-		set_irq_flags(i, IRQF_VALID);
-	}
-	irq_set_chained_handler(IRQ_DOVE_PMU, pmu_irq_handler);
 }
diff --git a/arch/arm/mach-dove/pmu.c b/arch/arm/mach-dove/pmu.c
new file mode 100644
index 000000000000..0b3201fa2d5c
--- /dev/null
+++ b/arch/arm/mach-dove/pmu.c
@@ -0,0 +1,457 @@
+/*
+ * Marvell Dove PMU support
+ */
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/reset.h>
+#include <linux/reset-controller.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include <asm/mach/irq.h>
+
+#include <mach/hardware.h>
+#include <mach/pm.h>
+
+#define PMC_SW_RST		0x30
+#define PMC_IRQ_CAUSE		0x50
+#define PMC_IRQ_MASK		0x54
+
+#define PMU_PWR			0x10
+#define  PMU_PWR_DOWN_GPU	BIT(2)
+#define  PMU_PWR_DOWN_VPU	BIT(3)
+#define PMU_ISO			0x58
+#define  PMU_ISO_VPU		BIT(0)
+#define  PMU_ISO_GPU		BIT(1)
+#define  PMU_ISO_CPU		BIT(2)
+#define  PMU_ISO_CORE		BIT(3)
+
+struct pmu_data {
+	spinlock_t lock;
+	struct device_node *of_node;
+	void __iomem *pmc_base;
+	void __iomem *pmu_base;
+	struct irq_chip_generic *irq_gc;
+	struct irq_domain *irq_domain;
+#ifdef CONFIG_RESET_CONTROLLER
+	struct reset_controller_dev reset;
+#endif
+};
+
+/*
+ * The PMU contains a register to reset various subsystems within the
+ * SoC.  Export this as a reset controller.
+ */
+#ifdef CONFIG_RESET_CONTROLLER
+#define rcdev_to_pmu(rcdev) container_of(rcdev, struct pmu_data, reset)
+
+static int pmu_reset_reset(struct reset_controller_dev *rc, unsigned long id)
+{
+	struct pmu_data *pmu = rcdev_to_pmu(rc);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&pmu->lock, flags);
+	val = readl_relaxed(pmu->pmc_base + PMC_SW_RST);
+	writel_relaxed(val & ~BIT(id), pmu->pmc_base + PMC_SW_RST);
+	writel_relaxed(val | BIT(id), pmu->pmc_base + PMC_SW_RST);
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static int pmu_reset_assert(struct reset_controller_dev *rc, unsigned long id)
+{
+	struct pmu_data *pmu = rcdev_to_pmu(rc);
+	unsigned long flags;
+	u32 val = ~BIT(id);
+
+	spin_lock_irqsave(&pmu->lock, flags);
+	val &= readl_relaxed(pmu->pmc_base + PMC_SW_RST);
+	writel_relaxed(val, pmu->pmc_base + PMC_SW_RST);
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static int pmu_reset_deassert(struct reset_controller_dev *rc, unsigned long id)
+{
+	struct pmu_data *pmu = rcdev_to_pmu(rc);
+	unsigned long flags;
+	u32 val = BIT(id);
+
+	spin_lock_irqsave(&pmu->lock, flags);
+	val |= readl_relaxed(pmu->pmc_base + PMC_SW_RST);
+	writel_relaxed(val, pmu->pmc_base + PMC_SW_RST);
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static struct reset_control_ops pmu_reset_ops = {
+	.reset = pmu_reset_reset,
+	.assert = pmu_reset_assert,
+	.deassert = pmu_reset_deassert,
+};
+
+static struct reset_controller_dev pmu_reset __initdata = {
+	.ops = &pmu_reset_ops,
+	.owner = THIS_MODULE,
+	.nr_resets = 32,
+};
+
+static void __init pmu_reset_init(struct pmu_data *pmu)
+{
+	int ret;
+
+	pmu->reset = pmu_reset;
+	pmu->reset.of_node = pmu->of_node;
+
+	ret = reset_controller_register(&pmu->reset);
+	if (ret)
+		pr_err("pmu: %s failed: %d\n", "reset_controller_register", ret);
+}
+#else
+static void __init pmu_reset_init(struct pmu_data *pmu)
+{
+}
+#endif
+
+struct pmu_domain {
+	struct pmu_data *pmu;
+	u32 pwr_mask;
+	u32 rst_mask;
+	u32 iso_mask;
+	struct generic_pm_domain base;
+};
+
+#define to_pmu_domain(dom) container_of(dom, struct pmu_domain, base)
+
+/*
+ * This deals with the "old" Marvell sequence of bringing a power domain
+ * down/up, which is: apply power, release reset, disable isolators.
+ *
+ * Later devices apparantly use a different sequence: power up, disable
+ * isolators, assert repair signal, enable SRMA clock, enable AXI clock,
+ * enable module clock, deassert reset.
+ *
+ * Note: reading the assembly, it seems that the IO accessors have an
+ * unfortunate side-effect - they cause memory already read into registers
+ * for the if () to be re-read for the bit-set or bit-clear operation.
+ * The code is written to avoid this.
+ */
+static int pmu_domain_power_off(struct generic_pm_domain *domain)
+{
+	struct pmu_domain *pmu_dom = to_pmu_domain(domain);
+	struct pmu_data *pmu = pmu_dom->pmu;
+	unsigned long flags;
+	unsigned int val;
+	void __iomem *pmu_base = pmu->pmu_base;
+	void __iomem *pmc_base = pmu->pmc_base;
+
+	spin_lock_irqsave(&pmu->lock, flags);
+
+	/* Enable isolators */
+	if (pmu_dom->iso_mask) {
+		val = ~pmu_dom->iso_mask;
+		val &= readl_relaxed(pmu_base + PMU_ISO);
+		writel_relaxed(val, pmu_base + PMU_ISO);
+	}
+
+	/* Reset unit */
+	if (pmu_dom->rst_mask) {
+		val = ~pmu_dom->rst_mask;
+		val &= readl_relaxed(pmc_base + PMC_SW_RST);
+		writel_relaxed(val, pmc_base + PMC_SW_RST);
+	}
+
+	/* Power down */
+	val = readl_relaxed(pmu_base + PMU_PWR) | pmu_dom->pwr_mask;
+	writel_relaxed(val, pmu_base + PMU_PWR);
+
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static int pmu_domain_power_on(struct generic_pm_domain *domain)
+{
+	struct pmu_domain *pmu_dom = to_pmu_domain(domain);
+	struct pmu_data *pmu = pmu_dom->pmu;
+	unsigned long flags;
+	unsigned int val;
+	void __iomem *pmu_base = pmu->pmu_base;
+	void __iomem *pmc_base = pmu->pmc_base;
+
+	spin_lock_irqsave(&pmu->lock, flags);
+
+	/* Power on */
+	val = ~pmu_dom->pwr_mask & readl_relaxed(pmu_base + PMU_PWR);
+	writel_relaxed(val, pmu_base + PMU_PWR);
+
+	/* Release reset */
+	if (pmu_dom->rst_mask) {
+		val = pmu_dom->rst_mask;
+		val |= readl_relaxed(pmc_base + PMC_SW_RST);
+		writel_relaxed(val, pmc_base + PMC_SW_RST);
+	}
+
+	/* Disable isolators */
+	if (pmu_dom->iso_mask) {
+		val = pmu_dom->iso_mask;
+		val |= readl_relaxed(pmu_base + PMU_ISO);
+		writel_relaxed(val, pmu_base + PMU_ISO);
+	}
+
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static void __pmu_domain_register(struct pmu_domain *domain)
+{
+	unsigned int val = readl_relaxed(domain->pmu->pmu_base + PMU_PWR);
+
+	domain->base.dev_irq_safe = true;
+	domain->base.power_off = pmu_domain_power_off;
+	domain->base.power_on = pmu_domain_power_on;
+
+	pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask));
+}
+
+static void pmu_add_genpd_of(struct device *dev)
+{
+	struct device_node *node;
+
+	node = of_parse_phandle(dev->of_node, "marvell,power-domain", 0);
+	if (!node)
+		return;
+
+	while (1) {
+		if (pm_genpd_of_add_device(node, dev) != -EAGAIN)
+			break;
+		cond_resched();
+	}
+}
+
+static void pmu_remove_genpd(struct device *dev)
+{
+	struct generic_pm_domain *genpd = dev_to_genpd(dev);
+
+	while (1) {
+		if (pm_genpd_remove_device(genpd, dev) != -EAGAIN)
+			break;
+		cond_resched();
+	}
+}
+
+static int pmu_platform_call(struct notifier_block *nb,
+	unsigned long event, void *data)
+{
+	struct device *dev = data;
+
+	switch (event) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		if (dev->of_node)
+			pmu_add_genpd_of(dev);
+		break;
+
+	case BUS_NOTIFY_DEL_DEVICE:
+		pmu_remove_genpd(dev);
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block platform_nb = {
+	.notifier_call = pmu_platform_call,
+};
+
+/* PMU IRQ controller */
+static void pmu_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+	struct pmu_data *pmu = irq_get_handler_data(irq);
+	struct irq_chip_generic *gc = pmu->irq_gc;
+	struct irq_domain *domain = pmu->irq_domain;
+	void __iomem *base = gc->reg_base;
+	u32 stat = readl_relaxed(base + PMC_IRQ_CAUSE) & gc->mask_cache;
+	u32 done = ~0;
+
+	if (stat == 0) {
+		do_bad_IRQ(irq, desc);
+		return;
+	}
+
+	while (stat) {
+		u32 hwirq = fls(stat) - 1;
+
+		stat &= ~(1 << hwirq);
+		done &= ~(1 << hwirq);
+
+		generic_handle_irq(irq_find_mapping(domain, hwirq));
+	}
+
+	/*
+	 * The PMU mask register is not RW0C: it is RW.  This means that
+	 * the bits take whatever value is written to them; if you write
+	 * a '1', you will set the interrupt.
+	 *
+	 * Unfortunately this means there is NO race free way to clear
+	 * these interrupts.
+	 *
+	 * So, let's structure the code so that the window is as small as
+	 * possible.
+	 */
+	irq_gc_lock(gc);
+	done &= readl_relaxed(base + PMC_IRQ_CAUSE);
+	writel_relaxed(done, base + PMC_IRQ_CAUSE);
+	irq_gc_unlock(gc);
+}
+
+static int __init dove_init_pmu_irq(struct pmu_data *pmu, int irq)
+{
+	const char *name = "pmu_irq";
+	struct irq_chip_generic *gc;
+	struct irq_domain *domain;
+	int ret;
+
+	/* mask and clear all interrupts */
+	writel(0, pmu->pmc_base + PMC_IRQ_MASK);
+	writel(0, pmu->pmc_base + PMC_IRQ_CAUSE);
+
+	domain = irq_domain_add_linear(pmu->of_node, NR_PMU_IRQS,
+				       &irq_generic_chip_ops, NULL);
+	if (!domain) {
+		pr_err("%s: unable to add irq domain\n", name);
+		return -ENOMEM;
+	}
+
+	ret = irq_alloc_domain_generic_chips(domain, NR_PMU_IRQS, 1, name,
+					     handle_level_irq,
+					     IRQ_NOREQUEST | IRQ_NOPROBE, 0,
+					     IRQ_GC_INIT_MASK_CACHE);
+	if (ret) {
+		pr_err("%s: unable to alloc irq domain gc: %d\n", name, ret);
+		irq_domain_remove(domain);
+		return ret;
+	}
+
+	gc = irq_get_domain_generic_chip(domain, 0);
+	gc->reg_base = pmu->pmc_base;
+	gc->chip_types[0].regs.mask = PMC_IRQ_MASK;
+	gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
+	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit;
+
+	pmu->irq_domain = domain;
+	pmu->irq_gc = gc;
+
+	/* If no of_node, populate the domain */
+	if (!pmu->of_node)
+		irq_domain_associate_many(pmu->irq_domain, IRQ_DOVE_PMU_START,
+					  0, NR_PMU_IRQS);
+
+	irq_set_handler_data(irq, pmu);
+	irq_set_chained_handler(irq, pmu_irq_handler);
+
+	return 0;
+}
+
+/*
+ * pmu {
+ *	compatible = "marvell,pmu";
+ *	reg = <0xd0000 0x8000> <0xd8000 0x8000>;
+ *	interrupts = <33>;
+ *	#reset-cells = 1;
+ *	vpu_domain: vpu-domain {
+ *		marvell,pmu_pwr_mask = <0x00000008>;
+ *		marvell,pmu_iso_mask = <0x00000001>;
+ *		resets = <&pmu 16>;
+ *	};
+ *	gpu_domain: gpu-domain {
+ *		marvell,pmu_pwr_mask = <0x00000004>;
+ *		marvell,pmu_iso_mask = <0x00000002>;
+ *		resets = <&pmu 18>;
+ *	};
+ * };
+ */
+int __init dove_init_pmu(void)
+{
+	struct device_node *np_pmu, *np;
+	struct pmu_data *pmu;
+	int ret, parent_irq;
+
+	/* Lookup the PMU node */
+	np_pmu = of_find_compatible_node(NULL, NULL, "marvell,pmu");
+	if (!np_pmu)
+		return 0;
+
+	pmu = kzalloc(sizeof(*pmu), GFP_KERNEL);
+	if (!pmu)
+		return -ENOMEM;
+
+	spin_lock_init(&pmu->lock);
+	pmu->of_node = np_pmu;
+	pmu->pmc_base = of_iomap(pmu->of_node, 0);
+	pmu->pmu_base = of_iomap(pmu->of_node, 1);
+	if (!pmu->pmc_base || !pmu->pmu_base) {
+		pr_err("%s: failed to map PMU\n", np_pmu->name);
+		iounmap(pmu->pmu_base);
+		iounmap(pmu->pmc_base);
+		kfree(pmu);
+		return -ENOMEM;
+	}
+
+	parent_irq = irq_of_parse_and_map(pmu->of_node, 0);
+	if (!parent_irq)
+		pr_err("%s: no interrupt specified\n", np_pmu->name);
+
+	pmu_reset_init(pmu);
+
+	for_each_available_child_of_node(pmu->of_node, np) {
+		struct of_phandle_args args;
+		struct pmu_domain *domain;
+
+		domain = kzalloc(sizeof(*domain), GFP_KERNEL);
+		if (!domain)
+			break;
+
+		domain->pmu = pmu;
+		domain->base.of_node = np;
+		domain->base.name = kstrdup(np->name, GFP_KERNEL);
+		if (!domain->base.name) {
+			kfree(domain);
+			break;
+		}
+
+		of_property_read_u32(np, "marvell,pmu_pwr_mask",
+				     &domain->pwr_mask);
+		of_property_read_u32(np, "marvell,pmu_iso_mask",
+				     &domain->iso_mask);
+
+		ret = of_parse_phandle_with_args(np, "resets", "#reset-cells",
+						 0, &args);
+		if (ret == 0) {
+			if (args.np == pmu->of_node)
+				domain->rst_mask = BIT(args.args[0]);
+			of_node_put(args.np);
+		}
+
+		__pmu_domain_register(domain);
+	}
+	pm_genpd_poweroff_unused();
+
+	ret = dove_init_pmu_irq(pmu, parent_irq);
+	if (ret)
+		pr_err("dove_init_pmu_irq() failed: %d\n", ret);
+
+	bus_register_notifier(&platform_bus_type, &platform_nb);
+
+	return 0;
+}
-- 
1.8.3.1

[PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King]

The PMU device contains an interrupt controller, power control and
resets.  The interrupt controller is a little sub-standard in that
there is no race free way to clear down pending interrupts, so we try
to avoid problems by reducing the window as much as possible, and
clearing as infrequently as possible.

The interrupt support is implemented using an IRQ domain, and the
parent interrupt referenced in the standard DT way.

The power domains and reset support is closely related - there is a
defined sequence for powering down a domain which is tightly coupled
with asserting the reset.  Hence, it makes sense to group these two
together.

This patch adds the core PMU driver: power domains must be defined in
the DT file in order to make use of them.  The reset controller can
be referenced in the standard way for reset controllers.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/Kconfig                     |   1 +
 arch/arm/mach-dove/Makefile          |   1 +
 arch/arm/mach-dove/common.c          |   2 +
 arch/arm/mach-dove/common.h          |   1 +
 arch/arm/mach-dove/include/mach/pm.h |  17 --
 arch/arm/mach-dove/irq.c             |  87 -------
 arch/arm/mach-dove/pmu.c             | 457 +++++++++++++++++++++++++++++++++++
 7 files changed, 462 insertions(+), 104 deletions(-)
 create mode 100644 arch/arm/mach-dove/pmu.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 15949459611f..cec3ff2dfad4 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -534,6 +534,7 @@ config ARCH_DOVE
 	select PINCTRL
 	select PINCTRL_DOVE
 	select PLAT_ORION_LEGACY
+	select PM_GENERIC_DOMAINS if PM
 	select USB_ARCH_HAS_EHCI
 	help
 	  Support for the Marvell Dove SoC 88AP510
diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile
index cbc5c0618788..8e59c57dfa3c 100644
--- a/arch/arm/mach-dove/Makefile
+++ b/arch/arm/mach-dove/Makefile
@@ -1,4 +1,5 @@
 obj-y				+= common.o
+obj-$(CONFIG_PM_GENERIC_DOMAINS)+= pmu.o
 obj-$(CONFIG_DOVE_LEGACY)	+= irq.o mpp.o
 obj-$(CONFIG_PCI)		+= pcie.o
 obj-$(CONFIG_MACH_DOVE_DB)	+= dove-db-setup.o
diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c
index 0d1a89298ece..195871c87819 100644
--- a/arch/arm/mach-dove/common.c
+++ b/arch/arm/mach-dove/common.c
@@ -377,6 +377,8 @@ void __init dove_setup_cpu_wins(void)
 
 void __init dove_init(void)
 {
+	dove_init_pmu();
+
 	pr_info("Dove 88AP510 SoC, TCLK = %d MHz.\n",
 		(dove_tclk + 499999) / 1000000);
 
diff --git a/arch/arm/mach-dove/common.h b/arch/arm/mach-dove/common.h
index 1d725224d146..261e0e995daa 100644
--- a/arch/arm/mach-dove/common.h
+++ b/arch/arm/mach-dove/common.h
@@ -45,5 +45,6 @@ void dove_i2c_init(void);
 void dove_sdio0_init(void);
 void dove_sdio1_init(void);
 void dove_restart(enum reboot_mode, const char *);
+int dove_init_pmu(void);
 
 #endif
diff --git a/arch/arm/mach-dove/include/mach/pm.h b/arch/arm/mach-dove/include/mach/pm.h
index b47f75038686..625a89c15c1f 100644
--- a/arch/arm/mach-dove/include/mach/pm.h
+++ b/arch/arm/mach-dove/include/mach/pm.h
@@ -51,22 +51,5 @@
 #define  CLOCK_GATING_GIGA_PHY_MASK	(1 << CLOCK_GATING_BIT_GIGA_PHY)
 
 #define PMU_INTERRUPT_CAUSE	(DOVE_PMU_VIRT_BASE + 0x50)
-#define PMU_INTERRUPT_MASK	(DOVE_PMU_VIRT_BASE + 0x54)
-
-static inline int pmu_to_irq(int pin)
-{
-	if (pin < NR_PMU_IRQS)
-		return pin + IRQ_DOVE_PMU_START;
-
-	return -EINVAL;
-}
-
-static inline int irq_to_pmu(int irq)
-{
-	if (IRQ_DOVE_PMU_START <= irq && irq < NR_IRQS)
-		return irq - IRQ_DOVE_PMU_START;
-
-	return -EINVAL;
-}
 
 #endif
diff --git a/arch/arm/mach-dove/irq.c b/arch/arm/mach-dove/irq.c
index bc4344aa1009..ca14d45a699b 100644
--- a/arch/arm/mach-dove/irq.c
+++ b/arch/arm/mach-dove/irq.c
@@ -7,86 +7,14 @@
  * License version 2.  This program is licensed "as is" without any
  * warranty of any kind, whether express or implied.
  */
-
-#include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/irq.h>
-#include <linux/gpio.h>
 #include <linux/io.h>
-#include <asm/mach/arch.h>
 #include <plat/irq.h>
-#include <asm/mach/irq.h>
-#include <mach/pm.h>
 #include <mach/bridge-regs.h>
 #include <plat/orion-gpio.h>
 #include "common.h"
 
-static void pmu_irq_mask(struct irq_data *d)
-{
-	int pin = irq_to_pmu(d->irq);
-	u32 u;
-
-	u = readl(PMU_INTERRUPT_MASK);
-	u &= ~(1 << (pin & 31));
-	writel(u, PMU_INTERRUPT_MASK);
-}
-
-static void pmu_irq_unmask(struct irq_data *d)
-{
-	int pin = irq_to_pmu(d->irq);
-	u32 u;
-
-	u = readl(PMU_INTERRUPT_MASK);
-	u |= 1 << (pin & 31);
-	writel(u, PMU_INTERRUPT_MASK);
-}
-
-static void pmu_irq_ack(struct irq_data *d)
-{
-	int pin = irq_to_pmu(d->irq);
-	u32 u;
-
-	/*
-	 * The PMU mask register is not RW0C: it is RW.  This means that
-	 * the bits take whatever value is written to them; if you write
-	 * a '1', you will set the interrupt.
-	 *
-	 * Unfortunately this means there is NO race free way to clear
-	 * these interrupts.
-	 *
-	 * So, let's structure the code so that the window is as small as
-	 * possible.
-	 */
-	u = ~(1 << (pin & 31));
-	u &= readl_relaxed(PMU_INTERRUPT_CAUSE);
-	writel_relaxed(u, PMU_INTERRUPT_CAUSE);
-}
-
-static struct irq_chip pmu_irq_chip = {
-	.name		= "pmu_irq",
-	.irq_mask	= pmu_irq_mask,
-	.irq_unmask	= pmu_irq_unmask,
-	.irq_ack	= pmu_irq_ack,
-};
-
-static void pmu_irq_handler(unsigned int irq, struct irq_desc *desc)
-{
-	unsigned long cause = readl(PMU_INTERRUPT_CAUSE);
-
-	cause &= readl(PMU_INTERRUPT_MASK);
-	if (cause == 0) {
-		do_bad_IRQ(irq, desc);
-		return;
-	}
-
-	for (irq = 0; irq < NR_PMU_IRQS; irq++) {
-		if (!(cause & (1 << irq)))
-			continue;
-		irq = pmu_to_irq(irq);
-		generic_handle_irq(irq);
-	}
-}
-
 static int __initdata gpio0_irqs[4] = {
 	IRQ_DOVE_GPIO_0_7,
 	IRQ_DOVE_GPIO_8_15,
@@ -110,8 +38,6 @@ static int __initdata gpio2_irqs[4] = {
 
 void __init dove_init_irq(void)
 {
-	int i;
-
 	orion_irq_init(0, IRQ_VIRT_BASE + IRQ_MASK_LOW_OFF);
 	orion_irq_init(32, IRQ_VIRT_BASE + IRQ_MASK_HIGH_OFF);
 
@@ -126,17 +52,4 @@ void __init dove_init_irq(void)
 
 	orion_gpio_init(NULL, 64, 8, DOVE_GPIO2_VIRT_BASE, 0,
 			IRQ_DOVE_GPIO_START + 64, gpio2_irqs);
-
-	/*
-	 * Mask and clear PMU interrupts
-	 */
-	writel(0, PMU_INTERRUPT_MASK);
-	writel(0, PMU_INTERRUPT_CAUSE);
-
-	for (i = IRQ_DOVE_PMU_START; i < NR_IRQS; i++) {
-		irq_set_chip_and_handler(i, &pmu_irq_chip, handle_level_irq);
-		irq_set_status_flags(i, IRQ_LEVEL);
-		set_irq_flags(i, IRQF_VALID);
-	}
-	irq_set_chained_handler(IRQ_DOVE_PMU, pmu_irq_handler);
 }
diff --git a/arch/arm/mach-dove/pmu.c b/arch/arm/mach-dove/pmu.c
new file mode 100644
index 000000000000..0b3201fa2d5c
--- /dev/null
+++ b/arch/arm/mach-dove/pmu.c
@@ -0,0 +1,457 @@
+/*
+ * Marvell Dove PMU support
+ */
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/reset.h>
+#include <linux/reset-controller.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include <asm/mach/irq.h>
+
+#include <mach/hardware.h>
+#include <mach/pm.h>
+
+#define PMC_SW_RST		0x30
+#define PMC_IRQ_CAUSE		0x50
+#define PMC_IRQ_MASK		0x54
+
+#define PMU_PWR			0x10
+#define  PMU_PWR_DOWN_GPU	BIT(2)
+#define  PMU_PWR_DOWN_VPU	BIT(3)
+#define PMU_ISO			0x58
+#define  PMU_ISO_VPU		BIT(0)
+#define  PMU_ISO_GPU		BIT(1)
+#define  PMU_ISO_CPU		BIT(2)
+#define  PMU_ISO_CORE		BIT(3)
+
+struct pmu_data {
+	spinlock_t lock;
+	struct device_node *of_node;
+	void __iomem *pmc_base;
+	void __iomem *pmu_base;
+	struct irq_chip_generic *irq_gc;
+	struct irq_domain *irq_domain;
+#ifdef CONFIG_RESET_CONTROLLER
+	struct reset_controller_dev reset;
+#endif
+};
+
+/*
+ * The PMU contains a register to reset various subsystems within the
+ * SoC.  Export this as a reset controller.
+ */
+#ifdef CONFIG_RESET_CONTROLLER
+#define rcdev_to_pmu(rcdev) container_of(rcdev, struct pmu_data, reset)
+
+static int pmu_reset_reset(struct reset_controller_dev *rc, unsigned long id)
+{
+	struct pmu_data *pmu = rcdev_to_pmu(rc);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&pmu->lock, flags);
+	val = readl_relaxed(pmu->pmc_base + PMC_SW_RST);
+	writel_relaxed(val & ~BIT(id), pmu->pmc_base + PMC_SW_RST);
+	writel_relaxed(val | BIT(id), pmu->pmc_base + PMC_SW_RST);
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static int pmu_reset_assert(struct reset_controller_dev *rc, unsigned long id)
+{
+	struct pmu_data *pmu = rcdev_to_pmu(rc);
+	unsigned long flags;
+	u32 val = ~BIT(id);
+
+	spin_lock_irqsave(&pmu->lock, flags);
+	val &= readl_relaxed(pmu->pmc_base + PMC_SW_RST);
+	writel_relaxed(val, pmu->pmc_base + PMC_SW_RST);
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static int pmu_reset_deassert(struct reset_controller_dev *rc, unsigned long id)
+{
+	struct pmu_data *pmu = rcdev_to_pmu(rc);
+	unsigned long flags;
+	u32 val = BIT(id);
+
+	spin_lock_irqsave(&pmu->lock, flags);
+	val |= readl_relaxed(pmu->pmc_base + PMC_SW_RST);
+	writel_relaxed(val, pmu->pmc_base + PMC_SW_RST);
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static struct reset_control_ops pmu_reset_ops = {
+	.reset = pmu_reset_reset,
+	.assert = pmu_reset_assert,
+	.deassert = pmu_reset_deassert,
+};
+
+static struct reset_controller_dev pmu_reset __initdata = {
+	.ops = &pmu_reset_ops,
+	.owner = THIS_MODULE,
+	.nr_resets = 32,
+};
+
+static void __init pmu_reset_init(struct pmu_data *pmu)
+{
+	int ret;
+
+	pmu->reset = pmu_reset;
+	pmu->reset.of_node = pmu->of_node;
+
+	ret = reset_controller_register(&pmu->reset);
+	if (ret)
+		pr_err("pmu: %s failed: %d\n", "reset_controller_register", ret);
+}
+#else
+static void __init pmu_reset_init(struct pmu_data *pmu)
+{
+}
+#endif
+
+struct pmu_domain {
+	struct pmu_data *pmu;
+	u32 pwr_mask;
+	u32 rst_mask;
+	u32 iso_mask;
+	struct generic_pm_domain base;
+};
+
+#define to_pmu_domain(dom) container_of(dom, struct pmu_domain, base)
+
+/*
+ * This deals with the "old" Marvell sequence of bringing a power domain
+ * down/up, which is: apply power, release reset, disable isolators.
+ *
+ * Later devices apparantly use a different sequence: power up, disable
+ * isolators, assert repair signal, enable SRMA clock, enable AXI clock,
+ * enable module clock, deassert reset.
+ *
+ * Note: reading the assembly, it seems that the IO accessors have an
+ * unfortunate side-effect - they cause memory already read into registers
+ * for the if () to be re-read for the bit-set or bit-clear operation.
+ * The code is written to avoid this.
+ */
+static int pmu_domain_power_off(struct generic_pm_domain *domain)
+{
+	struct pmu_domain *pmu_dom = to_pmu_domain(domain);
+	struct pmu_data *pmu = pmu_dom->pmu;
+	unsigned long flags;
+	unsigned int val;
+	void __iomem *pmu_base = pmu->pmu_base;
+	void __iomem *pmc_base = pmu->pmc_base;
+
+	spin_lock_irqsave(&pmu->lock, flags);
+
+	/* Enable isolators */
+	if (pmu_dom->iso_mask) {
+		val = ~pmu_dom->iso_mask;
+		val &= readl_relaxed(pmu_base + PMU_ISO);
+		writel_relaxed(val, pmu_base + PMU_ISO);
+	}
+
+	/* Reset unit */
+	if (pmu_dom->rst_mask) {
+		val = ~pmu_dom->rst_mask;
+		val &= readl_relaxed(pmc_base + PMC_SW_RST);
+		writel_relaxed(val, pmc_base + PMC_SW_RST);
+	}
+
+	/* Power down */
+	val = readl_relaxed(pmu_base + PMU_PWR) | pmu_dom->pwr_mask;
+	writel_relaxed(val, pmu_base + PMU_PWR);
+
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static int pmu_domain_power_on(struct generic_pm_domain *domain)
+{
+	struct pmu_domain *pmu_dom = to_pmu_domain(domain);
+	struct pmu_data *pmu = pmu_dom->pmu;
+	unsigned long flags;
+	unsigned int val;
+	void __iomem *pmu_base = pmu->pmu_base;
+	void __iomem *pmc_base = pmu->pmc_base;
+
+	spin_lock_irqsave(&pmu->lock, flags);
+
+	/* Power on */
+	val = ~pmu_dom->pwr_mask & readl_relaxed(pmu_base + PMU_PWR);
+	writel_relaxed(val, pmu_base + PMU_PWR);
+
+	/* Release reset */
+	if (pmu_dom->rst_mask) {
+		val = pmu_dom->rst_mask;
+		val |= readl_relaxed(pmc_base + PMC_SW_RST);
+		writel_relaxed(val, pmc_base + PMC_SW_RST);
+	}
+
+	/* Disable isolators */
+	if (pmu_dom->iso_mask) {
+		val = pmu_dom->iso_mask;
+		val |= readl_relaxed(pmu_base + PMU_ISO);
+		writel_relaxed(val, pmu_base + PMU_ISO);
+	}
+
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
+static void __pmu_domain_register(struct pmu_domain *domain)
+{
+	unsigned int val = readl_relaxed(domain->pmu->pmu_base + PMU_PWR);
+
+	domain->base.dev_irq_safe = true;
+	domain->base.power_off = pmu_domain_power_off;
+	domain->base.power_on = pmu_domain_power_on;
+
+	pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask));
+}
+
+static void pmu_add_genpd_of(struct device *dev)
+{
+	struct device_node *node;
+
+	node = of_parse_phandle(dev->of_node, "marvell,power-domain", 0);
+	if (!node)
+		return;
+
+	while (1) {
+		if (pm_genpd_of_add_device(node, dev) != -EAGAIN)
+			break;
+		cond_resched();
+	}
+}
+
+static void pmu_remove_genpd(struct device *dev)
+{
+	struct generic_pm_domain *genpd = dev_to_genpd(dev);
+
+	while (1) {
+		if (pm_genpd_remove_device(genpd, dev) != -EAGAIN)
+			break;
+		cond_resched();
+	}
+}
+
+static int pmu_platform_call(struct notifier_block *nb,
+	unsigned long event, void *data)
+{
+	struct device *dev = data;
+
+	switch (event) {
+	case BUS_NOTIFY_ADD_DEVICE:
+		if (dev->of_node)
+			pmu_add_genpd_of(dev);
+		break;
+
+	case BUS_NOTIFY_DEL_DEVICE:
+		pmu_remove_genpd(dev);
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block platform_nb = {
+	.notifier_call = pmu_platform_call,
+};
+
+/* PMU IRQ controller */
+static void pmu_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+	struct pmu_data *pmu = irq_get_handler_data(irq);
+	struct irq_chip_generic *gc = pmu->irq_gc;
+	struct irq_domain *domain = pmu->irq_domain;
+	void __iomem *base = gc->reg_base;
+	u32 stat = readl_relaxed(base + PMC_IRQ_CAUSE) & gc->mask_cache;
+	u32 done = ~0;
+
+	if (stat == 0) {
+		do_bad_IRQ(irq, desc);
+		return;
+	}
+
+	while (stat) {
+		u32 hwirq = fls(stat) - 1;
+
+		stat &= ~(1 << hwirq);
+		done &= ~(1 << hwirq);
+
+		generic_handle_irq(irq_find_mapping(domain, hwirq));
+	}
+
+	/*
+	 * The PMU mask register is not RW0C: it is RW.  This means that
+	 * the bits take whatever value is written to them; if you write
+	 * a '1', you will set the interrupt.
+	 *
+	 * Unfortunately this means there is NO race free way to clear
+	 * these interrupts.
+	 *
+	 * So, let's structure the code so that the window is as small as
+	 * possible.
+	 */
+	irq_gc_lock(gc);
+	done &= readl_relaxed(base + PMC_IRQ_CAUSE);
+	writel_relaxed(done, base + PMC_IRQ_CAUSE);
+	irq_gc_unlock(gc);
+}
+
+static int __init dove_init_pmu_irq(struct pmu_data *pmu, int irq)
+{
+	const char *name = "pmu_irq";
+	struct irq_chip_generic *gc;
+	struct irq_domain *domain;
+	int ret;
+
+	/* mask and clear all interrupts */
+	writel(0, pmu->pmc_base + PMC_IRQ_MASK);
+	writel(0, pmu->pmc_base + PMC_IRQ_CAUSE);
+
+	domain = irq_domain_add_linear(pmu->of_node, NR_PMU_IRQS,
+				       &irq_generic_chip_ops, NULL);
+	if (!domain) {
+		pr_err("%s: unable to add irq domain\n", name);
+		return -ENOMEM;
+	}
+
+	ret = irq_alloc_domain_generic_chips(domain, NR_PMU_IRQS, 1, name,
+					     handle_level_irq,
+					     IRQ_NOREQUEST | IRQ_NOPROBE, 0,
+					     IRQ_GC_INIT_MASK_CACHE);
+	if (ret) {
+		pr_err("%s: unable to alloc irq domain gc: %d\n", name, ret);
+		irq_domain_remove(domain);
+		return ret;
+	}
+
+	gc = irq_get_domain_generic_chip(domain, 0);
+	gc->reg_base = pmu->pmc_base;
+	gc->chip_types[0].regs.mask = PMC_IRQ_MASK;
+	gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
+	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit;
+
+	pmu->irq_domain = domain;
+	pmu->irq_gc = gc;
+
+	/* If no of_node, populate the domain */
+	if (!pmu->of_node)
+		irq_domain_associate_many(pmu->irq_domain, IRQ_DOVE_PMU_START,
+					  0, NR_PMU_IRQS);
+
+	irq_set_handler_data(irq, pmu);
+	irq_set_chained_handler(irq, pmu_irq_handler);
+
+	return 0;
+}
+
+/*
+ * pmu {
+ *	compatible = "marvell,pmu";
+ *	reg = <0xd0000 0x8000> <0xd8000 0x8000>;
+ *	interrupts = <33>;
+ *	#reset-cells = 1;
+ *	vpu_domain: vpu-domain {
+ *		marvell,pmu_pwr_mask = <0x00000008>;
+ *		marvell,pmu_iso_mask = <0x00000001>;
+ *		resets = <&pmu 16>;
+ *	};
+ *	gpu_domain: gpu-domain {
+ *		marvell,pmu_pwr_mask = <0x00000004>;
+ *		marvell,pmu_iso_mask = <0x00000002>;
+ *		resets = <&pmu 18>;
+ *	};
+ * };
+ */
+int __init dove_init_pmu(void)
+{
+	struct device_node *np_pmu, *np;
+	struct pmu_data *pmu;
+	int ret, parent_irq;
+
+	/* Lookup the PMU node */
+	np_pmu = of_find_compatible_node(NULL, NULL, "marvell,pmu");
+	if (!np_pmu)
+		return 0;
+
+	pmu = kzalloc(sizeof(*pmu), GFP_KERNEL);
+	if (!pmu)
+		return -ENOMEM;
+
+	spin_lock_init(&pmu->lock);
+	pmu->of_node = np_pmu;
+	pmu->pmc_base = of_iomap(pmu->of_node, 0);
+	pmu->pmu_base = of_iomap(pmu->of_node, 1);
+	if (!pmu->pmc_base || !pmu->pmu_base) {
+		pr_err("%s: failed to map PMU\n", np_pmu->name);
+		iounmap(pmu->pmu_base);
+		iounmap(pmu->pmc_base);
+		kfree(pmu);
+		return -ENOMEM;
+	}
+
+	parent_irq = irq_of_parse_and_map(pmu->of_node, 0);
+	if (!parent_irq)
+		pr_err("%s: no interrupt specified\n", np_pmu->name);
+
+	pmu_reset_init(pmu);
+
+	for_each_available_child_of_node(pmu->of_node, np) {
+		struct of_phandle_args args;
+		struct pmu_domain *domain;
+
+		domain = kzalloc(sizeof(*domain), GFP_KERNEL);
+		if (!domain)
+			break;
+
+		domain->pmu = pmu;
+		domain->base.of_node = np;
+		domain->base.name = kstrdup(np->name, GFP_KERNEL);
+		if (!domain->base.name) {
+			kfree(domain);
+			break;
+		}
+
+		of_property_read_u32(np, "marvell,pmu_pwr_mask",
+				     &domain->pwr_mask);
+		of_property_read_u32(np, "marvell,pmu_iso_mask",
+				     &domain->iso_mask);
+
+		ret = of_parse_phandle_with_args(np, "resets", "#reset-cells",
+						 0, &args);
+		if (ret == 0) {
+			if (args.np == pmu->of_node)
+				domain->rst_mask = BIT(args.args[0]);
+			of_node_put(args.np);
+		}
+
+		__pmu_domain_register(domain);
+	}
+	pm_genpd_poweroff_unused();
+
+	ret = dove_init_pmu_irq(pmu, parent_irq);
+	if (ret)
+		pr_err("dove_init_pmu_irq() failed: %d\n", ret);
+
+	bus_register_notifier(&platform_bus_type, &platform_nb);
+
+	return 0;
+}
-- 
1.8.3.1

[PATCH 4/5] ARM: dove: add Dove PMU DT entries to dove.dtsi

[Russell King]

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/boot/dts/dove.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 187fd46b7b5e..64773e96e7f6 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -367,6 +367,13 @@
 				status = "disabled";
 			};
 
+			pmu: power-management@d0000 {
+				compatible = "marvell,pmu";
+				reg = <0xd0000 0x8000>, <0xd8000 0x8000>;
+				interrupts = <33>;
+				#reset-cells = <1>;
+			};
+
 			thermal: thermal-diode@d001c {
 				compatible = "marvell,dove-thermal";
 				reg = <0xd001c 0x0c>, <0xd005c 0x08>;
-- 
1.8.3.1

[PATCH 4/5] ARM: dove: add Dove PMU DT entries to dove.dtsi

[Russell King]

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/boot/dts/dove.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 187fd46b7b5e..64773e96e7f6 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -367,6 +367,13 @@
 				status = "disabled";
 			};
 
+			pmu: power-management at d0000 {
+				compatible = "marvell,pmu";
+				reg = <0xd0000 0x8000>, <0xd8000 0x8000>;
+				interrupts = <33>;
+				#reset-cells = <1>;
+			};
+
 			thermal: thermal-diode at d001c {
 				compatible = "marvell,dove-thermal";
 				reg = <0xd001c 0x0c>, <0xd005c 0x08>;
-- 
1.8.3.1

[PATCH 5/5] ARM: dove: add non-DT PMU support

[Russell King]

Since Dove has non-DT support still, this patch adds the static data
to the PMU driver in order to initialise PMU domains.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mach-dove/pmu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-dove/pmu.c b/arch/arm/mach-dove/pmu.c
index 0b3201fa2d5c..9832044dbab9 100644
--- a/arch/arm/mach-dove/pmu.c
+++ b/arch/arm/mach-dove/pmu.c
@@ -226,6 +226,20 @@ static void __pmu_domain_register(struct pmu_domain *domain)
 	pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask));
 }
 
+static void pmu_domain_register(struct pmu_data *pmu,
+	const struct pmu_domain *pmu_dom)
+{
+	struct pmu_domain *domain;
+
+	domain = kmemdup(pmu_dom, sizeof(*domain), GFP_KERNEL);
+	if (!domain)
+		return;
+
+	domain->pmu = pmu;
+
+	__pmu_domain_register(domain);
+}
+
 static void pmu_add_genpd_of(struct device *dev)
 {
 	struct device_node *node;
@@ -241,6 +255,15 @@ static void pmu_add_genpd_of(struct device *dev)
 	}
 }
 
+static void pmu_add_genpd_name(const char *name, struct device *dev)
+{
+	while (1) {
+		if (pm_genpd_name_add_device(name, dev) != -EAGAIN)
+			break;
+		cond_resched();
+	}
+}
+
 static void pmu_remove_genpd(struct device *dev)
 {
 	struct generic_pm_domain *genpd = dev_to_genpd(dev);
@@ -256,11 +279,19 @@ static int pmu_platform_call(struct notifier_block *nb,
 	unsigned long event, void *data)
 {
 	struct device *dev = data;
+	const char *name = NULL;
+
+	if (strcmp(dev_name(dev), "ap510-vmeta.0") == 0)
+		name = "vpu";
+	else if (strcmp(dev_name(dev), "galcore.0") == 0)
+		name = "gpu";
 
 	switch (event) {
 	case BUS_NOTIFY_ADD_DEVICE:
 		if (dev->of_node)
 			pmu_add_genpd_of(dev);
+		else if (name)
+			pmu_add_genpd_name(name, dev);
 		break;
 
 	case BUS_NOTIFY_DEL_DEVICE:
@@ -363,6 +394,49 @@ static int __init dove_init_pmu_irq(struct pmu_data *pmu, int irq)
 	return 0;
 }
 
+static struct pmu_data pmu_data = {
+	.lock = __SPIN_LOCK_UNLOCKED(&pmu.lock),
+	.pmc_base = DOVE_PMU_VIRT_BASE,
+	.pmu_base = DOVE_PMU_VIRT_BASE + 0x8000,
+};
+
+static struct pmu_domain vpu_domain = {
+	.pwr_mask = PMU_PWR_DOWN_VPU,
+	.rst_mask = PMU_SW_RST_VIDEO_MASK,
+	.iso_mask = PMU_ISO_VPU,
+	.base = {
+		.name = "vpu",
+	},
+};
+
+static struct pmu_domain gpu_domain = {
+	.pwr_mask = PMU_PWR_DOWN_GPU,
+	.rst_mask = PMU_SW_RST_GPU_MASK,
+	.iso_mask = PMU_ISO_GPU,
+	.base = {
+		.name = "gpu",
+	},
+};
+
+static int __init dove_pmu_init_legacy(void)
+{
+	struct pmu_data *pmu = &pmu_data;
+	int ret, parent_irq;
+
+	pmu_reset_init(pmu);
+	pmu_domain_register(pmu, &vpu_domain);
+	pmu_domain_register(pmu, &gpu_domain);
+	pm_genpd_poweroff_unused();
+
+	ret = dove_init_pmu_irq(pmu, IRQ_DOVE_PMU);
+	if (ret)
+		pr_err("dove_init_pmu_irq() failed: %d\n", ret);
+
+	bus_register_notifier(&platform_bus_type, &platform_nb);
+
+	return 0;
+}
+
 /*
  * pmu {
  *	compatible = "marvell,pmu";
@@ -390,7 +464,7 @@ int __init dove_init_pmu(void)
 	/* Lookup the PMU node */
 	np_pmu = of_find_compatible_node(NULL, NULL, "marvell,pmu");
 	if (!np_pmu)
-		return 0;
+		return dove_pmu_init_legacy();
 
 	pmu = kzalloc(sizeof(*pmu), GFP_KERNEL);
 	if (!pmu)
-- 
1.8.3.1

[PATCH 5/5] ARM: dove: add non-DT PMU support

[Russell King]

Since Dove has non-DT support still, this patch adds the static data
to the PMU driver in order to initialise PMU domains.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mach-dove/pmu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-dove/pmu.c b/arch/arm/mach-dove/pmu.c
index 0b3201fa2d5c..9832044dbab9 100644
--- a/arch/arm/mach-dove/pmu.c
+++ b/arch/arm/mach-dove/pmu.c
@@ -226,6 +226,20 @@ static void __pmu_domain_register(struct pmu_domain *domain)
 	pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask));
 }
 
+static void pmu_domain_register(struct pmu_data *pmu,
+	const struct pmu_domain *pmu_dom)
+{
+	struct pmu_domain *domain;
+
+	domain = kmemdup(pmu_dom, sizeof(*domain), GFP_KERNEL);
+	if (!domain)
+		return;
+
+	domain->pmu = pmu;
+
+	__pmu_domain_register(domain);
+}
+
 static void pmu_add_genpd_of(struct device *dev)
 {
 	struct device_node *node;
@@ -241,6 +255,15 @@ static void pmu_add_genpd_of(struct device *dev)
 	}
 }
 
+static void pmu_add_genpd_name(const char *name, struct device *dev)
+{
+	while (1) {
+		if (pm_genpd_name_add_device(name, dev) != -EAGAIN)
+			break;
+		cond_resched();
+	}
+}
+
 static void pmu_remove_genpd(struct device *dev)
 {
 	struct generic_pm_domain *genpd = dev_to_genpd(dev);
@@ -256,11 +279,19 @@ static int pmu_platform_call(struct notifier_block *nb,
 	unsigned long event, void *data)
 {
 	struct device *dev = data;
+	const char *name = NULL;
+
+	if (strcmp(dev_name(dev), "ap510-vmeta.0") == 0)
+		name = "vpu";
+	else if (strcmp(dev_name(dev), "galcore.0") == 0)
+		name = "gpu";
 
 	switch (event) {
 	case BUS_NOTIFY_ADD_DEVICE:
 		if (dev->of_node)
 			pmu_add_genpd_of(dev);
+		else if (name)
+			pmu_add_genpd_name(name, dev);
 		break;
 
 	case BUS_NOTIFY_DEL_DEVICE:
@@ -363,6 +394,49 @@ static int __init dove_init_pmu_irq(struct pmu_data *pmu, int irq)
 	return 0;
 }
 
+static struct pmu_data pmu_data = {
+	.lock = __SPIN_LOCK_UNLOCKED(&pmu.lock),
+	.pmc_base = DOVE_PMU_VIRT_BASE,
+	.pmu_base = DOVE_PMU_VIRT_BASE + 0x8000,
+};
+
+static struct pmu_domain vpu_domain = {
+	.pwr_mask = PMU_PWR_DOWN_VPU,
+	.rst_mask = PMU_SW_RST_VIDEO_MASK,
+	.iso_mask = PMU_ISO_VPU,
+	.base = {
+		.name = "vpu",
+	},
+};
+
+static struct pmu_domain gpu_domain = {
+	.pwr_mask = PMU_PWR_DOWN_GPU,
+	.rst_mask = PMU_SW_RST_GPU_MASK,
+	.iso_mask = PMU_ISO_GPU,
+	.base = {
+		.name = "gpu",
+	},
+};
+
+static int __init dove_pmu_init_legacy(void)
+{
+	struct pmu_data *pmu = &pmu_data;
+	int ret, parent_irq;
+
+	pmu_reset_init(pmu);
+	pmu_domain_register(pmu, &vpu_domain);
+	pmu_domain_register(pmu, &gpu_domain);
+	pm_genpd_poweroff_unused();
+
+	ret = dove_init_pmu_irq(pmu, IRQ_DOVE_PMU);
+	if (ret)
+		pr_err("dove_init_pmu_irq() failed: %d\n", ret);
+
+	bus_register_notifier(&platform_bus_type, &platform_nb);
+
+	return 0;
+}
+
 /*
  * pmu {
  *	compatible = "marvell,pmu";
@@ -390,7 +464,7 @@ int __init dove_init_pmu(void)
 	/* Lookup the PMU node */
 	np_pmu = of_find_compatible_node(NULL, NULL, "marvell,pmu");
 	if (!np_pmu)
-		return 0;
+		return dove_pmu_init_legacy();
 
 	pmu = kzalloc(sizeof(*pmu), GFP_KERNEL);
 	if (!pmu)
-- 
1.8.3.1

Re: [PATCH RFC 0/5] Dove PMU support

[Sebastian Hesselbarth]

On 04/27/2014 03:23 PM, Russell King - ARM Linux wrote:
> The following series of patches add better PMU support for Dove.  This
> has been developed on the Cubox, and tested in non-DT and DT modes.
>
> This also improves the interrupt handling over the existing code: the
> existing code ends up calling the interrupt handlers twice for every
> interrupt raised, because the interrupt clear-down is done at the
> wrong point - we need to clear down the interrupt in the device first,
> then clear it down in the controller.
>
> The problem this gives is that it can be racy (see comments in the
> driver) so we're careful about how we do that to minimise the window.
>
> I've included all patches here - the initial set are targetted towards
> adding DT support, with the final adding the non-DT support.  There is
> a call to the initialisation function missing for DT mode - I'd like
> the mvebu people to comment on how that should be handled, as it needs
> to be done pretty early.
>
> Also included are two PM domain changes: the first I've discussed with
> Rafael who seems happy with it.  The second is necessary because we
> have no way to know if a generic PM domain is associated with a device
> or whether something else making use of the PM domain is installed in
> the dev->pm_domain pointer, so this allows that decision to be made by
> core PM code.
>
> This is more a "this is where I'm at" with this stuff than a real
> submission, nevertheless comments on how to get it ready for submission
> would be welcome.  I'd like to get this off my plate ASAP.

Russell,

thanks for dropping those patches. I know you are packed with a bunch
of other patch sets, so if you agree, I can pick up your Dove related
patches and finish them.

One thing that comes into my mind is, that we moved Dove DT to
mach-mvebu starting with v3.15-rc1 so we need to find a better place
for the driver than mach-dove.

Sebastian

>   arch/arm/Kconfig                     |   1 +
>   arch/arm/boot/dts/dove.dtsi          |   7 +
>   arch/arm/mach-dove/Makefile          |   1 +
>   arch/arm/mach-dove/common.c          |   2 +
>   arch/arm/mach-dove/common.h          |   1 +
>   arch/arm/mach-dove/include/mach/pm.h |  17 --
>   arch/arm/mach-dove/irq.c             |  87 ------
>   arch/arm/mach-dove/pmu.c             | 531 +++++++++++++++++++++++++++++++++++
>   drivers/base/power/domain.c          |   8 +-
>   9 files changed, 547 insertions(+), 108 deletions(-)
>

[PATCH RFC 0/5] Dove PMU support

[Sebastian Hesselbarth]

On 04/27/2014 03:23 PM, Russell King - ARM Linux wrote:
> The following series of patches add better PMU support for Dove.  This
> has been developed on the Cubox, and tested in non-DT and DT modes.
>
> This also improves the interrupt handling over the existing code: the
> existing code ends up calling the interrupt handlers twice for every
> interrupt raised, because the interrupt clear-down is done at the
> wrong point - we need to clear down the interrupt in the device first,
> then clear it down in the controller.
>
> The problem this gives is that it can be racy (see comments in the
> driver) so we're careful about how we do that to minimise the window.
>
> I've included all patches here - the initial set are targetted towards
> adding DT support, with the final adding the non-DT support.  There is
> a call to the initialisation function missing for DT mode - I'd like
> the mvebu people to comment on how that should be handled, as it needs
> to be done pretty early.
>
> Also included are two PM domain changes: the first I've discussed with
> Rafael who seems happy with it.  The second is necessary because we
> have no way to know if a generic PM domain is associated with a device
> or whether something else making use of the PM domain is installed in
> the dev->pm_domain pointer, so this allows that decision to be made by
> core PM code.
>
> This is more a "this is where I'm at" with this stuff than a real
> submission, nevertheless comments on how to get it ready for submission
> would be welcome.  I'd like to get this off my plate ASAP.

Russell,

thanks for dropping those patches. I know you are packed with a bunch
of other patch sets, so if you agree, I can pick up your Dove related
patches and finish them.

One thing that comes into my mind is, that we moved Dove DT to
mach-mvebu starting with v3.15-rc1 so we need to find a better place
for the driver than mach-dove.

Sebastian

>   arch/arm/Kconfig                     |   1 +
>   arch/arm/boot/dts/dove.dtsi          |   7 +
>   arch/arm/mach-dove/Makefile          |   1 +
>   arch/arm/mach-dove/common.c          |   2 +
>   arch/arm/mach-dove/common.h          |   1 +
>   arch/arm/mach-dove/include/mach/pm.h |  17 --
>   arch/arm/mach-dove/irq.c             |  87 ------
>   arch/arm/mach-dove/pmu.c             | 531 +++++++++++++++++++++++++++++++++++
>   drivers/base/power/domain.c          |   8 +-
>   9 files changed, 547 insertions(+), 108 deletions(-)
>

Re: [PATCH RFC 0/5] Dove PMU support

[Andrew Lunn]

> Russell,
> 
> thanks for dropping those patches. I know you are packed with a bunch
> of other patch sets, so if you agree, I can pick up your Dove related
> patches and finish them.
> 
> One thing that comes into my mind is, that we moved Dove DT to
> mach-mvebu starting with v3.15-rc1 so we need to find a better place
> for the driver than mach-dove.

Create drivers/pmu ?

The cpufreq driver also needs access to registers within the pmu
range. Should it be part of pmu.c, or should we export a regmap which
cpufreq can use?

	Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 0/5] Dove PMU support

[Andrew Lunn]

> Russell,
> 
> thanks for dropping those patches. I know you are packed with a bunch
> of other patch sets, so if you agree, I can pick up your Dove related
> patches and finish them.
> 
> One thing that comes into my mind is, that we moved Dove DT to
> mach-mvebu starting with v3.15-rc1 so we need to find a better place
> for the driver than mach-dove.

Create drivers/pmu ?

The cpufreq driver also needs access to registers within the pmu
range. Should it be part of pmu.c, or should we export a regmap which
cpufreq can use?

	Andrew

Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Ulf Hansson]

On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> The PMU device contains an interrupt controller, power control and
> resets.  The interrupt controller is a little sub-standard in that
> there is no race free way to clear down pending interrupts, so we try
> to avoid problems by reducing the window as much as possible, and
> clearing as infrequently as possible.
>
> The interrupt support is implemented using an IRQ domain, and the
> parent interrupt referenced in the standard DT way.
>
> The power domains and reset support is closely related - there is a
> defined sequence for powering down a domain which is tightly coupled
> with asserting the reset.  Hence, it makes sense to group these two
> together.
>
> This patch adds the core PMU driver: power domains must be defined in
> the DT file in order to make use of them.  The reset controller can
> be referenced in the standard way for reset controllers.

Hi Russell,

This patch would be simplified if this was based upon the not yet
merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree
based power domain look-up".

For example you would likely not need to add some of the marvel
specific DT bindings, and you wouldn’t need the bus_notifiers to add
devices to the power domain. I guess I just though it could be useful
input to consider while going forward, unless you already knew.

Kind regards
Ulf Hansson

[PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Ulf Hansson]

On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> The PMU device contains an interrupt controller, power control and
> resets.  The interrupt controller is a little sub-standard in that
> there is no race free way to clear down pending interrupts, so we try
> to avoid problems by reducing the window as much as possible, and
> clearing as infrequently as possible.
>
> The interrupt support is implemented using an IRQ domain, and the
> parent interrupt referenced in the standard DT way.
>
> The power domains and reset support is closely related - there is a
> defined sequence for powering down a domain which is tightly coupled
> with asserting the reset.  Hence, it makes sense to group these two
> together.
>
> This patch adds the core PMU driver: power domains must be defined in
> the DT file in order to make use of them.  The reset controller can
> be referenced in the standard way for reset controllers.

Hi Russell,

This patch would be simplified if this was based upon the not yet
merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree
based power domain look-up".

For example you would likely not need to add some of the marvel
specific DT bindings, and you wouldn?t need the bus_notifiers to add
devices to the power domain. I guess I just though it could be useful
input to consider while going forward, unless you already knew.

Kind regards
Ulf Hansson

Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King - ARM Linux]

On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote:
> On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> > The PMU device contains an interrupt controller, power control and
> > resets.  The interrupt controller is a little sub-standard in that
> > there is no race free way to clear down pending interrupts, so we try
> > to avoid problems by reducing the window as much as possible, and
> > clearing as infrequently as possible.
> >
> > The interrupt support is implemented using an IRQ domain, and the
> > parent interrupt referenced in the standard DT way.
> >
> > The power domains and reset support is closely related - there is a
> > defined sequence for powering down a domain which is tightly coupled
> > with asserting the reset.  Hence, it makes sense to group these two
> > together.
> >
> > This patch adds the core PMU driver: power domains must be defined in
> > the DT file in order to make use of them.  The reset controller can
> > be referenced in the standard way for reset controllers.
> 
> Hi Russell,
> 
> This patch would be simplified if this was based upon the not yet
> merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree
> based power domain look-up".
> 
> For example you would likely not need to add some of the marvel
> specific DT bindings, and you wouldn’t need the bus_notifiers to add
> devices to the power domain. I guess I just though it could be useful
> input to consider while going forward, unless you already knew.

Does that apply to 3.14?

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King - ARM Linux]

On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote:
> On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> > The PMU device contains an interrupt controller, power control and
> > resets.  The interrupt controller is a little sub-standard in that
> > there is no race free way to clear down pending interrupts, so we try
> > to avoid problems by reducing the window as much as possible, and
> > clearing as infrequently as possible.
> >
> > The interrupt support is implemented using an IRQ domain, and the
> > parent interrupt referenced in the standard DT way.
> >
> > The power domains and reset support is closely related - there is a
> > defined sequence for powering down a domain which is tightly coupled
> > with asserting the reset.  Hence, it makes sense to group these two
> > together.
> >
> > This patch adds the core PMU driver: power domains must be defined in
> > the DT file in order to make use of them.  The reset controller can
> > be referenced in the standard way for reset controllers.
> 
> Hi Russell,
> 
> This patch would be simplified if this was based upon the not yet
> merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree
> based power domain look-up".
> 
> For example you would likely not need to add some of the marvel
> specific DT bindings, and you wouldn?t need the bus_notifiers to add
> devices to the power domain. I guess I just though it could be useful
> input to consider while going forward, unless you already knew.

Does that apply to 3.14?

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH RFC 0/5] Dove PMU support

[Sebastian Hesselbarth]

On 04/28/2014 10:31 AM, Andrew Lunn wrote:
>> Russell,
>>
>> thanks for dropping those patches. I know you are packed with a bunch
>> of other patch sets, so if you agree, I can pick up your Dove related
>> patches and finish them.
>>
>> One thing that comes into my mind is, that we moved Dove DT to
>> mach-mvebu starting with v3.15-rc1 so we need to find a better place
>> for the driver than mach-dove.
>
> Create drivers/pmu ?

Hmm, I see no other folder it could sit in. Maybe, yes.

> The cpufreq driver also needs access to registers within the pmu
> range. Should it be part of pmu.c, or should we export a regmap which
> cpufreq can use?

Not only cpufreq but also pinctrl as the first 16 mpps can have PMU
related functions mapped to them. Syscon should do the trick.

Sebastian

[PATCH RFC 0/5] Dove PMU support

[Sebastian Hesselbarth]

On 04/28/2014 10:31 AM, Andrew Lunn wrote:
>> Russell,
>>
>> thanks for dropping those patches. I know you are packed with a bunch
>> of other patch sets, so if you agree, I can pick up your Dove related
>> patches and finish them.
>>
>> One thing that comes into my mind is, that we moved Dove DT to
>> mach-mvebu starting with v3.15-rc1 so we need to find a better place
>> for the driver than mach-dove.
>
> Create drivers/pmu ?

Hmm, I see no other folder it could sit in. Maybe, yes.

> The cpufreq driver also needs access to registers within the pmu
> range. Should it be part of pmu.c, or should we export a regmap which
> cpufreq can use?

Not only cpufreq but also pinctrl as the first 16 mpps can have PMU
related functions mapped to them. Syscon should do the trick.

Sebastian

Re: [PATCH 1/5] pm: domains: quieten down generic pm domains

[Rafael J. Wysocki]

On Sunday, April 27, 2014 02:28:50 PM Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net>

Since you need this one and [2/5] for the rest of the patchset, please feel
free to take them through your tree if that's convenient.

> ---
>  drivers/base/power/domain.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index bfb8955c406c..ea91ea0e137b 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -42,7 +42,7 @@
>  	struct gpd_timing_data *__td = &dev_gpd_data(dev)->td;			\
>  	if (!__retval && __elapsed > __td->field) {				\
>  		__td->field = __elapsed;					\
> -		dev_warn(dev, name " latency exceeded, new value %lld ns\n",	\
> +		dev_dbg(dev, name " latency exceeded, new value %lld ns\n",	\
>  			__elapsed);						\
>  		genpd->max_off_time_changed = true;				\
>  		__td->constraint_changed = true;				\
> @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
>  			genpd->max_off_time_changed = true;
>  			genpd_recalc_cpu_exit_latency(genpd);
>  			if (genpd->name)
> -				pr_warning("%s: Power-on latency exceeded, "
> +				pr_debug("%s: Power-on latency exceeded, "
>  					"new value %lld ns\n", genpd->name,
>  					elapsed_ns);
>  		}
> @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
>  			genpd->power_off_latency_ns = elapsed_ns;
>  			genpd->max_off_time_changed = true;
>  			if (genpd->name)
> -				pr_warning("%s: Power-off latency exceeded, "
> +				pr_debug("%s: Power-off latency exceeded, "
>  					"new value %lld ns\n", genpd->name,
>  					elapsed_ns);
>  		}
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 1/5] pm: domains: quieten down generic pm domains

[Rafael J. Wysocki]

On Sunday, April 27, 2014 02:28:50 PM Russell King wrote:
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net>

Since you need this one and [2/5] for the rest of the patchset, please feel
free to take them through your tree if that's convenient.

> ---
>  drivers/base/power/domain.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index bfb8955c406c..ea91ea0e137b 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -42,7 +42,7 @@
>  	struct gpd_timing_data *__td = &dev_gpd_data(dev)->td;			\
>  	if (!__retval && __elapsed > __td->field) {				\
>  		__td->field = __elapsed;					\
> -		dev_warn(dev, name " latency exceeded, new value %lld ns\n",	\
> +		dev_dbg(dev, name " latency exceeded, new value %lld ns\n",	\
>  			__elapsed);						\
>  		genpd->max_off_time_changed = true;				\
>  		__td->constraint_changed = true;				\
> @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
>  			genpd->max_off_time_changed = true;
>  			genpd_recalc_cpu_exit_latency(genpd);
>  			if (genpd->name)
> -				pr_warning("%s: Power-on latency exceeded, "
> +				pr_debug("%s: Power-on latency exceeded, "
>  					"new value %lld ns\n", genpd->name,
>  					elapsed_ns);
>  		}
> @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
>  			genpd->power_off_latency_ns = elapsed_ns;
>  			genpd->max_off_time_changed = true;
>  			if (genpd->name)
> -				pr_warning("%s: Power-off latency exceeded, "
> +				pr_debug("%s: Power-off latency exceeded, "
>  					"new value %lld ns\n", genpd->name,
>  					elapsed_ns);
>  		}
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 2/5] pm: domains: avoid potential oops in pm_genpd_remove_device()

[Rafael J. Wysocki]

On Sunday, April 27, 2014 02:28:55 PM Russell King wrote:
> pm_genpd_remove_device() should only be called with valid and present
> pm domain.  There are circumstances where we may end up with something
> that isn't a generic PM domain in dev->pm_domain (eg, vga_switcheroo
> stuff.)
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net>

> ---
>  drivers/base/power/domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index ea91ea0e137b..9d8faecc060c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1531,7 +1531,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
> -	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev)
> +	if (!pm_genpd_present(genpd) || IS_ERR_OR_NULL(dev)
>  	    ||  IS_ERR_OR_NULL(dev->pm_domain)
>  	    ||  pd_to_genpd(dev->pm_domain) != genpd)
>  		return -EINVAL;
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 2/5] pm: domains: avoid potential oops in pm_genpd_remove_device()

[Rafael J. Wysocki]

On Sunday, April 27, 2014 02:28:55 PM Russell King wrote:
> pm_genpd_remove_device() should only be called with valid and present
> pm domain.  There are circumstances where we may end up with something
> that isn't a generic PM domain in dev->pm_domain (eg, vga_switcheroo
> stuff.)
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net>

> ---
>  drivers/base/power/domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index ea91ea0e137b..9d8faecc060c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1531,7 +1531,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
>  
>  	dev_dbg(dev, "%s()\n", __func__);
>  
> -	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev)
> +	if (!pm_genpd_present(genpd) || IS_ERR_OR_NULL(dev)
>  	    ||  IS_ERR_OR_NULL(dev->pm_domain)
>  	    ||  pd_to_genpd(dev->pm_domain) != genpd)
>  		return -EINVAL;
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 1/5] pm: domains: quieten down generic pm domains

[Ulf Hansson]

On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Sunday, April 27, 2014 02:28:50 PM Russell King wrote:
>> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
>
> Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net>
>
> Since you need this one and [2/5] for the rest of the patchset, please feel
> free to take them through your tree if that's convenient.

Unless it complicates things for Russell, I would prefer if this could
go via Rafael's tree.

I have a quite big patchset for the PM domain, it would help me if I
have a single point to base my work upon.

Additionally, you have Tomasz Figa patchset below to consider for the
PM domain code.

[PATCH v3 0/3] Generic Device Tree based power domain look-up

Kind regards
Ulf Hansson

>
>> ---
>>  drivers/base/power/domain.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index bfb8955c406c..ea91ea0e137b 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -42,7 +42,7 @@
>>       struct gpd_timing_data *__td = &dev_gpd_data(dev)->td;                  \
>>       if (!__retval && __elapsed > __td->field) {                             \
>>               __td->field = __elapsed;                                        \
>> -             dev_warn(dev, name " latency exceeded, new value %lld ns\n",    \
>> +             dev_dbg(dev, name " latency exceeded, new value %lld ns\n",     \
>>                       __elapsed);                                             \
>>               genpd->max_off_time_changed = true;                             \
>>               __td->constraint_changed = true;                                \
>> @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
>>                       genpd->max_off_time_changed = true;
>>                       genpd_recalc_cpu_exit_latency(genpd);
>>                       if (genpd->name)
>> -                             pr_warning("%s: Power-on latency exceeded, "
>> +                             pr_debug("%s: Power-on latency exceeded, "
>>                                       "new value %lld ns\n", genpd->name,
>>                                       elapsed_ns);
>>               }
>> @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
>>                       genpd->power_off_latency_ns = elapsed_ns;
>>                       genpd->max_off_time_changed = true;
>>                       if (genpd->name)
>> -                             pr_warning("%s: Power-off latency exceeded, "
>> +                             pr_debug("%s: Power-off latency exceeded, "
>>                                       "new value %lld ns\n", genpd->name,
>>                                       elapsed_ns);
>>               }
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] pm: domains: quieten down generic pm domains

[Ulf Hansson]

On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Sunday, April 27, 2014 02:28:50 PM Russell King wrote:
>> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
>
> Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net>
>
> Since you need this one and [2/5] for the rest of the patchset, please feel
> free to take them through your tree if that's convenient.

Unless it complicates things for Russell, I would prefer if this could
go via Rafael's tree.

I have a quite big patchset for the PM domain, it would help me if I
have a single point to base my work upon.

Additionally, you have Tomasz Figa patchset below to consider for the
PM domain code.

[PATCH v3 0/3] Generic Device Tree based power domain look-up

Kind regards
Ulf Hansson

>
>> ---
>>  drivers/base/power/domain.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index bfb8955c406c..ea91ea0e137b 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -42,7 +42,7 @@
>>       struct gpd_timing_data *__td = &dev_gpd_data(dev)->td;                  \
>>       if (!__retval && __elapsed > __td->field) {                             \
>>               __td->field = __elapsed;                                        \
>> -             dev_warn(dev, name " latency exceeded, new value %lld ns\n",    \
>> +             dev_dbg(dev, name " latency exceeded, new value %lld ns\n",     \
>>                       __elapsed);                                             \
>>               genpd->max_off_time_changed = true;                             \
>>               __td->constraint_changed = true;                                \
>> @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
>>                       genpd->max_off_time_changed = true;
>>                       genpd_recalc_cpu_exit_latency(genpd);
>>                       if (genpd->name)
>> -                             pr_warning("%s: Power-on latency exceeded, "
>> +                             pr_debug("%s: Power-on latency exceeded, "
>>                                       "new value %lld ns\n", genpd->name,
>>                                       elapsed_ns);
>>               }
>> @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
>>                       genpd->power_off_latency_ns = elapsed_ns;
>>                       genpd->max_off_time_changed = true;
>>                       if (genpd->name)
>> -                             pr_warning("%s: Power-off latency exceeded, "
>> +                             pr_debug("%s: Power-off latency exceeded, "
>>                                       "new value %lld ns\n", genpd->name,
>>                                       elapsed_ns);
>>               }
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] pm: domains: quieten down generic pm domains

[Rafael J. Wysocki]

On Friday, May 02, 2014 11:24:45 AM Ulf Hansson wrote:
> On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote:
> >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> >
> > Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net>
> >
> > Since you need this one and [2/5] for the rest of the patchset, please feel
> > free to take them through your tree if that's convenient.
> 
> Unless it complicates things for Russell, I would prefer if this could
> go via Rafael's tree.
> 
> I have a quite big patchset for the PM domain, it would help me if I
> have a single point to base my work upon.

No problem for me, but I'm not sure about what Russell things.

> Additionally, you have Tomasz Figa patchset below to consider for the
> PM domain code.
> 
> [PATCH v3 0/3] Generic Device Tree based power domain look-up

This one seems to have been under discussion recently, however.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 1/5] pm: domains: quieten down generic pm domains

[Rafael J. Wysocki]

On Friday, May 02, 2014 11:24:45 AM Ulf Hansson wrote:
> On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote:
> >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> >
> > Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net>
> >
> > Since you need this one and [2/5] for the rest of the patchset, please feel
> > free to take them through your tree if that's convenient.
> 
> Unless it complicates things for Russell, I would prefer if this could
> go via Rafael's tree.
> 
> I have a quite big patchset for the PM domain, it would help me if I
> have a single point to base my work upon.

No problem for me, but I'm not sure about what Russell things.

> Additionally, you have Tomasz Figa patchset below to consider for the
> PM domain code.
> 
> [PATCH v3 0/3] Generic Device Tree based power domain look-up

This one seems to have been under discussion recently, however.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH RFC 0/5] Dove PMU support

[Russell King - ARM Linux]

On Tue, Apr 29, 2014 at 11:15:27AM +0200, Sebastian Hesselbarth wrote:
> On 04/28/2014 10:31 AM, Andrew Lunn wrote:
>>> Russell,
>>>
>>> thanks for dropping those patches. I know you are packed with a bunch
>>> of other patch sets, so if you agree, I can pick up your Dove related
>>> patches and finish them.
>>>
>>> One thing that comes into my mind is, that we moved Dove DT to
>>> mach-mvebu starting with v3.15-rc1 so we need to find a better place
>>> for the driver than mach-dove.
>>
>> Create drivers/pmu ?
>
> Hmm, I see no other folder it could sit in. Maybe, yes.
>
>> The cpufreq driver also needs access to registers within the pmu
>> range. Should it be part of pmu.c, or should we export a regmap which
>> cpufreq can use?
>
> Not only cpufreq but also pinctrl as the first 16 mpps can have PMU
> related functions mapped to them. Syscon should do the trick.

The thing I don't like about regmap/syscon for this stuff is that it
seems pretty easy to violate the statement in 8.6.2.4 when you lock
individual register accesses.

This statement states that the power-up/power-down sequence for VMeta
and the GPU must be completed one at a time.  In other words, you can't
interleave the power control of these units (which is what could happen
if you rely on the locking provided by regmap/syscon.)

I'd prefer to take that a little further to avoid any unintended
consequences (as indeed I have done in this patch) and ensure that
these sequences are complete before any other system is controlled.

As for DT, I should probably state that I'm far from happy with the DT
situation on Dove.  When I've tried it, I've encountered problems with
the HDMI output - the picture spends more time blanked than displaying.
I've no idea what is causing that, I've been through the SI5351
registers, the TDA998x registers, the LCD controller registers, and I
can't find any reason for it.  Yet, boot the same kernel without DT
and it works fine.  Boot back using DT, and it's unstable again.

I can't detect any difference on the actual HDMI signals themselves
either.  The HDMI clock seems stable and of the correct frequency.

There is definitely some difference between booting with DT and booting
with legacy stuff that seems to upset HDMI.

For me, the /only/ reliably working system is one where DT is not
involved.  This means I remain opposed to any solutions which can't
be used in a non-DT environment - at least until the HDMI issue can
be resolved.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH RFC 0/5] Dove PMU support

[Russell King - ARM Linux]

On Tue, Apr 29, 2014 at 11:15:27AM +0200, Sebastian Hesselbarth wrote:
> On 04/28/2014 10:31 AM, Andrew Lunn wrote:
>>> Russell,
>>>
>>> thanks for dropping those patches. I know you are packed with a bunch
>>> of other patch sets, so if you agree, I can pick up your Dove related
>>> patches and finish them.
>>>
>>> One thing that comes into my mind is, that we moved Dove DT to
>>> mach-mvebu starting with v3.15-rc1 so we need to find a better place
>>> for the driver than mach-dove.
>>
>> Create drivers/pmu ?
>
> Hmm, I see no other folder it could sit in. Maybe, yes.
>
>> The cpufreq driver also needs access to registers within the pmu
>> range. Should it be part of pmu.c, or should we export a regmap which
>> cpufreq can use?
>
> Not only cpufreq but also pinctrl as the first 16 mpps can have PMU
> related functions mapped to them. Syscon should do the trick.

The thing I don't like about regmap/syscon for this stuff is that it
seems pretty easy to violate the statement in 8.6.2.4 when you lock
individual register accesses.

This statement states that the power-up/power-down sequence for VMeta
and the GPU must be completed one at a time.  In other words, you can't
interleave the power control of these units (which is what could happen
if you rely on the locking provided by regmap/syscon.)

I'd prefer to take that a little further to avoid any unintended
consequences (as indeed I have done in this patch) and ensure that
these sequences are complete before any other system is controlled.

As for DT, I should probably state that I'm far from happy with the DT
situation on Dove.  When I've tried it, I've encountered problems with
the HDMI output - the picture spends more time blanked than displaying.
I've no idea what is causing that, I've been through the SI5351
registers, the TDA998x registers, the LCD controller registers, and I
can't find any reason for it.  Yet, boot the same kernel without DT
and it works fine.  Boot back using DT, and it's unstable again.

I can't detect any difference on the actual HDMI signals themselves
either.  The HDMI clock seems stable and of the correct frequency.

There is definitely some difference between booting with DT and booting
with legacy stuff that seems to upset HDMI.

For me, the /only/ reliably working system is one where DT is not
involved.  This means I remain opposed to any solutions which can't
be used in a non-DT environment - at least until the HDMI issue can
be resolved.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support)

[Sebastian Hesselbarth]

On 06/15/2014 05:25 PM, Russell King - ARM Linux wrote:
> As for DT, I should probably state that I'm far from happy with the DT
> situation on Dove.  When I've tried it, I've encountered problems with
> the HDMI output - the picture spends more time blanked than displaying.
> I've no idea what is causing that, I've been through the SI5351
> registers, the TDA998x registers, the LCD controller registers, and I
> can't find any reason for it.  Yet, boot the same kernel without DT
> and it works fine.  Boot back using DT, and it's unstable again.
>
> I can't detect any difference on the actual HDMI signals themselves
> either.  The HDMI clock seems stable and of the correct frequency.
>
> There is definitely some difference between booting with DT and booting
> with legacy stuff that seems to upset HDMI.
>
> For me, the /only/ reliably working system is one where DT is not
> involved.  This means I remain opposed to any solutions which can't
> be used in a non-DT environment - at least until the HDMI issue can
> be resolved.

Russell,

as said on IRC, I really want this to be sorted out. I prepared a Dove
DT HDMI quick-hack and pushed it to

https://github.com/shesselba/linux-dove.git dove-drm-v3.16-rc1

Using your libdrm-armada and xf86-video-armada on Ubuntu raring armhf,
I can run Xfce4 on 1920x1080p60 on Dove Cubox without any blanking
issues. Using xrandr -s <mode> will fail after 2-3 times but that
shouldn't be related to the issues you see.

Please test above branch with kernel config at
https://dl.dropboxusercontent.com/u/59928252/config-dove-v3.16-rc1-hdmi
and report back if issues are still there.

Anyone having a Dove CuBox ready, please also test. Russell has a git
branch for the required video lib and drivers.

Sebastian

Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support)

[Ezequiel Garcia]

On 18 Jun 04:11 PM, Sebastian Hesselbarth wrote:
> On 06/15/2014 05:25 PM, Russell King - ARM Linux wrote:
> >As for DT, I should probably state that I'm far from happy with the DT
> >situation on Dove.  When I've tried it, I've encountered problems with
> >the HDMI output - the picture spends more time blanked than displaying.
> >I've no idea what is causing that, I've been through the SI5351
> >registers, the TDA998x registers, the LCD controller registers, and I
> >can't find any reason for it.  Yet, boot the same kernel without DT
> >and it works fine.  Boot back using DT, and it's unstable again.
> >
> >I can't detect any difference on the actual HDMI signals themselves
> >either.  The HDMI clock seems stable and of the correct frequency.
> >
> >There is definitely some difference between booting with DT and booting
> >with legacy stuff that seems to upset HDMI.
> >
> >For me, the /only/ reliably working system is one where DT is not
> >involved.  This means I remain opposed to any solutions which can't
> >be used in a non-DT environment - at least until the HDMI issue can
> >be resolved.
> 
> Russell,
> 
> as said on IRC, I really want this to be sorted out. I prepared a Dove
> DT HDMI quick-hack and pushed it to
> 
> https://github.com/shesselba/linux-dove.git dove-drm-v3.16-rc1
> 

I can probably give a quick test to this using Qt over FB, or just
plain fb-test / libdrm tests. But you are already doing a more involved
test, so maybe my tests are too simple.

> Using your libdrm-armada and xf86-video-armada on Ubuntu raring armhf,
> I can run Xfce4 on 1920x1080p60 on Dove Cubox without any blanking
> issues. Using xrandr -s <mode> will fail after 2-3 times but that
> shouldn't be related to the issues you see.
> 
> Please test above branch with kernel config at
> https://dl.dropboxusercontent.com/u/59928252/config-dove-v3.16-rc1-hdmi
> and report back if issues are still there.
> 
> Anyone having a Dove CuBox ready, please also test. Russell has a git
> branch for the required video lib and drivers.
> 

Can you point me at those?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support)

[Russell King - ARM Linux]

On Wed, Jun 18, 2014 at 11:34:48AM -0300, Ezequiel Garcia wrote:
> On 18 Jun 04:11 PM, Sebastian Hesselbarth wrote:
> > Using your libdrm-armada and xf86-video-armada on Ubuntu raring armhf,
> > I can run Xfce4 on 1920x1080p60 on Dove Cubox without any blanking
> > issues. Using xrandr -s <mode> will fail after 2-3 times but that
> > shouldn't be related to the issues you see.
> > 
> > Please test above branch with kernel config at
> > https://dl.dropboxusercontent.com/u/59928252/config-dove-v3.16-rc1-hdmi
> > and report back if issues are still there.
> > 
> > Anyone having a Dove CuBox ready, please also test. Russell has a git
> > branch for the required video lib and drivers.
> > 
> 
> Can you point me at those?

Beware - the machine is rather aged and slow by todays standards ((c)git
is rather heavy weight):

 http://ftp.arm.linux.org.uk/cgit/

The xf86-video-armada.git tree is not trivial to build as there's
dependencies on the Vivante galcore libraries - and even if you have
them, there's then there's non-trivial changes to the kernel-side driver
to support dmabuf imports.  Welcome to the world of closed source graphics
libraries making open source hard... I've been wishing for etnaviv for
a while now...

Even though the kernel side is supposed to be GPL, I'm really not happy
distributing it or publishing changes as there are a number of files with
headers which seem to be GPL-incompatible (which I've eliminated from my
tree through updates) but the problem is wonderful git keeps them as
history... and in my tree it's a massive 93 patches, against an old
version of the code (0.8.0.1998) which I then sort-of updated to the
version OLPC were carrying towards the start of 2013.

It's probably best if Sebastian walks you through getting it up and
running as my experience is with my kernels and userspace, which has all
the necessary hacks in.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Dove DT and HDMI on v3.16-rc1

[Sebastian Hesselbarth]

On 06/18/2014 05:01 PM, Russell King - ARM Linux wrote:
> On Wed, Jun 18, 2014 at 11:34:48AM -0300, Ezequiel Garcia wrote:
>> On 18 Jun 04:11 PM, Sebastian Hesselbarth wrote:
>>> Using your libdrm-armada and xf86-video-armada on Ubuntu raring armhf,
>>> I can run Xfce4 on 1920x1080p60 on Dove Cubox without any blanking
>>> issues. Using xrandr -s <mode> will fail after 2-3 times but that
>>> shouldn't be related to the issues you see.
>>>
>>> Please test above branch with kernel config at
>>> https://dl.dropboxusercontent.com/u/59928252/config-dove-v3.16-rc1-hdmi
>>> and report back if issues are still there.
>>>
>>> Anyone having a Dove CuBox ready, please also test. Russell has a git
>>> branch for the required video lib and drivers.
>>>
>>
>> Can you point me at those?
>
> Beware - the machine is rather aged and slow by todays standards ((c)git
> is rather heavy weight):
>
>   http://ftp.arm.linux.org.uk/cgit/
>
> The xf86-video-armada.git tree is not trivial to build as there's
> dependencies on the Vivante galcore libraries - and even if you have
> them, there's then there's non-trivial changes to the kernel-side driver
> to support dmabuf imports.  Welcome to the world of closed source graphics
> libraries making open source hard... I've been wishing for etnaviv for
> a while now...
>
> Even though the kernel side is supposed to be GPL, I'm really not happy
> distributing it or publishing changes as there are a number of files with
> headers which seem to be GPL-incompatible (which I've eliminated from my
> tree through updates) but the problem is wonderful git keeps them as
> history... and in my tree it's a massive 93 patches, against an old
> version of the code (0.8.0.1998) which I then sort-of updated to the
> version OLPC were carrying towards the start of 2013.
>
> It's probably best if Sebastian walks you through getting it up and
> running as my experience is with my kernels and userspace, which has all
> the necessary hacks in.

I basically followed Russell's guide from
http://www.home.arm.linux.org.uk/~rmk/cubox/carrier-1-readme.txt

with this binary/include package from SolidRun
http://download.solid-run.com/pub/solidrun/cubox/packages/marvell-opengl/marvell-opengl-hardfp-new-headers.zip

xf86-video-armada needs some missing
#include "picturestr.h"
for raring, but whole process has been very straight forward.

Feel free to bug me on IRC anytime you hit some build errors or
anything else.

Sebastian

Dove DT and HDMI on v3.16-rc1

[Russell King - ARM Linux]

On Wed, Jun 18, 2014 at 05:10:32PM +0200, Sebastian Hesselbarth wrote:
> xf86-video-armada needs some missing
> #include "picturestr.h"
> for raring, but whole process has been very straight forward.

I'm only aware of this (sorry, copy'n'pasted) which I found when trying
to build against ubuntu 14.04.  If further patches are needed to make it
compatible with more Xorg versions, please ensure that they find their
way into my inbox...

diff --git a/src/common_drm.h b/src/common_drm.h
index b0c3cd4..3bc5dc6 100644
--- a/src/common_drm.h
+++ b/src/common_drm.h
@@ -1,8 +1,8 @@
 #ifndef COMMON_DRM_H
 #define COMMON_DRM_H
 
-#include "compat-api.h"
 #include "xf86Crtc.h"
+#include "compat-api.h"
 
 struct common_crtc_info {
 	int drm_fd;
diff --git a/src/vivante_unaccel_render.c b/src/vivante_unaccel_render.c
index ad4d5cf..0496d7a 100644
--- a/src/vivante_unaccel_render.c
+++ b/src/vivante_unaccel_render.c
@@ -11,11 +11,11 @@
 #include "config.h"
 #endif
 
-#include "compat-api.h"
 #include "fb.h"
 #include "fbpict.h"
 #include "mipict.h"
 
+#include "compat-api.h"
 #include "vivante_unaccel.h"
 #include "vivante_utils.h"


-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Dove DT and HDMI on v3.16-rc1

[Sebastian Hesselbarth]

On 06/18/2014 05:30 PM, Russell King - ARM Linux wrote:
> On Wed, Jun 18, 2014 at 05:10:32PM +0200, Sebastian Hesselbarth wrote:
>> xf86-video-armada needs some missing
>> #include "picturestr.h"
>> for raring, but whole process has been very straight forward.
>
> I'm only aware of this (sorry, copy'n'pasted) which I found when trying
> to build against ubuntu 14.04.  If further patches are needed to make it
> compatible with more Xorg versions, please ensure that they find their
> way into my inbox...

Your patch also makes the xorg driver build fine on raring.
When I encountered the build error, I just tried the first hit on Google
- I didn't investigate it at all.

Sebastian

Dove DT and HDMI on v3.16-rc1

[Russell King - ARM Linux]

On Wed, Jun 18, 2014 at 05:39:08PM +0200, Sebastian Hesselbarth wrote:
> On 06/18/2014 05:30 PM, Russell King - ARM Linux wrote:
>> On Wed, Jun 18, 2014 at 05:10:32PM +0200, Sebastian Hesselbarth wrote:
>>> xf86-video-armada needs some missing
>>> #include "picturestr.h"
>>> for raring, but whole process has been very straight forward.
>>
>> I'm only aware of this (sorry, copy'n'pasted) which I found when trying
>> to build against ubuntu 14.04.  If further patches are needed to make it
>> compatible with more Xorg versions, please ensure that they find their
>> way into my inbox...
>
> Your patch also makes the xorg driver build fine on raring.
> When I encountered the build error, I just tried the first hit on Google
> - I didn't investigate it at all.

Thanks, just pushed that change out.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support)

[Nicolas Pitre]

On Wed, 18 Jun 2014, Russell King - ARM Linux wrote:

> Even though the kernel side is supposed to be GPL, I'm really not happy
> distributing it or publishing changes as there are a number of files with
> headers which seem to be GPL-incompatible (which I've eliminated from my
> tree through updates) but the problem is wonderful git keeps them as
> history... and in my tree it's a massive 93 patches, against an old
> version of the code (0.8.0.1998) which I then sort-of updated to the
> version OLPC were carrying towards the start of 2013.

If you want to remove some files from git history, you may look at the 
git-filter-branch man page.  The first provided examples are most likely 
what you'd need.


Nicolas

Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support)

[Ezequiel Garcia]

On 18 Jun 04:11 PM, Sebastian Hesselbarth wrote:
[..]
> 
> as said on IRC, I really want this to be sorted out. I prepared a Dove
> DT HDMI quick-hack and pushed it to
> 
> https://github.com/shesselba/linux-dove.git dove-drm-v3.16-rc1
> 

Tested the above branch using mvebu_v7_defconfig + Cubox-specific options
and ran a few basic tests. I know it's not much, but here they are:

* The bootlogo shows up

* Legacy fbdev tests (fb-test, fb-test-rect, mplayer) worked fine in the 
  default mode (1920x1080 in my case).

* Set a few different modes with LibDRM modetest tool:
  modetest -M armada-drm -s 18:640x480
  modetest -M armada-drm -s 18:800x600
  ...
  modetest -M armada-drm -s 18:1920x1080

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 1/5] pm: domains: quieten down generic pm domains

[Russell King - ARM Linux]

On Mon, May 05, 2014 at 12:36:25AM +0200, Rafael J. Wysocki wrote:
> On Friday, May 02, 2014 11:24:45 AM Ulf Hansson wrote:
> > On 1 May 2014 01:46, Rafael J. Wysocki <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org> wrote:
> > > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote:
> > >> Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> > >
> > > Acked-by: Rafael J. Wysocki <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>
> > >
> > > Since you need this one and [2/5] for the rest of the patchset, please feel
> > > free to take them through your tree if that's convenient.
> > 
> > Unless it complicates things for Russell, I would prefer if this could
> > go via Rafael's tree.
> > 
> > I have a quite big patchset for the PM domain, it would help me if I
> > have a single point to base my work upon.
> 
> No problem for me, but I'm not sure about what Russell things.

Well, it seems nothing happened with these patches, and they're still
something I have (and I've just updated them to changes in 3.19.)

What are we going to do about them?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] pm: domains: quieten down generic pm domains

[Russell King - ARM Linux]

On Mon, May 05, 2014 at 12:36:25AM +0200, Rafael J. Wysocki wrote:
> On Friday, May 02, 2014 11:24:45 AM Ulf Hansson wrote:
> > On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote:
> > >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > >
> > > Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net>
> > >
> > > Since you need this one and [2/5] for the rest of the patchset, please feel
> > > free to take them through your tree if that's convenient.
> > 
> > Unless it complicates things for Russell, I would prefer if this could
> > go via Rafael's tree.
> > 
> > I have a quite big patchset for the PM domain, it would help me if I
> > have a single point to base my work upon.
> 
> No problem for me, but I'm not sure about what Russell things.

Well, it seems nothing happened with these patches, and they're still
something I have (and I've just updated them to changes in 3.19.)

What are we going to do about them?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King - ARM Linux]

On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote:
> On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> > The PMU device contains an interrupt controller, power control and
> > resets.  The interrupt controller is a little sub-standard in that
> > there is no race free way to clear down pending interrupts, so we try
> > to avoid problems by reducing the window as much as possible, and
> > clearing as infrequently as possible.
> >
> > The interrupt support is implemented using an IRQ domain, and the
> > parent interrupt referenced in the standard DT way.
> >
> > The power domains and reset support is closely related - there is a
> > defined sequence for powering down a domain which is tightly coupled
> > with asserting the reset.  Hence, it makes sense to group these two
> > together.
> >
> > This patch adds the core PMU driver: power domains must be defined in
> > the DT file in order to make use of them.  The reset controller can
> > be referenced in the standard way for reset controllers.
> 
> Hi Russell,
> 
> This patch would be simplified if this was based upon the not yet
> merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree
> based power domain look-up".
> 
> For example you would likely not need to add some of the marvel
> specific DT bindings, and you wouldn’t need the bus_notifiers to add
> devices to the power domain. I guess I just though it could be useful
> input to consider while going forward, unless you already knew.

In 3.19, I notice something of an odd behaviour.

My vMeta driver has runtime PM support enabled.  When I explicitly register
the PM domain in the pmu driver via a bus notifier, I see:

root@cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
    domain                      status         slaves
           /device                                      runtime status
----------------------------------------------------------------------
gpu-domain                      on
    /devices/platform/vivante/etnaviv-gpu,2d            active
vpu-domain                      off
    /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder  suspended

But when I disable that, and let the generic code do the registration,
I instead get:

root@cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
    domain                      status         slaves
           /device                                      runtime status
----------------------------------------------------------------------
gpu-domain                      on
    /devices/platform/vivante/etnaviv-gpu,2d            active
vpu-domain                      on
    /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder  suspended

The difference being that the vpu domain remains powered.

The only difference code-wise seems to be when genpd_dev_pm_attach() is
called.  In the working case, it's before the device is considered for
probing.  In the non-working case, it's just before the device is probed.

With debugging enabled in the PM domain code, with the former case I get:

Added domain provider from /mbus/internal-regs/power-management@d0000/vpu-domain
platform f1c00000.video-decoder: adding to PM domain vpu-domain
platform f1c00000.video-decoder: __pm_genpd_add_device()

With the latter non-working case:

Added domain provider from /mbus/internal-regs/power-management@d0000/vpu-domain
...
ap510-vmeta f1c00000.video-decoder: adding to PM domain vpu-domain
ap510-vmeta f1c00000.video-decoder: __pm_genpd_add_device()
vpu-domain: Power-on latency exceeded, new value 1578 ns

Neither of these debug messages provide much hint as to what the
difference is, or the cause of the PM domain code being de-sync'd
with its devices.

Maybe the PM code needs more debugging in it, and maybe the debugfs
file should always be present if debugfs support is enabled?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King - ARM Linux]

On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote:
> On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> > The PMU device contains an interrupt controller, power control and
> > resets.  The interrupt controller is a little sub-standard in that
> > there is no race free way to clear down pending interrupts, so we try
> > to avoid problems by reducing the window as much as possible, and
> > clearing as infrequently as possible.
> >
> > The interrupt support is implemented using an IRQ domain, and the
> > parent interrupt referenced in the standard DT way.
> >
> > The power domains and reset support is closely related - there is a
> > defined sequence for powering down a domain which is tightly coupled
> > with asserting the reset.  Hence, it makes sense to group these two
> > together.
> >
> > This patch adds the core PMU driver: power domains must be defined in
> > the DT file in order to make use of them.  The reset controller can
> > be referenced in the standard way for reset controllers.
> 
> Hi Russell,
> 
> This patch would be simplified if this was based upon the not yet
> merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree
> based power domain look-up".
> 
> For example you would likely not need to add some of the marvel
> specific DT bindings, and you wouldn?t need the bus_notifiers to add
> devices to the power domain. I guess I just though it could be useful
> input to consider while going forward, unless you already knew.

In 3.19, I notice something of an odd behaviour.

My vMeta driver has runtime PM support enabled.  When I explicitly register
the PM domain in the pmu driver via a bus notifier, I see:

root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
    domain                      status         slaves
           /device                                      runtime status
----------------------------------------------------------------------
gpu-domain                      on
    /devices/platform/vivante/etnaviv-gpu,2d            active
vpu-domain                      off
    /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder  suspended

But when I disable that, and let the generic code do the registration,
I instead get:

root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
    domain                      status         slaves
           /device                                      runtime status
----------------------------------------------------------------------
gpu-domain                      on
    /devices/platform/vivante/etnaviv-gpu,2d            active
vpu-domain                      on
    /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder  suspended

The difference being that the vpu domain remains powered.

The only difference code-wise seems to be when genpd_dev_pm_attach() is
called.  In the working case, it's before the device is considered for
probing.  In the non-working case, it's just before the device is probed.

With debugging enabled in the PM domain code, with the former case I get:

Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain
platform f1c00000.video-decoder: adding to PM domain vpu-domain
platform f1c00000.video-decoder: __pm_genpd_add_device()

With the latter non-working case:

Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain
...
ap510-vmeta f1c00000.video-decoder: adding to PM domain vpu-domain
ap510-vmeta f1c00000.video-decoder: __pm_genpd_add_device()
vpu-domain: Power-on latency exceeded, new value 1578 ns

Neither of these debug messages provide much hint as to what the
difference is, or the cause of the PM domain code being de-sync'd
with its devices.

Maybe the PM code needs more debugging in it, and maybe the debugfs
file should always be present if debugfs support is enabled?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King - ARM Linux]

On Fri, Feb 13, 2015 at 01:29:25PM +0000, Russell King - ARM Linux wrote:
> On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote:
> > On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> > > The PMU device contains an interrupt controller, power control and
> > > resets.  The interrupt controller is a little sub-standard in that
> > > there is no race free way to clear down pending interrupts, so we try
> > > to avoid problems by reducing the window as much as possible, and
> > > clearing as infrequently as possible.
> > >
> > > The interrupt support is implemented using an IRQ domain, and the
> > > parent interrupt referenced in the standard DT way.
> > >
> > > The power domains and reset support is closely related - there is a
> > > defined sequence for powering down a domain which is tightly coupled
> > > with asserting the reset.  Hence, it makes sense to group these two
> > > together.
> > >
> > > This patch adds the core PMU driver: power domains must be defined in
> > > the DT file in order to make use of them.  The reset controller can
> > > be referenced in the standard way for reset controllers.
> > 
> > Hi Russell,
> > 
> > This patch would be simplified if this was based upon the not yet
> > merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree
> > based power domain look-up".
> > 
> > For example you would likely not need to add some of the marvel
> > specific DT bindings, and you wouldn’t need the bus_notifiers to add
> > devices to the power domain. I guess I just though it could be useful
> > input to consider while going forward, unless you already knew.
> 
> In 3.19, I notice something of an odd behaviour.
> 
> My vMeta driver has runtime PM support enabled.  When I explicitly register
> the PM domain in the pmu driver via a bus notifier, I see:
> 
> root@cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>     domain                      status         slaves
>            /device                                      runtime status
> ----------------------------------------------------------------------
> gpu-domain                      on
>     /devices/platform/vivante/etnaviv-gpu,2d            active
> vpu-domain                      off
>     /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder  suspended
> 
> But when I disable that, and let the generic code do the registration,
> I instead get:
> 
> root@cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>     domain                      status         slaves
>            /device                                      runtime status
> ----------------------------------------------------------------------
> gpu-domain                      on
>     /devices/platform/vivante/etnaviv-gpu,2d            active
> vpu-domain                      on
>     /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder  suspended
> 
> The difference being that the vpu domain remains powered.
> 
> The only difference code-wise seems to be when genpd_dev_pm_attach() is
> called.  In the working case, it's before the device is considered for
> probing.  In the non-working case, it's just before the device is probed.
> 
> With debugging enabled in the PM domain code, with the former case I get:
> 
> Added domain provider from /mbus/internal-regs/power-management@d0000/vpu-domain
> platform f1c00000.video-decoder: adding to PM domain vpu-domain
> platform f1c00000.video-decoder: __pm_genpd_add_device()
> 
> With the latter non-working case:
> 
> Added domain provider from /mbus/internal-regs/power-management@d0000/vpu-domain
> ...
> ap510-vmeta f1c00000.video-decoder: adding to PM domain vpu-domain
> ap510-vmeta f1c00000.video-decoder: __pm_genpd_add_device()
> vpu-domain: Power-on latency exceeded, new value 1578 ns
> 
> Neither of these debug messages provide much hint as to what the
> difference is, or the cause of the PM domain code being de-sync'd
> with its devices.
> 
> Maybe the PM code needs more debugging in it, and maybe the debugfs
> file should always be present if debugfs support is enabled?

The vmeta driver does this in its probe function:

        pm_runtime_use_autosuspend(vi->dev);
        pm_runtime_set_autosuspend_delay(vi->dev, 100);
        pm_runtime_enable(vi->dev);

since it doesn't touch the hardware, and the hardware starts off at
boot time in "suspended" mode.

I think what's going on is that there's a difference in the expectations
from the PM domain code vs the runtime PM code.  I refer to section 5
of the runtime PM documentation:

| 5. Runtime PM Initialization, Device Probing and Removal
| 
| Initially, the runtime PM is disabled for all devices, which means that the
| majority of the runtime PM helper functions described in Section 4 will return
| -EAGAIN until pm_runtime_enable() is called for the device.
| 
| In addition to that, the initial runtime PM status of all devices is
| 'suspended', but it need not reflect the actual physical state of the device.
| Thus, if the device is initially active (i.e. it is able to process I/O), its
| runtime PM status must be changed to 'active', with the help of
| pm_runtime_set_active(), before pm_runtime_enable() is called for the device.

However, the PM domain code seems to always power up the PM domain when
a device is attached to it:

int genpd_dev_pm_attach(struct device *dev)
{
...
        pm_genpd_poweron(pd);

        return 0;
}
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);

So, the PM domain code ends up disagreeing with the runtime PM code about
the state of the device.

I think your commit (2ed127697eb1 "PM / Domains: Power on the PM domain
right after attach completes") is fundamentally wrong.  The assertion
you make in there is built upon the assumption that every driver will
call pm_runtime_set_active(), which is not an assumption you can make.

Instead, you should be doing is to hook into __pm_runtime_set_status()
and use that to trigger the PM domain power up so that the runtime PM
and PM domain state is always in step with each other.

What I'm certain of is that the current situation is just totally crazy.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King - ARM Linux]

On Fri, Feb 13, 2015 at 01:29:25PM +0000, Russell King - ARM Linux wrote:
> On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote:
> > On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote:
> > > The PMU device contains an interrupt controller, power control and
> > > resets.  The interrupt controller is a little sub-standard in that
> > > there is no race free way to clear down pending interrupts, so we try
> > > to avoid problems by reducing the window as much as possible, and
> > > clearing as infrequently as possible.
> > >
> > > The interrupt support is implemented using an IRQ domain, and the
> > > parent interrupt referenced in the standard DT way.
> > >
> > > The power domains and reset support is closely related - there is a
> > > defined sequence for powering down a domain which is tightly coupled
> > > with asserting the reset.  Hence, it makes sense to group these two
> > > together.
> > >
> > > This patch adds the core PMU driver: power domains must be defined in
> > > the DT file in order to make use of them.  The reset controller can
> > > be referenced in the standard way for reset controllers.
> > 
> > Hi Russell,
> > 
> > This patch would be simplified if this was based upon the not yet
> > merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree
> > based power domain look-up".
> > 
> > For example you would likely not need to add some of the marvel
> > specific DT bindings, and you wouldn?t need the bus_notifiers to add
> > devices to the power domain. I guess I just though it could be useful
> > input to consider while going forward, unless you already knew.
> 
> In 3.19, I notice something of an odd behaviour.
> 
> My vMeta driver has runtime PM support enabled.  When I explicitly register
> the PM domain in the pmu driver via a bus notifier, I see:
> 
> root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>     domain                      status         slaves
>            /device                                      runtime status
> ----------------------------------------------------------------------
> gpu-domain                      on
>     /devices/platform/vivante/etnaviv-gpu,2d            active
> vpu-domain                      off
>     /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder  suspended
> 
> But when I disable that, and let the generic code do the registration,
> I instead get:
> 
> root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>     domain                      status         slaves
>            /device                                      runtime status
> ----------------------------------------------------------------------
> gpu-domain                      on
>     /devices/platform/vivante/etnaviv-gpu,2d            active
> vpu-domain                      on
>     /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder  suspended
> 
> The difference being that the vpu domain remains powered.
> 
> The only difference code-wise seems to be when genpd_dev_pm_attach() is
> called.  In the working case, it's before the device is considered for
> probing.  In the non-working case, it's just before the device is probed.
> 
> With debugging enabled in the PM domain code, with the former case I get:
> 
> Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain
> platform f1c00000.video-decoder: adding to PM domain vpu-domain
> platform f1c00000.video-decoder: __pm_genpd_add_device()
> 
> With the latter non-working case:
> 
> Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain
> ...
> ap510-vmeta f1c00000.video-decoder: adding to PM domain vpu-domain
> ap510-vmeta f1c00000.video-decoder: __pm_genpd_add_device()
> vpu-domain: Power-on latency exceeded, new value 1578 ns
> 
> Neither of these debug messages provide much hint as to what the
> difference is, or the cause of the PM domain code being de-sync'd
> with its devices.
> 
> Maybe the PM code needs more debugging in it, and maybe the debugfs
> file should always be present if debugfs support is enabled?

The vmeta driver does this in its probe function:

        pm_runtime_use_autosuspend(vi->dev);
        pm_runtime_set_autosuspend_delay(vi->dev, 100);
        pm_runtime_enable(vi->dev);

since it doesn't touch the hardware, and the hardware starts off at
boot time in "suspended" mode.

I think what's going on is that there's a difference in the expectations
from the PM domain code vs the runtime PM code.  I refer to section 5
of the runtime PM documentation:

| 5. Runtime PM Initialization, Device Probing and Removal
| 
| Initially, the runtime PM is disabled for all devices, which means that the
| majority of the runtime PM helper functions described in Section 4 will return
| -EAGAIN until pm_runtime_enable() is called for the device.
| 
| In addition to that, the initial runtime PM status of all devices is
| 'suspended', but it need not reflect the actual physical state of the device.
| Thus, if the device is initially active (i.e. it is able to process I/O), its
| runtime PM status must be changed to 'active', with the help of
| pm_runtime_set_active(), before pm_runtime_enable() is called for the device.

However, the PM domain code seems to always power up the PM domain when
a device is attached to it:

int genpd_dev_pm_attach(struct device *dev)
{
...
        pm_genpd_poweron(pd);

        return 0;
}
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);

So, the PM domain code ends up disagreeing with the runtime PM code about
the state of the device.

I think your commit (2ed127697eb1 "PM / Domains: Power on the PM domain
right after attach completes") is fundamentally wrong.  The assertion
you make in there is built upon the assumption that every driver will
call pm_runtime_set_active(), which is not an assumption you can make.

Instead, you should be doing is to hook into __pm_runtime_set_status()
and use that to trigger the PM domain power up so that the runtime PM
and PM domain state is always in step with each other.

What I'm certain of is that the current situation is just totally crazy.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King - ARM Linux]

On Fri, Feb 13, 2015 at 02:11:52PM +0000, Russell King - ARM Linux wrote:
> I think what's going on is that there's a difference in the expectations
> from the PM domain code vs the runtime PM code.  I refer to section 5
> of the runtime PM documentation:
> 
> | 5. Runtime PM Initialization, Device Probing and Removal
> | 
> | Initially, the runtime PM is disabled for all devices, which means that the
> | majority of the runtime PM helper functions described in Section 4 will return
> | -EAGAIN until pm_runtime_enable() is called for the device.
> | 
> | In addition to that, the initial runtime PM status of all devices is
> | 'suspended', but it need not reflect the actual physical state of the device.
> | Thus, if the device is initially active (i.e. it is able to process I/O), its
> | runtime PM status must be changed to 'active', with the help of
> | pm_runtime_set_active(), before pm_runtime_enable() is called for the device.
> 
> However, the PM domain code seems to always power up the PM domain when
> a device is attached to it:
> 
> int genpd_dev_pm_attach(struct device *dev)
> {
> ...
>         pm_genpd_poweron(pd);
> 
>         return 0;
> }
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
> 
> So, the PM domain code ends up disagreeing with the runtime PM code about
> the state of the device.
> 
> I think your commit (2ed127697eb1 "PM / Domains: Power on the PM domain
> right after attach completes") is fundamentally wrong.  The assertion
> you make in there is built upon the assumption that every driver will
> call pm_runtime_set_active(), which is not an assumption you can make.
> 
> Instead, you should be doing is to hook into __pm_runtime_set_status()
> and use that to trigger the PM domain power up so that the runtime PM
> and PM domain state is always in step with each other.
> 
> What I'm certain of is that the current situation is just totally crazy.

There are around 150 drivers in the kernel tree which do not call
pm_runtime_set_active() before calling pm_runtime_enable(), so the
assertion in the PM domains commit above is wrong.  Some of them are
only saved because they do a pm_runtime_get() immediately after
pm_runtime_enable(), but that's in no way guaranteed - others do
neither.  So, for this to work properly, we need to address this
issue _correctly_ rather than papering over the problem.

Here's a patch which solves the issue for me along the lines which I
described above.  I'm introducing a new callback - runtime_set_status()
which the PM domain code uses to be notified of the runtime PM status
updates while RPM is disabled.  This callback will only occur from
__pm_runtime_set_status(), which can only be used while runtime PM is
disabled.

I believe it's safe - if we think that a PM domain is powered down, and
the runtime PM status is then set active, it's clear that the PM domain
absolutely must transition to active mode as well.  If the runtime PM
status is set to suspended, then the PM domain code can transition to
off state.

I've left some of my debugging in place in the patch below as that's
exactly the code I've tested.

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 751a8859a4ab..1616faadf904 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -5,7 +5,7 @@
  *
  * This file is released under the GPLv2.
  */
-
+#define DEBUG
 #include <linux/kernel.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
@@ -158,6 +158,8 @@ static int genpd_power_on(struct generic_pm_domain *genpd)
 	s64 elapsed_ns;
 	int ret;
 
+	pr_debug("%s: %s()\n", genpd->name, __func__);
+
 	if (!genpd->power_on)
 		return 0;
 
@@ -219,6 +221,10 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
 	DEFINE_WAIT(wait);
 	int ret = 0;
 
+	pr_debug("%s: %s() status %u prepared %d spo %u\n",
+		genpd->name, __func__, genpd->status, genpd->prepared_count,
+		genpd->suspend_power_off);
+
 	/* If the domain's master is being waited for, we have to wait too. */
 	for (;;) {
 		prepare_to_wait(&genpd->status_wait_queue, &wait,
@@ -741,6 +747,20 @@ static int pm_genpd_runtime_resume(struct device *dev)
 	return 0;
 }
 
+static int pm_genpd_runtime_set_status(struct device *dev)
+{
+	int ret;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (pm_runtime_suspended(dev))
+		ret = pm_genpd_runtime_suspend(dev);
+	else
+		ret = pm_genpd_runtime_resume(dev);
+
+	return ret;
+}
+
 static bool pd_ignore_unused;
 static int __init pd_ignore_unused_setup(char *__unused)
 {
@@ -1907,6 +1927,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	genpd->max_off_time_changed = true;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
 	genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
+	genpd->domain.ops.runtime_set_status = pm_genpd_runtime_set_status;
 	genpd->domain.ops.prepare = pm_genpd_prepare;
 	genpd->domain.ops.suspend = pm_genpd_suspend;
 	genpd->domain.ops.suspend_late = pm_genpd_suspend_late;
@@ -2223,7 +2244,6 @@ int genpd_dev_pm_attach(struct device *dev)
 	}
 
 	dev->pm_domain->detach = genpd_dev_pm_detach;
-	pm_genpd_poweron(pd);
 
 	return 0;
 }
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 5070c4fe8542..a958cae67a37 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -981,6 +981,7 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
 	struct device *parent = dev->parent;
 	unsigned long flags;
 	bool notify_parent = false;
+	pm_callback_t callback;
 	int error = 0;
 
 	if (status != RPM_ACTIVE && status != RPM_SUSPENDED)
@@ -1029,6 +1030,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
  out_set:
 	__update_runtime_status(dev, status);
 	dev->power.runtime_error = 0;
+	if (dev->power.no_callbacks)
+		goto out;
+	callback = RPM_GET_CALLBACK(dev, runtime_set_status);
+	rpm_callback(callback, dev);
  out:
 	spin_unlock_irqrestore(&dev->power.lock, flags);
 
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 8b5976364619..ee098585d577 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -316,6 +316,7 @@ struct dev_pm_ops {
 	int (*runtime_suspend)(struct device *dev);
 	int (*runtime_resume)(struct device *dev);
 	int (*runtime_idle)(struct device *dev);
+	int (*runtime_set_status)(struct device *dev);
 };
 
 #ifdef CONFIG_PM_SLEEP


-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets

[Russell King - ARM Linux]

On Fri, Feb 13, 2015 at 02:11:52PM +0000, Russell King - ARM Linux wrote:
> I think what's going on is that there's a difference in the expectations
> from the PM domain code vs the runtime PM code.  I refer to section 5
> of the runtime PM documentation:
> 
> | 5. Runtime PM Initialization, Device Probing and Removal
> | 
> | Initially, the runtime PM is disabled for all devices, which means that the
> | majority of the runtime PM helper functions described in Section 4 will return
> | -EAGAIN until pm_runtime_enable() is called for the device.
> | 
> | In addition to that, the initial runtime PM status of all devices is
> | 'suspended', but it need not reflect the actual physical state of the device.
> | Thus, if the device is initially active (i.e. it is able to process I/O), its
> | runtime PM status must be changed to 'active', with the help of
> | pm_runtime_set_active(), before pm_runtime_enable() is called for the device.
> 
> However, the PM domain code seems to always power up the PM domain when
> a device is attached to it:
> 
> int genpd_dev_pm_attach(struct device *dev)
> {
> ...
>         pm_genpd_poweron(pd);
> 
>         return 0;
> }
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
> 
> So, the PM domain code ends up disagreeing with the runtime PM code about
> the state of the device.
> 
> I think your commit (2ed127697eb1 "PM / Domains: Power on the PM domain
> right after attach completes") is fundamentally wrong.  The assertion
> you make in there is built upon the assumption that every driver will
> call pm_runtime_set_active(), which is not an assumption you can make.
> 
> Instead, you should be doing is to hook into __pm_runtime_set_status()
> and use that to trigger the PM domain power up so that the runtime PM
> and PM domain state is always in step with each other.
> 
> What I'm certain of is that the current situation is just totally crazy.

There are around 150 drivers in the kernel tree which do not call
pm_runtime_set_active() before calling pm_runtime_enable(), so the
assertion in the PM domains commit above is wrong.  Some of them are
only saved because they do a pm_runtime_get() immediately after
pm_runtime_enable(), but that's in no way guaranteed - others do
neither.  So, for this to work properly, we need to address this
issue _correctly_ rather than papering over the problem.

Here's a patch which solves the issue for me along the lines which I
described above.  I'm introducing a new callback - runtime_set_status()
which the PM domain code uses to be notified of the runtime PM status
updates while RPM is disabled.  This callback will only occur from
__pm_runtime_set_status(), which can only be used while runtime PM is
disabled.

I believe it's safe - if we think that a PM domain is powered down, and
the runtime PM status is then set active, it's clear that the PM domain
absolutely must transition to active mode as well.  If the runtime PM
status is set to suspended, then the PM domain code can transition to
off state.

I've left some of my debugging in place in the patch below as that's
exactly the code I've tested.

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 751a8859a4ab..1616faadf904 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -5,7 +5,7 @@
  *
  * This file is released under the GPLv2.
  */
-
+#define DEBUG
 #include <linux/kernel.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
@@ -158,6 +158,8 @@ static int genpd_power_on(struct generic_pm_domain *genpd)
 	s64 elapsed_ns;
 	int ret;
 
+	pr_debug("%s: %s()\n", genpd->name, __func__);
+
 	if (!genpd->power_on)
 		return 0;
 
@@ -219,6 +221,10 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
 	DEFINE_WAIT(wait);
 	int ret = 0;
 
+	pr_debug("%s: %s() status %u prepared %d spo %u\n",
+		genpd->name, __func__, genpd->status, genpd->prepared_count,
+		genpd->suspend_power_off);
+
 	/* If the domain's master is being waited for, we have to wait too. */
 	for (;;) {
 		prepare_to_wait(&genpd->status_wait_queue, &wait,
@@ -741,6 +747,20 @@ static int pm_genpd_runtime_resume(struct device *dev)
 	return 0;
 }
 
+static int pm_genpd_runtime_set_status(struct device *dev)
+{
+	int ret;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (pm_runtime_suspended(dev))
+		ret = pm_genpd_runtime_suspend(dev);
+	else
+		ret = pm_genpd_runtime_resume(dev);
+
+	return ret;
+}
+
 static bool pd_ignore_unused;
 static int __init pd_ignore_unused_setup(char *__unused)
 {
@@ -1907,6 +1927,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	genpd->max_off_time_changed = true;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
 	genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
+	genpd->domain.ops.runtime_set_status = pm_genpd_runtime_set_status;
 	genpd->domain.ops.prepare = pm_genpd_prepare;
 	genpd->domain.ops.suspend = pm_genpd_suspend;
 	genpd->domain.ops.suspend_late = pm_genpd_suspend_late;
@@ -2223,7 +2244,6 @@ int genpd_dev_pm_attach(struct device *dev)
 	}
 
 	dev->pm_domain->detach = genpd_dev_pm_detach;
-	pm_genpd_poweron(pd);
 
 	return 0;
 }
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 5070c4fe8542..a958cae67a37 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -981,6 +981,7 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
 	struct device *parent = dev->parent;
 	unsigned long flags;
 	bool notify_parent = false;
+	pm_callback_t callback;
 	int error = 0;
 
 	if (status != RPM_ACTIVE && status != RPM_SUSPENDED)
@@ -1029,6 +1030,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
  out_set:
 	__update_runtime_status(dev, status);
 	dev->power.runtime_error = 0;
+	if (dev->power.no_callbacks)
+		goto out;
+	callback = RPM_GET_CALLBACK(dev, runtime_set_status);
+	rpm_callback(callback, dev);
  out:
 	spin_unlock_irqrestore(&dev->power.lock, flags);
 
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 8b5976364619..ee098585d577 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -316,6 +316,7 @@ struct dev_pm_ops {
 	int (*runtime_suspend)(struct device *dev);
 	int (*runtime_resume)(struct device *dev);
 	int (*runtime_idle)(struct device *dev);
+	int (*runtime_set_status)(struct device *dev);
 };
 
 #ifdef CONFIG_PM_SLEEP


-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.