[PATCH] mfd: palmas: Add support for optional wakeup

[PATCH] mfd: palmas: Add support for optional wakeup

[Nishanth Menon]

With the recent pinctrl-single changes, omaps can treat wake-up events
from deeper idle states as interrupts.

Let's add support for the optional second interrupt for wake-up
events. And then SoC can wakeup and handle the event using it's
regular handler.

Finally, to pass the wake-up interrupt in the dts file,
interrupts-extended property needs to be passed.

This is similar in approach to commit 2a0b965cfb6e ("serial: omap: Add
support for optional wake-up")

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 Documentation/devicetree/bindings/mfd/palmas.txt |   20 ++++++++
 drivers/mfd/palmas.c                             |   59 ++++++++++++++++++++++
 include/linux/mfd/palmas.h                       |    2 +
 3 files changed, 81 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
index eda8989..2627842 100644
--- a/Documentation/devicetree/bindings/mfd/palmas.txt
+++ b/Documentation/devicetree/bindings/mfd/palmas.txt
@@ -51,3 +51,23 @@ palmas {
 		....
 	};
 }
+
+Example: with interrupts extended
+ See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+ Use pinmux 0x418 as wakeup interrupt and gpio1_0 as interrupt source
+
+palmas {
+	compatible = "ti,twl6035", "ti,palmas";
+	reg = <0x48>
+	interrupt-parent = <&intc>;
+	interrupt-controller;
+	#interrupt-cells = <2>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	interrupts-extended = <&gpio1 0 IRQ_TYPE_LEVEL_HIGH
+			       &pinmux 0x418>;
+	pmic {
+		compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
+		....
+	};
+}
diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index 28cb048..11186ab 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -24,6 +24,7 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/palmas.h>
 #include <linux/of_device.h>
