[PATCH] mfd: palmas: provide irq flags through DT/platform data

[PATCH] mfd: palmas: provide irq flags through DT/platform data

[Laxman Dewangan]

Currently driver sets the irq type to IRQF_TRIGGER_LOW which is
causing interrupt registration failure in ARM based SoCs as:
[    0.208479] genirq: Setting trigger mode 8 for irq 118 failed (gic_set_type+0x0/0xf0)
[    0.208513] dummy 0-0059: Failed to request IRQ 118: -22

Provide the irq flags through platform data if device is registered
through board file or get the irq type from DT node property in place
of hardcoding the irq flag in driver to support multiple platforms.

Also configure the device to generate the interrupt signal according to
flag type.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
This patch was 1/3 of earlier patch and the 2/2 is not needed if child
nodes are registered in a recomended DTS file. The discussion is still
going on on the dts file.

However, this patch is independent of discussion and dts file and hence
separating this patch from series and resending as independent patch.

 drivers/mfd/palmas.c       |   42 +++++++++++++++++++++++++++++++++++++++---
 include/linux/mfd/palmas.h |    1 +
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index bbdbc50..25f0eab 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -257,9 +257,24 @@ static struct regmap_irq_chip palmas_irq_chip = {
 			PALMAS_INT1_MASK),
 };
 
-static void palmas_dt_to_pdata(struct device_node *node,
+static int palmas_set_pdata_irq_flag(struct i2c_client *i2c,
 		struct palmas_platform_data *pdata)
 {
+	struct irq_data *irq_data = irq_get_irq_data(i2c->irq);
+	if (!irq_data) {
+		dev_err(&i2c->dev, "Invalid IRQ: %d\n", i2c->irq);
+		return -EINVAL;
+	}
+
+	pdata->irq_flags = irqd_get_trigger_type(irq_data);
+	dev_info(&i2c->dev, "Irq flag is 0x%08x\n", pdata->irq_flags);
+	return 0;
+}
+
+static void palmas_dt_to_pdata(struct i2c_client *i2c,
+		struct palmas_platform_data *pdata)
+{
+	struct device_node *node = i2c->dev.of_node;
 	int ret;
 	u32 prop;
 
@@ -283,6 +298,8 @@ static void palmas_dt_to_pdata(struct device_node *node,
 		pdata->power_ctrl = PALMAS_POWER_CTRL_NSLEEP_MASK |
 					PALMAS_POWER_CTRL_ENABLE1_MASK |
 					PALMAS_POWER_CTRL_ENABLE2_MASK;
+	if (i2c->irq)
+		palmas_set_pdata_irq_flag(i2c, pdata);
 }
 
 static int palmas_i2c_probe(struct i2c_client *i2c,
@@ -304,7 +321,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 		if (!pdata)
 			return -ENOMEM;
 
-		palmas_dt_to_pdata(node, pdata);
+		palmas_dt_to_pdata(i2c, pdata);
 	}
 
 	if (!pdata)
@@ -344,6 +361,25 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 		}
 	}
 
+	/* Change interrupt line output polarity */
+	ret = palmas_read(palmas, PALMAS_PU_PD_OD_BASE,
+				PALMAS_POLARITY_CTRL, &reg);
+	if (ret < 0) {
+		dev_err(palmas->dev, "POLARITY_CTRL read failed: %d\n", ret);
+		goto err;
+	}
+
+	if (pdata->irq_flags & IRQ_TYPE_LEVEL_HIGH)
+		reg |= PALMAS_POLARITY_CTRL_INT_POLARITY;
+	else
+		reg &= ~PALMAS_POLARITY_CTRL_INT_POLARITY;
+	ret = palmas_write(palmas, PALMAS_PU_PD_OD_BASE,
+			PALMAS_POLARITY_CTRL, reg);
+	if (ret < 0) {
+		dev_err(palmas->dev, "POLARITY_CTRL write failed: %d\n", ret);
+		goto err;
+	}
+
 	/* Change IRQ into clear on read mode for efficiency */
 	slave = PALMAS_BASE_TO_SLAVE(PALMAS_INTERRUPT_BASE);
 	addr = PALMAS_BASE_TO_REG(PALMAS_INTERRUPT_BASE, PALMAS_INT_CTRL);
