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

[PATCH 1/2] 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>
---
 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

[PATCH 2/2] mfd: palmas: get rid of of_platform_populate() in DT registration

[Laxman Dewangan]

If driver is getting registered through DT then it look for
population of platform data which is not possible if platform
completely support the DT.

In this case, if device is registered through DT then just ignore
platform data population and continue the further registration.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/mfd/palmas.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index 25f0eab..6771d9b 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -465,18 +465,6 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 	if (ret)
 		goto err_irq;
 
-	/*
-	 * If we are probing with DT do this the DT way and return here
-	 * otherwise continue and add devices using mfd helpers.
-	 */
-	if (node) {
-		ret = of_platform_populate(node, NULL, NULL, &i2c->dev);
-		if (ret < 0)
-			goto err_irq;
-		else
-			return ret;
-	}
-
 	children = kmemdup(palmas_children, sizeof(palmas_children),
 			   GFP_KERNEL);
 	if (!children) {
@@ -484,6 +472,14 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 		goto err_irq;
 	}
 
+	/*
+	 * If we are probing with DT then skip the platform data initialisation
+	 * for children.
+	 */
+	if (node)
+		goto skip_platform_data_init;
+
+
 	children[PALMAS_PMIC_ID].platform_data = pdata->pmic_pdata;
 	children[PALMAS_PMIC_ID].pdata_size = sizeof(*pdata->pmic_pdata);
 
@@ -500,6 +496,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 	children[PALMAS_CLK_ID].platform_data = pdata->clk_pdata;
 	children[PALMAS_CLK_ID].pdata_size = sizeof(*pdata->clk_pdata);
 
