* [rtc-linux] [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 16:32 ` Marcin Niestroj 0 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-06 16:32 UTC (permalink / raw) To: rtc-linux, devicetree Cc: Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, Tony Lindgren, Marcin Niestroj Hi, This is second approach for adding support of ext_wakeup configuration in OMAP RTC. Developed and tested on ChiliBoard (am335x) using v4.6-rc2. Changes v1 -> v2: * Use gpiochip implementation to utilize existing device-tree bindings (as suggested by Tony Lindgren) Marcin Niestroj (1): rtc: omap: Support ext_wakeup configuration 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(-) -- 2.8.0 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 16:32 ` Marcin Niestroj 0 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-06 16:32 UTC (permalink / raw) To: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA Cc: Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, Tony Lindgren, Marcin Niestroj Hi, This is second approach for adding support of ext_wakeup configuration in OMAP RTC. Developed and tested on ChiliBoard (am335x) using v4.6-rc2. Changes v1 -> v2: * Use gpiochip implementation to utilize existing device-tree bindings (as suggested by Tony Lindgren) Marcin Niestroj (1): rtc: omap: Support ext_wakeup configuration 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(-) -- 2.8.0 -- 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 16:32 ` Marcin Niestroj 0 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-06 16:32 UTC (permalink / raw) To: rtc-linux, devicetree Cc: Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, Tony Lindgren, Marcin Niestroj 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). 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>; +}; 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 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 16:32 ` Marcin Niestroj 0 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-06 16:32 UTC (permalink / raw) To: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA Cc: Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, Tony Lindgren, Marcin Niestroj 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). 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>; +}; 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 -- 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 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 19:11 ` Tony Lindgren 0 siblings, 0 replies; 28+ messages in thread From: Tony Lindgren @ 2016-04-06 19:11 UTC (permalink / raw) To: Marcin Niestroj Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap * 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? 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>; > +}; > 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 > -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 19:11 ` Tony Lindgren 0 siblings, 0 replies; 28+ messages in thread From: Tony Lindgren @ 2016-04-06 19:11 UTC (permalink / raw) To: Marcin Niestroj Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap-u79uwXL29TY76Z2rM5mHXA * 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? 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>; > +}; > 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 > -- 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 19:28 ` Marcin Niestroj 0 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-06 19:28 UTC (permalink / raw) To: Tony Lindgren Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap 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>; >> +}; >> 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 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 19:28 ` Marcin Niestroj 0 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-06 19:28 UTC (permalink / raw) To: Tony Lindgren Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap-u79uwXL29TY76Z2rM5mHXA 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>; >> +}; >> 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 22:36 ` Tony Lindgren 0 siblings, 0 replies; 28+ messages in thread From: Tony Lindgren @ 2016-04-06 22:36 UTC (permalink / raw) To: Marcin Niestroj Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap * Marcin Niestroj <m.niestroj@grinn-global.com> [160406 12:29]: > 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. OK thanks for checking. Tony -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 22:36 ` Tony Lindgren 0 siblings, 0 replies; 28+ messages in thread From: Tony Lindgren @ 2016-04-06 22:36 UTC (permalink / raw) To: Marcin Niestroj Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap-u79uwXL29TY76Z2rM5mHXA * Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org> [160406 12:29]: > 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. OK thanks for checking. Tony -- 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration 2016-04-06 19:28 ` Marcin Niestroj @ 2016-04-07 10:48 ` Grygorii Strashko -1 siblings, 0 replies; 28+ messages in thread From: Grygorii Strashko @ 2016-04-07 10:48 UTC (permalink / raw) To: Marcin Niestroj, Tony Lindgren Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap, linux-gpio 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-07 10:48 ` Grygorii Strashko 0 siblings, 0 replies; 28+ messages in thread From: Grygorii Strashko @ 2016-04-07 10:48 UTC (permalink / raw) To: Marcin Niestroj, Tony Lindgren Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap, linux-gpio 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 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
[parent not found: <57063B1A.7080700-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration 2016-04-07 10:48 ` [rtc-linux] " Grygorii Strashko @ 2016-04-07 17:11 ` Marcin Niestroj -1 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-07 17:11 UTC (permalink / raw) To: Grygorii Strashko, Tony Lindgren Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-07 17:11 ` Marcin Niestroj 0 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-07 17:11 UTC (permalink / raw) To: Grygorii Strashko, Tony Lindgren Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap, linux-gpio 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. > > 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 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
[parent not found: <570694B0.4040500-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>]
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration 2016-04-07 17:11 ` [rtc-linux] " Marcin Niestroj @ 2016-04-08 9:18 ` Grygorii Strashko -1 siblings, 0 replies; 28+ messages in thread From: Grygorii Strashko @ 2016-04-08 9:18 UTC (permalink / raw) To: Marcin Niestroj, Tony Lindgren Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-08 9:18 ` Grygorii Strashko 0 siblings, 0 replies; 28+ messages in thread From: Grygorii Strashko @ 2016-04-08 9:18 UTC (permalink / raw) To: Marcin Niestroj, Tony Lindgren Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap, linux-gpio 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). 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 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
[parent not found: <57077769.1050101-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration 2016-04-08 9:18 ` [rtc-linux] " Grygorii Strashko @ 2016-04-08 10:08 ` Marcin Niestroj -1 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-08 10:08 UTC (permalink / raw) To: Grygorii Strashko, Tony Lindgren Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-08 10:08 ` Marcin Niestroj 0 siblings, 0 replies; 28+ messages in thread From: Marcin Niestroj @ 2016-04-08 10:08 UTC (permalink / raw) To: Grygorii Strashko, Tony Lindgren Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap, linux-gpio 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. > > 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 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration 2016-04-08 10:08 ` [rtc-linux] " Marcin Niestroj @ 2016-04-08 10:42 ` Grygorii Strashko -1 siblings, 0 replies; 28+ messages in thread From: Grygorii Strashko @ 2016-04-08 10:42 UTC (permalink / raw) To: Marcin Niestroj, Tony Lindgren Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap, linux-gpio 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-08 10:42 ` Grygorii Strashko 0 siblings, 0 replies; 28+ messages in thread From: Grygorii Strashko @ 2016-04-08 10:42 UTC (permalink / raw) To: Marcin Niestroj, Tony Lindgren Cc: rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap, linux-gpio 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 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration 2016-04-08 10:42 ` [rtc-linux] " Grygorii Strashko @ 2016-04-08 14:57 ` Tony Lindgren -1 siblings, 0 replies; 28+ messages in thread From: Tony Lindgren @ 2016-04-08 14:57 UTC (permalink / raw) To: Grygorii Strashko Cc: Marcin Niestroj, rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap, linux-gpio * 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-08 14:57 ` Tony Lindgren 0 siblings, 0 replies; 28+ messages in thread From: Tony Lindgren @ 2016-04-08 14:57 UTC (permalink / raw) To: Grygorii Strashko Cc: Marcin Niestroj, rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, linux-omap, linux-gpio * 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 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 22:09 ` Alexandre Belloni 0 siblings, 0 replies; 28+ messages in thread From: Alexandre Belloni @ 2016-04-06 22:09 UTC (permalink / raw) To: Tony Lindgren Cc: Marcin Niestroj, rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Keerthy, linux-omap On 06/04/2016 at 12:11:03 -0700, 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. > Seems good to me too. Tony, is that an ack? -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 22:09 ` Alexandre Belloni 0 siblings, 0 replies; 28+ messages in thread From: Alexandre Belloni @ 2016-04-06 22:09 UTC (permalink / raw) To: Tony Lindgren Cc: Marcin Niestroj, rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll, Alessandro Zummo, Keerthy, linux-omap-u79uwXL29TY76Z2rM5mHXA On 06/04/2016 at 12:11:03 -0700, 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. > Seems good to me too. Tony, is that an ack? -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 22:35 ` Tony Lindgren 0 siblings, 0 replies; 28+ messages in thread From: Tony Lindgren @ 2016-04-06 22:35 UTC (permalink / raw) To: Alexandre Belloni Cc: Marcin Niestroj, rtc-linux, devicetree, Rob Herring, Pawel Moll, Alessandro Zummo, Keerthy, linux-omap * Alexandre Belloni <alexandre.belloni@free-electrons.com> [160406 15:10]: > On 06/04/2016 at 12:11:03 -0700, 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. > > > > Seems good to me too. Tony, is that an ack? Yup looks good to me: Acked-by: Tony Lindgren <tony@atomide.com> -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-06 22:35 ` Tony Lindgren 0 siblings, 0 replies; 28+ messages in thread From: Tony Lindgren @ 2016-04-06 22:35 UTC (permalink / raw) To: Alexandre Belloni Cc: Marcin Niestroj, rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll, Alessandro Zummo, Keerthy, linux-omap-u79uwXL29TY76Z2rM5mHXA * Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> [160406 15:10]: > On 06/04/2016 at 12:11:03 -0700, 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. > > > > Seems good to me too. Tony, is that an ack? Yup looks good to me: Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> -- 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [rtc-linux] Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-11 13:57 ` Rob Herring 0 siblings, 0 replies; 28+ messages in thread From: Rob Herring @ 2016-04-11 13:57 UTC (permalink / raw) To: Marcin Niestroj Cc: rtc-linux, devicetree, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, Tony Lindgren On Wed, Apr 06, 2016 at 06:32:47PM +0200, Marcin Niestroj wrote: > 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). > > Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com> > --- Why are you doing a cover letter for a single patch. Put any commentary and version history here instead. > 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) This is not how this property is intended to be used. > +- ext-wakeup-gpios: List of ext_wakeup sources to configure This doesn't belong here. It should be in the PMIC node if that is where it is attached. Rob -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] rtc: omap: Support ext_wakeup configuration @ 2016-04-11 13:57 ` Rob Herring 0 siblings, 0 replies; 28+ messages in thread From: Rob Herring @ 2016-04-11 13:57 UTC (permalink / raw) To: Marcin Niestroj Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, devicetree-u79uwXL29TY76Z2rM5mHXA, Pawel Moll, Alessandro Zummo, Alexandre Belloni, Keerthy, Tony Lindgren On Wed, Apr 06, 2016 at 06:32:47PM +0200, Marcin Niestroj wrote: > 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). > > Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org> > --- Why are you doing a cover letter for a single patch. Put any commentary and version history here instead. > 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) This is not how this property is intended to be used. > +- ext-wakeup-gpios: List of ext_wakeup sources to configure This doesn't belong here. It should be in the PMIC node if that is where it is attached. Rob -- 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2016-04-11 13:57 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-06 16:32 [rtc-linux] [PATCH v2] rtc: omap: Support ext_wakeup configuration Marcin Niestroj
2016-04-06 16:32 ` Marcin Niestroj
2016-04-06 16:32 ` [rtc-linux] " Marcin Niestroj
2016-04-06 16:32 ` Marcin Niestroj
2016-04-06 19:11 ` [rtc-linux] " Tony Lindgren
2016-04-06 19:11 ` Tony Lindgren
2016-04-06 19:28 ` [rtc-linux] " Marcin Niestroj
2016-04-06 19:28 ` Marcin Niestroj
2016-04-06 22:36 ` [rtc-linux] " Tony Lindgren
2016-04-06 22:36 ` Tony Lindgren
2016-04-07 10:48 ` Grygorii Strashko
2016-04-07 10:48 ` [rtc-linux] " Grygorii Strashko
[not found] ` <57063B1A.7080700-l0cyMroinI0@public.gmane.org>
2016-04-07 17:11 ` Marcin Niestroj
2016-04-07 17:11 ` [rtc-linux] " Marcin Niestroj
[not found] ` <570694B0.4040500-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
2016-04-08 9:18 ` Grygorii Strashko
2016-04-08 9:18 ` [rtc-linux] " Grygorii Strashko
[not found] ` <57077769.1050101-l0cyMroinI0@public.gmane.org>
2016-04-08 10:08 ` Marcin Niestroj
2016-04-08 10:08 ` [rtc-linux] " Marcin Niestroj
2016-04-08 10:42 ` Grygorii Strashko
2016-04-08 10:42 ` [rtc-linux] " Grygorii Strashko
2016-04-08 14:57 ` Tony Lindgren
2016-04-08 14:57 ` [rtc-linux] " Tony Lindgren
2016-04-06 22:09 ` Alexandre Belloni
2016-04-06 22:09 ` Alexandre Belloni
2016-04-06 22:35 ` [rtc-linux] " Tony Lindgren
2016-04-06 22:35 ` Tony Lindgren
2016-04-11 13:57 ` [rtc-linux] " Rob Herring
2016-04-11 13:57 ` Rob Herring
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.