linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [FOR DISCUSSION 0/8] Dove PMU support
@ 2015-02-14 15:26 Russell King - ARM Linux
  2015-02-14 15:27 ` [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets Russell King
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Russell King - ARM Linux @ 2015-02-14 15:26 UTC (permalink / raw)
  To: linux-arm-kernel

This is a re-posting of the patch set which I posted almost 10 months
ago to support the Dove PMU, with a few additional changes.  This set
is based upon 3.19.

In this set are:

* two patches which Rafael originally acked, but there was indecision
  last time around how to handle them due to potential conflicts with
  work that Ulf was doing.  These patches have been updated to apply
  cleanly to 3.19.  I don't know if people want to take these as
  fixes to the PM domain code or not (hence why I'm posting this
  series during the merge window - if it weren't for this, I'd hold
  it off.)

* what I regard as a fix to the PM domain code; as a result of a
  previous commit, the PM domain code mismatches the runtime PM state,
  which leads to the PM domain being unexpectedly left on.  This patch
  works around that.  (It's been sent recently as well but in an old
  thread.)

* the addition of the core Dove PMU driver, which consists of a reset,
  IRQ controller, and power domains.  The reset and power domain code
  has to be closely related due to the power up/down requirements of
  the GPU/VPU subsystems needing to be performed atomically.  (This
  requirement prevents it using the MFD infrastructure, because we
  would need to hold spinlocks while calling several different
  sub-drivers.)

* addition of the RTC interrupt, so we can now receive and act on
  alarms generated by the Dove RTC.

* addition of the DT descriptions for the GPU and VPU power domains.
  These patches do not themselves add the DT descriptions for these
  units, so these patches serve as illustrations how these should be
  described.

There are a few things which are missing from this driver, such sa the
DT binding documentation.  This will follow once people are happy with
the first four patches, in particular where to locate the PMU driver
in the kernel tree.  Currently, I have it in arch/arm/mach-dove, which
is where the legacy Dove code lives, so it's not ideal.

There are some gotchas with it - PM domains need to be created prior
to any device which uses them being probed, so it is best if the driver
is initialised really early in the kernel boot.  We may be able to get
away with a core_initcall() or a postcore_initcall().

I'd ideally like to get these queued for merging in the _next_ merge
window if at all possible, if only to reduce the number of patches I've
been carrying for the last few years.

 arch/arm/Kconfig                     |   1 +
 arch/arm/boot/dts/dove.dtsi          |  25 +++
 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             | 410 +++++++++++++++++++++++++++++++++++
 drivers/base/power/domain.c          |  26 ++-
 drivers/base/power/runtime.c         |   5 +
 include/linux/pm.h                   |   1 +
 11 files changed, 466 insertions(+), 110 deletions(-)

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
  2015-02-14 15:26 [FOR DISCUSSION 0/8] Dove PMU support Russell King - ARM Linux
@ 2015-02-14 15:27 ` Russell King
  2015-02-14 17:02   ` Sebastian Hesselbarth
  2015-02-14 17:09   ` Andrew Lunn
  2015-02-14 15:27 ` [PATCH 5/8] ARM: dt: dove: add Dove PMU DT entry to dove.dtsi Russell King
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 15+ messages in thread
From: Russell King @ 2015-02-14 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

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             | 410 +++++++++++++++++++++++++++++++++++
 7 files changed, 415 insertions(+), 104 deletions(-)
 create mode 100644 arch/arm/mach-dove/pmu.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 97d07ed60a0b..08e7608d1c52 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -519,6 +519,7 @@ config ARCH_DOVE
 	select PINCTRL
 	select PINCTRL_DOVE
 	select PLAT_ORION_LEGACY
+	select PM_GENERIC_DOMAINS if PM
 	help
 	  Support for the Marvell Dove SoC 88AP510
 
diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile
index b608a21919fb..b039b91c97b9 100644
--- a/arch/arm/mach-dove/Makefile
+++ b/arch/arm/mach-dove/Makefile
@@ -1,5 +1,6 @@
 obj-y				+= common.o
 obj-$(CONFIG_DOVE_LEGACY)	+= irq.o mpp.o
 obj-$(CONFIG_PCI)		+= pcie.o
+obj-$(CONFIG_PM_GENERIC_DOMAINS)+= pmu.o
 obj-$(CONFIG_MACH_DOVE_DB)	+= dove-db-setup.o
 obj-$(CONFIG_MACH_CM_A510)	+= cm-a510.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..3e4a4178efa6 100644
--- a/arch/arm/mach-dove/common.h
+++ b/arch/arm/mach-dove/common.h
@@ -25,6 +25,7 @@ void dove_map_io(void);
 void dove_init(void);
 void dove_init_early(void);
 void dove_init_irq(void);
+int dove_init_pmu(void);
 void dove_setup_cpu_wins(void);
 void dove_ge00_init(struct mv643xx_eth_platform_data *eth_data);
 void dove_sata_init(struct mv_sata_platform_data *sata_data);
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 4a5a7aedcb76..924d8afe4597 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,
@@ -142,8 +70,6 @@ __exception_irq_entry dove_legacy_handle_irq(struct pt_regs *regs)
 
 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);
 
@@ -162,17 +88,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..2d1325995a68
--- /dev/null
+++ b/arch/arm/mach-dove/pmu.c
@@ -0,0 +1,410 @@
+/*
+ * 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,
+	struct device_node *np)
+{
+	unsigned int val = readl_relaxed(domain->pmu->pmu_base + PMU_PWR);
+
+	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));
+
+	if (np)
+		of_genpd_add_provider_simple(np, &domain->base);
+}
+
+/* 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;
+ *	#power-domain-cells = <0>;
+ *	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.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, np);
+	}
+	pm_genpd_poweroff_unused();
+
+	ret = dove_init_pmu_irq(pmu, parent_irq);
+	if (ret)
+		pr_err("dove_init_pmu_irq() failed: %d\n", ret);
+
+	return 0;
+}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/8] ARM: dt: dove: add Dove PMU DT entry to dove.dtsi
  2015-02-14 15:26 [FOR DISCUSSION 0/8] Dove PMU support Russell King - ARM Linux
  2015-02-14 15:27 ` [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets Russell King
@ 2015-02-14 15:27 ` Russell King
  2015-02-14 15:27 ` [PATCH 6/8] ARM: dt: dove: wire up RTC interrupt Russell King
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Russell King @ 2015-02-14 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

Add the PMU description to the Dove DT file.  The PMU provides multiple
features, including an interrupt, reset, power and isolation controller.

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

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/8] ARM: dt: dove: wire up RTC interrupt
  2015-02-14 15:26 [FOR DISCUSSION 0/8] Dove PMU support Russell King - ARM Linux
  2015-02-14 15:27 ` [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets Russell King
  2015-02-14 15:27 ` [PATCH 5/8] ARM: dt: dove: add Dove PMU DT entry to dove.dtsi Russell King
