linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] MFD: twl6040: Device tree support
@ 2012-05-03 12:54 Peter Ujfalusi
  2012-05-03 12:54 ` [PATCH 1/3] MFD: twl6040: Code cleanup in interrupt initialization part Peter Ujfalusi
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-03 12:54 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

Hello,

The following series adds device tree support for the twl6040 MFD core driver.
Support for the child drivers (vibra, ASoC codec) will be submitted via the
corresponding subsystem since there is no compile time dependency between them.

The first patch is a minor clean up patch to remove wrapped lines in the
twl6040-irq.c. The resulting code is more natural to read to me.

The series depends on the regulator support patch for the twl6040:
http://marc.info/?l=linux-kernel&m=133596703610515&w=2

Regards,
Peter
---
Peter Ujfalusi (3):
  MFD: twl6040: Code cleanup in interrupt initialization part
  MFD: twl6040: Allocate IRQ numbers dynamically
  MFD: twl6040: Support for DT

 Documentation/devicetree/bindings/mfd/twl6040.txt |   51 +++++++++++++++++++++
 drivers/mfd/twl6040-core.c                        |   26 +++++++---
 drivers/mfd/twl6040-irq.c                         |   32 +++++++++----
 3 files changed, 92 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/twl6040.txt

-- 
1.7.8.6


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

* [PATCH 1/3] MFD: twl6040: Code cleanup in interrupt initialization part
  2012-05-03 12:54 [PATCH 0/3] MFD: twl6040: Device tree support Peter Ujfalusi
@ 2012-05-03 12:54 ` Peter Ujfalusi
  2012-05-03 12:54 ` [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically Peter Ujfalusi
  2012-05-03 12:54 ` [PATCH 3/3] MFD: twl6040: Support for DT Peter Ujfalusi
  2 siblings, 0 replies; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-03 12:54 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

