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).
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. "
>
>>
>> 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
[...]
--
regards,
-grygorii
--
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