@ 2015-02-14 15:27 ` Russell King
  2015-02-14 15:28 ` [PATCH 7/8] ARM: dt: dove: add video decoder power domain description Russell King
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Russell King @ 2015-02-14 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

Now that we have a PMU driver, we can wire up the RTC interrupt in the
DT description for Dove.

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

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 1e664eb8bdcb..00b61d2be66b 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -624,6 +624,8 @@
 			rtc: real-time-clock at d8500 {
 				compatible = "marvell,orion-rtc";
 				reg = <0xd8500 0x20>;
+				interrupt-parent = <&pmu>;
+				interrupts = <5>;
 			};
 
 			gconf: global-config at e802c {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/8] ARM: dt: dove: add video decoder power domain description
  2015-02-14 15:26 [FOR DISCUSSION 0/8] Dove PMU support Russell King - ARM Linux
                   ` (2 preceding siblings ...)
  2015-02-14 15:27 ` [PATCH 6/8] ARM: dt: dove: wire up RTC interrupt Russell King
@ 2015-02-14 15:28 ` Russell King
  2015-02-14 15:28 ` [PATCH 8/8] ARM: dt: dove: add GPU " Russell King
  2015-02-14 16:49 ` [FOR DISCUSSION 0/8] Dove PMU support Andrew Lunn
  5 siblings, 0 replies; 15+ messages in thread
From: Russell King @ 2015-02-14 15:28 UTC (permalink / raw)
  To: linux-arm-kernel

Add the description of the video decoder power domain to the PMU DT
entry.

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 00b61d2be66b..a47c7cef380f 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -387,6 +387,13 @@
 				interrupt-controller;
 				#interrupt-cells = <1>;
 				#reset-cells = <1>;
+
+				vpu_domain: vpu-domain {
+					#power-domain-cells = <0>;
+					marvell,pmu_pwr_mask = <0x00000008>;
+					marvell,pmu_iso_mask = <0x00000001>;
+					resets = <&pmu 16>;
+				};
 			};
 
 			thermal: thermal-diode at d001c {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/8] ARM: dt: dove: add GPU power domain description
  2015-02-14 15:26 [FOR DISCUSSION 0/8] Dove PMU support Russell King - ARM Linux
                   ` (3 preceding siblings ...)
  2015-02-14 15:28 ` [PATCH 7/8] ARM: dt: dove: add video decoder power domain description Russell King
@ 2015-02-14 15:28 ` Russell King
  2015-02-14 16:49 ` [FOR DISCUSSION 0/8] Dove PMU support Andrew Lunn
  5 siblings, 0 replies; 15+ messages in thread
From: Russell King @ 2015-02-14 15:28 UTC (permalink / raw)
  To: linux-arm-kernel

Add the description of the GPU power domain to the PMU DT entry.

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 a47c7cef380f..081f968c521a 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -394,6 +394,13 @@
 					marvell,pmu_iso_mask = <0x00000001>;
 					resets = <&pmu 16>;
 				};
+
+				gpu_domain: gpu-domain {
+					#power-domain-cells = <0>;
+					marvell,pmu_pwr_mask = <0x00000004>;
+					marvell,pmu_iso_mask = <0x00000002>;
+					resets = <&pmu 18>;
+				};
 			};
 
 			thermal: thermal-diode at d001c {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [FOR DISCUSSION 0/8] Dove PMU support
  2015-02-14 15:26 [FOR DISCUSSION 0/8] Dove PMU support Russell King - ARM Linux
                   ` (4 preceding siblings ...)
  2015-02-14 15:28 ` [PATCH 8/8] ARM: dt: dove: add GPU " Russell King
@ 2015-02-14 16:49 ` Andrew Lunn
  5 siblings, 0 replies; 15+ messages in thread
From: Andrew Lunn @ 2015-02-14 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell

Thanks for posting these. Lets try to get them merged.

> There are a few things which are missing from this driver, such sa the
> DT binding documentation.  This will follow once people are happy with
> the first four patches, in particular where to locate the PMU driver
> in the kernel tree.  Currently, I have it in arch/arm/mach-dove, which
> is where the legacy Dove code lives, so it's not ideal.

Maybe time for a drivers/pmu? arch/arm/mach-dove is not good, we want
to be able to use this code from arch/arm/mach-mvebu.

   Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
  2015-02-14 15:27 ` [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets Russell King
@ 2015-02-14 17:02   ` Sebastian Hesselbarth
  2015-02-15 16:36     ` Russell King - ARM Linux
  2015-02-16 15:58     ` Russell King - ARM Linux
  2015-02-14 17:09   ` Andrew Lunn
  1 sibling, 2 replies; 15+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-14 17:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 14.02.2015 16:27, Russell King 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.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
[...]
> diff --git a/arch/arm/mach-dove/pmu.c b/arch/arm/mach-dove/pmu.c
> new file mode 100644
> index 000000000000..2d1325995a68
> --- /dev/null
> +++ b/arch/arm/mach-dove/pmu.c
> @@ -0,0 +1,410 @@
> +/*
> + * Marvell Dove PMU support
> + */
[...]
> +/*
> + * pmu {
> + *	compatible = "marvell,pmu";

Russell,

Should we be more precise and call it "marvell,dove-pmu" ?

Also, can we have an additional "syscon" compatible for that node?

That will allow us to get rid of the messy iomem stuff current
pinctrl-dove is doing to get access to the pmu pinctrl registers.

> + *	reg = <0xd0000 0x8000> <0xd8000 0x8000>;
> + *	interrupts = <33>;
> + *	#reset-cells = 1;
> + *	#power-domain-cells = <0>;
> + *	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)
> +{

How about we copy the clk subsystem way of installing early probed
pm for DT here?

For example:

#define PM_OF_DECLARE(name, compat, fn) OF_DECLARE_1(clk, name, compat, fn)

and

static int __init dove_pmu_init(struct device_node *np) { ... }
PM_OF_DECLARE(dove_pmu, "marvell,dove-pmu", dove_init_pmu);

Sebastian

> +	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.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, np);
> +	}
> +	pm_genpd_poweroff_unused();
> +
> +	ret = dove_init_pmu_irq(pmu, parent_irq);
> +	if (ret)
> +		pr_err("dove_init_pmu_irq() failed: %d\n", ret);
> +
> +	return 0;
> +}
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
  2015-02-14 15:27 ` [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets Russell King
  2015-02-14 17:02   ` Sebastian Hesselbarth
@ 2015-02-14 17:09   ` Andrew Lunn
  2015-02-15 16:26     ` Russell King - ARM Linux
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Lunn @ 2015-02-14 17:09 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell

> +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,

Dumb question: Is this still needed? There have been a lot of patches
from Wolfram Sang removing similar THIS_MODULE statements.

> + * pmu {
> + *	compatible = "marvell,pmu";

Is this maybe too generic? I'm sure Marvell has more than one pmu.
Maybe "marvell,dove-pmu"

> +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);
> +

Is it not fatal to be missing the interrupt? Seems like return -EINVAL
would be a good idea?

      Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
  2015-02-14 17:09   ` Andrew Lunn
@ 2015-02-15 16:26     ` Russell King - ARM Linux
  0 siblings, 0 replies; 15+ messages in thread
From: Russell King - ARM Linux @ 2015-02-15 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 14, 2015 at 06:09:39PM +0100, Andrew Lunn wrote:
> Hi Russell
> 
> > +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,
> 
> Dumb question: Is this still needed? There have been a lot of patches
> from Wolfram Sang removing similar THIS_MODULE statements.

If this is not set, then .owner remains NULL; reset_controller_register()
is not wrapped to hide the initialisation of this member.  (IMHO, that's
exactly why hiding it is bad - it has created inconsistencies because
some APIs need an explicit initialisation, others don't.)

> > + * pmu {
> > + *	compatible = "marvell,pmu";
> 
> Is this maybe too generic? I'm sure Marvell has more than one pmu.
> Maybe "marvell,dove-pmu"

Changed.

> > +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);
> > +
> 
> Is it not fatal to be missing the interrupt? Seems like return -EINVAL
> would be a good idea?

I don't think so.  The lack of parent interrupt shouldn't stop the
rest of the PMU code initialising - least of all only because the
only user of this right now is the RTC.

It may make sense to avoid initialising the PMU's IRQ support in
that case though.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
  2015-02-14 17:02   ` Sebastian Hesselbarth
@ 2015-02-15 16:36     ` Russell King - ARM Linux
  2015-02-16 15:58     ` Russell King - ARM Linux
  1 sibling, 0 replies; 15+ messages in thread
From: Russell King - ARM Linux @ 2015-02-15 16:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 14, 2015 at 06:02:11PM +0100, Sebastian Hesselbarth wrote:
> Should we be more precise and call it "marvell,dove-pmu" ?

Andrew made the same comment, consider it done.

> Also, can we have an additional "syscon" compatible for that node?

I'll look into it, thanks.

> How about we copy the clk subsystem way of installing early probed
> pm for DT here?
> 
> For example:
> 
> #define PM_OF_DECLARE(name, compat, fn) OF_DECLARE_1(clk, name, compat, fn)
> 
> and
> 
> static int __init dove_pmu_init(struct device_node *np) { ... }
> PM_OF_DECLARE(dove_pmu, "marvell,dove-pmu", dove_init_pmu);

Not sure it's worth the overhead.  Firstly, sticking the PMU into the
clock OF tables doesn't seem like the right thing to do.  Secondly,
it means modifying the asm-generic vmlinux.lds.S, adding another
config symbol for PMUs to enable that table, adding a sentinel entry,
and finally adding some code somewhere to scan this new table which
right now is only going to have at most one entry.

That seems like an awful lot of overhead.

It would have been easier if there was a generic mechanism, like the
initcall mechanism, where it's possible to add these kinds of
compatible matches automatically without having to mess around with
so many different files.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
  2015-02-14 17:02   ` Sebastian Hesselbarth
  2015-02-15 16:36     ` Russell King - ARM Linux
@ 2015-02-16 15:58     ` Russell King - ARM Linux
  2015-02-16 16:30       ` Sebastian Hesselbarth
  1 sibling, 1 reply; 15+ messages in thread
From: Russell King - ARM Linux @ 2015-02-16 15:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 14, 2015 at 06:02:11PM +0100, Sebastian Hesselbarth wrote:
> How about we copy the clk subsystem way of installing early probed
> pm for DT here?
> 
> For example:
> 
> #define PM_OF_DECLARE(name, compat, fn) OF_DECLARE_1(clk, name, compat, fn)
> 
> and
> 
> static int __init dove_pmu_init(struct device_node *np) { ... }
> PM_OF_DECLARE(dove_pmu, "marvell,dove-pmu", dove_init_pmu);

Well, Rob's response was basically "use the machine descriptor" so I
guess it needs to be explicitly called from
arch/arm/mach-mvebu/dove.c:dove_init().

If you disagree, please discuss with Rob (on To:) and try and find a
way forward and let me know how to deal with this.

Thanks.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
  2015-02-16 15:58     ` Russell King - ARM Linux
@ 2015-02-16 16:30       ` Sebastian Hesselbarth
  2015-02-16 16:52         ` Russell King - ARM Linux
  0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-16 16:30 UTC (permalink / raw)
  To: linux-arm-kernel

On 16.02.2015 16:58, Russell King - ARM Linux wrote:
> On Sat, Feb 14, 2015 at 06:02:11PM +0100, Sebastian Hesselbarth wrote:
>> How about we copy the clk subsystem way of installing early probed
>> pm for DT here?
>>
>> For example:
>>
>> #define PM_OF_DECLARE(name, compat, fn) OF_DECLARE_1(clk, name, compat, fn)
>>
>> and
>>
>> static int __init dove_pmu_init(struct device_node *np) { ... }
>> PM_OF_DECLARE(dove_pmu, "marvell,dove-pmu", dove_init_pmu);
>
> Well, Rob's response was basically "use the machine descriptor" so I
> guess it needs to be explicitly called from
> arch/arm/mach-mvebu/dove.c:dove_init().

Ok, I am very fine with that, too.

Still, we'd have to find a proper place for the driver, don't we?

Sebastian

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
  2015-02-16 16:30       ` Sebastian Hesselbarth
@ 2015-02-16 16:52         ` Russell King - ARM Linux
  2015-02-16 17:54           ` Andrew Lunn
  0 siblings, 1 reply; 15+ messages in thread
From: Russell King - ARM Linux @ 2015-02-16 16:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 16, 2015 at 05:30:24PM +0100, Sebastian Hesselbarth wrote:
> On 16.02.2015 16:58, Russell King - ARM Linux wrote:
> >On Sat, Feb 14, 2015 at 06:02:11PM +0100, Sebastian Hesselbarth wrote:
> >>How about we copy the clk subsystem way of installing early probed
> >>pm for DT here?
> >>
> >>For example:
> >>
> >>#define PM_OF_DECLARE(name, compat, fn) OF_DECLARE_1(clk, name, compat, fn)
> >>
> >>and
> >>
> >>static int __init dove_pmu_init(struct device_node *np) { ... }
> >>PM_OF_DECLARE(dove_pmu, "marvell,dove-pmu", dove_init_pmu);
> >
> >Well, Rob's response was basically "use the machine descriptor" so I
> >guess it needs to be explicitly called from
> >arch/arm/mach-mvebu/dove.c:dove_init().
> 
> Ok, I am very fine with that, too.
> 
> Still, we'd have to find a proper place for the driver, don't we?

Yep - I'm not sure creating drivers/pmu for (at the moment) one driver
is a particularly good idea.  Maybe something in drivers/soc/ ?

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets
  2015-02-16 16:52         ` Russell King - ARM Linux
@ 2015-02-16 17:54           ` Andrew Lunn
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Lunn @ 2015-02-16 17:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 16, 2015 at 04:52:54PM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 16, 2015 at 05:30:24PM +0100, Sebastian Hesselbarth wrote:
> > On 16.02.2015 16:58, Russell King - ARM Linux wrote:
> > >On Sat, Feb 14, 2015 at 06:02:11PM +0100, Sebastian Hesselbarth wrote:
> > >>How about we copy the clk subsystem way of installing early probed
> > >>pm for DT here?
> > >>
> > >>For example:
> > >>
> > >>#define PM_OF_DECLARE(name, compat, fn) OF_DECLARE_1(clk, name, compat, fn)
> > >>
> > >>and
> > >>
> > >>static int __init dove_pmu_init(struct device_node *np) { ... }
> > >>PM_OF_DECLARE(dove_pmu, "marvell,dove-pmu", dove_init_pmu);
> > >
> > >Well, Rob's response was basically "use the machine descriptor" so I
> > >guess it needs to be explicitly called from
> > >arch/arm/mach-mvebu/dove.c:dove_init().
> > 
> > Ok, I am very fine with that, too.
> > 
> > Still, we'd have to find a proper place for the driver, don't we?
> 
> Yep - I'm not sure creating drivers/pmu for (at the moment) one driver
> is a particularly good idea.  Maybe something in drivers/soc/ ?

If you did create drivers/pmu, you could move drives/soc/pmc.c into
it.

But drivers/soc also seems like a good place for the dove PMU code.

    Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-02-16 17:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-14 15:26 [FOR DISCUSSION 0/8] Dove PMU support Russell King - ARM Linux
2015-02-14 15:27 ` [PATCH 4/8] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets Russell King
2015-02-14 17:02   ` Sebastian Hesselbarth
2015-02-15 16:36     ` Russell King - ARM Linux
2015-02-16 15:58     ` Russell King - ARM Linux
2015-02-16 16:30       ` Sebastian Hesselbarth
2015-02-16 16:52         ` Russell King - ARM Linux
2015-02-16 17:54           ` Andrew Lunn
2015-02-14 17:09   ` Andrew Lunn
2015-02-15 16:26     ` Russell King - ARM Linux
2015-02-14 15:27 ` [PATCH 5/8] ARM: dt: dove: add Dove PMU DT entry to dove.dtsi Russell King
2015-02-14 15:27 ` [PATCH 6/8] ARM: dt: dove: wire up RTC interrupt Russell King
2015-02-14 15:28 ` [PATCH 7/8] ARM: dt: dove: add video decoder power domain description Russell King
2015-02-14 15:28 ` [PATCH 8/8] ARM: dt: dove: add GPU " Russell King
2015-02-14 16:49 ` [FOR DISCUSSION 0/8] Dove PMU support Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).