Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






On 08.04.2016 11:18, Grygorii Strashko wrote:
On 04/07/2016 08:11 PM, Marcin Niestroj wrote:

On 07.04.2016 12:48, Grygorii Strashko wrote:
On 04/06/2016 10:28 PM, Marcin Niestroj wrote:
Hi,

On 06.04.2016 21:11, Tony Lindgren wrote:
* Marcin Niestroj <m.niestroj@xxxxxxxxxxxxxxxx> [160406 09:34]:
Support configuration of ext_wakeup sources. This patch makes it
possible to enable ext_wakeup (and set it's polarity), depending on
board configuration. AM335x's dedicated PMIC (tps65217) uses
ext_wakeup
in SLEEP mode (RTC-only) to notify about power-button presses.
Handling
power-button presses enables to recover from RTC-only power states
correctly.

Implementation uses gpiochip to utilize standard bindings. However,
configuration is possible only using device-tree (no runtime changes).

Hey looks good to me, adding linux-omap list to Cc.

Can you please check that the "depends on GPIOLIB" does not disable
the RTC driver for omap1?

Looks ok for omap1. ARCH_OMAP1 selects ARCH_REQUIRE_GPIOLIB, which
selects GPIOLIB.

Best regards,
Marcin


Regards,

Tony

Signed-off-by: Marcin Niestroj <m.niestroj@xxxxxxxxxxxxxxxx>
---
   Documentation/devicetree/bindings/rtc/rtc-omap.txt |  18 ++-
   drivers/rtc/Kconfig                                |   2 +-
   drivers/rtc/rtc-omap.c                             | 137
++++++++++++++++++++-
   3 files changed, 154 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
index bf7d11a..4a7738e 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
@@ -18,8 +18,12 @@ Optional properties:
     through pmic_power_en
   - clocks: Any internal or external clocks feeding in to rtc
   - clock-names: Corresponding names of the clocks
+- gpio-controller: Mark as gpio controller when using ext_wakeup
+- #gpio-cells: Should be set to 2
+- ngpios: Number of ext_wakeup sources supported by processor (board)
+- ext-wakeup-gpios: List of ext_wakeup sources to configure

-Example:
+Examples:

   rtc@1c23000 {
       compatible = "ti,da830-rtc";
@@ -31,3 +35,15 @@ rtc@1c23000 {
       clocks = <&clk_32k_rtc>, <&clk_32768_ck>;
       clock-names = "ext-clk", "int-clk";
   };
+
+rtc: rtc@44e3e000 {
+    compatible = "ti,am3352-rtc", "ti,da830-rtc";
+    reg = <0x44e3e000 0x1000>;
+    interrupts = <75
+              76>;
+    system-power-controller;
+    gpio-controller;
+    #gpio-cells = <2>;
+    ngpios = <1>;
+    ext-wakeup-gpios = <&rtc 0 GPIO_ACTIVE_LOW>;
+};

I'm not sure that rtc can request gpios by itself in this
way (if i rememberer this can break modules isnmod/rmmod).

cc: linux-gpio

The gpio-hog is more correct way follow if I'm not mistaken)
- see gpiochip_request_own_desc().

You are right. It is not possible to rmmod module, as it is "in use"
all the time. I'll try with gpio_requst_own_desc().


Another question, in commit message you refer to  power-button,
but i do not see anything related in binding description.

We don't have power-button connected right to the processor. It is
connected to PMIC. During runtime we receive IRQs about power-button
from PMIC using i2c bus. The only purpose of this patch is to
configure processor's ext_wakeup line, which is triggered during
RTC-only mode (for example when power-button is pressed), causing
device wakeup. On the other hand, it is not possible to use ext_wakeup
during runtime, as we are only able to read it's status, but it
cannot trigger any interrupts.

Sry, but I don't like this approach - it could make sense if RTC
EXT_WAKEUP will be at least partially mapped on gpiolib interface.
But your gpiochip is fake, you do not/can't use GPIO hogging mechanism
and you're even parsing DT on your own (in V3).

With gpio hogging we can't pass polarity to the driver. It is hidden
in gpiolib.


In general it's more like irqchip than gpiochip, but RTC can
trigger IRQ on ext_wakeup :(

As per above, your first version of the patch looks more sensible to me.

Additional note. Shouldn't EXT_WAKEUP_STATUS be cleared after wake up?
This is requested by am57x trm at least: "SW must clear the events before
PMIC_PWR_ENABLE can go from 1 - 0. "

I haven't seen that in am335x TRM. In current implementation if
EXT_WAKEUP_STATUS is set, we read it and write it back together with EXT_WAKEUP_EN and EXT_WAKEUP_POL, which results in clearing of this
event.




Shouldn't some gpio-key node be here, which will consume rtc-gpio?


diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 3e84315..f013346 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig

[...]



--
Marcin Niestroj
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux