linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Changes surrounding IRQs and IRQ domains
@ 2012-08-09 15:53 Lee Jones
  2012-08-09 15:53 ` [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF Lee Jones
                   ` (7 more replies)
  0 siblings, 8 replies; 50+ messages in thread
From: Lee Jones @ 2012-08-09 15:53 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: STEricsson_nomadik_linux, linus.walleij, arnd, broonie

Here we apply some fixes to the way the AB8500 handles IRQs and
provide the DB8500 with an IRQ domain. We also handle hwirq to
virq conversion slightly differently when registering MFD devices.
The other changes pertain to the way IRQs are requested and dealt
with by AB8500 child devices.

 arch/arm/boot/dts/dbx5x0.dtsi      |    3 ++
 drivers/input/misc/ab8500-ponkey.c |    4 +--
 drivers/mfd/ab8500-core.c          |    3 +-
 drivers/mfd/db8500-prcmu.c         |   54 +++++++++++++++++++++++++++---------
 drivers/mfd/mfd-core.c             |    2 +-
 include/linux/mfd/db8500-prcmu.h   |    2 ++
 include/linux/of_irq.h             |    5 ++++
 kernel/irq/irqdomain.c             |    7 +++++
 8 files changed, 63 insertions(+), 17 deletions(-)


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

