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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140901093206.GJ7374@lee--X1>
Date:	Mon, 1 Sep 2014 10:32:06 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	Nishanth Menon <nm@...com>
Cc:	Samuel Ortiz <sameo@...ux.intel.com>,
	Mark Brown <broonie@...aro.org>,
	Tony Lindgren <tony@...mide.com>, Keerthy <j-keerthy@...com>,
	devicetree@...r.kernel.org, linux-omap@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mfd: palmas: Add support for optional wakeup

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@...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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/