No functional change, just to make the code a bit more uniform and
remove wrapped lines.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl6040-irq.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/twl6040-irq.c b/drivers/mfd/twl6040-irq.c
index b3f8dda..008022c 100644
--- a/drivers/mfd/twl6040-irq.c
+++ b/drivers/mfd/twl6040-irq.c
@@ -138,7 +138,7 @@ static irqreturn_t twl6040_irq_thread(int irq, void *data)
 
 int twl6040_irq_init(struct twl6040 *twl6040)
 {
-	int cur_irq, ret;
+	int i, nr_irqs, ret;
 	u8 val;
 
 	mutex_init(&twl6040->irq_mutex);
@@ -148,21 +148,20 @@ int twl6040_irq_init(struct twl6040 *twl6040)
 	twl6040->irq_masks_cache = TWL6040_ALLINT_MSK;
 	twl6040_reg_write(twl6040, TWL6040_REG_INTMR, TWL6040_ALLINT_MSK);
 
+	nr_irqs = ARRAY_SIZE(twl6040_irqs);
 	/* Register them with genirq */
-	for (cur_irq = twl6040->irq_base;
-	     cur_irq < twl6040->irq_base + ARRAY_SIZE(twl6040_irqs);
-	     cur_irq++) {
-		irq_set_chip_data(cur_irq, twl6040);
-		irq_set_chip_and_handler(cur_irq, &twl6040_irq_chip,
+	for (i = twl6040->irq_base; i < twl6040->irq_base + nr_irqs; i++) {
+		irq_set_chip_data(i, twl6040);
+		irq_set_chip_and_handler(i, &twl6040_irq_chip,
 					 handle_level_irq);
-		irq_set_nested_thread(cur_irq, 1);
+		irq_set_nested_thread(i, 1);
 
 		/* ARM needs us to explicitly flag the IRQ as valid
 		 * and will set them noprobe when we do so. */
 #ifdef CONFIG_ARM
-		set_irq_flags(cur_irq, IRQF_VALID);
+		set_irq_flags(i, IRQF_VALID);
 #else
-		irq_set_noprobe(cur_irq);
+		irq_set_noprobe(i);
 #endif
 	}
 
-- 
1.7.8.6


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

* [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-03 12:54 [PATCH 0/3] MFD: twl6040: Device tree support Peter Ujfalusi
  2012-05-03 12:54 ` [PATCH 1/3] MFD: twl6040: Code cleanup in interrupt initialization part Peter Ujfalusi
@ 2012-05-03 12:54 ` Peter Ujfalusi
  2012-05-03 13:20   ` Mark Brown
  2012-05-03 12:54 ` [PATCH 3/3] MFD: twl6040: Support for DT Peter Ujfalusi
  2 siblings, 1 reply; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-03 12:54 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

Use irq_alloc_descs() to get the IRQ number range dynamically instead of
the hardwired use if pdata->irq_base.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/mfd/twl6040-core.c |    3 +--
 drivers/mfd/twl6040-irq.c  |   13 +++++++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/twl6040-core.c b/drivers/mfd/twl6040-core.c
index 7a92d95..c50fba7 100644
--- a/drivers/mfd/twl6040-core.c
+++ b/drivers/mfd/twl6040-core.c
@@ -515,7 +515,7 @@ static int __devinit twl6040_probe(struct i2c_client *client,
 	}
 
 	/* In order to operate correctly we need valid interrupt config */
-	if (!client->irq || !pdata->irq_base) {
+	if (!client->irq) {
 		dev_err(&client->dev, "Invalid IRQ configuration\n");
 		return -EINVAL;
 	}
@@ -552,7 +552,6 @@ static int __devinit twl6040_probe(struct i2c_client *client,
 
 	twl6040->dev = &client->dev;
 	twl6040->irq = client->irq;
-	twl6040->irq_base = pdata->irq_base;
 
 	mutex_init(&twl6040->mutex);
 	mutex_init(&twl6040->io_mutex);
diff --git a/drivers/mfd/twl6040-irq.c b/drivers/mfd/twl6040-irq.c
index 008022c..914978e 100644
--- a/drivers/mfd/twl6040-irq.c
+++ b/drivers/mfd/twl6040-irq.c
@@ -23,6 +23,7 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/err.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/core.h>
@@ -138,7 +139,7 @@ static irqreturn_t twl6040_irq_thread(int irq, void *data)
 
 int twl6040_irq_init(struct twl6040 *twl6040)
 {
-	int i, nr_irqs, ret;
+	int i, nr_irqs, irq_base, ret;
 	u8 val;
 
 	mutex_init(&twl6040->irq_mutex);
@@ -149,8 +150,16 @@ int twl6040_irq_init(struct twl6040 *twl6040)
 	twl6040_reg_write(twl6040, TWL6040_REG_INTMR, TWL6040_ALLINT_MSK);
 
 	nr_irqs = ARRAY_SIZE(twl6040_irqs);
+
+	irq_base = irq_alloc_descs(-1, 0, nr_irqs, 0);
+	if (IS_ERR_VALUE(irq_base)) {
+		dev_err(twl6040->dev, "Fail to allocate IRQ descs\n");
+		return irq_base;
+	}
+	twl6040->irq_base = irq_base;
+
 	/* Register them with genirq */
-	for (i = twl6040->irq_base; i < twl6040->irq_base + nr_irqs; i++) {
+	for (i = irq_base; i < irq_base + nr_irqs; i++) {
 		irq_set_chip_data(i, twl6040);
 		irq_set_chip_and_handler(i, &twl6040_irq_chip,
 					 handle_level_irq);
-- 
1.7.8.6


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

* [PATCH 3/3] MFD: twl6040: Support for DT
  2012-05-03 12:54 [PATCH 0/3] MFD: twl6040: Device tree support Peter Ujfalusi
  2012-05-03 12:54 ` [PATCH 1/3] MFD: twl6040: Code cleanup in interrupt initialization part Peter Ujfalusi
  2012-05-03 12:54 ` [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically Peter Ujfalusi
@ 2012-05-03 12:54 ` Peter Ujfalusi
  2 siblings, 0 replies; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-03 12:54 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

Add support for DT based probing of twl6040 core.

Example of dts section for twl6040:
twl6040: twl6040@4b {
	compatible = "ti,twl6040";
	reg = <0x4b>;
	interrupts = <0 119 4>; /* IRQ_SYS_2N cascaded to gic */
	interrupt-parent = <&gic>;
	twl6040,audpwron_gpio = <&gpio4 31 0>;  /* gpio line 127 */
	enable-active-high;

	interrupt-controller;
	#interrupt-cells = <1>;

	vio-supply = <&v1v8>;
	v2v1-supply = <&v2v1>;

	twl6040_codec: twl6040@0 {
		compatible = "ti,twl6040-codec";
		interrupts = <1>;
	};
};

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 Documentation/devicetree/bindings/mfd/twl6040.txt |   51 +++++++++++++++++++++
 drivers/mfd/twl6040-core.c                        |   23 +++++++---
 drivers/mfd/twl6040-irq.c                         |    6 +++
 3 files changed, 74 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/twl6040.txt

diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
new file mode 100644
index 0000000..6d2399c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
@@ -0,0 +1,51 @@
+Texas Instruments TWL6040 family
+
+The TWL6040s are 8-channel high quality low-power audio codecs providing audio
+and vibra functionality on OMAP4+ platforms.
+They are connected ot the host processor via i2c for commands, McPDM for audio
+data and commands.
+
+Required properties:
+- compatible : Must be "ti,twl6040";
+- reg: must be 0x4b for i2c address
+- interrupts: twl6040 has one interrupt line connecteded to the main SoC
+- interrupt-parent: The parent interrupt controller
+- interrupt-controller: twl6040 support several interrupts internally, it is
+  considered as an interupt controller cascaded to the SoC core.
+#interrupt-cells = <1>;
+- twl6040,audpwron_gpio: Power on GPIO line for the twl6040
+
+- vio-supply: Regulator for the twl6040 VIO supply
+- v2v1-supply: Regulator for the twl6040 V2V1 supply
+
+Optional properties, nodes:
+- enable-active-high: To power on the twl6040 during boot.
+- Child nodes for audio functionality and for the vibra driver
+  For details on the child nodes see:
+  Documentation/devicetree/bindings/input/twl6040-vibra.txt - Vibra driver
+  Documentation/devicetree/bindings/sound/twl6040.txt - ASoC codec driver
+
+Example:
+/*
+ * 8-channel high quality low-power audio codec
+ * http://www.ti.com/lit/ds/symlink/twl6040.pdf
+ */
+twl6040: twl6040@4b {
+	compatible = "ti,twl6040";
+	reg = <0x4b>;
+	interrupts = <0 119 4>; /* IRQ_SYS_2N cascaded to gic */
+	interrupt-parent = <&gic>;
+	twl6040,audpwron_gpio = <&gpio4 31 0>;  /* gpio line 127 */
+	enable-active-high;
+
+	interrupt-controller;
+	#interrupt-cells = <1>;
+
+	vio-supply = <&v1v8>;
+	v2v1-supply = <&v2v1>;
+
+	twl6040_codec: twl6040@0 {
+		compatible = "ti,twl6040-codec";
+		interrupts = <1>;
+	};
+};
diff --git a/drivers/mfd/twl6040-core.c b/drivers/mfd/twl6040-core.c
index c50fba7..23b9771 100644
--- a/drivers/mfd/twl6040-core.c
+++ b/drivers/mfd/twl6040-core.c
@@ -29,6 +29,10 @@
 #include <linux/kernel.h>
 #include <linux/err.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_gpio.h>
+#include <linux/of_platform.h>
 #include <linux/gpio.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
@@ -505,11 +509,12 @@ static int __devinit twl6040_probe(struct i2c_client *client,
 				     const struct i2c_device_id *id)
 {
 	struct twl6040_platform_data *pdata = client->dev.platform_data;
+	struct device_node *node = client->dev.of_node;
 	struct twl6040 *twl6040;
 	struct mfd_cell *cell = NULL;
 	int ret, children = 0;
 
-	if (!pdata) {
+	if (!pdata && !node) {
 		dev_err(&client->dev, "Platform data is missing\n");
 		return -EINVAL;
 	}
@@ -560,9 +565,13 @@ static int __devinit twl6040_probe(struct i2c_client *client,
 	twl6040->rev = twl6040_reg_read(twl6040, TWL6040_REG_ASICREV);
 
 	/* ERRATA: Automatic power-up is not possible in ES1.0 */
-	if (twl6040_get_revid(twl6040) > TWL6040_REV_ES1_0)
-		twl6040->audpwron = pdata->audpwron_gpio;
-	else
+	if (twl6040_get_revid(twl6040) > TWL6040_REV_ES1_0) {
+		if (pdata)
+			twl6040->audpwron = pdata->audpwron_gpio;
+		else
+			twl6040->audpwron = of_get_named_gpio(node,
+						"twl6040,audpwron_gpio", 0);
+	} else
 		twl6040->audpwron = -EINVAL;
 
 	if (gpio_is_valid(twl6040->audpwron)) {
@@ -589,7 +598,7 @@ static int __devinit twl6040_probe(struct i2c_client *client,
 	/* dual-access registers controlled by I2C only */
 	twl6040_set_bits(twl6040, TWL6040_REG_ACCCTL, TWL6040_I2CSEL);
 
-	if (pdata->codec) {
+	if (pdata && pdata->codec) {
 		int irq = twl6040->irq_base + TWL6040_IRQ_PLUG;
 
 		cell = &twl6040->cells[children];
@@ -603,7 +612,7 @@ static int __devinit twl6040_probe(struct i2c_client *client,
 		children++;
 	}
 
-	if (pdata->vibra) {
+	if (pdata && pdata->vibra) {
 		int irq = twl6040->irq_base + TWL6040_IRQ_VIB;
 
 		cell = &twl6040->cells[children];
@@ -623,6 +632,8 @@ static int __devinit twl6040_probe(struct i2c_client *client,
 				      children, NULL, 0);
 		if (ret)
 			goto mfd_err;
+	} else if (node) {
+		ret = of_platform_populate(node, NULL, NULL, &client->dev);
 	} else {
 		dev_err(&client->dev, "No platform data found for children\n");
 		ret = -ENODEV;
diff --git a/drivers/mfd/twl6040-irq.c b/drivers/mfd/twl6040-irq.c
index 914978e..4b42543 100644
--- a/drivers/mfd/twl6040-irq.c
+++ b/drivers/mfd/twl6040-irq.c
@@ -25,6 +25,8 @@
 #include <linux/module.h>
 #include <linux/err.h>
 #include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/irqdomain.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/twl6040.h>
@@ -139,6 +141,7 @@ static irqreturn_t twl6040_irq_thread(int irq, void *data)
 
 int twl6040_irq_init(struct twl6040 *twl6040)
 {
+	struct device_node *node = twl6040->dev->of_node;
 	int i, nr_irqs, irq_base, ret;
 	u8 val;
 
@@ -158,6 +161,9 @@ int twl6040_irq_init(struct twl6040 *twl6040)
 	}
 	twl6040->irq_base = irq_base;
 
+	irq_domain_add_legacy(node, ARRAY_SIZE(twl6040_irqs), irq_base, 0,
+			      &irq_domain_simple_ops, NULL);
+
 	/* Register them with genirq */
 	for (i = irq_base; i < irq_base + nr_irqs; i++) {
 		irq_set_chip_data(i, twl6040);
-- 
1.7.8.6


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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-03 12:54 ` [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically Peter Ujfalusi
@ 2012-05-03 13:20   ` Mark Brown
  2012-05-03 13:28     ` Peter Ujfalusi
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2012-05-03 13:20 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

On Thu, May 03, 2012 at 03:54:24PM +0300, Peter Ujfalusi wrote:

>  	/* In order to operate correctly we need valid interrupt config */
> -	if (!client->irq || !pdata->irq_base) {
> +	if (!client->irq) {

It looks like you're totally removing the use of irq_base which will
break any boards that didn't convert to DT.  The usual idiom is to use
irq_base as the base for the range of requested IRQs if it's supplied,
otherwise set it to -1 to allow dynamic allocation.  This should keep
existing users working without disruption.

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-03 13:20   ` Mark Brown
@ 2012-05-03 13:28     ` Peter Ujfalusi
  2012-05-03 14:52       ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-03 13:28 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

On 05/03/2012 04:20 PM, Mark Brown wrote:
> On Thu, May 03, 2012 at 03:54:24PM +0300, Peter Ujfalusi wrote:
> 
>>  	/* In order to operate correctly we need valid interrupt config */
>> -	if (!client->irq || !pdata->irq_base) {
>> +	if (!client->irq) {
> 
> It looks like you're totally removing the use of irq_base which will
> break any boards that didn't convert to DT.  The usual idiom is to use
> irq_base as the base for the range of requested IRQs if it's supplied,
> otherwise set it to -1 to allow dynamic allocation.  This should keep
> existing users working without disruption.

In the current board files the pdata->irq_base set to some #defined
value (which is a comfortably big number).
With irq_alloc_descs(-1, 0, nr_irqs, 0); the range which can accommodate
the number of IRQs twl6040 will be provided so there is no need to me to
use the board specified irq_base.
The irq_base will be removed from the pdata structure, but that change
will go via linux-omap to avoid compile breakage.
This part is in preparation for DT, yes, but it is working without DT.
The twl6040 irq range is mapped to different numbers, that's all.

-- 
Péter

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-03 13:28     ` Peter Ujfalusi
@ 2012-05-03 14:52       ` Mark Brown
  2012-05-03 15:13         ` Peter Ujfalusi
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2012-05-03 14:52 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

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

On Thu, May 03, 2012 at 04:28:59PM +0300, Peter Ujfalusi wrote:
> On 05/03/2012 04:20 PM, Mark Brown wrote:

> > It looks like you're totally removing the use of irq_base which will
> > break any boards that didn't convert to DT.  The usual idiom is to use

> In the current board files the pdata->irq_base set to some #defined
> value (which is a comfortably big number).

Are you sure there aren't any boards out there which rely on the
interrupt base (eg, using a GPIO with the IRQ output of a chip)?

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

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-03 14:52       ` Mark Brown
@ 2012-05-03 15:13         ` Peter Ujfalusi
  2012-05-03 15:26           ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-03 15:13 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

On 05/03/2012 05:52 PM, Mark Brown wrote:
> Are you sure there aren't any boards out there which rely on the
> interrupt base (eg, using a GPIO with the IRQ output of a chip)?

Yes, I'm sure.

-- 
Péter

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-03 15:13         ` Peter Ujfalusi
@ 2012-05-03 15:26           ` Mark Brown
  2012-05-04  8:38             ` Peter Ujfalusi
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2012-05-03 15:26 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

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

On Thu, May 03, 2012 at 06:13:16PM +0300, Peter Ujfalusi wrote:
> On 05/03/2012 05:52 PM, Mark Brown wrote:
> > Are you sure there aren't any boards out there which rely on the
> > interrupt base (eg, using a GPIO with the IRQ output of a chip)?

> Yes, I'm sure.

Because...?

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

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-03 15:26           ` Mark Brown
@ 2012-05-04  8:38             ` Peter Ujfalusi
  2012-05-04  9:08               ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-04  8:38 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

On 05/03/2012 06:26 PM, Mark Brown wrote:
> On Thu, May 03, 2012 at 06:13:16PM +0300, Peter Ujfalusi wrote:
>> On 05/03/2012 05:52 PM, Mark Brown wrote:
>>> Are you sure there aren't any boards out there which rely on the
>>> interrupt base (eg, using a GPIO with the IRQ output of a chip)?
> 
>> Yes, I'm sure.
> 
> Because...?

The irq_base was used to map the nested interrupt numbers somewhere high
enough. twl6040 has one irq line towards the CPU (comes via
i2c_client->irq).
With this change we just change the mapping of the nested interrupt
range provided by twl6040 (instead of hardwired number we ask for
suitable range).
We have sdp4430 and PandaBoard with twl6040. They are fine. The other
non upstream boards should be fine as well with this change.

-- 
Péter

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-04  8:38             ` Peter Ujfalusi
@ 2012-05-04  9:08               ` Mark Brown
  2012-05-04 10:37                 ` Peter Ujfalusi
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2012-05-04  9:08 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

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

On Fri, May 04, 2012 at 11:38:34AM +0300, Peter Ujfalusi wrote:

> The irq_base was used to map the nested interrupt numbers somewhere high
> enough. twl6040 has one irq line towards the CPU (comes via
> i2c_client->irq).
> With this change we just change the mapping of the nested interrupt
> range provided by twl6040 (instead of hardwired number we ask for
> suitable range).

You're not understanding the issue at all - the issue is that if
some driver outside the twl6040 driver is using an interrupt in that
range based off the irq_base that they supplied then you'll break them.
The most common case here is using GPIOs on the device as interrupts.

If this is safe you should at least be making it clear why...

> We have sdp4430 and PandaBoard with twl6040. They are fine. The other
> non upstream boards should be fine as well with this change.

...ideally in a more concrete way than just saying it works on your
reference boards.

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

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-04  9:08               ` Mark Brown
@ 2012-05-04 10:37                 ` Peter Ujfalusi
  2012-05-04 11:22                   ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-04 10:37 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

On 05/04/2012 12:08 PM, Mark Brown wrote:
> You're not understanding the issue at all - the issue is that if
> some driver outside the twl6040 driver is using an interrupt in that
> range based off the irq_base that they supplied then you'll break them.
> The most common case here is using GPIOs on the device as interrupts.

The OMAP platform related drives has been already converted to use
irq_alloc_descs(-1, 0, nr_irqs, 0); to map their range (including GPIO,
twl6030, etc).
Neither of these are using or have irq_base passed from board files for
some time now. We still have the defines for the legacy define based
mapping of the ranges, but it is no longer in use.
To be fair: in arch/arm/mach-omap2/ there's one driver which has not
been converted. It is the gpmc (Flash).

> If this is safe you should at least be making it clear why...

I'll add short explanation to the commit message.

-- 
Péter

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-04 10:37                 ` Peter Ujfalusi
@ 2012-05-04 11:22                   ` Mark Brown
  2012-05-04 11:55                     ` Peter Ujfalusi
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2012-05-04 11:22 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

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

On Fri, May 04, 2012 at 01:37:54PM +0300, Peter Ujfalusi wrote:
> On 05/04/2012 12:08 PM, Mark Brown wrote:

> > You're not understanding the issue at all - the issue is that if
> > some driver outside the twl6040 driver is using an interrupt in that
> > range based off the irq_base that they supplied then you'll break them.
> > The most common case here is using GPIOs on the device as interrupts.

> The OMAP platform related drives has been already converted to use
> irq_alloc_descs(-1, 0, nr_irqs, 0); to map their range (including GPIO,
> twl6030, etc).

How does this work for interrupts on things like SPI and I2C devices?

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

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-04 11:22                   ` Mark Brown
@ 2012-05-04 11:55                     ` Peter Ujfalusi
  2012-05-04 12:17                       ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-04 11:55 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

On 05/04/2012 02:22 PM, Mark Brown wrote:
>> The OMAP platform related drives has been already converted to use
>> irq_alloc_descs(-1, 0, nr_irqs, 0); to map their range (including GPIO,
>> twl6030, etc).
> 
> How does this work for interrupts on things like SPI and I2C devices?

You mean on devices like twl6040, twl6030, twl4030 (I2C MFD devices)?
Or "irq expander" type of devices?

For the later I'm not sure.

-- 
Péter

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-04 11:55                     ` Peter Ujfalusi
@ 2012-05-04 12:17                       ` Mark Brown
  2012-05-04 12:33                         ` Peter Ujfalusi
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2012-05-04 12:17 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

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

On Fri, May 04, 2012 at 02:55:37PM +0300, Peter Ujfalusi wrote:
> On 05/04/2012 02:22 PM, Mark Brown wrote:
> >> The OMAP platform related drives has been already converted to use
> >> irq_alloc_descs(-1, 0, nr_irqs, 0); to map their range (including GPIO,
> >> twl6030, etc).

> > How does this work for interrupts on things like SPI and I2C devices?

> You mean on devices like twl6040, twl6030, twl4030 (I2C MFD devices)?
> Or "irq expander" type of devices?

The latter, and also just any driver that delivers an interrupt via a
GPIO on OMAP - if the GPIO IRQ numbers are all dynamically allocated
then it gets hard to register an off-chip device and tell it which
interrupt to request.

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

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-04 12:17                       ` Mark Brown
@ 2012-05-04 12:33                         ` Peter Ujfalusi
  2012-05-04 12:47                           ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-04 12:33 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

On 05/04/2012 03:17 PM, Mark Brown wrote:
>> You mean on devices like twl6040, twl6030, twl4030 (I2C MFD devices)?
>> Or "irq expander" type of devices?
> 
> The latter, and also just any driver that delivers an interrupt via a
> GPIO on OMAP - if the GPIO IRQ numbers are all dynamically allocated
> then it gets hard to register an off-chip device and tell it which
> interrupt to request.

For GPIO IRQ there's the gpio_to_irq() call which returns the mapped IRQ
number for the given GPIO.
If there is "irq expander" type of chip I assume it would have similar
way to get the IRQ number based on either GPIO number or some other
enumeration value.
The twl6040 does not have such a feature. The interrupts are generated
internally and it has one IRQ line towards the host. We have nested
interrupts for the childs (plug detect is handled by ASoC codec, Vibra
overcurrent is handled by the vibra driver, etc).
Client drivers got their interrupts via platform_get_irq(); they does
not need to know anything about irq_base, or where the IRQ number has
been mapped.

-- 
Péter

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-04 12:33                         ` Peter Ujfalusi
@ 2012-05-04 12:47                           ` Mark Brown
  2012-05-07  6:49                             ` Peter Ujfalusi
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2012-05-04 12:47 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

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

On Fri, May 04, 2012 at 03:33:51PM +0300, Peter Ujfalusi wrote:
> On 05/04/2012 03:17 PM, Mark Brown wrote:

> > The latter, and also just any driver that delivers an interrupt via a
> > GPIO on OMAP - if the GPIO IRQ numbers are all dynamically allocated
> > then it gets hard to register an off-chip device and tell it which
> > interrupt to request.

> For GPIO IRQ there's the gpio_to_irq() call which returns the mapped IRQ
> number for the given GPIO.

This doesn't really work for board files, though - you can only call
gpio_to_irq() at runtime so if you're statically registering the devices
on the board you'd need to make the structures non-const, manage to find
the GPIO controller at runtime then do the lookup, update the struct and
finally register the device.

> If there is "irq expander" type of chip I assume it would have similar
> way to get the IRQ number based on either GPIO number or some other
> enumeration value.

No, there's no generic interface for this.

> The twl6040 does not have such a feature. The interrupts are generated
> internally and it has one IRQ line towards the host. We have nested
> interrupts for the childs (plug detect is handled by ASoC codec, Vibra
> overcurrent is handled by the vibra driver, etc).

OK, that's fine then - you should put this in the changelog to make it
clear that there are no external users who could be affected.

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

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-04 12:47                           ` Mark Brown
@ 2012-05-07  6:49                             ` Peter Ujfalusi
  2012-05-07  9:49                               ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Ujfalusi @ 2012-05-07  6:49 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

On 05/04/2012 03:47 PM, Mark Brown wrote:
> This doesn't really work for board files, though - you can only call
> gpio_to_irq() at runtime so if you're statically registering the devices
> on the board you'd need to make the structures non-const, manage to find
> the GPIO controller at runtime then do the lookup, update the struct and
> finally register the device.

I've seen some board files doing this already (obviously not for the
twl6040). Which is not the nicest thing...

>> If there is "irq expander" type of chip I assume it would have similar
>> way to get the IRQ number based on either GPIO number or some other
>> enumeration value.
> 
> No, there's no generic interface for this.

This dynamic range allocation for IRQ, GPIO will work nicely with DT at
the end. In legacy way we will need to have workarounds. Or just forget
about booting without DT.

>> The twl6040 does not have such a feature. The interrupts are generated
>> internally and it has one IRQ line towards the host. We have nested
>> interrupts for the childs (plug detect is handled by ASoC codec, Vibra
>> overcurrent is handled by the vibra driver, etc).
> 
> OK, that's fine then - you should put this in the changelog to make it
> clear that there are no external users who could be affected.

I'll do that.

Thank you,
Péter

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

* Re: [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically
  2012-05-07  6:49                             ` Peter Ujfalusi
@ 2012-05-07  9:49                               ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2012-05-07  9:49 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Samuel Ortiz, linux-kernel, Misael Lopez Cruz, Benoit Cousson,
	devicetree-discuss, Liam Girdwood

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

On Mon, May 07, 2012 at 09:49:55AM +0300, Peter Ujfalusi wrote:
> On 05/04/2012 03:47 PM, Mark Brown wrote:

> >> If there is "irq expander" type of chip I assume it would have similar
> >> way to get the IRQ number based on either GPIO number or some other
> >> enumeration value.

> > No, there's no generic interface for this.

> This dynamic range allocation for IRQ, GPIO will work nicely with DT at
> the end. In legacy way we will need to have workarounds. Or just forget
> about booting without DT.

Device tree is only available for a small minority of architectures.
Just as you shouldn't assume that people only ever use the reference
boards you work on so you should also not assume that people are using
ARM (and ARM systems that support DT at at that).

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

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

end of thread, other threads:[~2012-05-07  9:49 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-03 12:54 [PATCH 0/3] MFD: twl6040: Device tree support Peter Ujfalusi
2012-05-03 12:54 ` [PATCH 1/3] MFD: twl6040: Code cleanup in interrupt initialization part Peter Ujfalusi
2012-05-03 12:54 ` [PATCH 2/3] MFD: twl6040: Allocate IRQ numbers dynamically Peter Ujfalusi
2012-05-03 13:20   ` Mark Brown
2012-05-03 13:28     ` Peter Ujfalusi
2012-05-03 14:52       ` Mark Brown
2012-05-03 15:13         ` Peter Ujfalusi
2012-05-03 15:26           ` Mark Brown
2012-05-04  8:38             ` Peter Ujfalusi
2012-05-04  9:08               ` Mark Brown
2012-05-04 10:37                 ` Peter Ujfalusi
2012-05-04 11:22                   ` Mark Brown
2012-05-04 11:55                     ` Peter Ujfalusi
2012-05-04 12:17                       ` Mark Brown
2012-05-04 12:33                         ` Peter Ujfalusi
2012-05-04 12:47                           ` Mark Brown
2012-05-07  6:49                             ` Peter Ujfalusi
2012-05-07  9:49                               ` Mark Brown
2012-05-03 12:54 ` [PATCH 3/3] MFD: twl6040: Support for DT Peter Ujfalusi

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