+skip_platform_data_init:
 	ret = mfd_add_devices(palmas->dev, -1,
 			      children, ARRAY_SIZE(palmas_children),
 			      NULL, 0,
-- 
1.7.1.1

Re: [PATCH 2/2] mfd: palmas: get rid of of_platform_populate() in DT registration

[Graeme Gregory]

On 27/02/13 14:08, Laxman Dewangan wrote:
> If driver is getting registered through DT then it look for
> population of platform data which is not possible if platform
> completely support the DT.
No it doesnt!
> In this case, if device is registered through DT then just ignore
> platform data population and continue the further registration.
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  drivers/mfd/palmas.c |   21 +++++++++------------
>  1 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
> index 25f0eab..6771d9b 100644
> --- a/drivers/mfd/palmas.c
> +++ b/drivers/mfd/palmas.c
> @@ -465,18 +465,6 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
>  	if (ret)
>  		goto err_irq;
>  
> -	/*
> -	 * If we are probing with DT do this the DT way and return here
> -	 * otherwise continue and add devices using mfd helpers.
> -	 */
> -	if (node) {
> -		ret = of_platform_populate(node, NULL, NULL, &i2c->dev);
> -		if (ret < 0)
> -			goto err_irq;
> -		else
> -			return ret;
> -	}
> -
At this point we exit the function and do nothing with the platform data
which should work on wholy DT systems. I have tested it!
>  	children = kmemdup(palmas_children, sizeof(palmas_children),
>  			   GFP_KERNEL);
>  	if (!children) {
> @@ -484,6 +472,14 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
>  		goto err_irq;
>  	}
>  
> +	/*
> +	 * If we are probing with DT then skip the platform data initialisation
> +	 * for children.
> +	 */
> +	if (node)
> +		goto skip_platform_data_init;
> +
> +
>  	children[PALMAS_PMIC_ID].platform_data = pdata->pmic_pdata;
>  	children[PALMAS_PMIC_ID].pdata_size = sizeof(*pdata->pmic_pdata);
>  
> @@ -500,6 +496,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
>  	children[PALMAS_CLK_ID].platform_data = pdata->clk_pdata;
>  	children[PALMAS_CLK_ID].pdata_size = sizeof(*pdata->clk_pdata);
>  
> +skip_platform_data_init:
>  	ret = mfd_add_devices(palmas->dev, -1,
>  			      children, ARRAY_SIZE(palmas_children),
>  			      NULL, 0,
I think this change is wrong! It seems to be based on a misreading of
the code!

Graeme

Re: [PATCH 2/2] mfd: palmas: get rid of of_platform_populate() in DT registration

[Laxman Dewangan]

On Wednesday 27 February 2013 07:51 PM, Graeme Gregory wrote:
> On 27/02/13 14:08, Laxman Dewangan wrote:
>> If driver is getting registered through DT then it look for
>> population of platform data which is not possible if platform
>> completely support the DT.
> No it doesnt!
>> In this case, if device is registered through DT then just ignore
>> platform data population and continue the further registration.
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>> ---
>>   drivers/mfd/palmas.c |   21 +++++++++------------
>>   1 files changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
>> index 25f0eab..6771d9b 100644
>> --- a/drivers/mfd/palmas.c
>> +++ b/drivers/mfd/palmas.c
>> @@ -465,18 +465,6 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
>>   	if (ret)
>>   		goto err_irq;
>>   
>> -	/*
>> -	 * If we are probing with DT do this the DT way and return here
>> -	 * otherwise continue and add devices using mfd helpers.
>> -	 */
>> -	if (node) {
>> -		ret = of_platform_populate(node, NULL, NULL, &i2c->dev);
>> -		if (ret < 0)
>> -			goto err_irq;
>> -		else
>> -			return ret;
>> -	}
>> -
> At this point we exit the function and do nothing with the platform data
> which should work on wholy DT systems. I have tested it!


I think then dt files are different in this case. I did not saw the 
documentation of dt file.
But following is my entry in dts file which we follow for other devices:

         i2c@7000d000 {
                 status = "okay";
                 clock-frequency = <100000>;

   ::::::::::::::
                 tps65913@58 {
                         compatible = "ti,palmas";
                         reg = <0x58>;
                         interrupts = <0 86 0x4>;

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

                         #gpio-cells = <2>;
                         gpio-controller;

                         regulators {
                                 smps12 {
                                         regulator-name = "palmas-smps12";
                                         regulator-min-microvolt = 
<1350000>;
                                         regulator-max-microvolt = 
<1350000>;
                                         regulator-always-on;
                                         regulator-boot-on;
                                 };

                                 smps3 {
                                         regulator-name = "palmas-smps3";
                                         regulator-min-microvolt = 
<1800000>;
                                         regulator-max-microvolt = 
<1800000>;
                                         regulator-always-on;
                                         regulator-boot-on;
                                 };
:::::::::::::::
                     };
             };
     };


In the above DT file, it does not work.

Re: [PATCH 2/2] mfd: palmas: get rid of of_platform_populate() in DT registration

[Graeme Gregory]

On 27/02/13 14:26, Laxman Dewangan wrote:
> On Wednesday 27 February 2013 07:51 PM, Graeme Gregory wrote:
>> On 27/02/13 14:08, Laxman Dewangan wrote:
>>> If driver is getting registered through DT then it look for
>>> population of platform data which is not possible if platform
>>> completely support the DT.
>> No it doesnt!
>>> In this case, if device is registered through DT then just ignore
>>> platform data population and continue the further registration.
>>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>>> ---
>>>   drivers/mfd/palmas.c |   21 +++++++++------------
>>>   1 files changed, 9 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
>>> index 25f0eab..6771d9b 100644
>>> --- a/drivers/mfd/palmas.c
>>> +++ b/drivers/mfd/palmas.c
>>> @@ -465,18 +465,6 @@ static int palmas_i2c_probe(struct i2c_client
>>> *i2c,
>>>       if (ret)
>>>           goto err_irq;
>>>   -    /*
>>> -     * If we are probing with DT do this the DT way and return here
>>> -     * otherwise continue and add devices using mfd helpers.
>>> -     */
>>> -    if (node) {
>>> -        ret = of_platform_populate(node, NULL, NULL, &i2c->dev);
>>> -        if (ret < 0)
>>> -            goto err_irq;
>>> -        else
>>> -            return ret;
>>> -    }
>>> -
>> At this point we exit the function and do nothing with the platform data
>> which should work on wholy DT systems. I have tested it!
>
>
> I think then dt files are different in this case. I did not saw the
> documentation of dt file.
> But following is my entry in dts file which we follow for other devices:
>
>         i2c@7000d000 {
>                 status = "okay";
>                 clock-frequency = <100000>;
>
>   ::::::::::::::
>                 tps65913@58 {
>                         compatible = "ti,palmas";
>                         reg = <0x58>;
>                         interrupts = <0 86 0x4>;
>
>                         #interrupt-cells = <2>;
>                         interrupt-controller;
>
>                         #gpio-cells = <2>;
>                         gpio-controller;
>
>                         regulators {
>                                 smps12 {
>                                         regulator-name = "palmas-smps12";
>                                         regulator-min-microvolt =
> <1350000>;
>                                         regulator-max-microvolt =
> <1350000>;
>                                         regulator-always-on;
>                                         regulator-boot-on;
>                                 };
>
>                                 smps3 {
>                                         regulator-name = "palmas-smps3";
>                                         regulator-min-microvolt =
> <1800000>;
>                                         regulator-max-microvolt =
> <1800000>;
>                                         regulator-always-on;
>                                         regulator-boot-on;
>                                 };
> :::::::::::::::
>                     };
>             };
>     };
>
>
> In the above DT file, it does not work.
>
>
I did slip up and forget to push the documentation file, it has since
started the upstream here

http://article.gmane.org/gmane.linux.documentation/9491/match=palmas+dts

The design of the palmas was to make it properly hierarchical to stop
nasty hacks like accessing parent of parent. As a MFD children should
not be tightly bonded to the MFD device!
 
Graeme