* [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF
  2012-08-09 15:53 [PATCH 0/8] Changes surrounding IRQs and IRQ domains Lee Jones
@ 2012-08-09 15:53 ` Lee Jones
  2012-08-09 16:20   ` Rob Herring
                     ` (2 more replies)
  2012-08-09 15:53 ` [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified Lee Jones
                   ` (6 subsequent siblings)
  7 siblings, 3 replies; 50+ messages in thread
From: Lee Jones @ 2012-08-09 15:53 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	Lee Jones, Rob Herring, devicetree-discuss

of_irq_find_parent is a handy function to use outside the confines of
the Open Firmware subsystem. One such use-case is when the IRQ Domain
wishes to find an IRQ domain for a given device node. Currently it can
not take any notice of the 'interrupt-parent' property. Instead it
just uses the first IRQ controller as it climbs the Device Tree. If
we were to use this as a precursor the resultant controller is more
likely to be correct.

CC: Rob Herring <rob.herring@calxeda.com>
CC: devicetree-discuss@lists.ozlabs.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 include/linux/of_irq.h |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index 1717cd9..b8e2411 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -83,6 +83,11 @@ static inline unsigned int irq_of_parse_and_map(struct device_node *dev,
 {
 	return 0;
 }
+
+static inline void *of_irq_find_parent(struct device_node *child)
+{
+	return NULL;
+}
 #endif /* !CONFIG_OF */
 
 #endif /* __OF_IRQ_H */
-- 
1.7.9.5


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

* [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified
  2012-08-09 15:53 [PATCH 0/8] Changes surrounding IRQs and IRQ domains Lee Jones
  2012-08-09 15:53 ` [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF Lee Jones
@ 2012-08-09 15:53 ` Lee Jones
  2012-08-14  8:19   ` Linus Walleij
  2012-08-09 15:53 ` [PATCH 3/8] ARM: ux500: Identify the PRCMU as an interrupt controller Lee Jones
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-09 15:53 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	Lee Jones, Benjamin Herrenschmidt, Grant Likely

irq_find_host() currently ignores the 'interrupt-parent' property
even if it's specified in the Device Tree. Meaning that a node can
match to a domain in its hierarchy even if it doesn't belong to it.
By searching for the parent first using of_irq_find_parent() we
insist that the 'interrupt-parent' property is taken into account
ensuring a greater chance of returning the correct domain.

CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 kernel/irq/irqdomain.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 49a7772..db63b9b 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/topology.h>
 #include <linux/seq_file.h>
@@ -323,8 +324,14 @@ EXPORT_SYMBOL_GPL(irq_domain_add_tree);
 struct irq_domain *irq_find_host(struct device_node *node)
 {
 	struct irq_domain *h, *found = NULL;
+	struct device_node *parent_node;
 	int rc;
 
+	/* Take heed if an 'interrupt-parent' was specified. */
+	parent_node = of_irq_find_parent(node);
+	if (parent_node)
+		node = parent_node;
+
 	/* We might want to match the legacy controller last since
 	 * it might potentially be set to match all interrupts in
 	 * the absence of a device node. This isn't a problem so far
-- 
1.7.9.5


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

* [PATCH 3/8] ARM: ux500: Identify the PRCMU as an interrupt controller
  2012-08-09 15:53 [PATCH 0/8] Changes surrounding IRQs and IRQ domains Lee Jones
  2012-08-09 15:53 ` [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF Lee Jones
  2012-08-09 15:53 ` [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified Lee Jones
@ 2012-08-09 15:53 ` Lee Jones
  2012-08-14  8:19   ` Linus Walleij
  2012-08-09 15:53 ` [PATCH 4/8] ARM: ux500: Force AB8500 to use the GIC as its " Lee Jones
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-09 15:53 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: STEricsson_nomadik_linux, linus.walleij, arnd, broonie, Lee Jones

We're just about to provide the DB8500-PRCMU with its own IRQ domain,
so that its subordinate drivers can use it as an interrupt controller.
It's obligatory for all IRQ controllers to reference themselves as
such from its own node in Device Tree. This patch does just that.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/dbx5x0.dtsi |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 32e063d..6da7ccb 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -194,6 +194,8 @@
 			interrupts = <0 47 0x4>;
 			#address-cells = <1>;
 			#size-cells = <1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
 			ranges;
 
 			prcmu-timer-4@80157450 {
-- 
1.7.9.5


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

* [PATCH 4/8] ARM: ux500: Force AB8500 to use the GIC as its interrupt controller
  2012-08-09 15:53 [PATCH 0/8] Changes surrounding IRQs and IRQ domains Lee Jones
                   ` (2 preceding siblings ...)
  2012-08-09 15:53 ` [PATCH 3/8] ARM: ux500: Identify the PRCMU as an interrupt controller Lee Jones
@ 2012-08-09 15:53 ` Lee Jones
  2012-08-14  8:20   ` Linus Walleij
  2012-08-09 15:53 ` [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain Lee Jones
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-09 15:53 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: STEricsson_nomadik_linux, linus.walleij, arnd, broonie, Lee Jones

It's understood that the AB8500 should be subordinate to the DB8500;
however, the AB8500 uses the GIC as it's interrupt controller. If
we do not specify which IRQ controller to use the default is to use
the next encountered IRQ controller as we climb the tree. This would
be the DB8500. This patch ensures the AB8500 makes use of the correct
interrupt controller.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/dbx5x0.dtsi |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index 6da7ccb..5106662 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -332,6 +332,7 @@
 			ab8500@5 {
 				compatible = "stericsson,ab8500";
 				reg = <5>; /* mailbox 5 is i2c */
+				interrupt-parent = <&intc>;
 				interrupts = <0 40 0x4>;
 				interrupt-controller;
 				#interrupt-cells = <2>;
-- 
1.7.9.5


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

* [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-09 15:53 [PATCH 0/8] Changes surrounding IRQs and IRQ domains Lee Jones
                   ` (3 preceding siblings ...)
  2012-08-09 15:53 ` [PATCH 4/8] ARM: ux500: Force AB8500 to use the GIC as its " Lee Jones
@ 2012-08-09 15:53 ` Lee Jones
  2012-08-14  8:29   ` Linus Walleij
  2012-08-09 15:53 ` [PATCH 6/8] mfd: Use interrupt-parent as IRQ controller if specified in DT Lee Jones
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-09 15:53 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	Lee Jones, Samuel Ortiz

The PRCMU has its own USB, Thermal, GPIO, Modem, HSI and RTC drivers,
amongst other things. This patch allows those subordinate devices to
use it as an interrupt controller as and when they are DT enabled.

CC: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/db8500-prcmu.c       |   54 +++++++++++++++++++++++++++++---------
 include/linux/mfd/db8500-prcmu.h |    2 ++
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 7040a00..9899b3f 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -270,6 +270,8 @@ static struct {
 	struct prcmu_fw_version version;
 } fw_info;
 
+static struct irq_domain *db8500_irq_domain;
+
 /*
  * This vector maps irq numbers to the bits in the bit field used in
  * communication with the PRCMU firmware.
@@ -2583,7 +2585,7 @@ static void prcmu_irq_mask(struct irq_data *d)
 
 	spin_lock_irqsave(&mb0_transfer.dbb_irqs_lock, flags);
 
-	mb0_transfer.req.dbb_irqs &= ~prcmu_irq_bit[d->irq - IRQ_PRCMU_BASE];
+	mb0_transfer.req.dbb_irqs &= ~prcmu_irq_bit[d->hwirq];
 
 	spin_unlock_irqrestore(&mb0_transfer.dbb_irqs_lock, flags);
 
@@ -2597,7 +2599,7 @@ static void prcmu_irq_unmask(struct irq_data *d)
 
 	spin_lock_irqsave(&mb0_transfer.dbb_irqs_lock, flags);
 
-	mb0_transfer.req.dbb_irqs |= prcmu_irq_bit[d->irq - IRQ_PRCMU_BASE];
+	mb0_transfer.req.dbb_irqs |= prcmu_irq_bit[d->hwirq];
 
 	spin_unlock_irqrestore(&mb0_transfer.dbb_irqs_lock, flags);
 
@@ -2637,9 +2639,43 @@ static char *fw_project_name(u8 project)
 	}
 }
 
+int db8500_irq_get_virq(int irq)
+{
+	return irq_create_mapping(db8500_irq_domain, irq);
+}
+EXPORT_SYMBOL_GPL(db8500_irq_get_virq);
+
+static int db8500_irq_map(struct irq_domain *d, unsigned int virq,
+				irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(virq, &prcmu_irq_chip,
+				handle_simple_irq);
+	set_irq_flags(virq, IRQF_VALID);
+
+	return 0;
+}
+
+static struct irq_domain_ops db8500_irq_ops = {
+        .map    = db8500_irq_map,
+        .xlate  = irq_domain_xlate_twocell,
+};
+
+static int db8500_irq_init(struct device_node *np)
+{
+	db8500_irq_domain = irq_domain_add_legacy(
+		np, NUM_PRCMU_WAKEUPS, IRQ_PRCMU_BASE,
+		0, &db8500_irq_ops, NULL);
+
+	if (!db8500_irq_domain) {
+		pr_err("Failed to create irqdomain\n");
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
 void __init db8500_prcmu_early_init(void)
 {
-	unsigned int i;
 	if (cpu_is_u8500v2()) {
 		void *tcpm_base = ioremap_nocache(U8500_PRCMU_TCPM_BASE, SZ_4K);
 
@@ -2683,16 +2719,6 @@ void __init db8500_prcmu_early_init(void)
 	init_completion(&mb5_transfer.work);
 
 	INIT_WORK(&mb0_transfer.mask_work, prcmu_mask_work);
-
-	/* Initalize irqs. */
-	for (i = 0; i < NUM_PRCMU_WAKEUPS; i++) {
-		unsigned int irq;
-
-		irq = IRQ_PRCMU_BASE + i;
-		irq_set_chip_and_handler(irq, &prcmu_irq_chip,
-					 handle_simple_irq);
-		set_irq_flags(irq, IRQF_VALID);
-	}
 }
 
 static void __init init_prcm_registers(void)
@@ -2999,6 +3025,8 @@ static int __devinit db8500_prcmu_probe(struct platform_device *pdev)
 		goto no_irq_return;
 	}
 
+	db8500_irq_init(np);
+
 	for (i = 0; i < ARRAY_SIZE(db8500_prcmu_devs); i++) {
 		if (!strcmp(db8500_prcmu_devs[i].name, "ab8500-core")) {
 			db8500_prcmu_devs[i].platform_data = ab8500_platdata;
diff --git a/include/linux/mfd/db8500-prcmu.h b/include/linux/mfd/db8500-prcmu.h
index b82f6ee..38494d9 100644
--- a/include/linux/mfd/db8500-prcmu.h
+++ b/include/linux/mfd/db8500-prcmu.h
@@ -571,6 +571,8 @@ u32 db8500_prcmu_read(unsigned int reg);
 void db8500_prcmu_write(unsigned int reg, u32 value);
 void db8500_prcmu_write_masked(unsigned int reg, u32 mask, u32 value);
 
+int db8500_irq_get_virq(int irq);
+
 #else /* !CONFIG_MFD_DB8500_PRCMU */
 
 static inline void db8500_prcmu_early_init(void) {}
-- 
1.7.9.5


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

* [PATCH 6/8] mfd: Use interrupt-parent as IRQ controller if specified in DT
  2012-08-09 15:53 [PATCH 0/8] Changes surrounding IRQs and IRQ domains Lee Jones
                   ` (4 preceding siblings ...)
  2012-08-09 15:53 ` [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain Lee Jones
@ 2012-08-09 15:53 ` Lee Jones
  2012-08-14  8:22   ` Linus Walleij
  2012-08-09 15:53 ` [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq Lee Jones
  2012-08-09 15:53 ` [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual Lee Jones
  7 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-09 15:53 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	Lee Jones, Samuel Ortiz

Without this patch the default behaviour is to climb the Device
Tree and use the first encountered interrupt controller. This
does not take into account if a device node has specified to use
a particular IRQ controller using the interrupt-parent property.
This patch ensures that property is adhered to.

CC: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/mfd-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 0c3a01c..bb5e753 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -97,7 +97,7 @@ static int mfd_add_device(struct device *parent, int id,
 		for_each_child_of_node(parent->of_node, np) {
 			if (of_device_is_compatible(np, cell->of_compatible)) {
 				pdev->dev.of_node = np;
-				domain = irq_find_host(parent->of_node);
+				domain = irq_find_host(np);
 				break;
 			}
 		}
-- 
1.7.9.5


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

* [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq
  2012-08-09 15:53 [PATCH 0/8] Changes surrounding IRQs and IRQ domains Lee Jones
                   ` (5 preceding siblings ...)
  2012-08-09 15:53 ` [PATCH 6/8] mfd: Use interrupt-parent as IRQ controller if specified in DT Lee Jones
@ 2012-08-09 15:53 ` Lee Jones
  2012-08-14  8:25   ` Linus Walleij
  2012-09-19  0:00   ` Samuel Ortiz
  2012-08-09 15:53 ` [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual Lee Jones
  7 siblings, 2 replies; 50+ messages in thread
From: Lee Jones @ 2012-08-09 15:53 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	Lee Jones, Samuel Ortiz

Before the AB8500 had its own IRQ domain, the IRQ handler would take
the fired local IRQ (hwirq) and add it to the irq_base to convert it
to an IRQ number which Linux would understand (virq). However, the
IRQ base is not always used anymore since we can make use of Linear
domains. It's better to use the AB8500 hwirq -> virq mapping helper
function to convert them instead. That's what we do here.

CC: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/ab8500-core.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
index 0c5b70f..71a7757 100644
--- a/drivers/mfd/ab8500-core.c
+++ b/drivers/mfd/ab8500-core.c
@@ -501,8 +501,9 @@ static irqreturn_t ab8500_irq(int irq, void *dev)
 		do {
 			int bit = __ffs(value);
 			int line = i * 8 + bit;
+			int virq = ab8500_irq_get_virq(ab8500, line);
 
-			handle_nested_irq(ab8500->irq_base + line);
+			handle_nested_irq(virq);
 			value &= ~(1 << bit);
 
 		} while (value);
-- 
1.7.9.5


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

* [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
  2012-08-09 15:53 [PATCH 0/8] Changes surrounding IRQs and IRQ domains Lee Jones
                   ` (6 preceding siblings ...)
  2012-08-09 15:53 ` [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq Lee Jones
@ 2012-08-09 15:53 ` Lee Jones
  2012-08-14  8:31   ` Linus Walleij
  7 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-09 15:53 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	Lee Jones, Dmitry Torokhov, linux-input

There was a plan to place ab8500_irq_get_virq() calls in each AB8500
child device prior to requesting an IRQ, but as we're no longer using
Device Tree to collect our IRQ numbers, it's actually better to allow
the core to do this during device registration time. So the IRQ number
we pull from its resource has already been converted to a virtual IRQ.

CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: linux-input@vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/input/misc/ab8500-ponkey.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/ab8500-ponkey.c b/drivers/input/misc/ab8500-ponkey.c
index f06231b..84ec691 100644
--- a/drivers/input/misc/ab8500-ponkey.c
+++ b/drivers/input/misc/ab8500-ponkey.c
@@ -74,8 +74,8 @@ static int __devinit ab8500_ponkey_probe(struct platform_device *pdev)
 
 	ponkey->idev = input;
 	ponkey->ab8500 = ab8500;
-	ponkey->irq_dbf = ab8500_irq_get_virq(ab8500, irq_dbf);
-	ponkey->irq_dbr = ab8500_irq_get_virq(ab8500, irq_dbr);
+	ponkey->irq_dbf = irq_dbf;
+	ponkey->irq_dbr = irq_dbr;
 
 	input->name = "AB8500 POn(PowerOn) Key";
 	input->dev.parent = &pdev->dev;
-- 
1.7.9.5


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

* Re: [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF
  2012-08-09 15:53 ` [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF Lee Jones
@ 2012-08-09 16:20   ` Rob Herring
  2012-08-09 19:44     ` Lee Jones
  2012-08-09 19:53   ` Rob Herring
  2012-08-14  8:17   ` Linus Walleij
  2 siblings, 1 reply; 50+ messages in thread
From: Rob Herring @ 2012-08-09 16:20 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, linus.walleij, arnd,
	devicetree-discuss, broonie, STEricsson_nomadik_linux

On 08/09/2012 10:53 AM, Lee Jones wrote:
> of_irq_find_parent is a handy function to use outside the confines of
> the Open Firmware subsystem. One such use-case is when the IRQ Domain
> wishes to find an IRQ domain for a given device node. Currently it can
> not take any notice of the 'interrupt-parent' property. Instead it
> just uses the first IRQ controller as it climbs the Device Tree. If
> we were to use this as a precursor the resultant controller is more
> likely to be correct.

Where are you using this? I don't have all the patches in the series.

Rob

> 
> CC: Rob Herring <rob.herring@calxeda.com>
> CC: devicetree-discuss@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  include/linux/of_irq.h |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index 1717cd9..b8e2411 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -83,6 +83,11 @@ static inline unsigned int irq_of_parse_and_map(struct device_node *dev,
>  {
>  	return 0;
>  }
> +
> +static inline void *of_irq_find_parent(struct device_node *child)
> +{
> +	return NULL;
> +}
>  #endif /* !CONFIG_OF */
>  
>  #endif /* __OF_IRQ_H */
> 


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

* Re: [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF
  2012-08-09 16:20   ` Rob Herring
@ 2012-08-09 19:44     ` Lee Jones
  0 siblings, 0 replies; 50+ messages in thread
From: Lee Jones @ 2012-08-09 19:44 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-arm-kernel, linux-kernel, linus.walleij, arnd,
	devicetree-discuss, broonie, STEricsson_nomadik_linux

On Thu, Aug 09, 2012 at 11:20:54AM -0500, Rob Herring wrote:
> On 08/09/2012 10:53 AM, Lee Jones wrote:
> > of_irq_find_parent is a handy function to use outside the confines of
> > the Open Firmware subsystem. One such use-case is when the IRQ Domain
> > wishes to find an IRQ domain for a given device node. Currently it can
> > not take any notice of the 'interrupt-parent' property. Instead it
> > just uses the first IRQ controller as it climbs the Device Tree. If
> > we were to use this as a precursor the resultant controller is more
> > likely to be correct.
> 
> Where are you using this? I don't have all the patches in the series.

Sorry Rob. Here:

https://lkml.org/lkml/2012/8/9/373

> > CC: Rob Herring <rob.herring@calxeda.com>
> > CC: devicetree-discuss@lists.ozlabs.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  include/linux/of_irq.h |    5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> > index 1717cd9..b8e2411 100644
> > --- a/include/linux/of_irq.h
> > +++ b/include/linux/of_irq.h
> > @@ -83,6 +83,11 @@ static inline unsigned int irq_of_parse_and_map(struct device_node *dev,
> >  {
> >  	return 0;
> >  }
> > +
> > +static inline void *of_irq_find_parent(struct device_node *child)
> > +{
> > +	return NULL;
> > +}
> >  #endif /* !CONFIG_OF */
> >  
> >  #endif /* __OF_IRQ_H */
> > 
> 

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF
  2012-08-09 15:53 ` [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF Lee Jones
  2012-08-09 16:20   ` Rob Herring
@ 2012-08-09 19:53   ` Rob Herring
  2012-08-14  8:17   ` Linus Walleij
  2 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2012-08-09 19:53 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, linus.walleij, arnd,
	devicetree-discuss, broonie, Rob Herring,
	STEricsson_nomadik_linux

On 08/09/2012 10:53 AM, Lee Jones wrote:
> of_irq_find_parent is a handy function to use outside the confines of
> the Open Firmware subsystem. One such use-case is when the IRQ Domain
> wishes to find an IRQ domain for a given device node. Currently it can
> not take any notice of the 'interrupt-parent' property. Instead it
> just uses the first IRQ controller as it climbs the Device Tree. If
> we were to use this as a precursor the resultant controller is more
> likely to be correct.
> 
> CC: Rob Herring <rob.herring@calxeda.com>
> CC: devicetree-discuss@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Rob Herring <rob.herring@calxeda.com>

Rob

> ---
>  include/linux/of_irq.h |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index 1717cd9..b8e2411 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -83,6 +83,11 @@ static inline unsigned int irq_of_parse_and_map(struct device_node *dev,
>  {
>  	return 0;
>  }
> +
> +static inline void *of_irq_find_parent(struct device_node *child)
> +{
> +	return NULL;
> +}
>  #endif /* !CONFIG_OF */
>  
>  #endif /* __OF_IRQ_H */
> 


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

* Re: [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF
  2012-08-09 15:53 ` [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF Lee Jones
  2012-08-09 16:20   ` Rob Herring
  2012-08-09 19:53   ` Rob Herring
@ 2012-08-14  8:17   ` Linus Walleij
  2 siblings, 0 replies; 50+ messages in thread
From: Linus Walleij @ 2012-08-14  8:17 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Rob Herring, devicetree-discuss

On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:

> of_irq_find_parent is a handy function to use outside the confines of
> the Open Firmware subsystem. One such use-case is when the IRQ Domain
> wishes to find an IRQ domain for a given device node. Currently it can
> not take any notice of the 'interrupt-parent' property. Instead it
> just uses the first IRQ controller as it climbs the Device Tree. If
> we were to use this as a precursor the resultant controller is more
> likely to be correct.
>
> CC: Rob Herring <rob.herring@calxeda.com>
> CC: devicetree-discuss@lists.ozlabs.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

This is clever.

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified
  2012-08-09 15:53 ` [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified Lee Jones
@ 2012-08-14  8:19   ` Linus Walleij
  2012-08-31  9:44     ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Linus Walleij @ 2012-08-14  8:19 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Benjamin Herrenschmidt,
	Grant Likely

On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:

> irq_find_host() currently ignores the 'interrupt-parent' property
> even if it's specified in the Device Tree. Meaning that a node can
> match to a domain in its hierarchy even if it doesn't belong to it.
> By searching for the parent first using of_irq_find_parent() we
> insist that the 'interrupt-parent' property is taken into account
> ensuring a greater chance of returning the correct domain.
>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

This (with 1/8) is the right solution. Thanks, and I think
Mark may want to look at this too since I recognize he asked
you to work in this direction.

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH 3/8] ARM: ux500: Identify the PRCMU as an interrupt controller
  2012-08-09 15:53 ` [PATCH 3/8] ARM: ux500: Identify the PRCMU as an interrupt controller Lee Jones
@ 2012-08-14  8:19   ` Linus Walleij
  0 siblings, 0 replies; 50+ messages in thread
From: Linus Walleij @ 2012-08-14  8:19 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie

On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:

> We're just about to provide the DB8500-PRCMU with its own IRQ domain,
> so that its subordinate drivers can use it as an interrupt controller.
> It's obligatory for all IRQ controllers to reference themselves as
> such from its own node in Device Tree. This patch does just that.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH 4/8] ARM: ux500: Force AB8500 to use the GIC as its interrupt controller
  2012-08-09 15:53 ` [PATCH 4/8] ARM: ux500: Force AB8500 to use the GIC as its " Lee Jones
@ 2012-08-14  8:20   ` Linus Walleij
  0 siblings, 0 replies; 50+ messages in thread
From: Linus Walleij @ 2012-08-14  8:20 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie

On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:

> It's understood that the AB8500 should be subordinate to the DB8500;
> however, the AB8500 uses the GIC as it's interrupt controller. If
> we do not specify which IRQ controller to use the default is to use
> the next encountered IRQ controller as we climb the tree. This would
> be the DB8500. This patch ensures the AB8500 makes use of the correct
> interrupt controller.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH 6/8] mfd: Use interrupt-parent as IRQ controller if specified in DT
  2012-08-09 15:53 ` [PATCH 6/8] mfd: Use interrupt-parent as IRQ controller if specified in DT Lee Jones
@ 2012-08-14  8:22   ` Linus Walleij
  0 siblings, 0 replies; 50+ messages in thread
From: Linus Walleij @ 2012-08-14  8:22 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Samuel Ortiz, Rob Herring

On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Without this patch the default behaviour is to climb the Device
> Tree and use the first encountered interrupt controller. This
> does not take into account if a device node has specified to use
> a particular IRQ controller using the interrupt-parent property.
> This patch ensures that property is adhered to.
>
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Mark & Rob may want to check this out too.

Yours,
Linus Walleij

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

* Re: [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq
  2012-08-09 15:53 ` [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq Lee Jones
@ 2012-08-14  8:25   ` Linus Walleij
  2012-09-19  0:00   ` Samuel Ortiz
  1 sibling, 0 replies; 50+ messages in thread
From: Linus Walleij @ 2012-08-14  8:25 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Samuel Ortiz

On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Before the AB8500 had its own IRQ domain, the IRQ handler would take
> the fired local IRQ (hwirq) and add it to the irq_base to convert it
> to an IRQ number which Linux would understand (virq). However, the
> IRQ base is not always used anymore since we can make use of Linear
> domains. It's better to use the AB8500 hwirq -> virq mapping helper
> function to convert them instead. That's what we do here.
>
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

This looks better that what was in there before so
Acked-by: Linus Walleij <linus.walleij@linaro.org>

However:

> @@ -501,8 +501,9 @@ static irqreturn_t ab8500_irq(int irq, void *dev)
>                 do {
>                         int bit = __ffs(value);
>                         int line = i * 8 + bit;
> +                       int virq = ab8500_irq_get_virq(ab8500, line);
>
> -                       handle_nested_irq(ab8500->irq_base + line);
> +                       handle_nested_irq(virq);
>                         value &= ~(1 << bit);

Still this ab8500_irq_get_virq() business. But is this a static local function
in the ab8500-core.c now? Then it's fine, it's the kernel-wide interface
that is the problem.

Yours,
Linus Walleij

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-09 15:53 ` [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain Lee Jones
@ 2012-08-14  8:29   ` Linus Walleij
  2012-08-14  9:42     ` Arnd Bergmann
                       ` (2 more replies)
  0 siblings, 3 replies; 50+ messages in thread
From: Linus Walleij @ 2012-08-14  8:29 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Samuel Ortiz

On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:

> +static struct irq_domain *db8500_irq_domain;

So this is a good idea.

> +int db8500_irq_get_virq(int irq);

And I'm sceptic about this business. Why isn't this physical-to virtual
mapping business confined to the core MFD driver? But enlighten me.

Yours,
Linus Walleij

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

* Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
  2012-08-09 15:53 ` [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual Lee Jones
@ 2012-08-14  8:31   ` Linus Walleij
  2012-08-21  9:23     ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Linus Walleij @ 2012-08-14  8:31 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Dmitry Torokhov, linux-input

On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:

> There was a plan to place ab8500_irq_get_virq() calls in each AB8500
> child device prior to requesting an IRQ, but as we're no longer using
> Device Tree to collect our IRQ numbers, it's actually better to allow
> the core to do this during device registration time. So the IRQ number
> we pull from its resource has already been converted to a virtual IRQ.
>
> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> CC: linux-input@vger.kernel.org
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

This is looking good, I guess you need all patches to go in at the
same time so Dmitry's ACK is required.

FWIW:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

BTW: this makes me suspect that the public ab8500_irq_get_virq()
interface can be *deleted* and the function made static in the
AB8500 driver, right?

Yours,
Linus Walleij

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-14  8:29   ` Linus Walleij
@ 2012-08-14  9:42     ` Arnd Bergmann
  2012-08-14 10:44       ` Linus Walleij
  2012-08-20  9:36     ` Lee Jones
  2012-08-20 10:49     ` Lee Jones
  2 siblings, 1 reply; 50+ messages in thread
From: Arnd Bergmann @ 2012-08-14  9:42 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Lee Jones, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, broonie, Samuel Ortiz

On Tuesday 14 August 2012, Linus Walleij wrote:
> 
> On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > +static struct irq_domain *db8500_irq_domain;
> 
> So this is a good idea.
> 
> > +int db8500_irq_get_virq(int irq);
> 
> And I'm sceptic about this business. Why isn't this physical-to virtual
> mapping business confined to the core MFD driver? But enlighten me.
> 

I believe MFD does not (yet) know about IRQ domains. The wm8994
and 88pm80x drivers do this slightly different by exporting
a foo_request_irq() function that performs the mapping.

Traditionally, an MFD would add an offset to the local IRQ number
to put the VIRQ into the IRQ resource, but this doesn't work when
you have domains other than legacy.

	Arnd

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-14  9:42     ` Arnd Bergmann
@ 2012-08-14 10:44       ` Linus Walleij
  2012-08-20  8:36         ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Linus Walleij @ 2012-08-14 10:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Lee Jones, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, broonie, Samuel Ortiz

On Tue, Aug 14, 2012 at 11:42 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 14 August 2012, Linus Walleij wrote:
>> On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> > +int db8500_irq_get_virq(int irq);
>>
>> And I'm sceptic about this business. Why isn't this physical-to virtual
>> mapping business confined to the core MFD driver? But enlighten me.
>
> Traditionally, an MFD would add an offset to the local IRQ number
> to put the VIRQ into the IRQ resource, but this doesn't work when
> you have domains other than legacy.

Yes but I think I saw this other patch set from Lee, hitting
irqdomain, OF and MFD to actually fix this ... or did I get
it wrong?

Yours,
Linus Walleij

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-14 10:44       ` Linus Walleij
@ 2012-08-20  8:36         ` Lee Jones
  2012-08-20 12:10           ` Mark Brown
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-20  8:36 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, broonie, Samuel Ortiz

On Tue, Aug 14, 2012 at 12:44:37PM +0200, Linus Walleij wrote:
> On Tue, Aug 14, 2012 at 11:42 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 14 August 2012, Linus Walleij wrote:
> >> On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >>
> >> > +int db8500_irq_get_virq(int irq);
> >>
> >> And I'm sceptic about this business. Why isn't this physical-to virtual
> >> mapping business confined to the core MFD driver? But enlighten me.
> >
> > Traditionally, an MFD would add an offset to the local IRQ number
> > to put the VIRQ into the IRQ resource, but this doesn't work when
> > you have domains other than legacy.
> 
> Yes but I think I saw this other patch set from Lee, hitting
> irqdomain, OF and MFD to actually fix this ... or did I get
> it wrong?

No, you're not wrong.

Historically (in my patches) xb8500_irq_get_virq() was used by drivers
to obtain a VIRQ when not using Device Tree. Now the MFD core handles
conversion there is little requirement for it. In fact there are no
more users for db8500_irq_get_virq() and only one user for 
ab8500_irq_get_virq() and that's itself. I guess we can rid them and
call irq_get_mapping() directly instead.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-14  8:29   ` Linus Walleij
  2012-08-14  9:42     ` Arnd Bergmann
@ 2012-08-20  9:36     ` Lee Jones
  2012-08-20 10:49     ` Lee Jones
  2 siblings, 0 replies; 50+ messages in thread
From: Lee Jones @ 2012-08-20  9:36 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Samuel Ortiz

On Tue, Aug 14, 2012 at 10:29:14AM +0200, Linus Walleij wrote:
> On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > +static struct irq_domain *db8500_irq_domain;
> 
> So this is a good idea.

Did you mean this, or did you mean that it's _not_ a good idea?

If the latter is true, where would you prefer to see it?

> > +int db8500_irq_get_virq(int irq);
> 
> And I'm sceptic about this business. Why isn't this physical-to virtual
> mapping business confined to the core MFD driver? But enlighten me.
> 
> Yours,
> Linus Walleij

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-14  8:29   ` Linus Walleij
  2012-08-14  9:42     ` Arnd Bergmann
  2012-08-20  9:36     ` Lee Jones
@ 2012-08-20 10:49     ` Lee Jones
  2 siblings, 0 replies; 50+ messages in thread
From: Lee Jones @ 2012-08-20 10:49 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Samuel Ortiz

From: Lee Jones <lee.jones@linaro.org>
Date: Fri, 3 Aug 2012 15:45:50 +0100
Subject: [PATCH 1/1] mfd: Provide the PRCMU with its own IRQ domain

The PRCMU has its own USB, Thermal, GPIO, Modem, HSI and RTC drivers,
amongst other things. This patch allows those subordinate devices to
use it as an interrupt controller as and when they are DT enabled.

CC: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/db8500-prcmu.c |   48 ++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 7040a00..de09113 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -270,6 +270,8 @@ static struct {
 	struct prcmu_fw_version version;
 } fw_info;
 
+static struct irq_domain *db8500_irq_domain;
+
 /*
  * This vector maps irq numbers to the bits in the bit field used in
  * communication with the PRCMU firmware.
@@ -2583,7 +2585,7 @@ static void prcmu_irq_mask(struct irq_data *d)
 
 	spin_lock_irqsave(&mb0_transfer.dbb_irqs_lock, flags);
 
-	mb0_transfer.req.dbb_irqs &= ~prcmu_irq_bit[d->irq - IRQ_PRCMU_BASE];
+	mb0_transfer.req.dbb_irqs &= ~prcmu_irq_bit[d->hwirq];
 
 	spin_unlock_irqrestore(&mb0_transfer.dbb_irqs_lock, flags);
 
@@ -2597,7 +2599,7 @@ static void prcmu_irq_unmask(struct irq_data *d)
 
 	spin_lock_irqsave(&mb0_transfer.dbb_irqs_lock, flags);
 
-	mb0_transfer.req.dbb_irqs |= prcmu_irq_bit[d->irq - IRQ_PRCMU_BASE];
+	mb0_transfer.req.dbb_irqs |= prcmu_irq_bit[d->hwirq];
 
 	spin_unlock_irqrestore(&mb0_transfer.dbb_irqs_lock, flags);
 
@@ -2637,9 +2639,37 @@ static char *fw_project_name(u8 project)
 	}
 }
 
+static int db8500_irq_map(struct irq_domain *d, unsigned int virq,
+				irq_hw_number_t hwirq)
+{
+	irq_set_chip_and_handler(virq, &prcmu_irq_chip,
+				handle_simple_irq);
+	set_irq_flags(virq, IRQF_VALID);
+
+	return 0;
+}
+
+static struct irq_domain_ops db8500_irq_ops = {
+        .map    = db8500_irq_map,
+        .xlate  = irq_domain_xlate_twocell,
+};
+
+static int db8500_irq_init(struct device_node *np)
+{
+	db8500_irq_domain = irq_domain_add_legacy(
+		np, NUM_PRCMU_WAKEUPS, IRQ_PRCMU_BASE,
+		0, &db8500_irq_ops, NULL);
+
+	if (!db8500_irq_domain) {
+		pr_err("Failed to create irqdomain\n");
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
 void __init db8500_prcmu_early_init(void)
 {
-	unsigned int i;
 	if (cpu_is_u8500v2()) {
 		void *tcpm_base = ioremap_nocache(U8500_PRCMU_TCPM_BASE, SZ_4K);
 
@@ -2683,16 +2713,6 @@ void __init db8500_prcmu_early_init(void)
 	init_completion(&mb5_transfer.work);
 
 	INIT_WORK(&mb0_transfer.mask_work, prcmu_mask_work);
-
-	/* Initalize irqs. */
-	for (i = 0; i < NUM_PRCMU_WAKEUPS; i++) {
-		unsigned int irq;
-
-		irq = IRQ_PRCMU_BASE + i;
-		irq_set_chip_and_handler(irq, &prcmu_irq_chip,
-					 handle_simple_irq);
-		set_irq_flags(irq, IRQF_VALID);
-	}
 }
 
 static void __init init_prcm_registers(void)
@@ -2999,6 +3019,8 @@ static int __devinit db8500_prcmu_probe(struct platform_device *pdev)
 		goto no_irq_return;
 	}
 
+	db8500_irq_init(np);
+
 	for (i = 0; i < ARRAY_SIZE(db8500_prcmu_devs); i++) {
 		if (!strcmp(db8500_prcmu_devs[i].name, "ab8500-core")) {
 			db8500_prcmu_devs[i].platform_data = ab8500_platdata;
-- 
1.7.9.5

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-20  8:36         ` Lee Jones
@ 2012-08-20 12:10           ` Mark Brown
  2012-08-20 12:55             ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-08-20 12:10 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]

On Mon, Aug 20, 2012 at 09:36:43AM +0100, Lee Jones wrote:
> On Tue, Aug 14, 2012 at 12:44:37PM +0200, Linus Walleij wrote:

> > Yes but I think I saw this other patch set from Lee, hitting
> > irqdomain, OF and MFD to actually fix this ... or did I get
> > it wrong?

> No, you're not wrong.

> Historically (in my patches) xb8500_irq_get_virq() was used by drivers
> to obtain a VIRQ when not using Device Tree. Now the MFD core handles
> conversion there is little requirement for it. In fact there are no
> more users for db8500_irq_get_virq() and only one user for 
> ab8500_irq_get_virq() and that's itself. I guess we can rid them and
> call irq_get_mapping() directly instead.

Oh dear.  Unfortunately whoever added this support to the MFD core did
so in such a manner that it's only supported for device tree systems
and only for devices which express the MFD cells as device tree nodes
which means that most devices can't it - db8500 has got a reasonably
unusual combination there.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-20 12:10           ` Mark Brown
@ 2012-08-20 12:55             ` Lee Jones
  2012-08-20 16:29               ` Mark Brown
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-20 12:55 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

On Mon, Aug 20, 2012 at 01:10:55PM +0100, Mark Brown wrote:
> On Mon, Aug 20, 2012 at 09:36:43AM +0100, Lee Jones wrote:
> > On Tue, Aug 14, 2012 at 12:44:37PM +0200, Linus Walleij wrote:
> 
> > > Yes but I think I saw this other patch set from Lee, hitting
> > > irqdomain, OF and MFD to actually fix this ... or did I get
> > > it wrong?
> 
> > No, you're not wrong.
> 
> > Historically (in my patches) xb8500_irq_get_virq() was used by drivers
> > to obtain a VIRQ when not using Device Tree. Now the MFD core handles
> > conversion there is little requirement for it. In fact there are no
> > more users for db8500_irq_get_virq() and only one user for 
> > ab8500_irq_get_virq() and that's itself. I guess we can rid them and
> > call irq_get_mapping() directly instead.
> 
> Oh dear.  Unfortunately whoever added this support to the MFD core did
> so in such a manner that it's only supported for device tree systems
> and only for devices which express the MFD cells as device tree nodes
> which means that most devices can't it - db8500 has got a reasonably
> unusual combination there.

Right, that was the initial intention. It would be a trivial semantic
change if drivers without DT support wished to use the functionality
though. However, the only examples I found of a non-DT enabled driver
that could make good use of it in order to strip out some cruft would
be the Arizona and one of the Samsung drivers, and they each have
their own hand-rolled methods of hwirq -> virq conversion now, so any
change to support them would result in multiple invocations of
irq_create_mapping which would likely cause breakage.
 
-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-20 12:55             ` Lee Jones
@ 2012-08-20 16:29               ` Mark Brown
  2012-08-20 16:49                 ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-08-20 16:29 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

On Mon, Aug 20, 2012 at 01:55:32PM +0100, Lee Jones wrote:

> Right, that was the initial intention. It would be a trivial semantic
> change if drivers without DT support wished to use the functionality
> though. However, the only examples I found of a non-DT enabled driver
> that could make good use of it in order to strip out some cruft would
> be the Arizona and one of the Samsung drivers, and they each have

All of the regmap devices could use this.

> their own hand-rolled methods of hwirq -> virq conversion now, so any
> change to support them would result in multiple invocations of
> irq_create_mapping which would likely cause breakage.

Multiple calls to irq_create_mapping() are totally fine.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-20 16:29               ` Mark Brown
@ 2012-08-20 16:49                 ` Lee Jones
  2012-08-20 17:51                   ` Mark Brown
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-20 16:49 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

On Mon, Aug 20, 2012 at 05:29:23PM +0100, Mark Brown wrote:
> On Mon, Aug 20, 2012 at 01:55:32PM +0100, Lee Jones wrote:
> 
> > Right, that was the initial intention. It would be a trivial semantic
> > change if drivers without DT support wished to use the functionality
> > though. However, the only examples I found of a non-DT enabled driver
> > that could make good use of it in order to strip out some cruft would
> > be the Arizona and one of the Samsung drivers, and they each have
> 
> All of the regmap devices could use this.

Only if they have linear domains and don't support DT.

If they don't have linear domains there's no point, if they support DT
then they can use it as it is.

> > their own hand-rolled methods of hwirq -> virq conversion now, so any
> > change to support them would result in multiple invocations of
> > irq_create_mapping which would likely cause breakage.
> 
> Multiple calls to irq_create_mapping() are totally fine.

Sorry, perhaps I wasn't clear.

<mfd_parent_device>_probe()
|
+-> mfd_add_device()
    |
    +-> res->start = res->end = irq_create_mapping(res->start); // Now a VIRQ


<mfd_child_device>_probe()
|
+-> <mfd_parent_device>_irq_get_virq(res->start) // Ahhh, double VIRQ conversion

To stop being DT dependent we'd need to remove all hand-rolled stuff 
that these drivers are currently using, or else they will be attempting
to convert and already converted VIRQ value. 

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-20 16:49                 ` Lee Jones
@ 2012-08-20 17:51                   ` Mark Brown
  2012-08-21  8:56                     ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-08-20 17:51 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

On Mon, Aug 20, 2012 at 05:49:50PM +0100, Lee Jones wrote:
> On Mon, Aug 20, 2012 at 05:29:23PM +0100, Mark Brown wrote:

> > All of the regmap devices could use this.

> Only if they have linear domains and don't support DT.

Neither of those restrictions really apply...

> If they don't have linear domains there's no point, if they support DT
> then they can use it as it is.

All this stuff just works for any IRQ domain type, there's no
requirement for a particular one.  It's not urgently exciting for legacy
domains but it's not harmful either and pushes all the handling of this
stuff out of the MFD core and into the irqdomain code which is
definitely an abstraction win.

As far as DT goes supporting DT isn't enough, the current code will only
work on systems that actively use DT and do so using a particular style
of binding.  Since the majority of Linux architectures don't support DT
at all and it's not universally deployed on those that do this means
that generic drivers can't rely on it, and even drivers that use DT
might not be using the binding pattern which the framework code now
uses.

Indeed now that I think about the above isn't this going to be actively
harmful for generic drivers if they use this pattern for their bindings?
On DT systems the framework will unconditionally do the mapping but on
others no mapping will be done.  The function drivers don't know if the
mapping has been performed or not.

Unless I'm missing something here we need to fix this before the merge
window...

> To stop being DT dependent we'd need to remove all hand-rolled stuff 
> that these drivers are currently using, or else they will be attempting
> to convert and already converted VIRQ value. 

Well, yes - using framework code would be the goal here...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-20 17:51                   ` Mark Brown
@ 2012-08-21  8:56                     ` Lee Jones
  2012-08-21  9:50                       ` Mark Brown
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-21  8:56 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

> > If they don't have linear domains there's no point, if they support DT
> > then they can use it as it is.
> 
> All this stuff just works for any IRQ domain type, there's no
> requirement for a particular one.  It's not urgently exciting for legacy
> domains but it's not harmful either and pushes all the handling of this
> stuff out of the MFD core and into the irqdomain code which is
> definitely an abstraction win.

Wherever we do this from to be able to obtain the IRQ domain pointer, 
which is where I'm currently struggling. Our options are:

- Call into a helper function based in the IRQ controller from each 
child device. In turn the IRQ controller will be responsible for creating
the mapping necessary to obtain a virq. Using this method the child
device doesn't need to know if we're using an IRQ domain or not, or
whether we're using Device Tree or not. The drawback is that each child
device will be required to call the helper function prior to requesting
an IRQ. 

- If we're only talking MFD here, we can handle this stuff in the MFD
core, but we need more information. The IRQ domain subsystem only allows
domain look-up via a Device Tree node, so we need to get our hands on
the domain another way in the case of non-DT enabled devices. Either we
add another parameter to mfd_add_device(irq_domain, ...), or we
standardise the 'irq_domain' variable name and use:
        irq_domain = container_of(parent, struct irq_domain, irq_domain);

- I know that you have interest in pushing the functionality into the
IRQ domain subsystem, but I'm struggling to see how. It's calling into
the IRQ domain where we're seeing issues in the first place, specifically
irq_create_mapping(). How about if we passed 'irq_domain' as a parameter
when requesting the IRQ? That way we can pass the correct IRQ without
worry of conversion. If 'irq_domain' is !NULL the IRQ management subsystem
can do the necessary conversions. If 'irq_domain' is NULL it continues to
use the requested IRQ as a virq.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
  2012-08-14  8:31   ` Linus Walleij
@ 2012-08-21  9:23     ` Lee Jones
  2012-08-21 16:42       ` Dmitry Torokhov
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-21  9:23 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Dmitry Torokhov, linux-input

On Tue, Aug 14, 2012 at 10:31:08AM +0200, Linus Walleij wrote:
> On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > There was a plan to place ab8500_irq_get_virq() calls in each AB8500
> > child device prior to requesting an IRQ, but as we're no longer using
> > Device Tree to collect our IRQ numbers, it's actually better to allow
> > the core to do this during device registration time. So the IRQ number
> > we pull from its resource has already been converted to a virtual IRQ.
> >
> > CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > CC: linux-input@vger.kernel.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> This is looking good, I guess you need all patches to go in at the
> same time so Dmitry's ACK is required.

Yep, just waiting for that now.
 
> FWIW:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> BTW: this makes me suspect that the public ab8500_irq_get_virq()
> interface can be *deleted* and the function made static in the
> AB8500 driver, right?

Right. Already taken care of.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-21  8:56                     ` Lee Jones
@ 2012-08-21  9:50                       ` Mark Brown
  2012-08-21 10:54                         ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-08-21  9:50 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]

On Tue, Aug 21, 2012 at 09:56:19AM +0100, Lee Jones wrote:

> Wherever we do this from to be able to obtain the IRQ domain pointer, 
> which is where I'm currently struggling. Our options are:

> - If we're only talking MFD here, we can handle this stuff in the MFD
> core, but we need more information. The IRQ domain subsystem only allows
> domain look-up via a Device Tree node, so we need to get our hands on

What makes you say this?  This is just a convenience for finding a
domain, irqdomains are *completely* indepentant of device tree.

> the domain another way in the case of non-DT enabled devices. Either we
> add another parameter to mfd_add_device(irq_domain, ...), or we
> standardise the 'irq_domain' variable name and use:
>         irq_domain = container_of(parent, struct irq_domain, irq_domain);

Passing the domain into mfd seems like the obvious solution here - it's
exactly what we currently do for legacy stuff (where we pass in the irq
base), ideally what we'd end up with over time is that we could just
remove the irq_base parameter entirely as everything would in time be
moved over to using irqdomains.

> - I know that you have interest in pushing the functionality into the
> IRQ domain subsystem, but I'm struggling to see how. It's calling into
> the IRQ domain where we're seeing issues in the first place, specifically
> irq_create_mapping(). How about if we passed 'irq_domain' as a parameter
> when requesting the IRQ? That way we can pass the correct IRQ without
> worry of conversion. If 'irq_domain' is !NULL the IRQ management subsystem
> can do the necessary conversions. If 'irq_domain' is NULL it continues to
> use the requested IRQ as a virq.

This is totally orthogonal to doing the mapping in the MFD subsystem
which is the issue here.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-21  9:50                       ` Mark Brown
@ 2012-08-21 10:54                         ` Lee Jones
  2012-08-21 11:03                           ` Mark Brown
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-21 10:54 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

On Tue, Aug 21, 2012 at 10:50:27AM +0100, Mark Brown wrote:
> On Tue, Aug 21, 2012 at 09:56:19AM +0100, Lee Jones wrote:
> 
> > Wherever we do this from to be able to obtain the IRQ domain pointer, 
> > which is where I'm currently struggling. Our options are:
> 
> > - If we're only talking MFD here, we can handle this stuff in the MFD
> > core, but we need more information. The IRQ domain subsystem only allows
> > domain look-up via a Device Tree node, so we need to get our hands on
> 
> What makes you say this?  This is just a convenience for finding a
> domain, irqdomains are *completely* indepentant of device tree.

How can you say that? I think you mean _can_ be independent of DT. If
that's what you mean then yes, that's true. All I'm saying is we need
another way to get hold of the domain, because the only way to obtain
it without having direct access is via a device node.
 
> > the domain another way in the case of non-DT enabled devices. Either we
> > add another parameter to mfd_add_device(irq_domain, ...), or we
> > standardise the 'irq_domain' variable name and use:
> >         irq_domain = container_of(parent, struct irq_domain, irq_domain);
> 
> Passing the domain into mfd seems like the obvious solution here - it's
> exactly what we currently do for legacy stuff (where we pass in the irq
> base), ideally what we'd end up with over time is that we could just
> remove the irq_base parameter entirely as everything would in time be
> moved over to using irqdomains.

Right. That's fine (and easy) then. You threw me when you said you wanted
it handled higher-up in the framework, as I didn't see how we could do 
this in the irqdomain itself.
 
> > - I know that you have interest in pushing the functionality into the
> > IRQ domain subsystem, but I'm struggling to see how. It's calling into
> > the IRQ domain where we're seeing issues in the first place, specifically
> > irq_create_mapping(). How about if we passed 'irq_domain' as a parameter
> > when requesting the IRQ? That way we can pass the correct IRQ without
> > worry of conversion. If 'irq_domain' is !NULL the IRQ management subsystem
> > can do the necessary conversions. If 'irq_domain' is NULL it continues to
> > use the requested IRQ as a virq.
> 
> This is totally orthogonal to doing the mapping in the MFD subsystem
> which is the issue here.

Again, I only mentioned this because you said you wanted it to be handled
by the irqdomain.

I'll code up the second suggestion now.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-21 10:54                         ` Lee Jones
@ 2012-08-21 11:03                           ` Mark Brown
  2012-08-21 12:02                             ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-08-21 11:03 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]

On Tue, Aug 21, 2012 at 11:54:14AM +0100, Lee Jones wrote:
> On Tue, Aug 21, 2012 at 10:50:27AM +0100, Mark Brown wrote:

> > What makes you say this?  This is just a convenience for finding a
> > domain, irqdomains are *completely* indepentant of device tree.

> How can you say that? I think you mean _can_ be independent of DT. If
> that's what you mean then yes, that's true. All I'm saying is we need

No, I really mean what I'm saying.  Device tree builds on irqdomains,
not the other way around.

> another way to get hold of the domain, because the only way to obtain
> it without having direct access is via a device node.

This doesn't actually hold.

> > > - I know that you have interest in pushing the functionality into the
> > > IRQ domain subsystem, but I'm struggling to see how. It's calling into
> > > the IRQ domain where we're seeing issues in the first place, specifically
> > > irq_create_mapping(). How about if we passed 'irq_domain' as a parameter
> > > when requesting the IRQ? That way we can pass the correct IRQ without
> > > worry of conversion. If 'irq_domain' is !NULL the IRQ management subsystem
> > > can do the necessary conversions. If 'irq_domain' is NULL it continues to
> > > use the requested IRQ as a virq.

> > This is totally orthogonal to doing the mapping in the MFD subsystem
> > which is the issue here.

> Again, I only mentioned this because you said you wanted it to be handled
> by the irqdomain.

The *mapping* should be being handled in irqdomain.

> I'll code up the second suggestion now.

I've already done this.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-21 11:03                           ` Mark Brown
@ 2012-08-21 12:02                             ` Lee Jones
  2012-08-21 16:52                               ` Mark Brown
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-21 12:02 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

On Tue, Aug 21, 2012 at 12:03:29PM +0100, Mark Brown wrote:
> On Tue, Aug 21, 2012 at 11:54:14AM +0100, Lee Jones wrote:
> > On Tue, Aug 21, 2012 at 10:50:27AM +0100, Mark Brown wrote:
> 
> > > What makes you say this?  This is just a convenience for finding a
> > > domain, irqdomains are *completely* indepentant of device tree.
> 
> > How can you say that? I think you mean _can_ be independent of DT. If
> > that's what you mean then yes, that's true. All I'm saying is we need
> 
> No, I really mean what I'm saying.  Device tree builds on irqdomains,
> not the other way around.

This is just semantics.
 
> > another way to get hold of the domain, because the only way to obtain
> > it without having direct access is via a device node.
> 
> This doesn't actually hold.

Okay, besides irq_find_host(struct device_node *np), how else can you fetch
a domain from the irqdomain?

> > > > - I know that you have interest in pushing the functionality into the
> > > > IRQ domain subsystem, but I'm struggling to see how. It's calling into
> > > > the IRQ domain where we're seeing issues in the first place, specifically
> > > > irq_create_mapping(). How about if we passed 'irq_domain' as a parameter
> > > > when requesting the IRQ? That way we can pass the correct IRQ without
> > > > worry of conversion. If 'irq_domain' is !NULL the IRQ management subsystem
> > > > can do the necessary conversions. If 'irq_domain' is NULL it continues to
> > > > use the requested IRQ as a virq.
> 
> > > This is totally orthogonal to doing the mapping in the MFD subsystem
> > > which is the issue here.
> 
> > Again, I only mentioned this because you said you wanted it to be handled
> > by the irqdomain.
> 
> The *mapping* should be being handled in irqdomain.
> 
> > I'll code up the second suggestion now.
> 
> I've already done this.

What have you done already? 

Why make suggestions if you're just going to do the work yourself?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
  2012-08-21  9:23     ` Lee Jones
@ 2012-08-21 16:42       ` Dmitry Torokhov
  2012-08-30 13:12         ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Torokhov @ 2012-08-21 16:42 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	linux-input

On Tue, Aug 21, 2012 at 10:23:29AM +0100, Lee Jones wrote:
> On Tue, Aug 14, 2012 at 10:31:08AM +0200, Linus Walleij wrote:
> > On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > 
> > > There was a plan to place ab8500_irq_get_virq() calls in each AB8500
> > > child device prior to requesting an IRQ, but as we're no longer using
> > > Device Tree to collect our IRQ numbers, it's actually better to allow
> > > the core to do this during device registration time. So the IRQ number
> > > we pull from its resource has already been converted to a virtual IRQ.
> > >
> > > CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > CC: linux-input@vger.kernel.org
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > This is looking good, I guess you need all patches to go in at the
> > same time so Dmitry's ACK is required.
> 
> Yep, just waiting for that now.

Sorry for the delay. Yes, this shoudl be fine, but since it is
essentially a revert of the original patch it should be pushed in as
such.

Thanks.

-- 
Dmitry

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-21 12:02                             ` Lee Jones
@ 2012-08-21 16:52                               ` Mark Brown
  2012-08-22  8:17                                 ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-08-21 16:52 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

On Tue, Aug 21, 2012 at 01:02:46PM +0100, Lee Jones wrote:
> On Tue, Aug 21, 2012 at 12:03:29PM +0100, Mark Brown wrote:

> > > another way to get hold of the domain, because the only way to obtain
> > > it without having direct access is via a device node.

> > This doesn't actually hold.

> Okay, besides irq_find_host(struct device_node *np), how else can you fetch
> a domain from the irqdomain?

I'm not sure I can parse the above, sorry - I'm not sure I can
distinguish "domain" and "irqdomain".

> What have you done already? 

Implemented a patch for this which I've now tested a bit and will
probably post in the next hour or so.

> Why make suggestions if you're just going to do the work yourself?

I made the suggestion then later on realised that this was actively
going to break things I care about so I actually need it fixing.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-21 16:52                               ` Mark Brown
@ 2012-08-22  8:17                                 ` Lee Jones
  2012-08-22 11:19                                   ` Mark Brown
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-22  8:17 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

On Tue, Aug 21, 2012 at 05:52:07PM +0100, Mark Brown wrote:
> On Tue, Aug 21, 2012 at 01:02:46PM +0100, Lee Jones wrote:
> > On Tue, Aug 21, 2012 at 12:03:29PM +0100, Mark Brown wrote:
> 
> > > > another way to get hold of the domain, because the only way to obtain
> > > > it without having direct access is via a device node.
> 
> > > This doesn't actually hold.
> 
> > Okay, besides irq_find_host(struct device_node *np), how else can you fetch
> > a domain from the irqdomain?
>
> I'm not sure I can parse the above, sorry - I'm not sure I can
> distinguish "domain" and "irqdomain".

Are you being intentionally awkward on this? Picking holes when you know
exactly what I'm trying to say? Not that it matters now, but just out of
principle, let me try to put it in a very clear way so there can be no
misinterpretation.

I was saying that in order for the MFD core to carry out the hwirq->virq
conversion, it needed to obtain the irqdomain pointer pertaining to the
provided hwirq. The only helper function the irqdomain subsystem provides
requires a device node pointer to be passed as an argument, hence the
mention of 'irq_find_host(struct device_node *np)'. Then the Device Tree
is traversed until a specified 'interrupt-controller' is stumbled upon
or is pointed to by the 'interrupt-parent' property. Hence, we have to 
find another way to find the irqdomain pointers for non-DT based MFDs. To
which we now have a solution.

> > What have you done already? 
> 
> Implemented a patch for this which I've now tested a bit and will
> probably post in the next hour or so.
> 
> > Why make suggestions if you're just going to do the work yourself?
> 
> I made the suggestion then later on realised that this was actively
> going to break things I care about so I actually need it fixing.

I'm a little taken aback and annoyed by this. In a previous email thread
you categorically requested that I discuss some of the important changes
with maintainers and people in-the-know prior to actually writing any
code. I was obviously actively working on, had put time into, and was in
the mist of discussing this with you. Then you just go ahead and code it
(the easy part) yourself, essentially wasting my time. Surely there's
some kind of etiquette surrounding such things?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-22  8:17                                 ` Lee Jones
@ 2012-08-22 11:19                                   ` Mark Brown
  2012-08-22 11:55                                     ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Mark Brown @ 2012-08-22 11:19 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

[-- Attachment #1: Type: text/plain, Size: 2373 bytes --]

On Wed, Aug 22, 2012 at 09:17:50AM +0100, Lee Jones wrote:

> I was saying that in order for the MFD core to carry out the hwirq->virq
> conversion, it needed to obtain the irqdomain pointer pertaining to the
> provided hwirq. The only helper function the irqdomain subsystem provides
> requires a device node pointer to be passed as an argument, hence the
> mention of 'irq_find_host(struct device_node *np)'. Then the Device Tree
> is traversed until a specified 'interrupt-controller' is stumbled upon
> or is pointed to by the 'interrupt-parent' property. Hence, we have to 
> find another way to find the irqdomain pointers for non-DT based MFDs. To
> which we now have a solution.

Oh, right.  Yes, there's no way to get an irqdomain if you don't already
have it or something which has a direct mapping to one like an irqdomain.

> > I made the suggestion then later on realised that this was actively
> > going to break things I care about so I actually need it fixing.

> I'm a little taken aback and annoyed by this. In a previous email thread
> you categorically requested that I discuss some of the important changes
> with maintainers and people in-the-know prior to actually writing any
> code.

No, that's not something I've ever said to do.

I *have* asked you to communicate more clearly about what you're doing
but that doesn't mean to stop sending code, it means to have clearer
words around what you're sending.  The really bad pattern here is that
you're frequently working around issues in your drivers with changes in
the subsystem without mentioning that the driver issues even exist -
this makes it much harder understand what you are trying to achieve,
especially when there is a problem with your subsystem changes and/or
the urgency you're attaching to them.

>       I was obviously actively working on, had put time into, and was in
> the mist of discussing this with you. Then you just go ahead and code it
> (the easy part) yourself, essentially wasting my time. Surely there's
> some kind of etiquette surrounding such things?

To be honest in this case I had expected to send the patch out much
sooner than I did - several priority interrupts stopped me testing it.
Like I say I realised that I really needed a fix and it seemed like the
quickest way to accomplish that was to just send the code.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-22 11:19                                   ` Mark Brown
@ 2012-08-22 11:55                                     ` Lee Jones
  2012-08-22 15:48                                       ` Mark Brown
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-22 11:55 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

> > > I made the suggestion then later on realised that this was actively
> > > going to break things I care about so I actually need it fixing.
> 
> > I'm a little taken aback and annoyed by this. In a previous email thread
> > you categorically requested that I discuss some of the important changes
> > with maintainers and people in-the-know prior to actually writing any
> > code.
> 
> No, that's not something I've ever said to do.
> 
> I *have* asked you to communicate more clearly about what you're doing
> but that doesn't mean to stop sending code, it means to have clearer
> words around what you're sending. 

That's not how I interpreted your words:

"What you can do here is to commmunicate about what you're doing more.
Don't just think about the code, think about the communication
surrounding the code - this is the core of the issue."

> The really bad pattern here is that
> you're frequently working around issues in your drivers with changes in
> the subsystem without mentioning that the driver issues even exist -
> this makes it much harder understand what you are trying to achieve,
> especially when there is a problem with your subsystem changes and/or
> the urgency you're attaching to them.

Frequently? I've done this once, and yes I did push hard because I
thought the subsystem was incorrect (I still think folding an entire
driver because of a minor failure is wrong, but we digress).

This is completely different to that. This was a subsystem change
designed to aid DT enabled MFDs which 'chose' to register themselves
in a specific way, by passing their compatible string through the 
mfd_cell only. It doesn't affect any other MFD unless they wrongly
assume the conversion would be automatically done for them. However,
the author would know because they would have tested it. All of this
was discussed at length with Arnd and this is what we came up with. 

I think it's great that you like the idea and want to extend that
functionality to other MFDs which perhaps don't support DT, or the
ones that do but don't want to provide compatible strings or device
nodes for all the MFD's child devices. But that is all we're doing
here. There was no breakage. It served a purpose and it worked well.
So well in fact that you've now provided the intended functionality
to other devices.

> >       I was obviously actively working on, had put time into, and was in
> > the mist of discussing this with you. Then you just go ahead and code it
> > (the easy part) yourself, essentially wasting my time. Surely there's
> > some kind of etiquette surrounding such things?
> 
> To be honest in this case I had expected to send the patch out much
> sooner than I did - several priority interrupts stopped me testing it.
> Like I say I realised that I really needed a fix and it seemed like the
> quickest way to accomplish that was to just send the code.

You only noticed it 2 days ago and I had a patch ready to go yesterday.
I'm confused because I don't understand why would you even complain about
it if you intended to work on it yourself? Surely, "Ah, I see an issue with
xyz, patch to follow." Would have been more appropriate, instead of 
complaining about it, then I go and waste my time trying to fix something
you intend on fixing yourself.

I guess what's done is done now.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain
  2012-08-22 11:55                                     ` Lee Jones
@ 2012-08-22 15:48                                       ` Mark Brown
  0 siblings, 0 replies; 50+ messages in thread
From: Mark Brown @ 2012-08-22 15:48 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Arnd Bergmann, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, Samuel Ortiz

[-- Attachment #1: Type: text/plain, Size: 2107 bytes --]

On Wed, Aug 22, 2012 at 12:55:25PM +0100, Lee Jones wrote:

> > I *have* asked you to communicate more clearly about what you're doing
> > but that doesn't mean to stop sending code, it means to have clearer
> > words around what you're sending. 

> That's not how I interpreted your words:

> "What you can do here is to commmunicate about what you're doing more.
> Don't just think about the code, think about the communication
> surrounding the code - this is the core of the issue."

Just to be clear I'd include things like commit messages, cover letters
and so on in the general area of communication.

> I think it's great that you like the idea and want to extend that
> functionality to other MFDs which perhaps don't support DT, or the
> ones that do but don't want to provide compatible strings or device
> nodes for all the MFD's child devices. But that is all we're doing
> here. There was no breakage. It served a purpose and it worked well.
> So well in fact that you've now provided the intended functionality
> to other devices.

I'm looking at this from the point of view of adding the compatible
strings and then finding that the core starts remapping things in the
case where you're probing from DT - and this behaviour will vary
depending on the device tree that the user is using so the driver can't
even make a decision based on if device tree is being used by the
system.

> You only noticed it 2 days ago and I had a patch ready to go yesterday.
> I'm confused because I don't understand why would you even complain about
> it if you intended to work on it yourself? Surely, "Ah, I see an issue with
> xyz, patch to follow." Would have been more appropriate, instead of 
> complaining about it, then I go and waste my time trying to fix something
> you intend on fixing yourself.

I didn't originally intend to do anything, if I had done I'd just have
sent a patch as you say.  Originally I'd just noticed it as being an
awkwardly designed interface at the wrong abstraction layer, it was only
later on that I realised how it could blow up.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
  2012-08-21 16:42       ` Dmitry Torokhov
@ 2012-08-30 13:12         ` Lee Jones
  2012-08-30 23:02           ` Dmitry Torokhov
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-30 13:12 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	linux-input

> Sorry for the delay. Yes, this shoudl be fine, but since it is
> essentially a revert of the original patch it should be pushed in as
> such.

How's this?

Author: Lee Jones <lee.jones@linaro.org>
Date:   Thu Aug 30 14:08:19 2012 +0100

    Revert "input: ab8500-ponkey: Create AB8500 domain IRQ mapping"
    
    This reverts commit ca3b3faf9bee4dc5df4f10eae2d1e48f7de0a8ad.
    
    There was a plan to place ab8500_irq_get_virq() calls in each AB8500
    child device prior to requesting an IRQ, but as we're no longer using
    Device Tree to collect our IRQ numbers, it's actually better to allow
    the core to do this during device registration time. So the IRQ number
    we pull from its resource has already been converted to a virtual IRQ.
    
    CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
    CC: linux-input@vger.kernel.org
    Acked-by: Linus Walleij <linus.walleij@linaro.org>
    Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/drivers/input/misc/ab8500-ponkey.c b/drivers/input/misc/ab8500-ponkey.c
index f06231b..84ec691 100644
--- a/drivers/input/misc/ab8500-ponkey.c
+++ b/drivers/input/misc/ab8500-ponkey.c
@@ -74,8 +74,8 @@ static int __devinit ab8500_ponkey_probe(struct platform_device *pdev)
 
        ponkey->idev = input;
        ponkey->ab8500 = ab8500;
-       ponkey->irq_dbf = ab8500_irq_get_virq(ab8500, irq_dbf);
-       ponkey->irq_dbr = ab8500_irq_get_virq(ab8500, irq_dbr);
+       ponkey->irq_dbf = irq_dbf;
+       ponkey->irq_dbr = irq_dbr;
 
        input->name = "AB8500 POn(PowerOn) Key";
        input->dev.parent = &pdev->dev;

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

* Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
  2012-08-30 13:12         ` Lee Jones
@ 2012-08-30 23:02           ` Dmitry Torokhov
  2012-08-30 23:03             ` Dmitry Torokhov
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Torokhov @ 2012-08-30 23:02 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	linux-input

On Thu, Aug 30, 2012 at 02:12:04PM +0100, Lee Jones wrote:
> > Sorry for the delay. Yes, this shoudl be fine, but since it is
> > essentially a revert of the original patch it should be pushed in as
> > such.
> 
> How's this?
> 

Excellent.

> Author: Lee Jones <lee.jones@linaro.org>
> Date:   Thu Aug 30 14:08:19 2012 +0100
> 
>     Revert "input: ab8500-ponkey: Create AB8500 domain IRQ mapping"
>     
>     This reverts commit ca3b3faf9bee4dc5df4f10eae2d1e48f7de0a8ad.
>     
>     There was a plan to place ab8500_irq_get_virq() calls in each AB8500
>     child device prior to requesting an IRQ, but as we're no longer using
>     Device Tree to collect our IRQ numbers, it's actually better to allow
>     the core to do this during device registration time. So the IRQ number
>     we pull from its resource has already been converted to a virtual IRQ.
>     
>     CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>     CC: linux-input@vger.kernel.org
>     Acked-by: Linus Walleij <linus.walleij@linaro.org>

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

>     Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> diff --git a/drivers/input/misc/ab8500-ponkey.c b/drivers/input/misc/ab8500-ponkey.c
> index f06231b..84ec691 100644
> --- a/drivers/input/misc/ab8500-ponkey.c
> +++ b/drivers/input/misc/ab8500-ponkey.c
> @@ -74,8 +74,8 @@ static int __devinit ab8500_ponkey_probe(struct platform_device *pdev)
>  
>         ponkey->idev = input;
>         ponkey->ab8500 = ab8500;
> -       ponkey->irq_dbf = ab8500_irq_get_virq(ab8500, irq_dbf);
> -       ponkey->irq_dbr = ab8500_irq_get_virq(ab8500, irq_dbr);
> +       ponkey->irq_dbf = irq_dbf;
> +       ponkey->irq_dbr = irq_dbr;
>  
>         input->name = "AB8500 POn(PowerOn) Key";
>         input->dev.parent = &pdev->dev;

-- 
Dmitry

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

* Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
  2012-08-30 23:02           ` Dmitry Torokhov
@ 2012-08-30 23:03             ` Dmitry Torokhov
  2012-08-31  7:31               ` Lee Jones
  0 siblings, 1 reply; 50+ messages in thread
From: Dmitry Torokhov @ 2012-08-30 23:03 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	linux-input

On Thu, Aug 30, 2012 at 04:02:21PM -0700, Dmitry Torokhov wrote:
> On Thu, Aug 30, 2012 at 02:12:04PM +0100, Lee Jones wrote:
> > > Sorry for the delay. Yes, this shoudl be fine, but since it is
> > > essentially a revert of the original patch it should be pushed in as
> > > such.
> > 
> > How's this?
> > 
> 
> Excellent.

I assume you will be merging it with the rest of AB8500 patches, right?

-- 
Dmitry

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

* Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
  2012-08-30 23:03             ` Dmitry Torokhov
@ 2012-08-31  7:31               ` Lee Jones
  2012-08-31 14:50                 ` Dmitry Torokhov
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-31  7:31 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	linux-input

On Thu, Aug 30, 2012 at 04:03:23PM -0700, Dmitry Torokhov wrote:
> On Thu, Aug 30, 2012 at 04:02:21PM -0700, Dmitry Torokhov wrote:
> > On Thu, Aug 30, 2012 at 02:12:04PM +0100, Lee Jones wrote:
> > > > Sorry for the delay. Yes, this shoudl be fine, but since it is
> > > > essentially a revert of the original patch it should be pushed in as
> > > > such.
> > > 
> > > How's this?
> > > 
> > 
> > Excellent.
> 
> I assume you will be merging it with the rest of AB8500 patches, right?

Yes, if that's okay with you of course?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified
  2012-08-14  8:19   ` Linus Walleij
@ 2012-08-31  9:44     ` Lee Jones
  2012-08-31 13:58       ` Rob Herring
  0 siblings, 1 reply; 50+ messages in thread
From: Lee Jones @ 2012-08-31  9:44 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie, Benjamin Herrenschmidt,
	Grant Likely

On Tue, Aug 14, 2012 at 10:19:08AM +0200, Linus Walleij wrote:
> On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > irq_find_host() currently ignores the 'interrupt-parent' property
> > even if it's specified in the Device Tree. Meaning that a node can
> > match to a domain in its hierarchy even if it doesn't belong to it.
> > By searching for the parent first using of_irq_find_parent() we
> > insist that the 'interrupt-parent' property is taken into account
> > ensuring a greater chance of returning the correct domain.
> >
> > CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > CC: Grant Likely <grant.likely@secretlab.ca>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> This (with 1/8) is the right solution. Thanks, and I think
> Mark may want to look at this too since I recognize he asked
> you to work in this direction.
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Rob, did this get your Ack too?

Ben, Grant, Mark, would you like to take a look also?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified
  2012-08-31  9:44     ` Lee Jones
@ 2012-08-31 13:58       ` Rob Herring
  0 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2012-08-31 13:58 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, linus.walleij, arnd, Benjamin Herrenschmidt,
	broonie, linux-kernel, Grant Likely, STEricsson_nomadik_linux,
	linux-arm-kernel

On 08/31/2012 04:44 AM, Lee Jones wrote:
> On Tue, Aug 14, 2012 at 10:19:08AM +0200, Linus Walleij wrote:
>> On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>>> irq_find_host() currently ignores the 'interrupt-parent' property
>>> even if it's specified in the Device Tree. Meaning that a node can
>>> match to a domain in its hierarchy even if it doesn't belong to it.
>>> By searching for the parent first using of_irq_find_parent() we
>>> insist that the 'interrupt-parent' property is taken into account
>>> ensuring a greater chance of returning the correct domain.
>>>
>>> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> CC: Grant Likely <grant.likely@secretlab.ca>
>>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>
>> This (with 1/8) is the right solution. Thanks, and I think
>> Mark may want to look at this too since I recognize he asked
>> you to work in this direction.
>>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Rob, did this get your Ack too?

Acked-by: Rob Herring <rob.herring@calxeda.com>

Rob

> 
> Ben, Grant, Mark, would you like to take a look also?
> 


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

* Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
  2012-08-31  7:31               ` Lee Jones
@ 2012-08-31 14:50                 ` Dmitry Torokhov
  0 siblings, 0 replies; 50+ messages in thread
From: Dmitry Torokhov @ 2012-08-31 14:50 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, linux-arm-kernel, linux-kernel,
	STEricsson_nomadik_linux, linus.walleij, arnd, broonie,
	linux-input

On Fri, Aug 31, 2012 at 08:31:33AM +0100, Lee Jones wrote:
> On Thu, Aug 30, 2012 at 04:03:23PM -0700, Dmitry Torokhov wrote:
> > On Thu, Aug 30, 2012 at 04:02:21PM -0700, Dmitry Torokhov wrote:
> > > On Thu, Aug 30, 2012 at 02:12:04PM +0100, Lee Jones wrote:
> > > > > Sorry for the delay. Yes, this shoudl be fine, but since it is
> > > > > essentially a revert of the original patch it should be pushed in as
> > > > > such.
> > > > 
> > > > How's this?
> > > > 
> > > 
> > > Excellent.
> > 
> > I assume you will be merging it with the rest of AB8500 patches, right?
> 
> Yes, if that's okay with you of course?

Sure, please go ahead,

Thanks.

-- 
Dmitry

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

* Re: [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq
  2012-08-09 15:53 ` [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq Lee Jones
  2012-08-14  8:25   ` Linus Walleij
@ 2012-09-19  0:00   ` Samuel Ortiz
  1 sibling, 0 replies; 50+ messages in thread
From: Samuel Ortiz @ 2012-09-19  0:00 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, STEricsson_nomadik_linux,
	linus.walleij, arnd, broonie

Hi Lee,

On Thu, Aug 09, 2012 at 04:53:54PM +0100, Lee Jones wrote:
> Before the AB8500 had its own IRQ domain, the IRQ handler would take
> the fired local IRQ (hwirq) and add it to the irq_base to convert it
> to an IRQ number which Linux would understand (virq). However, the
> IRQ base is not always used anymore since we can make use of Linear
> domains. It's better to use the AB8500 hwirq -> virq mapping helper
> function to convert them instead. That's what we do here.
> 
> CC: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/ab8500-core.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
Patch applied now, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

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

end of thread, other threads:[~2012-09-19  0:01 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-09 15:53 [PATCH 0/8] Changes surrounding IRQs and IRQ domains Lee Jones
2012-08-09 15:53 ` [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF Lee Jones
2012-08-09 16:20   ` Rob Herring
2012-08-09 19:44     ` Lee Jones
2012-08-09 19:53   ` Rob Herring
2012-08-14  8:17   ` Linus Walleij
2012-08-09 15:53 ` [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified Lee Jones
2012-08-14  8:19   ` Linus Walleij
2012-08-31  9:44     ` Lee Jones
2012-08-31 13:58       ` Rob Herring
2012-08-09 15:53 ` [PATCH 3/8] ARM: ux500: Identify the PRCMU as an interrupt controller Lee Jones
2012-08-14  8:19   ` Linus Walleij
2012-08-09 15:53 ` [PATCH 4/8] ARM: ux500: Force AB8500 to use the GIC as its " Lee Jones
2012-08-14  8:20   ` Linus Walleij
2012-08-09 15:53 ` [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain Lee Jones
2012-08-14  8:29   ` Linus Walleij
2012-08-14  9:42     ` Arnd Bergmann
2012-08-14 10:44       ` Linus Walleij
2012-08-20  8:36         ` Lee Jones
2012-08-20 12:10           ` Mark Brown
2012-08-20 12:55             ` Lee Jones
2012-08-20 16:29               ` Mark Brown
2012-08-20 16:49                 ` Lee Jones
2012-08-20 17:51                   ` Mark Brown
2012-08-21  8:56                     ` Lee Jones
2012-08-21  9:50                       ` Mark Brown
2012-08-21 10:54                         ` Lee Jones
2012-08-21 11:03                           ` Mark Brown
2012-08-21 12:02                             ` Lee Jones
2012-08-21 16:52                               ` Mark Brown
2012-08-22  8:17                                 ` Lee Jones
2012-08-22 11:19                                   ` Mark Brown
2012-08-22 11:55                                     ` Lee Jones
2012-08-22 15:48                                       ` Mark Brown
2012-08-20  9:36     ` Lee Jones
2012-08-20 10:49     ` Lee Jones
2012-08-09 15:53 ` [PATCH 6/8] mfd: Use interrupt-parent as IRQ controller if specified in DT Lee Jones
2012-08-14  8:22   ` Linus Walleij
2012-08-09 15:53 ` [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq Lee Jones
2012-08-14  8:25   ` Linus Walleij
2012-09-19  0:00   ` Samuel Ortiz
2012-08-09 15:53 ` [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual Lee Jones
2012-08-14  8:31   ` Linus Walleij
2012-08-21  9:23     ` Lee Jones
2012-08-21 16:42       ` Dmitry Torokhov
2012-08-30 13:12         ` Lee Jones
2012-08-30 23:02           ` Dmitry Torokhov
2012-08-30 23:03             ` Dmitry Torokhov
2012-08-31  7:31               ` Lee Jones
2012-08-31 14:50                 ` Dmitry Torokhov

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).