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

2014-09-02 Thread 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 n...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-09-01 Thread 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 n...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-09-01 Thread 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 n...@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


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-08-29 Thread 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 n...@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

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

2014-08-29 Thread 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 n...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2014-08-19 Thread 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 n...@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 =