[PATCH 0/8] Changes surrounding IRQs and IRQ domains

[PATCH 0/8] Changes surrounding IRQs and IRQ domains

[Lee Jones]

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

[PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF

[Lee Jones]

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

Re: [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF

[Rob Herring]

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 */
> 

Re: [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF

[Lee Jones]

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

Re: [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF

[Rob Herring]

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 */
> 

Re: [PATCH 1/8] of/irq: Create stub for of_irq_find_parent when !CONFIG_OF

[Linus Walleij]

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

[PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified

[Lee Jones]

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

Re: [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified

[Linus Walleij]

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

Re: [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified

[Lee Jones]

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

Re: [PATCH 2/8] irqdomain: Take interrupt-parent property into account if specified

[Rob Herring]

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?
> 

[PATCH 3/8] ARM: ux500: Identify the PRCMU as an interrupt controller

[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

Re: [PATCH 3/8] ARM: ux500: Identify the PRCMU as an interrupt controller

[Linus Walleij]

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

[PATCH 4/8] ARM: ux500: Force AB8500 to use the GIC as its interrupt controller

[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

Re: [PATCH 4/8] ARM: ux500: Force AB8500 to use the GIC as its interrupt controller

[Linus Walleij]

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

[PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Linus Walleij]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Arnd Bergmann]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Linus Walleij]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Mark Brown]

[-- 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 --]

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Mark Brown]

[-- 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 --]

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Mark Brown]

[-- 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 --]

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Mark Brown]

[-- 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 --]

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Mark Brown]

[-- 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 --]

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Mark Brown]

[-- 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 --]

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Mark Brown]

[-- 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 --]

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Mark Brown]

[-- 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 --]

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

Re: [PATCH 5/8] mfd: Provide the PRCMU with its own IRQ domain

[Lee Jones]

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

[PATCH 6/8] mfd: Use interrupt-parent as IRQ controller if specified in DT

[Lee Jones]

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

Re: [PATCH 6/8] mfd: Use interrupt-parent as IRQ controller if specified in DT

[Linus Walleij]

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

[PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq

[Lee Jones]

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

Re: [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq

[Linus Walleij]

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

Re: [PATCH 7/8] mfd: Use the AB8500's IRQ domain to convert hwirq to virq

[Samuel Ortiz]

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/

[PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

[Lee Jones]

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

Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

[Linus Walleij]

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

Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

[Lee Jones]

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

Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

[Dmitry Torokhov]

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

Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

[Lee Jones]

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

Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

[Dmitry Torokhov]

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

Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

[Dmitry Torokhov]

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

Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

[Lee Jones]

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

Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual

[Dmitry Torokhov]

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