+#include <linux/of_irq.h>
 
 static const struct regmap_config palmas_regmap_config[PALMAS_NUM_CLIENTS] = {
 	{
@@ -326,6 +327,16 @@ static struct regmap_irq_chip tps65917_irq_chip = {
 			PALMAS_INT1_MASK),
 };
 
+static irqreturn_t palmas_wake_irq(int irq, void *_palmas)
+{
+	/*
+	 * Return Not handled so that interrupt is disabled.
+	 * Level event ensures that the event is eventually handled
+	 * by the appropriate chip handler already registered
+	 */
+	return IRQ_NONE;
+}
+
 int palmas_ext_control_req_config(struct palmas *palmas,
 	enum palmas_external_requestor_id id,  int ext_ctrl, bool enable)
 {
@@ -409,6 +420,7 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
 		pdata->mux_from_pdata = 1;
 		pdata->pad2 = prop;
 	}
+	pdata->wakeirq = irq_of_parse_and_map(node, 1);
 
 	/* The default for this register is all masked */
 	ret = of_property_read_u32(node, "ti,power-ctrl", &prop);
@@ -521,6 +533,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 	i2c_set_clientdata(i2c, palmas);
 	palmas->dev = &i2c->dev;
 	palmas->irq = i2c->irq;
+	palmas->wakeirq = pdata->wakeirq;
 
 	match = of_match_device(of_palmas_match_tbl, &i2c->dev);
 
@@ -587,6 +600,22 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 	if (ret < 0)
 		goto err_i2c;
 
+	if (!palmas->wakeirq)
+		goto no_wake_irq;
+
+	ret = devm_request_irq(palmas->dev, palmas->wakeirq,
+			       palmas_wake_irq,
+			       IRQF_ONESHOT | pdata->irq_flags,
+			       dev_name(palmas->dev),
+			       &palmas);
+	if (ret < 0)
+		goto err_i2c;
+
+	/* We use wakeirq only during suspend-resume path */
+	device_set_wakeup_capable(palmas->dev, true);
+	disable_irq_nosync(palmas->wakeirq);
+
+no_wake_irq:
 no_irq:
 	slave = PALMAS_BASE_TO_SLAVE(PALMAS_PU_PD_OD_BASE);
 	addr = PALMAS_BASE_TO_REG(PALMAS_PU_PD_OD_BASE,
@@ -706,6 +735,34 @@ static int palmas_i2c_remove(struct i2c_client *i2c)
 	return 0;
 }
 
+static int palmas_i2c_suspend(struct i2c_client *i2c,  pm_message_t mesg)
+{
+	struct palmas *palmas = i2c_get_clientdata(i2c);
+	struct device *dev = &i2c->dev;
+
+	if (!palmas->wakeirq)
+		return 0;
+
+	if (device_may_wakeup(dev))
+		enable_irq(palmas->wakeirq);
+
+	return 0;
+}
+
+static int palmas_i2c_resume(struct i2c_client *i2c)
+{
+	struct palmas *palmas = i2c_get_clientdata(i2c);
+	struct device *dev = &i2c->dev;
+
+	if (!palmas->wakeirq)
+		return 0;
+
+	if (device_may_wakeup(dev))
+		disable_irq_nosync(palmas->wakeirq);
+
+	return 0;
+}
+
 static const struct i2c_device_id palmas_i2c_id[] = {
 	{ "palmas", },
 	{ "twl6035", },
@@ -721,6 +778,8 @@ static struct i2c_driver palmas_i2c_driver = {
 		   .of_match_table = of_palmas_match_tbl,
 		   .owner = THIS_MODULE,
 	},
+	.suspend = palmas_i2c_suspend,
+	.resume = palmas_i2c_resume,
 	.probe = palmas_i2c_probe,
 	.remove = palmas_i2c_remove,
 	.id_table = palmas_i2c_id,
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index fb0390a..e8cf4c2 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -75,6 +75,7 @@ struct palmas {
 	/* IRQ Data */
 	int irq;
 	u32 irq_mask;
+	int wakeirq;
 	struct mutex irq_lock;
 	struct regmap_irq_chip_data *irq_data;
 
@@ -377,6 +378,7 @@ struct palmas_clk_platform_data {
 
 struct palmas_platform_data {
 	int irq_flags;
+	int wakeirq;
 	int gpio_base;
 
 	/* bit value to be loaded to the POWER_CTRL register */
-- 
1.7.9.5

Re: [PATCH] mfd: palmas: Add support for optional wakeup

[Lee Jones]

On Tue, 19 Aug 2014, Nishanth Menon wrote:

> With the recent pinctrl-single changes, omaps can treat wake-up events
> from deeper idle states as interrupts.
> 
> Let's add support for the optional second interrupt for wake-up
> events. And then SoC can wakeup and handle the event using it's
> regular handler.
> 
> Finally, to pass the wake-up interrupt in the dts file,
> interrupts-extended property needs to be passed.
> 
> This is similar in approach to commit 2a0b965cfb6e ("serial: omap: Add
> support for optional wake-up")
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  Documentation/devicetree/bindings/mfd/palmas.txt |   20 ++++++++

DT Ack please.

>  drivers/mfd/palmas.c                             |   59 ++++++++++++++++++++++
>  include/linux/mfd/palmas.h                       |    2 +
>  3 files changed, 81 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
> index eda8989..2627842 100644
> --- a/Documentation/devicetree/bindings/mfd/palmas.txt
> +++ b/Documentation/devicetree/bindings/mfd/palmas.txt
> @@ -51,3 +51,23 @@ palmas {
>  		....
>  	};
>  }
> +
> +Example: with interrupts extended
> + See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> + Use pinmux 0x418 as wakeup interrupt and gpio1_0 as interrupt source
> +
> +palmas {

Should this be 'palmas@40 {'?

> +	compatible = "ti,twl6035", "ti,palmas";
> +	reg = <0x48>
> +	interrupt-parent = <&intc>;
> +	interrupt-controller;
> +	#interrupt-cells = <2>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	interrupts-extended = <&gpio1 0 IRQ_TYPE_LEVEL_HIGH
> +			       &pinmux 0x418>;

Can I get a DT Ack, that this is being used correctly?  It doesn't
match the syntax given in:

  Documentation/devicetree/bindings/interrupt-controller/interrupts.txt

> +	pmic {
> +		compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
> +		....
> +	};
> +}
> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
> index 28cb048..11186ab 100644
> --- a/drivers/mfd/palmas.c
> +++ b/drivers/mfd/palmas.c
> @@ -24,6 +24,7 @@
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/palmas.h>
>  #include <linux/of_device.h>
> +#include <linux/of_irq.h>
>  
>  static const struct regmap_config palmas_regmap_config[PALMAS_NUM_CLIENTS] = {
>  	{
> @@ -326,6 +327,16 @@ static struct regmap_irq_chip tps65917_irq_chip = {
>  			PALMAS_INT1_MASK),
>  };
>  
> +static irqreturn_t palmas_wake_irq(int irq, void *_palmas)
> +{
> +	/*
> +	 * Return Not handled so that interrupt is disabled.
> +	 * Level event ensures that the event is eventually handled
> +	 * by the appropriate chip handler already registered
> +	 */

This looks okay to me, but could do with a second opinion from someone
who is a little more familier with this kind of h/w.

How does this differ from threading IRQs?

> +	return IRQ_NONE;
> +}
> +
>  int palmas_ext_control_req_config(struct palmas *palmas,
>  	enum palmas_external_requestor_id id,  int ext_ctrl, bool enable)
>  {
> @@ -409,6 +420,7 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c,
>  		pdata->mux_from_pdata = 1;
>  		pdata->pad2 = prop;
>  	}
> +	pdata->wakeirq = irq_of_parse_and_map(node, 1);
>  
>  	/* The default for this register is all masked */
>  	ret = of_property_read_u32(node, "ti,power-ctrl", &prop);
> @@ -521,6 +533,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
>  	i2c_set_clientdata(i2c, palmas);
>  	palmas->dev = &i2c->dev;
>  	palmas->irq = i2c->irq;
> +	palmas->wakeirq = pdata->wakeirq;
>  
>  	match = of_match_device(of_palmas_match_tbl, &i2c->dev);
>  
> @@ -587,6 +600,22 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
>  	if (ret < 0)
>  		goto err_i2c;
>  
> +	if (!palmas->wakeirq)
> +		goto no_wake_irq;
> +
> +	ret = devm_request_irq(palmas->dev, palmas->wakeirq,
> +			       palmas_wake_irq,
> +			       IRQF_ONESHOT | pdata->irq_flags,
> +			       dev_name(palmas->dev),
> +			       &palmas);
> +	if (ret < 0)
> +		goto err_i2c;
> +
> +	/* We use wakeirq only during suspend-resume path */
> +	device_set_wakeup_capable(palmas->dev, true);
> +	disable_irq_nosync(palmas->wakeirq);
> +
> +no_wake_irq:
>  no_irq:
>  	slave = PALMAS_BASE_TO_SLAVE(PALMAS_PU_PD_OD_BASE);
>  	addr = PALMAS_BASE_TO_REG(PALMAS_PU_PD_OD_BASE,
> @@ -706,6 +735,34 @@ static int palmas_i2c_remove(struct i2c_client *i2c)
>  	return 0;
>  }
>  
> +static int palmas_i2c_suspend(struct i2c_client *i2c,  pm_message_t mesg)
> +{
> +	struct palmas *palmas = i2c_get_clientdata(i2c);
> +	struct device *dev = &i2c->dev;
> +
> +	if (!palmas->wakeirq)
> +		return 0;
> +
> +	if (device_may_wakeup(dev))
> +		enable_irq(palmas->wakeirq);
> +
> +	return 0;
> +}
> +
> +static int palmas_i2c_resume(struct i2c_client *i2c)
> +{
> +	struct palmas *palmas = i2c_get_clientdata(i2c);
> +	struct device *dev = &i2c->dev;
> +
> +	if (!palmas->wakeirq)
> +		return 0;
> +
> +	if (device_may_wakeup(dev))
> +		disable_irq_nosync(palmas->wakeirq);
> +
> +	return 0;
> +}
> +
>  static const struct i2c_device_id palmas_i2c_id[] = {
>  	{ "palmas", },
>  	{ "twl6035", },
> @@ -721,6 +778,8 @@ static struct i2c_driver palmas_i2c_driver = {
>  		   .of_match_table = of_palmas_match_tbl,
>  		   .owner = THIS_MODULE,
>  	},
> +	.suspend = palmas_i2c_suspend,
> +	.resume = palmas_i2c_resume,
>  	.probe = palmas_i2c_probe,
>  	.remove = palmas_i2c_remove,
>  	.id_table = palmas_i2c_id,
> diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
> index fb0390a..e8cf4c2 100644
> --- a/include/linux/mfd/palmas.h
> +++ b/include/linux/mfd/palmas.h
> @@ -75,6 +75,7 @@ struct palmas {
>  	/* IRQ Data */
>  	int irq;
>  	u32 irq_mask;
> +	int wakeirq;
>  	struct mutex irq_lock;
>  	struct regmap_irq_chip_data *irq_data;
>  
> @@ -377,6 +378,7 @@ struct palmas_clk_platform_data {
>  
>  struct palmas_platform_data {
>  	int irq_flags;
> +	int wakeirq;
>  	int gpio_base;
>  
>  	/* bit value to be loaded to the POWER_CTRL register */

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

Re: [PATCH] mfd: palmas: Add support for optional wakeup

[Nishanth Menon]

On 08/29/2014 05:56 AM, Lee Jones wrote:
> On Tue, 19 Aug 2014, Nishanth Menon wrote:
> 
>> With the recent pinctrl-single changes, omaps can treat wake-up events
>> from deeper idle states as interrupts.
>>
>> Let's add support for the optional second interrupt for wake-up
>> events. And then SoC can wakeup and handle the event using it's
>> regular handler.
>>
>> Finally, to pass the wake-up interrupt in the dts file,
>> interrupts-extended property needs to be passed.
>>
>> This is similar in approach to commit 2a0b965cfb6e ("serial: omap: Add
>> support for optional wake-up")
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/palmas.txt |   20 ++++++++
> 
> DT Ack please.
> 
>>  drivers/mfd/palmas.c                             |   59 ++++++++++++++++++++++
>>  include/linux/mfd/palmas.h                       |    2 +
>>  3 files changed, 81 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
>> index eda8989..2627842 100644
>> --- a/Documentation/devicetree/bindings/mfd/palmas.txt
>> +++ b/Documentation/devicetree/bindings/mfd/palmas.txt
>> @@ -51,3 +51,23 @@ palmas {
>>  		....
>>  	};
>>  }
>> +
>> +Example: with interrupts extended
>> + See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>> + Use pinmux 0x418 as wakeup interrupt and gpio1_0 as interrupt source
>> +
>> +palmas {
> 
> Should this be 'palmas@40 {'?

I might have preferred that as well.. I kept the existing style in the
documentation. Would you like me to change existing doc style too?

> 
>> +	compatible = "ti,twl6035", "ti,palmas";
>> +	reg = <0x48>
>> +	interrupt-parent = <&intc>;
>> +	interrupt-controller;
>> +	#interrupt-cells = <2>;
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	interrupts-extended = <&gpio1 0 IRQ_TYPE_LEVEL_HIGH
>> +			       &pinmux 0x418>;
> 
> Can I get a DT Ack, that this is being used correctly?  It doesn't
> match the syntax given in:
> 
>   Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
Did you mean:
<&gpio1 0 IRQ_TYPE_LEVEL_HIGH>, <&pinmux 0x418>;

Yes, I can fix that - sorry, both usage seem to be functional.

> 
>> +	pmic {
>> +		compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
>> +		....
>> +	};
>> +}
>> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
>> index 28cb048..11186ab 100644
>> --- a/drivers/mfd/palmas.c
>> +++ b/drivers/mfd/palmas.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/mfd/core.h>
>>  #include <linux/mfd/palmas.h>
>>  #include <linux/of_device.h>
>> +#include <linux/of_irq.h>
>>  
>>  static const struct regmap_config palmas_regmap_config[PALMAS_NUM_CLIENTS] = {
>>  	{
>> @@ -326,6 +327,16 @@ static struct regmap_irq_chip tps65917_irq_chip = {
>>  			PALMAS_INT1_MASK),
>>  };
>>  
>> +static irqreturn_t palmas_wake_irq(int irq, void *_palmas)
>> +{
>> +	/*
>> +	 * Return Not handled so that interrupt is disabled.
>> +	 * Level event ensures that the event is eventually handled
>> +	 * by the appropriate chip handler already registered
>> +	 */
> 
> This looks okay to me, but could do with a second opinion from someone
> who is a little more familier with this kind of h/w.
> 
> How does this differ from threading IRQs?

I could try with an example:
consider a GPIO block 7 gpio 4 connected to a pinctrl pin 234 as the
interrupt source for palmas.

When the system is active, the GPIO block 7, gpio 4 happily functions
as the interrupt source. However, the SoC might not capable of
achieving SoC wide deepsleep when GPIO block 7 is active, So we have
to power off GPIO block 7. However on achieving low power, the system
needs to be capable of waking backup, for this, SoC uses the hardware
at the pin itself(TI calls it control module, others have other names,
lets for the discussion, call it pinctrl), on going to sleep the
action of enabling the pinctrl irq - enables the wakeup capability of
the pin, and disabling it disabled the wakeup capability. when the
wakeup event does take place, in some cases, it might be a edge event,
where by the time we have recofigured GPIO block, the interrupt event
is long gone - to support this, pinctrl invokes the driver interrupt
handler to ensure this functions. in our case(palmas), we are level
event and can depend on GPIO block to handle it when it is configured.

Basically two interrupt sources when SoC is in deep sleep(1 to exit
from deepsleep, and other from the module handling the actual event) -
Example: powerbutton press OR palmas RTC wakeup OR Palmas GPIO
generated wakeup.

However, this is not the same as threading IRQ as the wakeup event is
involved only during suspend path.

commit 2a0b965cfb6e ("serial: omap: Add support for optional wake-up")

is a good reference from serial port handling perspective.

-- 
Regards,
Nishanth Menon

Re: [PATCH] mfd: palmas: Add support for optional wakeup

[Lee Jones]

On Fri, 29 Aug 2014, Nishanth Menon wrote:
> On 08/29/2014 05:56 AM, Lee Jones wrote:
> > On Tue, 19 Aug 2014, Nishanth Menon wrote:
> > 
> >> With the recent pinctrl-single changes, omaps can treat wake-up events
> >> from deeper idle states as interrupts.
> >>
> >> Let's add support for the optional second interrupt for wake-up
> >> events. And then SoC can wakeup and handle the event using it's
> >> regular handler.
> >>
> >> Finally, to pass the wake-up interrupt in the dts file,
> >> interrupts-extended property needs to be passed.
> >>
> >> This is similar in approach to commit 2a0b965cfb6e ("serial: omap: Add
> >> support for optional wake-up")
> >>
> >> Signed-off-by: Nishanth Menon <nm@ti.com>
> >> ---
> >>  Documentation/devicetree/bindings/mfd/palmas.txt |   20 ++++++++
> > 
> > DT Ack please.

Please read Documentation/devicetree/bindings/submittingpatches.txt

> >>  drivers/mfd/palmas.c                             |   59 ++++++++++++++++++++++
> >>  include/linux/mfd/palmas.h                       |    2 +
> >>  3 files changed, 81 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
> >> index eda8989..2627842 100644
> >> --- a/Documentation/devicetree/bindings/mfd/palmas.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/palmas.txt
> >> @@ -51,3 +51,23 @@ palmas {
> >>  		....
> >>  	};
> >>  }
> >> +
> >> +Example: with interrupts extended
> >> + See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> >> + Use pinmux 0x418 as wakeup interrupt and gpio1_0 as interrupt source
> >> +
> >> +palmas {
> > 
> > Should this be 'palmas@40 {'?
> 
> I might have preferred that as well.. I kept the existing style in the
> documentation. Would you like me to change existing doc style too?

Yes please.  Although you can do this subseqently.

[...]

> >> +static irqreturn_t palmas_wake_irq(int irq, void *_palmas)
> >> +{
> >> +	/*
> >> +	 * Return Not handled so that interrupt is disabled.
> >> +	 * Level event ensures that the event is eventually handled
> >> +	 * by the appropriate chip handler already registered
> >> +	 */
> > 
> > This looks okay to me, but could do with a second opinion from someone
> > who is a little more familier with this kind of h/w.
> > 
> > How does this differ from threading IRQs?
> 
> I could try with an example:
> consider a GPIO block 7 gpio 4 connected to a pinctrl pin 234 as the
> interrupt source for palmas.
> 
> When the system is active, the GPIO block 7, gpio 4 happily functions
> as the interrupt source. However, the SoC might not capable of
> achieving SoC wide deepsleep when GPIO block 7 is active, So we have
> to power off GPIO block 7. However on achieving low power, the system
> needs to be capable of waking backup, for this, SoC uses the hardware
> at the pin itself(TI calls it control module, others have other names,
> lets for the discussion, call it pinctrl), on going to sleep the
> action of enabling the pinctrl irq - enables the wakeup capability of
> the pin, and disabling it disabled the wakeup capability. when the
> wakeup event does take place, in some cases, it might be a edge event,
> where by the time we have recofigured GPIO block, the interrupt event
> is long gone - to support this, pinctrl invokes the driver interrupt
> handler to ensure this functions. in our case(palmas), we are level
> event and can depend on GPIO block to handle it when it is configured.
> 
> Basically two interrupt sources when SoC is in deep sleep(1 to exit
> from deepsleep, and other from the module handling the actual event) -
> Example: powerbutton press OR palmas RTC wakeup OR Palmas GPIO
> generated wakeup.
> 
> However, this is not the same as threading IRQ as the wakeup event is
> involved only during suspend path.
> 
> commit 2a0b965cfb6e ("serial: omap: Add support for optional wake-up")
> 
> is a good reference from serial port handling perspective.

Thanks for the explanation.  This makes sense now.

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

Re: [PATCH] mfd: palmas: Add support for optional wakeup

[Nishanth Menon]

On 09/01/2014 04:32 AM, Lee Jones wrote:
> On Fri, 29 Aug 2014, Nishanth Menon wrote:
>> On 08/29/2014 05:56 AM, Lee Jones wrote:
>>> On Tue, 19 Aug 2014, Nishanth Menon wrote:
>>>
>>>> With the recent pinctrl-single changes, omaps can treat wake-up events
>>>> from deeper idle states as interrupts.
>>>>
>>>> Let's add support for the optional second interrupt for wake-up
>>>> events. And then SoC can wakeup and handle the event using it's
>>>> regular handler.
>>>>
>>>> Finally, to pass the wake-up interrupt in the dts file,
>>>> interrupts-extended property needs to be passed.
>>>>
>>>> This is similar in approach to commit 2a0b965cfb6e ("serial: omap: Add
>>>> support for optional wake-up")
>>>>
>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>> ---
>>>>   Documentation/devicetree/bindings/mfd/palmas.txt |   20 ++++++++
>>>
>>> DT Ack please.
>
> Please read Documentation/devicetree/bindings/submittingpatches.txt

I assume you wanted me to note the following:
"
The Documentation/ portion of the patch should be a separate patch.
"

Many maintainers prefer that when the bindings for the device is new, 
and when additional properties are added, they prefer it part of same 
patch.. Anyways.. if the above is what you prefer, I can follow that.

In short, I assume you'd like three patches:
a) fix up style of current documentation - palmas to palmas@40
b) Split documentation for interrupt-extended from the current patch 
into it's own patch.
c) remainder of this patch as is..

Does that sound right?

---
Regards,
Nishanth Menon

Re: [PATCH] mfd: palmas: Add support for optional wakeup

[Lee Jones]

On Mon, 01 Sep 2014, Nishanth Menon wrote:

> On 09/01/2014 04:32 AM, Lee Jones wrote:
> >On Fri, 29 Aug 2014, Nishanth Menon wrote:
> >>On 08/29/2014 05:56 AM, Lee Jones wrote:
> >>>On Tue, 19 Aug 2014, Nishanth Menon wrote:
> >>>
> >>>>With the recent pinctrl-single changes, omaps can treat wake-up events
> >>>>from deeper idle states as interrupts.
> >>>>
> >>>>Let's add support for the optional second interrupt for wake-up
> >>>>events. And then SoC can wakeup and handle the event using it's
> >>>>regular handler.
> >>>>
> >>>>Finally, to pass the wake-up interrupt in the dts file,
> >>>>interrupts-extended property needs to be passed.
> >>>>
> >>>>This is similar in approach to commit 2a0b965cfb6e ("serial: omap: Add
> >>>>support for optional wake-up")
> >>>>
> >>>>Signed-off-by: Nishanth Menon <nm@ti.com>
> >>>>---
> >>>>  Documentation/devicetree/bindings/mfd/palmas.txt |   20 ++++++++
> >>>
> >>>DT Ack please.
> >
> >Please read Documentation/devicetree/bindings/submittingpatches.txt
> 
> I assume you wanted me to note the following:
> "
> The Documentation/ portion of the patch should be a separate patch.
> "
> 
> Many maintainers prefer that when the bindings for the device is
> new, and when additional properties are added, they prefer it part
> of same patch.. Anyways.. if the above is what you prefer, I can
> follow that.

I tend to like it done properly please.

> In short, I assume you'd like three patches:
> a) fix up style of current documentation - palmas to palmas@40
> b) Split documentation for interrupt-extended from the current patch
> into it's own patch.
> c) remainder of this patch as is..
> 
> Does that sound right?

That does, thanks.

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