@@ -352,7 +388,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 	regmap_write(palmas->regmap[slave], addr, reg);
 
 	ret = regmap_add_irq_chip(palmas->regmap[slave], palmas->irq,
-			IRQF_ONESHOT | IRQF_TRIGGER_LOW, 0, &palmas_irq_chip,
+			IRQF_ONESHOT | pdata->irq_flags, 0, &palmas_irq_chip,
 			&palmas->irq_data);
 	if (ret < 0)
 		goto err;
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index a4d13d7..3bbda22 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -221,6 +221,7 @@ struct palmas_clk_platform_data {
 };
 
 struct palmas_platform_data {
+	int irq_flags;
 	int gpio_base;
 
 	/* bit value to be loaded to the POWER_CTRL register */
-- 
1.7.1.1

Re: [PATCH] mfd: palmas: provide irq flags through DT/platform data

[Mark Brown]

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

On Fri, Mar 01, 2013 at 06:04:56PM +0530, Laxman Dewangan wrote:

> Currently driver sets the irq type to IRQF_TRIGGER_LOW which is
> causing interrupt registration failure in ARM based SoCs as:
> [    0.208479] genirq: Setting trigger mode 8 for irq 118 failed (gic_set_type+0x0/0xf0)
> [    0.208513] dummy 0-0059: Failed to request IRQ 118: -22

This can't be a generic problem on ARM systems, I'm pretty sure the
primary users of palmas would've noticed, this is more of a new feature
isn't it?

> +	/* Change interrupt line output polarity */
> +	ret = palmas_read(palmas, PALMAS_PU_PD_OD_BASE,
> +				PALMAS_POLARITY_CTRL, &reg);
> +	if (ret < 0) {
> +		dev_err(palmas->dev, "POLARITY_CTRL read failed: %d\n", ret);
> +		goto err;
> +	}
> +
> +	if (pdata->irq_flags & IRQ_TYPE_LEVEL_HIGH)
> +		reg |= PALMAS_POLARITY_CTRL_INT_POLARITY;
> +	else
> +		reg &= ~PALMAS_POLARITY_CTRL_INT_POLARITY;
> +	ret = palmas_write(palmas, PALMAS_PU_PD_OD_BASE,
> +			PALMAS_POLARITY_CTRL, reg);
> +	if (ret < 0) {
> +		dev_err(palmas->dev, "POLARITY_CTRL write failed: %d\n", ret);
> +		goto err;
> +	}

Isn't there a read/modify/write call for palmas?

Otherwise looks good; I wonder if we even need the platform data though
I can't see it hurting.

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

Re: [PATCH] mfd: palmas: provide irq flags through DT/platform data

[Laxman Dewangan]

On Friday 01 March 2013 06:13 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Fri, Mar 01, 2013 at 06:04:56PM +0530, Laxman Dewangan wrote:
>
>> Currently driver sets the irq type to IRQF_TRIGGER_LOW which is
>> causing interrupt registration failure in ARM based SoCs as:
>> [    0.208479] genirq: Setting trigger mode 8 for irq 118 failed (gic_set_type+0x0/0xf0)
>> [    0.208513] dummy 0-0059: Failed to request IRQ 118: -22
> This can't be a generic problem on ARM systems, I'm pretty sure the
> primary users of palmas would've noticed, this is more of a new feature
> isn't it?

I think it is tested with eval board and connected to gpio interrupt and 
hence it is not noticed.


>
>> +	if (ret < 0) {
>> +		dev_err(palmas->dev, "POLARITY_CTRL write failed: %d\n", ret);
>> +		goto err;
>> +	}
> Isn't there a read/modify/write call for palmas?

Yaah, there is call but forget as I took this fix from my downstream code.
I will respin the next patch.

Re: [PATCH] mfd: palmas: provide irq flags through DT/platform data

[Mark Brown]

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

On Fri, Mar 01, 2013 at 06:25:24PM +0530, Laxman Dewangan wrote:
> On Friday 01 March 2013 06:13 PM, Mark Brown wrote:

> >This can't be a generic problem on ARM systems, I'm pretty sure the
> >primary users of palmas would've noticed, this is more of a new feature
> >isn't it?

> I think it is tested with eval board and connected to gpio interrupt
> and hence it is not noticed.

One of the palmas chips is the default PMIC for OMAP5 isn't it?

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

Re: [PATCH] mfd: palmas: provide irq flags through DT/platform data

[Stephen Warren]

On 03/01/2013 06:16 AM, Mark Brown wrote:
> On Fri, Mar 01, 2013 at 06:25:24PM +0530, Laxman Dewangan wrote:
>> On Friday 01 March 2013 06:13 PM, Mark Brown wrote:
> 
>>> This can't be a generic problem on ARM systems, I'm pretty sure
>>> the primary users of palmas would've noticed, this is more of a
>>> new feature isn't it?
> 
>> I think it is tested with eval board and connected to gpio
>> interrupt and hence it is not noticed.
> 
> One of the palmas chips is the default PMIC for OMAP5 isn't it?

A tangential question more re: DT bindings for it:

Is Palmas a family of chips rather than a single chip then? That
implies that the DT would need two compatible values, e.g.:

compatible = "ti,12345", "ti,palmas";

... where "12345" is the actual chip name.

... rather than just the following which IIRC was in the example in
the DT binding document in another patch series:

compatible = "ti,palmas";

Re: [PATCH] mfd: palmas: provide irq flags through DT/platform data

[Mark Brown]

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

On Fri, Mar 01, 2013 at 12:34:40PM -0700, Stephen Warren wrote:

> Is Palmas a family of chips rather than a single chip then? That
> implies that the DT would need two compatible values, e.g.:

Yes.

> compatible = "ti,12345", "ti,palmas";

> ... where "12345" is the actual chip name.

> ... rather than just the following which IIRC was in the example in
> the DT binding document in another patch series:

> compatible = "ti,palmas";

Indeed, and in fact this has already been done for the I2C device ID
table.  We should have the same list of devices in the OF IDs.

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

Re: [PATCH] mfd: palmas: provide irq flags through DT/platform data

[Graeme Gregory]

On 02/03/13 11:35, Mark Brown wrote:
> On Fri, Mar 01, 2013 at 12:34:40PM -0700, Stephen Warren wrote:
>
>> Is Palmas a family of chips rather than a single chip then? That
>> implies that the DT would need two compatible values, e.g.:
> Yes.
>
>> compatible = "ti,12345", "ti,palmas";
>> ... where "12345" is the actual chip name.
>> ... rather than just the following which IIRC was in the example in
>> the DT binding document in another patch series:
>> compatible = "ti,palmas";
> Indeed, and in fact this has already been done for the I2C device ID
> table.  We should have the same list of devices in the OF IDs.
Currently all members of the palmas family I know about (from memory).

For palmas :-
twl6035, twl6037, tps65913, tps65914

For palmas-charger :-
twl6036, tps80036

All with varying IP blocks or hardware configuration.

Mainly this is due to misunderstanding I had of DT definitions when I
originally read docs, when driver project commenced. I will work with
Ian, Laxman, Keerthy and get these updated.

Should these also flow down into the various drivers for the IP blocks? eg.

compatible = "ti,twl6035-regulator", "ti,palmas-regulator";

Graeme

Re: [PATCH] mfd: palmas: provide irq flags through DT/platform data

[Mark Brown]

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

On Sat, Mar 02, 2013 at 08:13:35PM +0800, Graeme Gregory wrote:

> Should these also flow down into the various drivers for the IP blocks? eg.

> compatible = "ti,twl6035-regulator", "ti,palmas-regulator";

Ideally, in case there's chip specific issues.

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