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

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

[Grygorii Strashko]

On 04/06/2016 10:28 PM, Marcin Niestroj wrote:
> Hi,
>
> On 06.04.2016 21:11, Tony Lindgren wrote:
>> * Marcin Niestroj <m.niestroj@grinn-global.com> [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@grinn-global.com>
>>> ---
>>>   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().

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

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
>>> @@ -1208,7 +1208,7 @@ config RTC_DRV_IMXDI
>>>
>>>   config RTC_DRV_OMAP
>>>       tristate "TI OMAP Real Time Clock"
>>> -    depends on ARCH_OMAP || ARCH_DAVINCI || COMPILE_TEST
>>> +    depends on (ARCH_OMAP || ARCH_DAVINCI || COMPILE_TEST) && GPIOLIB
>>>       help
>>>         Say "yes" here to support the on chip real time clock
>>>         present on TI OMAP1, AM33xx, DA8xx/OMAP-L13x, AM43xx and DRA7xx.
>>> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
>>> index ec2e9c5..56c7155 100644
>>> --- a/drivers/rtc/rtc-omap.c
>>> +++ b/drivers/rtc/rtc-omap.c
>>> @@ -26,6 +26,8 @@
>>>   #include <linux/pm_runtime.h>
>>>   #include <linux/io.h>
>>>   #include <linux/clk.h>
>>> +#include <linux/gpio/driver.h>
>>> +#include <linux/gpio/consumer.h>
>>>
>>>   /*
>>>    * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
>>> @@ -114,7 +116,11 @@
>>>   #define OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN    BIT(1)
>>>
>>>   /* OMAP_RTC_PMIC bit fields: */
>>> -#define OMAP_RTC_PMIC_POWER_EN_EN    BIT(16)
>>> +#define OMAP_RTC_PMIC_POWER_EN_EN        BIT(16)
>>> +#define OMAP_RTC_PMIC_EXT_WAKEUP_EN(x)        (BIT(x))
>>> +#define OMAP_RTC_PMIC_EXT_WAKEUP_POL(x)        (BIT(x) << 4)
>>> +#define OMAP_RTC_PMIC_EXT_WAKEUP_EN_MASK    (0x0F)
>>> +#define OMAP_RTC_PMIC_EXT_WAKEUP_POL_MASK    (0x0F << 4)
>>>
>>>   /* OMAP_RTC_KICKER values */
>>>   #define    KICK0_VALUE            0x83e70b13
>>> @@ -141,6 +147,7 @@ struct omap_rtc {
>>>       bool is_pmic_controller;
>>>       bool has_ext_clk;
>>>       const struct omap_rtc_device_type *type;
>>> +    struct gpio_chip gpio_chip;
>>>   };
>>>
>>>   static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
>>> @@ -183,6 +190,104 @@ static void default_rtc_lock(struct omap_rtc *rtc)
>>>   {
>>>   }
>>>
>>> +static int omap_rtc_gpio_request(struct gpio_chip *chip,
>>> +                unsigned int offset)
>>> +{
>>> +    struct omap_rtc *rtc = gpiochip_get_data(chip);
>>> +    u32 val;
>>> +
>>> +    rtc->type->unlock(rtc);
>>> +
>>> +    val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
>>> +    val |= OMAP_RTC_PMIC_EXT_WAKEUP_EN(offset);
>>> +    rtc_writel(rtc, OMAP_RTC_PMIC_REG, val);
>>> +
>>> +    rtc->type->lock(rtc);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void omap_rtc_gpio_free(struct gpio_chip *chip,
>>> +            unsigned int offset)
>>> +{
>>> +    struct omap_rtc *rtc = gpiochip_get_data(chip);
>>> +    u32 val;
>>> +
>>> +    rtc->type->unlock(rtc);
>>> +
>>> +    val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
>>> +    val &= ~OMAP_RTC_PMIC_EXT_WAKEUP_EN(offset);
>>> +    rtc_writel(rtc, OMAP_RTC_PMIC_REG, val);
>>> +
>>> +    rtc->type->lock(rtc);
>>> +}
>>> +
>>> +static int omap_rtc_gpio_get_direction(struct gpio_chip *chip,
>>> +                unsigned int offset)
>>> +{
>>> +    return 1; /* Always in */
>>> +}
>>> +
>>> +
>>> +static int omap_rtc_gpio_direction_input(struct gpio_chip *chip,
>>> +                    unsigned int offset)
>>> +{
>>> +    return 0;
>>> +}
>>> +
>>> +static int omap_rtc_gpio_get(struct gpio_chip *chip, unsigned int
>>> offset)
>>> +{
>>> +    return 0;
>>> +}
>>> +
>>> +/*
>>> + * Note: This function is called only when setting ext_wakeup polarity
>>> + * with omap_rtc_gpio_set_polarity
>>> + */
>>> +static void omap_rtc_gpio_set(struct gpio_chip *chip, unsigned int
>>> offset,
>>> +            int value)
>>> +{
>>> +    struct omap_rtc *rtc = gpiochip_get_data(chip);
>>> +    u32 val;
>>> +
>>> +    rtc->type->unlock(rtc);
>>> +
>>> +    val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
>>> +    if (value)
>>> +        val |= OMAP_RTC_PMIC_EXT_WAKEUP_POL(offset);
>>> +    else
>>> +        val &= ~OMAP_RTC_PMIC_EXT_WAKEUP_POL(offset);
>>> +    rtc_writel(rtc, OMAP_RTC_PMIC_REG, val);
>>> +
>>> +    rtc->type->lock(rtc);
>>> +}
>>> +
>>> +static void omap_rtc_gpio_set_polarity(struct gpio_descs *ext_wakeup)
>>> +{
>>> +    struct gpio_desc *desc;
>>> +    int i;
>>> +
>>> +    for (i = 0; i < ext_wakeup->ndescs; i++) {
>>> +        desc = ext_wakeup->desc[i];
>>> +        gpiod_set_raw_value_cansleep(desc,
>>> +                    gpiod_is_active_low(desc));
>>> +    }
>>> +}
>>> +
>>> +static struct gpio_chip template_chip = {
>>> +    .label            = "omap-rtc-gpio",
>>> +    .owner            = THIS_MODULE,
>>> +    .request        = omap_rtc_gpio_request,
>>> +    .free            = omap_rtc_gpio_free,
>>> +    .get_direction        = omap_rtc_gpio_get_direction,
>>> +    .direction_input    = omap_rtc_gpio_direction_input,
>>> +    .get            = omap_rtc_gpio_get,
>>> +    .set            = omap_rtc_gpio_set,
>>> +    .base            = -1,
>>> +    .ngpio            = 4,
>>> +    .can_sleep        = true,
>>> +};
>>> +
>>>   /*
>>>    * We rely on the rtc framework to handle locking (rtc->ops_lock),
>>>    * so the only other requirement is that register accesses which
>>> @@ -533,6 +638,8 @@ static int omap_rtc_probe(struct platform_device
>>> *pdev)
>>>       const struct platform_device_id *id_entry;
>>>       const struct of_device_id *of_id;
>>>       int ret;
>>> +    struct gpio_descs *ext_wakeup;
>>> +    u32 ngpios = 0;
>>>
>>>       rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
>>>       if (!rtc)
>>> @@ -544,6 +651,10 @@ static int omap_rtc_probe(struct platform_device
>>> *pdev)
>>>           rtc->is_pmic_controller = rtc->type->has_pmic_mode &&
>>>                   of_property_read_bool(pdev->dev.of_node,
>>>                           "system-power-controller");
>>> +        ret = of_property_read_u32(pdev->dev.of_node, "ngpios",
>>> +                    &ngpios);
>>> +        if (ret)
>>> +            ngpios = 0;
>>>       } else {
>>>           id_entry = platform_get_device_id(pdev);
>>>           rtc->type = (void *)id_entry->driver_data;
>>> @@ -577,6 +688,30 @@ static int omap_rtc_probe(struct platform_device
>>> *pdev)
>>>       pm_runtime_enable(&pdev->dev);
>>>       pm_runtime_get_sync(&pdev->dev);
>>>
>>> +    if (ngpios > 0) {
>>> +        rtc->gpio_chip = template_chip;
>>> +        rtc->gpio_chip.parent = &pdev->dev;
>>> +        rtc->gpio_chip.ngpio = ngpios;
>>> +        ret = devm_gpiochip_add_data(&pdev->dev, &rtc->gpio_chip,
>>> +                    rtc);
>>> +        if (ret < 0) {
>>> +            dev_err(&pdev->dev, "Could not register gpiochip, %d\n",
>>> +                ret);
>>> +            return ret;
>>> +        }
>>> +
>>> +        ext_wakeup = devm_gpiod_get_array_optional(&pdev->dev,
>>> +                        "ext-wakeup", GPIOD_IN);
>>> +        if (IS_ERR(ext_wakeup)) {
>>> +            ret = PTR_ERR(ext_wakeup);
>>> +            dev_err(&pdev->dev,
>>> +                "ext-wakeup request failed, ret %d\n", ret);
>>> +            return ret;
>>> +        }
>>> +        if (ext_wakeup)
>>> +            omap_rtc_gpio_set_polarity(ext_wakeup);
>>> +    }
>>> +
>>>       rtc->type->unlock(rtc);
>>>
>>>       /*
>>> --
>>> 2.8.0
>>>
>


-- 
regards,
-grygorii

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

[Marcin Niestroj]

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-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org> [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-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
>>>> ---
>>>>   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.

>
> 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
>>>> @@ -1208,7 +1208,7 @@ config RTC_DRV_IMXDI
>>>>
>>>>   config RTC_DRV_OMAP
>>>>       tristate "TI OMAP Real Time Clock"
>>>> -    depends on ARCH_OMAP || ARCH_DAVINCI || COMPILE_TEST
>>>> +    depends on (ARCH_OMAP || ARCH_DAVINCI || COMPILE_TEST) && GPIOLIB
>>>>       help
>>>>         Say "yes" here to support the on chip real time clock
>>>>         present on TI OMAP1, AM33xx, DA8xx/OMAP-L13x, AM43xx and
>>>> DRA7xx.
>>>> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
>>>> index ec2e9c5..56c7155 100644
>>>> --- a/drivers/rtc/rtc-omap.c
>>>> +++ b/drivers/rtc/rtc-omap.c
>>>> @@ -26,6 +26,8 @@
>>>>   #include <linux/pm_runtime.h>
>>>>   #include <linux/io.h>
>>>>   #include <linux/clk.h>
>>>> +#include <linux/gpio/driver.h>
>>>> +#include <linux/gpio/consumer.h>
>>>>
>>>>   /*
>>>>    * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
>>>> @@ -114,7 +116,11 @@
>>>>   #define OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN    BIT(1)
>>>>
>>>>   /* OMAP_RTC_PMIC bit fields: */
>>>> -#define OMAP_RTC_PMIC_POWER_EN_EN    BIT(16)
>>>> +#define OMAP_RTC_PMIC_POWER_EN_EN        BIT(16)
>>>> +#define OMAP_RTC_PMIC_EXT_WAKEUP_EN(x)        (BIT(x))
>>>> +#define OMAP_RTC_PMIC_EXT_WAKEUP_POL(x)        (BIT(x) << 4)
>>>> +#define OMAP_RTC_PMIC_EXT_WAKEUP_EN_MASK    (0x0F)
>>>> +#define OMAP_RTC_PMIC_EXT_WAKEUP_POL_MASK    (0x0F << 4)
>>>>
>>>>   /* OMAP_RTC_KICKER values */
>>>>   #define    KICK0_VALUE            0x83e70b13
>>>> @@ -141,6 +147,7 @@ struct omap_rtc {
>>>>       bool is_pmic_controller;
>>>>       bool has_ext_clk;
>>>>       const struct omap_rtc_device_type *type;
>>>> +    struct gpio_chip gpio_chip;
>>>>   };
>>>>
>>>>   static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
>>>> @@ -183,6 +190,104 @@ static void default_rtc_lock(struct omap_rtc
>>>> *rtc)
>>>>   {
>>>>   }
>>>>
>>>> +static int omap_rtc_gpio_request(struct gpio_chip *chip,
>>>> +                unsigned int offset)
>>>> +{
>>>> +    struct omap_rtc *rtc = gpiochip_get_data(chip);
>>>> +    u32 val;
>>>> +
>>>> +    rtc->type->unlock(rtc);
>>>> +
>>>> +    val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
>>>> +    val |= OMAP_RTC_PMIC_EXT_WAKEUP_EN(offset);
>>>> +    rtc_writel(rtc, OMAP_RTC_PMIC_REG, val);
>>>> +
>>>> +    rtc->type->lock(rtc);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void omap_rtc_gpio_free(struct gpio_chip *chip,
>>>> +            unsigned int offset)
>>>> +{
>>>> +    struct omap_rtc *rtc = gpiochip_get_data(chip);
>>>> +    u32 val;
>>>> +
>>>> +    rtc->type->unlock(rtc);
>>>> +
>>>> +    val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
>>>> +    val &= ~OMAP_RTC_PMIC_EXT_WAKEUP_EN(offset);
>>>> +    rtc_writel(rtc, OMAP_RTC_PMIC_REG, val);
>>>> +
>>>> +    rtc->type->lock(rtc);
>>>> +}
>>>> +
>>>> +static int omap_rtc_gpio_get_direction(struct gpio_chip *chip,
>>>> +                unsigned int offset)
>>>> +{
>>>> +    return 1; /* Always in */
>>>> +}
>>>> +
>>>> +
>>>> +static int omap_rtc_gpio_direction_input(struct gpio_chip *chip,
>>>> +                    unsigned int offset)
>>>> +{
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int omap_rtc_gpio_get(struct gpio_chip *chip, unsigned int
>>>> offset)
>>>> +{
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Note: This function is called only when setting ext_wakeup polarity
>>>> + * with omap_rtc_gpio_set_polarity
>>>> + */
>>>> +static void omap_rtc_gpio_set(struct gpio_chip *chip, unsigned int
>>>> offset,
>>>> +            int value)
>>>> +{
>>>> +    struct omap_rtc *rtc = gpiochip_get_data(chip);
>>>> +    u32 val;
>>>> +
>>>> +    rtc->type->unlock(rtc);
>>>> +
>>>> +    val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
>>>> +    if (value)
>>>> +        val |= OMAP_RTC_PMIC_EXT_WAKEUP_POL(offset);
>>>> +    else
>>>> +        val &= ~OMAP_RTC_PMIC_EXT_WAKEUP_POL(offset);
>>>> +    rtc_writel(rtc, OMAP_RTC_PMIC_REG, val);
>>>> +
>>>> +    rtc->type->lock(rtc);
>>>> +}
>>>> +
>>>> +static void omap_rtc_gpio_set_polarity(struct gpio_descs *ext_wakeup)
>>>> +{
>>>> +    struct gpio_desc *desc;
>>>> +    int i;
>>>> +
>>>> +    for (i = 0; i < ext_wakeup->ndescs; i++) {
>>>> +        desc = ext_wakeup->desc[i];
>>>> +        gpiod_set_raw_value_cansleep(desc,
>>>> +                    gpiod_is_active_low(desc));
>>>> +    }
>>>> +}
>>>> +
>>>> +static struct gpio_chip template_chip = {
>>>> +    .label            = "omap-rtc-gpio",
>>>> +    .owner            = THIS_MODULE,
>>>> +    .request        = omap_rtc_gpio_request,
>>>> +    .free            = omap_rtc_gpio_free,
>>>> +    .get_direction        = omap_rtc_gpio_get_direction,
>>>> +    .direction_input    = omap_rtc_gpio_direction_input,
>>>> +    .get            = omap_rtc_gpio_get,
>>>> +    .set            = omap_rtc_gpio_set,
>>>> +    .base            = -1,
>>>> +    .ngpio            = 4,
>>>> +    .can_sleep        = true,
>>>> +};
>>>> +
>>>>   /*
>>>>    * We rely on the rtc framework to handle locking (rtc->ops_lock),
>>>>    * so the only other requirement is that register accesses which
>>>> @@ -533,6 +638,8 @@ static int omap_rtc_probe(struct platform_device
>>>> *pdev)
>>>>       const struct platform_device_id *id_entry;
>>>>       const struct of_device_id *of_id;
>>>>       int ret;
>>>> +    struct gpio_descs *ext_wakeup;
>>>> +    u32 ngpios = 0;
>>>>
>>>>       rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
>>>>       if (!rtc)
>>>> @@ -544,6 +651,10 @@ static int omap_rtc_probe(struct platform_device
>>>> *pdev)
>>>>           rtc->is_pmic_controller = rtc->type->has_pmic_mode &&
>>>>                   of_property_read_bool(pdev->dev.of_node,
>>>>                           "system-power-controller");
>>>> +        ret = of_property_read_u32(pdev->dev.of_node, "ngpios",
>>>> +                    &ngpios);
>>>> +        if (ret)
>>>> +            ngpios = 0;
>>>>       } else {
>>>>           id_entry = platform_get_device_id(pdev);
>>>>           rtc->type = (void *)id_entry->driver_data;
>>>> @@ -577,6 +688,30 @@ static int omap_rtc_probe(struct platform_device
>>>> *pdev)
>>>>       pm_runtime_enable(&pdev->dev);
>>>>       pm_runtime_get_sync(&pdev->dev);
>>>>
>>>> +    if (ngpios > 0) {
>>>> +        rtc->gpio_chip = template_chip;
>>>> +        rtc->gpio_chip.parent = &pdev->dev;
>>>> +        rtc->gpio_chip.ngpio = ngpios;
>>>> +        ret = devm_gpiochip_add_data(&pdev->dev, &rtc->gpio_chip,
>>>> +                    rtc);
>>>> +        if (ret < 0) {
>>>> +            dev_err(&pdev->dev, "Could not register gpiochip, %d\n",
>>>> +                ret);
>>>> +            return ret;
>>>> +        }
>>>> +
>>>> +        ext_wakeup = devm_gpiod_get_array_optional(&pdev->dev,
>>>> +                        "ext-wakeup", GPIOD_IN);
>>>> +        if (IS_ERR(ext_wakeup)) {
>>>> +            ret = PTR_ERR(ext_wakeup);
>>>> +            dev_err(&pdev->dev,
>>>> +                "ext-wakeup request failed, ret %d\n", ret);
>>>> +            return ret;
>>>> +        }
>>>> +        if (ext_wakeup)
>>>> +            omap_rtc_gpio_set_polarity(ext_wakeup);
>>>> +    }
>>>> +
>>>>       rtc->type->unlock(rtc);
>>>>
>>>>       /*
>>>> --
>>>> 2.8.0
>>>>
>>
>
>

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

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

[Grygorii Strashko]

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-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org> [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-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
>>>>> ---
>>>>>   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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Marcin Niestroj]

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-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org> [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-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
>>>>>> ---
>>>>>>    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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Grygorii Strashko]

On 04/08/2016 01:08 PM, Marcin Niestroj wrote:
> 
> 
> 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@grinn-global.com> [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@grinn-global.com>
>>>>>>> ---
>>>>>>>    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.

kirkwood-openrd.dtsi-			p2 {
kirkwood-openrd.dtsi:				gpio-hog;
kirkwood-openrd.dtsi-				gpios = <2 GPIO_ACTIVE_HIGH>;
						[input;]

Sry, if you can't do smth like above - it's just prove that this approach is not right.

> 
>>
>> 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.

ok. I found this in omap_rtc_power_off(). Thanks for your explanation.

> 
>>
>>>
>>>>
>>>> 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

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

[Tony Lindgren]

* Grygorii Strashko <grygorii.strashko@ti.com> [160408 03:43]:
> On 04/08/2016 01:08 PM, Marcin Niestroj wrote:
> >>>>>>> @@ -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

I think the naming standard here should be gpio-*, so in this
case gpio-wakeup.

> >>> 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.
> 
> kirkwood-openrd.dtsi-			p2 {
> kirkwood-openrd.dtsi:				gpio-hog;
> kirkwood-openrd.dtsi-				gpios = <2 GPIO_ACTIVE_HIGH>;
> 						[input;]
> 
> Sry, if you can't do smth like above - it's just prove that this approach is not right.

Yes let's stick to the standards. It should be using gpiolib interface.

Regards,

Tony