* [PATCHv2 0/3] omap gpio add level idle, cpu_pm and drop runtime_irq_safe
@ 2018-09-20 19:35 ` Tony Lindgren
0 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-20 19:35 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Here's v2 of the patches to add support for omap4 and later level interrupt
idle support, get rid of the custom pm calls, and finally after these we can
drop pm_runtime_irq_safe().
Linus, assuming people are happy with these, I'd appreciate an immutable branch
with these in case we run into merge conflicts for arch/arm/mach-omap2 changes.
Regards,
Tony
Changes since v1:
- Fix issue noted in v1 set and update comments accordingly
- Don't call pm_runtime from cpu_pm to avoid further dependencies to
pm_runtime_irq_safe() as discussed offline with Grygorii
- Add a patch to drop pm_runtime_irq_safe() as it's no longer needed
Tony Lindgren (3):
gpio: omap: Add level wakeup handling for omap4 based SoCs
gpio: omap: Remove custom PM calls and use cpu_pm instead
gpio: omap: Get rid of pm_runtime_irq_safe()
arch/arm/mach-omap2/pm24xx.c | 7 +-
arch/arm/mach-omap2/pm34xx.c | 14 +-
drivers/gpio/gpio-omap.c | 331 ++++++++++++++++--------
include/linux/platform_data/gpio-omap.h | 15 +-
4 files changed, 237 insertions(+), 130 deletions(-)
--
2.19.0
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/3] gpio: omap: Add level wakeup handling for omap4 based SoCs
2018-09-20 19:35 ` Tony Lindgren
@ 2018-09-20 19:35 ` Tony Lindgren
-1 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-20 19:35 UTC (permalink / raw)
To: Linus Walleij, Alexandre Courbot
Cc: Grygorii Strashko, Aaro Koskinen, Keerthy, Tero Kristo,
linux-gpio, Ladislav Michl, linux-omap, linux-arm-kernel
I noticed that unlike omap2 and 3 based SoCs, omap4 based SoCs keep
the GPIO clocks enabled for GPIO level interrupts with wakeup enabled.
This blocks deeper idle states as the whole domain will stay busy.
The GPIO functional clock seems to stay enabled if the wakeup register
is enabled and a level interrupt is triggered. In that case the only
way to have the GPIO module idle is to reset it. It is possible this
has gone unnoticed with OSWR (Open SWitch Retention) and off mode
during idle resetting GPIO context most GPIO instances in the earlier
Android trees for example.
Looks like the way to deal with this is to have omap4 based SoCs
only set wake for the duration of idle for level interrupts, and clear
level registers for the idle. With level interrupts we can do this as
the level interrupt from device will be still there on resume.
I've taken the long path to fixing this to avoid yet more hard to
read code. I've set up a quirks flag, and a struct for function
pointers so we can use these to clean up other quirk handling easier
in the later patches. The current level quirk handling is moved to
the new functions.
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/gpio/gpio-omap.c | 151 ++++++++++++++++++------
include/linux/platform_data/gpio-omap.h | 2 +
2 files changed, 120 insertions(+), 33 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -31,6 +31,8 @@
#define OFF_MODE 1
#define OMAP4_GPIO_DEBOUNCINGTIME_MASK 0xFF
+#define OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN BIT(1)
+
static LIST_HEAD(omap_gpio_list);
struct gpio_regs {
@@ -48,6 +50,13 @@ struct gpio_regs {
u32 debounce_en;
};
+struct gpio_bank;
+
+struct gpio_omap_funcs {
+ void (*idle_enable_level_quirk)(struct gpio_bank *bank);
+ void (*idle_disable_level_quirk)(struct gpio_bank *bank);
+};
+
struct gpio_bank {
struct list_head node;
void __iomem *base;
@@ -55,6 +64,7 @@ struct gpio_bank {
u32 non_wakeup_gpios;
u32 enabled_non_wakeup_gpios;
struct gpio_regs context;
+ struct gpio_omap_funcs funcs;
u32 saved_datain;
u32 level_mask;
u32 toggle_mask;
@@ -75,6 +85,7 @@ struct gpio_bank {
int context_loss_count;
int power_mode;
bool workaround_enabled;
+ u32 quirks;
void (*set_dataout)(struct gpio_bank *bank, unsigned gpio, int enable);
void (*set_dataout_multiple)(struct gpio_bank *bank,
@@ -368,9 +379,18 @@ static inline void omap_set_gpio_trigger(struct gpio_bank *bank, int gpio,
readl_relaxed(bank->base + bank->regs->fallingdetect);
if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
- omap_gpio_rmw(base, bank->regs->wkup_en, gpio_bit, trigger != 0);
- bank->context.wake_en =
- readl_relaxed(bank->base + bank->regs->wkup_en);
+ /* Defer wkup_en register update until we idle? */
+ if (bank->quirks & OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN) {
+ if (trigger)
+ bank->context.wake_en |= gpio_bit;
+ else
+ bank->context.wake_en &= ~gpio_bit;
+ } else {
+ omap_gpio_rmw(base, bank->regs->wkup_en, gpio_bit,
+ trigger != 0);
+ bank->context.wake_en =
+ readl_relaxed(bank->base + bank->regs->wkup_en);
+ }
}
/* This part needs to be executed always for OMAP{34xx, 44xx} */
@@ -899,6 +919,82 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
raw_spin_unlock_irqrestore(&bank->lock, flags);
}
+/*
+ * Only edges can generate a wakeup event to the PRCM.
+ *
+ * Therefore, ensure any wake-up capable GPIOs have
+ * edge-detection enabled before going idle to ensure a wakeup
+ * to the PRCM is generated on a GPIO transition. (c.f. 34xx
+ * NDA TRM 25.5.3.1)
+ *
+ * The normal values will be restored upon ->runtime_resume()
+ * by writing back the values saved in bank->context.
+ */
+static void __maybe_unused
+omap2_gpio_enable_level_quirk(struct gpio_bank *bank)
+{
+ u32 wake_low, wake_hi;
+
+ /* Enable additional edge detection for level gpios for idle */
+ wake_low = bank->context.leveldetect0 & bank->context.wake_en;
+ if (wake_low)
+ writel_relaxed(wake_low | bank->context.fallingdetect,
+ bank->base + bank->regs->fallingdetect);
+
+ wake_hi = bank->context.leveldetect1 & bank->context.wake_en;
+ if (wake_hi)
+ writel_relaxed(wake_hi | bank->context.risingdetect,
+ bank->base + bank->regs->risingdetect);
+}
+
+static void __maybe_unused
+omap2_gpio_disable_level_quirk(struct gpio_bank *bank)
+{
+ /* Disable edge detection for level gpios after idle */
+ writel_relaxed(bank->context.fallingdetect,
+ bank->base + bank->regs->fallingdetect);
+ writel_relaxed(bank->context.risingdetect,
+ bank->base + bank->regs->risingdetect);
+}
+
+/*
+ * On omap4 and later SoC variants a level interrupt with wkup_en
+ * enabled blocks the GPIO functional clock from idling until the GPIO
+ * instance has been reset. To avoid that, we must set wkup_en only for
+ * idle for level interrupts, and clear level registers for the duration
+ * of idle. The level interrupts will be still there on wakeup by their
+ * nature.
+ */
+static void __maybe_unused
+omap4_gpio_enable_level_quirk(struct gpio_bank *bank)
+{
+ /* Update wake register for idle, edge bits might be already set */
+ writel_relaxed(bank->context.wake_en,
+ bank->base + bank->regs->wkup_en);
+
+ /* Clear level registers for idle */
+ writel_relaxed(0, bank->base + bank->regs->leveldetect0);
+ writel_relaxed(0, bank->base + bank->regs->leveldetect1);
+}
+
+static void __maybe_unused
+omap4_gpio_disable_level_quirk(struct gpio_bank *bank)
+{
+ /* Restore level registers after idle */
+ writel_relaxed(bank->context.leveldetect0,
+ bank->base + bank->regs->leveldetect0);
+ writel_relaxed(bank->context.leveldetect1,
+ bank->base + bank->regs->leveldetect1);
+
+ /* Clear saved wkup_en for level, it will be set for next idle again */
+ bank->context.wake_en &= ~(bank->context.leveldetect0 |
+ bank->context.leveldetect1);
+
+ /* Update wake with only edge configuration */
+ writel_relaxed(bank->context.wake_en,
+ bank->base + bank->regs->wkup_en);
+}
+
/*---------------------------------------------------------------------*/
static int omap_mpuio_suspend_noirq(struct device *dev)
@@ -1270,6 +1366,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
bank->chip.parent = dev;
bank->chip.owner = THIS_MODULE;
bank->dbck_flag = pdata->dbck_flag;
+ bank->quirks = pdata->quirks;
bank->stride = pdata->bank_stride;
bank->width = pdata->bank_width;
bank->is_mpuio = pdata->is_mpuio;
@@ -1278,6 +1375,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
#ifdef CONFIG_OF_GPIO
bank->chip.of_node = of_node_get(node);
#endif
+
if (node) {
if (!of_property_read_bool(node, "ti,gpio-always-on"))
bank->loses_context = true;
@@ -1298,6 +1396,18 @@ static int omap_gpio_probe(struct platform_device *pdev)
omap_set_gpio_dataout_mask_multiple;
}
+ if (bank->quirks & OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN) {
+ bank->funcs.idle_enable_level_quirk =
+ omap4_gpio_enable_level_quirk;
+ bank->funcs.idle_disable_level_quirk =
+ omap4_gpio_disable_level_quirk;
+ } else {
+ bank->funcs.idle_enable_level_quirk =
+ omap2_gpio_enable_level_quirk;
+ bank->funcs.idle_disable_level_quirk =
+ omap2_gpio_disable_level_quirk;
+ }
+
raw_spin_lock_init(&bank->lock);
raw_spin_lock_init(&bank->wa_lock);
@@ -1372,29 +1482,11 @@ static int omap_gpio_runtime_suspend(struct device *dev)
struct gpio_bank *bank = platform_get_drvdata(pdev);
u32 l1 = 0, l2 = 0;
unsigned long flags;
- u32 wake_low, wake_hi;
raw_spin_lock_irqsave(&bank->lock, flags);
- /*
- * Only edges can generate a wakeup event to the PRCM.
- *
- * Therefore, ensure any wake-up capable GPIOs have
- * edge-detection enabled before going idle to ensure a wakeup
- * to the PRCM is generated on a GPIO transition. (c.f. 34xx
- * NDA TRM 25.5.3.1)
- *
- * The normal values will be restored upon ->runtime_resume()
- * by writing back the values saved in bank->context.
- */
- wake_low = bank->context.leveldetect0 & bank->context.wake_en;
- if (wake_low)
- writel_relaxed(wake_low | bank->context.fallingdetect,
- bank->base + bank->regs->fallingdetect);
- wake_hi = bank->context.leveldetect1 & bank->context.wake_en;
- if (wake_hi)
- writel_relaxed(wake_hi | bank->context.risingdetect,
- bank->base + bank->regs->risingdetect);
+ if (bank->funcs.idle_enable_level_quirk)
+ bank->funcs.idle_enable_level_quirk(bank);
if (!bank->enabled_non_wakeup_gpios)
goto update_gpio_context_count;
@@ -1459,16 +1551,8 @@ static int omap_gpio_runtime_resume(struct device *dev)
omap_gpio_dbck_enable(bank);
- /*
- * In ->runtime_suspend(), level-triggered, wakeup-enabled
- * GPIOs were set to edge trigger also in order to be able to
- * generate a PRCM wakeup. Here we restore the
- * pre-runtime_suspend() values for edge triggering.
- */
- writel_relaxed(bank->context.fallingdetect,
- bank->base + bank->regs->fallingdetect);
- writel_relaxed(bank->context.risingdetect,
- bank->base + bank->regs->risingdetect);
+ if (bank->funcs.idle_disable_level_quirk)
+ bank->funcs.idle_disable_level_quirk(bank);
if (bank->loses_context) {
if (!bank->get_context_loss_count) {
@@ -1706,6 +1790,7 @@ static const struct omap_gpio_platform_data omap4_pdata = {
.regs = &omap4_gpio_regs,
.bank_width = 32,
.dbck_flag = true,
+ .quirks = OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN,
};
static const struct of_device_id omap_gpio_match[] = {
diff --git a/include/linux/platform_data/gpio-omap.h b/include/linux/platform_data/gpio-omap.h
--- a/include/linux/platform_data/gpio-omap.h
+++ b/include/linux/platform_data/gpio-omap.h
@@ -197,6 +197,8 @@ struct omap_gpio_platform_data {
bool is_mpuio; /* whether the bank is of type MPUIO */
u32 non_wakeup_gpios;
+ u32 quirks; /* Version specific quirks mask */
+
struct omap_gpio_reg_offs *regs;
/* Return context loss count due to PM states changing */
--
2.19.0
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/3] gpio: omap: Add level wakeup handling for omap4 based SoCs
@ 2018-09-20 19:35 ` Tony Lindgren
0 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-20 19:35 UTC (permalink / raw)
To: linux-arm-kernel
I noticed that unlike omap2 and 3 based SoCs, omap4 based SoCs keep
the GPIO clocks enabled for GPIO level interrupts with wakeup enabled.
This blocks deeper idle states as the whole domain will stay busy.
The GPIO functional clock seems to stay enabled if the wakeup register
is enabled and a level interrupt is triggered. In that case the only
way to have the GPIO module idle is to reset it. It is possible this
has gone unnoticed with OSWR (Open SWitch Retention) and off mode
during idle resetting GPIO context most GPIO instances in the earlier
Android trees for example.
Looks like the way to deal with this is to have omap4 based SoCs
only set wake for the duration of idle for level interrupts, and clear
level registers for the idle. With level interrupts we can do this as
the level interrupt from device will be still there on resume.
I've taken the long path to fixing this to avoid yet more hard to
read code. I've set up a quirks flag, and a struct for function
pointers so we can use these to clean up other quirk handling easier
in the later patches. The current level quirk handling is moved to
the new functions.
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/gpio/gpio-omap.c | 151 ++++++++++++++++++------
include/linux/platform_data/gpio-omap.h | 2 +
2 files changed, 120 insertions(+), 33 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -31,6 +31,8 @@
#define OFF_MODE 1
#define OMAP4_GPIO_DEBOUNCINGTIME_MASK 0xFF
+#define OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN BIT(1)
+
static LIST_HEAD(omap_gpio_list);
struct gpio_regs {
@@ -48,6 +50,13 @@ struct gpio_regs {
u32 debounce_en;
};
+struct gpio_bank;
+
+struct gpio_omap_funcs {
+ void (*idle_enable_level_quirk)(struct gpio_bank *bank);
+ void (*idle_disable_level_quirk)(struct gpio_bank *bank);
+};
+
struct gpio_bank {
struct list_head node;
void __iomem *base;
@@ -55,6 +64,7 @@ struct gpio_bank {
u32 non_wakeup_gpios;
u32 enabled_non_wakeup_gpios;
struct gpio_regs context;
+ struct gpio_omap_funcs funcs;
u32 saved_datain;
u32 level_mask;
u32 toggle_mask;
@@ -75,6 +85,7 @@ struct gpio_bank {
int context_loss_count;
int power_mode;
bool workaround_enabled;
+ u32 quirks;
void (*set_dataout)(struct gpio_bank *bank, unsigned gpio, int enable);
void (*set_dataout_multiple)(struct gpio_bank *bank,
@@ -368,9 +379,18 @@ static inline void omap_set_gpio_trigger(struct gpio_bank *bank, int gpio,
readl_relaxed(bank->base + bank->regs->fallingdetect);
if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
- omap_gpio_rmw(base, bank->regs->wkup_en, gpio_bit, trigger != 0);
- bank->context.wake_en =
- readl_relaxed(bank->base + bank->regs->wkup_en);
+ /* Defer wkup_en register update until we idle? */
+ if (bank->quirks & OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN) {
+ if (trigger)
+ bank->context.wake_en |= gpio_bit;
+ else
+ bank->context.wake_en &= ~gpio_bit;
+ } else {
+ omap_gpio_rmw(base, bank->regs->wkup_en, gpio_bit,
+ trigger != 0);
+ bank->context.wake_en =
+ readl_relaxed(bank->base + bank->regs->wkup_en);
+ }
}
/* This part needs to be executed always for OMAP{34xx, 44xx} */
@@ -899,6 +919,82 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
raw_spin_unlock_irqrestore(&bank->lock, flags);
}
+/*
+ * Only edges can generate a wakeup event to the PRCM.
+ *
+ * Therefore, ensure any wake-up capable GPIOs have
+ * edge-detection enabled before going idle to ensure a wakeup
+ * to the PRCM is generated on a GPIO transition. (c.f. 34xx
+ * NDA TRM 25.5.3.1)
+ *
+ * The normal values will be restored upon ->runtime_resume()
+ * by writing back the values saved in bank->context.
+ */
+static void __maybe_unused
+omap2_gpio_enable_level_quirk(struct gpio_bank *bank)
+{
+ u32 wake_low, wake_hi;
+
+ /* Enable additional edge detection for level gpios for idle */
+ wake_low = bank->context.leveldetect0 & bank->context.wake_en;
+ if (wake_low)
+ writel_relaxed(wake_low | bank->context.fallingdetect,
+ bank->base + bank->regs->fallingdetect);
+
+ wake_hi = bank->context.leveldetect1 & bank->context.wake_en;
+ if (wake_hi)
+ writel_relaxed(wake_hi | bank->context.risingdetect,
+ bank->base + bank->regs->risingdetect);
+}
+
+static void __maybe_unused
+omap2_gpio_disable_level_quirk(struct gpio_bank *bank)
+{
+ /* Disable edge detection for level gpios after idle */
+ writel_relaxed(bank->context.fallingdetect,
+ bank->base + bank->regs->fallingdetect);
+ writel_relaxed(bank->context.risingdetect,
+ bank->base + bank->regs->risingdetect);
+}
+
+/*
+ * On omap4 and later SoC variants a level interrupt with wkup_en
+ * enabled blocks the GPIO functional clock from idling until the GPIO
+ * instance has been reset. To avoid that, we must set wkup_en only for
+ * idle for level interrupts, and clear level registers for the duration
+ * of idle. The level interrupts will be still there on wakeup by their
+ * nature.
+ */
+static void __maybe_unused
+omap4_gpio_enable_level_quirk(struct gpio_bank *bank)
+{
+ /* Update wake register for idle, edge bits might be already set */
+ writel_relaxed(bank->context.wake_en,
+ bank->base + bank->regs->wkup_en);
+
+ /* Clear level registers for idle */
+ writel_relaxed(0, bank->base + bank->regs->leveldetect0);
+ writel_relaxed(0, bank->base + bank->regs->leveldetect1);
+}
+
+static void __maybe_unused
+omap4_gpio_disable_level_quirk(struct gpio_bank *bank)
+{
+ /* Restore level registers after idle */
+ writel_relaxed(bank->context.leveldetect0,
+ bank->base + bank->regs->leveldetect0);
+ writel_relaxed(bank->context.leveldetect1,
+ bank->base + bank->regs->leveldetect1);
+
+ /* Clear saved wkup_en for level, it will be set for next idle again */
+ bank->context.wake_en &= ~(bank->context.leveldetect0 |
+ bank->context.leveldetect1);
+
+ /* Update wake with only edge configuration */
+ writel_relaxed(bank->context.wake_en,
+ bank->base + bank->regs->wkup_en);
+}
+
/*---------------------------------------------------------------------*/
static int omap_mpuio_suspend_noirq(struct device *dev)
@@ -1270,6 +1366,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
bank->chip.parent = dev;
bank->chip.owner = THIS_MODULE;
bank->dbck_flag = pdata->dbck_flag;
+ bank->quirks = pdata->quirks;
bank->stride = pdata->bank_stride;
bank->width = pdata->bank_width;
bank->is_mpuio = pdata->is_mpuio;
@@ -1278,6 +1375,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
#ifdef CONFIG_OF_GPIO
bank->chip.of_node = of_node_get(node);
#endif
+
if (node) {
if (!of_property_read_bool(node, "ti,gpio-always-on"))
bank->loses_context = true;
@@ -1298,6 +1396,18 @@ static int omap_gpio_probe(struct platform_device *pdev)
omap_set_gpio_dataout_mask_multiple;
}
+ if (bank->quirks & OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN) {
+ bank->funcs.idle_enable_level_quirk =
+ omap4_gpio_enable_level_quirk;
+ bank->funcs.idle_disable_level_quirk =
+ omap4_gpio_disable_level_quirk;
+ } else {
+ bank->funcs.idle_enable_level_quirk =
+ omap2_gpio_enable_level_quirk;
+ bank->funcs.idle_disable_level_quirk =
+ omap2_gpio_disable_level_quirk;
+ }
+
raw_spin_lock_init(&bank->lock);
raw_spin_lock_init(&bank->wa_lock);
@@ -1372,29 +1482,11 @@ static int omap_gpio_runtime_suspend(struct device *dev)
struct gpio_bank *bank = platform_get_drvdata(pdev);
u32 l1 = 0, l2 = 0;
unsigned long flags;
- u32 wake_low, wake_hi;
raw_spin_lock_irqsave(&bank->lock, flags);
- /*
- * Only edges can generate a wakeup event to the PRCM.
- *
- * Therefore, ensure any wake-up capable GPIOs have
- * edge-detection enabled before going idle to ensure a wakeup
- * to the PRCM is generated on a GPIO transition. (c.f. 34xx
- * NDA TRM 25.5.3.1)
- *
- * The normal values will be restored upon ->runtime_resume()
- * by writing back the values saved in bank->context.
- */
- wake_low = bank->context.leveldetect0 & bank->context.wake_en;
- if (wake_low)
- writel_relaxed(wake_low | bank->context.fallingdetect,
- bank->base + bank->regs->fallingdetect);
- wake_hi = bank->context.leveldetect1 & bank->context.wake_en;
- if (wake_hi)
- writel_relaxed(wake_hi | bank->context.risingdetect,
- bank->base + bank->regs->risingdetect);
+ if (bank->funcs.idle_enable_level_quirk)
+ bank->funcs.idle_enable_level_quirk(bank);
if (!bank->enabled_non_wakeup_gpios)
goto update_gpio_context_count;
@@ -1459,16 +1551,8 @@ static int omap_gpio_runtime_resume(struct device *dev)
omap_gpio_dbck_enable(bank);
- /*
- * In ->runtime_suspend(), level-triggered, wakeup-enabled
- * GPIOs were set to edge trigger also in order to be able to
- * generate a PRCM wakeup. Here we restore the
- * pre-runtime_suspend() values for edge triggering.
- */
- writel_relaxed(bank->context.fallingdetect,
- bank->base + bank->regs->fallingdetect);
- writel_relaxed(bank->context.risingdetect,
- bank->base + bank->regs->risingdetect);
+ if (bank->funcs.idle_disable_level_quirk)
+ bank->funcs.idle_disable_level_quirk(bank);
if (bank->loses_context) {
if (!bank->get_context_loss_count) {
@@ -1706,6 +1790,7 @@ static const struct omap_gpio_platform_data omap4_pdata = {
.regs = &omap4_gpio_regs,
.bank_width = 32,
.dbck_flag = true,
+ .quirks = OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN,
};
static const struct of_device_id omap_gpio_match[] = {
diff --git a/include/linux/platform_data/gpio-omap.h b/include/linux/platform_data/gpio-omap.h
--- a/include/linux/platform_data/gpio-omap.h
+++ b/include/linux/platform_data/gpio-omap.h
@@ -197,6 +197,8 @@ struct omap_gpio_platform_data {
bool is_mpuio; /* whether the bank is of type MPUIO */
u32 non_wakeup_gpios;
+ u32 quirks; /* Version specific quirks mask */
+
struct omap_gpio_reg_offs *regs;
/* Return context loss count due to PM states changing */
--
2.19.0
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/3] gpio: omap: Add level wakeup handling for omap4 based SoCs
2018-09-20 19:35 ` Tony Lindgren
@ 2018-09-21 5:45 ` Keerthy
-1 siblings, 0 replies; 22+ messages in thread
From: Keerthy @ 2018-09-21 5:45 UTC (permalink / raw)
To: Tony Lindgren, Linus Walleij, Alexandre Courbot
Cc: Grygorii Strashko, Aaro Koskinen, Tero Kristo, linux-gpio,
Ladislav Michl, linux-omap, linux-arm-kernel
On Friday 21 September 2018 01:05 AM, Tony Lindgren wrote:
> I noticed that unlike omap2 and 3 based SoCs, omap4 based SoCs keep
> the GPIO clocks enabled for GPIO level interrupts with wakeup enabled.
> This blocks deeper idle states as the whole domain will stay busy.
>
> The GPIO functional clock seems to stay enabled if the wakeup register
> is enabled and a level interrupt is triggered. In that case the only
> way to have the GPIO module idle is to reset it. It is possible this
> has gone unnoticed with OSWR (Open SWitch Retention) and off mode
> during idle resetting GPIO context most GPIO instances in the earlier
> Android trees for example.
>
> Looks like the way to deal with this is to have omap4 based SoCs
> only set wake for the duration of idle for level interrupts, and clear
> level registers for the idle. With level interrupts we can do this as
> the level interrupt from device will be still there on resume.
>
> I've taken the long path to fixing this to avoid yet more hard to
> read code. I've set up a quirks flag, and a struct for function
> pointers so we can use these to clean up other quirk handling easier
> in the later patches. The current level quirk handling is moved to
> the new functions.
>
Tested for Deep Sleep 0 on AM437x-GP-EVM and AM335x-Boneblack for all
the three patches of the series.
Tested-by: Keerthy <j-keerthy@ti.com>
> Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Ladislav Michl <ladis@linux-mips.org>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/gpio/gpio-omap.c | 151 ++++++++++++++++++------
> include/linux/platform_data/gpio-omap.h | 2 +
> 2 files changed, 120 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -31,6 +31,8 @@
> #define OFF_MODE 1
> #define OMAP4_GPIO_DEBOUNCINGTIME_MASK 0xFF
>
> +#define OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN BIT(1)
> +
> static LIST_HEAD(omap_gpio_list);
>
> struct gpio_regs {
> @@ -48,6 +50,13 @@ struct gpio_regs {
> u32 debounce_en;
> };
>
> +struct gpio_bank;
> +
> +struct gpio_omap_funcs {
> + void (*idle_enable_level_quirk)(struct gpio_bank *bank);
> + void (*idle_disable_level_quirk)(struct gpio_bank *bank);
> +};
> +
> struct gpio_bank {
> struct list_head node;
> void __iomem *base;
> @@ -55,6 +64,7 @@ struct gpio_bank {
> u32 non_wakeup_gpios;
> u32 enabled_non_wakeup_gpios;
> struct gpio_regs context;
> + struct gpio_omap_funcs funcs;
> u32 saved_datain;
> u32 level_mask;
> u32 toggle_mask;
> @@ -75,6 +85,7 @@ struct gpio_bank {
> int context_loss_count;
> int power_mode;
> bool workaround_enabled;
> + u32 quirks;
>
> void (*set_dataout)(struct gpio_bank *bank, unsigned gpio, int enable);
> void (*set_dataout_multiple)(struct gpio_bank *bank,
> @@ -368,9 +379,18 @@ static inline void omap_set_gpio_trigger(struct gpio_bank *bank, int gpio,
> readl_relaxed(bank->base + bank->regs->fallingdetect);
>
> if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
> - omap_gpio_rmw(base, bank->regs->wkup_en, gpio_bit, trigger != 0);
> - bank->context.wake_en =
> - readl_relaxed(bank->base + bank->regs->wkup_en);
> + /* Defer wkup_en register update until we idle? */
> + if (bank->quirks & OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN) {
> + if (trigger)
> + bank->context.wake_en |= gpio_bit;
> + else
> + bank->context.wake_en &= ~gpio_bit;
> + } else {
> + omap_gpio_rmw(base, bank->regs->wkup_en, gpio_bit,
> + trigger != 0);
> + bank->context.wake_en =
> + readl_relaxed(bank->base + bank->regs->wkup_en);
> + }
> }
>
> /* This part needs to be executed always for OMAP{34xx, 44xx} */
> @@ -899,6 +919,82 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
> raw_spin_unlock_irqrestore(&bank->lock, flags);
> }
>
> +/*
> + * Only edges can generate a wakeup event to the PRCM.
> + *
> + * Therefore, ensure any wake-up capable GPIOs have
> + * edge-detection enabled before going idle to ensure a wakeup
> + * to the PRCM is generated on a GPIO transition. (c.f. 34xx
> + * NDA TRM 25.5.3.1)
> + *
> + * The normal values will be restored upon ->runtime_resume()
> + * by writing back the values saved in bank->context.
> + */
> +static void __maybe_unused
> +omap2_gpio_enable_level_quirk(struct gpio_bank *bank)
> +{
> + u32 wake_low, wake_hi;
> +
> + /* Enable additional edge detection for level gpios for idle */
> + wake_low = bank->context.leveldetect0 & bank->context.wake_en;
> + if (wake_low)
> + writel_relaxed(wake_low | bank->context.fallingdetect,
> + bank->base + bank->regs->fallingdetect);
> +
> + wake_hi = bank->context.leveldetect1 & bank->context.wake_en;
> + if (wake_hi)
> + writel_relaxed(wake_hi | bank->context.risingdetect,
> + bank->base + bank->regs->risingdetect);
> +}
> +
> +static void __maybe_unused
> +omap2_gpio_disable_level_quirk(struct gpio_bank *bank)
> +{
> + /* Disable edge detection for level gpios after idle */
> + writel_relaxed(bank->context.fallingdetect,
> + bank->base + bank->regs->fallingdetect);
> + writel_relaxed(bank->context.risingdetect,
> + bank->base + bank->regs->risingdetect);
> +}
> +
> +/*
> + * On omap4 and later SoC variants a level interrupt with wkup_en
> + * enabled blocks the GPIO functional clock from idling until the GPIO
> + * instance has been reset. To avoid that, we must set wkup_en only for
> + * idle for level interrupts, and clear level registers for the duration
> + * of idle. The level interrupts will be still there on wakeup by their
> + * nature.
> + */
> +static void __maybe_unused
> +omap4_gpio_enable_level_quirk(struct gpio_bank *bank)
> +{
> + /* Update wake register for idle, edge bits might be already set */
> + writel_relaxed(bank->context.wake_en,
> + bank->base + bank->regs->wkup_en);
> +
> + /* Clear level registers for idle */
> + writel_relaxed(0, bank->base + bank->regs->leveldetect0);
> + writel_relaxed(0, bank->base + bank->regs->leveldetect1);
> +}
> +
> +static void __maybe_unused
> +omap4_gpio_disable_level_quirk(struct gpio_bank *bank)
> +{
> + /* Restore level registers after idle */
> + writel_relaxed(bank->context.leveldetect0,
> + bank->base + bank->regs->leveldetect0);
> + writel_relaxed(bank->context.leveldetect1,
> + bank->base + bank->regs->leveldetect1);
> +
> + /* Clear saved wkup_en for level, it will be set for next idle again */
> + bank->context.wake_en &= ~(bank->context.leveldetect0 |
> + bank->context.leveldetect1);
> +
> + /* Update wake with only edge configuration */
> + writel_relaxed(bank->context.wake_en,
> + bank->base + bank->regs->wkup_en);
> +}
> +
> /*---------------------------------------------------------------------*/
>
> static int omap_mpuio_suspend_noirq(struct device *dev)
> @@ -1270,6 +1366,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
> bank->chip.parent = dev;
> bank->chip.owner = THIS_MODULE;
> bank->dbck_flag = pdata->dbck_flag;
> + bank->quirks = pdata->quirks;
> bank->stride = pdata->bank_stride;
> bank->width = pdata->bank_width;
> bank->is_mpuio = pdata->is_mpuio;
> @@ -1278,6 +1375,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
> #ifdef CONFIG_OF_GPIO
> bank->chip.of_node = of_node_get(node);
> #endif
> +
> if (node) {
> if (!of_property_read_bool(node, "ti,gpio-always-on"))
> bank->loses_context = true;
> @@ -1298,6 +1396,18 @@ static int omap_gpio_probe(struct platform_device *pdev)
> omap_set_gpio_dataout_mask_multiple;
> }
>
> + if (bank->quirks & OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN) {
> + bank->funcs.idle_enable_level_quirk =
> + omap4_gpio_enable_level_quirk;
> + bank->funcs.idle_disable_level_quirk =
> + omap4_gpio_disable_level_quirk;
> + } else {
> + bank->funcs.idle_enable_level_quirk =
> + omap2_gpio_enable_level_quirk;
> + bank->funcs.idle_disable_level_quirk =
> + omap2_gpio_disable_level_quirk;
> + }
> +
> raw_spin_lock_init(&bank->lock);
> raw_spin_lock_init(&bank->wa_lock);
>
> @@ -1372,29 +1482,11 @@ static int omap_gpio_runtime_suspend(struct device *dev)
> struct gpio_bank *bank = platform_get_drvdata(pdev);
> u32 l1 = 0, l2 = 0;
> unsigned long flags;
> - u32 wake_low, wake_hi;
>
> raw_spin_lock_irqsave(&bank->lock, flags);
>
> - /*
> - * Only edges can generate a wakeup event to the PRCM.
> - *
> - * Therefore, ensure any wake-up capable GPIOs have
> - * edge-detection enabled before going idle to ensure a wakeup
> - * to the PRCM is generated on a GPIO transition. (c.f. 34xx
> - * NDA TRM 25.5.3.1)
> - *
> - * The normal values will be restored upon ->runtime_resume()
> - * by writing back the values saved in bank->context.
> - */
> - wake_low = bank->context.leveldetect0 & bank->context.wake_en;
> - if (wake_low)
> - writel_relaxed(wake_low | bank->context.fallingdetect,
> - bank->base + bank->regs->fallingdetect);
> - wake_hi = bank->context.leveldetect1 & bank->context.wake_en;
> - if (wake_hi)
> - writel_relaxed(wake_hi | bank->context.risingdetect,
> - bank->base + bank->regs->risingdetect);
> + if (bank->funcs.idle_enable_level_quirk)
> + bank->funcs.idle_enable_level_quirk(bank);
>
> if (!bank->enabled_non_wakeup_gpios)
> goto update_gpio_context_count;
> @@ -1459,16 +1551,8 @@ static int omap_gpio_runtime_resume(struct device *dev)
>
> omap_gpio_dbck_enable(bank);
>
> - /*
> - * In ->runtime_suspend(), level-triggered, wakeup-enabled
> - * GPIOs were set to edge trigger also in order to be able to
> - * generate a PRCM wakeup. Here we restore the
> - * pre-runtime_suspend() values for edge triggering.
> - */
> - writel_relaxed(bank->context.fallingdetect,
> - bank->base + bank->regs->fallingdetect);
> - writel_relaxed(bank->context.risingdetect,
> - bank->base + bank->regs->risingdetect);
> + if (bank->funcs.idle_disable_level_quirk)
> + bank->funcs.idle_disable_level_quirk(bank);
>
> if (bank->loses_context) {
> if (!bank->get_context_loss_count) {
> @@ -1706,6 +1790,7 @@ static const struct omap_gpio_platform_data omap4_pdata = {
> .regs = &omap4_gpio_regs,
> .bank_width = 32,
> .dbck_flag = true,
> + .quirks = OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN,
> };
>
> static const struct of_device_id omap_gpio_match[] = {
> diff --git a/include/linux/platform_data/gpio-omap.h b/include/linux/platform_data/gpio-omap.h
> --- a/include/linux/platform_data/gpio-omap.h
> +++ b/include/linux/platform_data/gpio-omap.h
> @@ -197,6 +197,8 @@ struct omap_gpio_platform_data {
> bool is_mpuio; /* whether the bank is of type MPUIO */
> u32 non_wakeup_gpios;
>
> + u32 quirks; /* Version specific quirks mask */
> +
> struct omap_gpio_reg_offs *regs;
>
> /* Return context loss count due to PM states changing */
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/3] gpio: omap: Add level wakeup handling for omap4 based SoCs
@ 2018-09-21 5:45 ` Keerthy
0 siblings, 0 replies; 22+ messages in thread
From: Keerthy @ 2018-09-21 5:45 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 21 September 2018 01:05 AM, Tony Lindgren wrote:
> I noticed that unlike omap2 and 3 based SoCs, omap4 based SoCs keep
> the GPIO clocks enabled for GPIO level interrupts with wakeup enabled.
> This blocks deeper idle states as the whole domain will stay busy.
>
> The GPIO functional clock seems to stay enabled if the wakeup register
> is enabled and a level interrupt is triggered. In that case the only
> way to have the GPIO module idle is to reset it. It is possible this
> has gone unnoticed with OSWR (Open SWitch Retention) and off mode
> during idle resetting GPIO context most GPIO instances in the earlier
> Android trees for example.
>
> Looks like the way to deal with this is to have omap4 based SoCs
> only set wake for the duration of idle for level interrupts, and clear
> level registers for the idle. With level interrupts we can do this as
> the level interrupt from device will be still there on resume.
>
> I've taken the long path to fixing this to avoid yet more hard to
> read code. I've set up a quirks flag, and a struct for function
> pointers so we can use these to clean up other quirk handling easier
> in the later patches. The current level quirk handling is moved to
> the new functions.
>
Tested for Deep Sleep 0 on AM437x-GP-EVM and AM335x-Boneblack for all
the three patches of the series.
Tested-by: Keerthy <j-keerthy@ti.com>
> Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Ladislav Michl <ladis@linux-mips.org>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> drivers/gpio/gpio-omap.c | 151 ++++++++++++++++++------
> include/linux/platform_data/gpio-omap.h | 2 +
> 2 files changed, 120 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -31,6 +31,8 @@
> #define OFF_MODE 1
> #define OMAP4_GPIO_DEBOUNCINGTIME_MASK 0xFF
>
> +#define OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN BIT(1)
> +
> static LIST_HEAD(omap_gpio_list);
>
> struct gpio_regs {
> @@ -48,6 +50,13 @@ struct gpio_regs {
> u32 debounce_en;
> };
>
> +struct gpio_bank;
> +
> +struct gpio_omap_funcs {
> + void (*idle_enable_level_quirk)(struct gpio_bank *bank);
> + void (*idle_disable_level_quirk)(struct gpio_bank *bank);
> +};
> +
> struct gpio_bank {
> struct list_head node;
> void __iomem *base;
> @@ -55,6 +64,7 @@ struct gpio_bank {
> u32 non_wakeup_gpios;
> u32 enabled_non_wakeup_gpios;
> struct gpio_regs context;
> + struct gpio_omap_funcs funcs;
> u32 saved_datain;
> u32 level_mask;
> u32 toggle_mask;
> @@ -75,6 +85,7 @@ struct gpio_bank {
> int context_loss_count;
> int power_mode;
> bool workaround_enabled;
> + u32 quirks;
>
> void (*set_dataout)(struct gpio_bank *bank, unsigned gpio, int enable);
> void (*set_dataout_multiple)(struct gpio_bank *bank,
> @@ -368,9 +379,18 @@ static inline void omap_set_gpio_trigger(struct gpio_bank *bank, int gpio,
> readl_relaxed(bank->base + bank->regs->fallingdetect);
>
> if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
> - omap_gpio_rmw(base, bank->regs->wkup_en, gpio_bit, trigger != 0);
> - bank->context.wake_en =
> - readl_relaxed(bank->base + bank->regs->wkup_en);
> + /* Defer wkup_en register update until we idle? */
> + if (bank->quirks & OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN) {
> + if (trigger)
> + bank->context.wake_en |= gpio_bit;
> + else
> + bank->context.wake_en &= ~gpio_bit;
> + } else {
> + omap_gpio_rmw(base, bank->regs->wkup_en, gpio_bit,
> + trigger != 0);
> + bank->context.wake_en =
> + readl_relaxed(bank->base + bank->regs->wkup_en);
> + }
> }
>
> /* This part needs to be executed always for OMAP{34xx, 44xx} */
> @@ -899,6 +919,82 @@ static void omap_gpio_unmask_irq(struct irq_data *d)
> raw_spin_unlock_irqrestore(&bank->lock, flags);
> }
>
> +/*
> + * Only edges can generate a wakeup event to the PRCM.
> + *
> + * Therefore, ensure any wake-up capable GPIOs have
> + * edge-detection enabled before going idle to ensure a wakeup
> + * to the PRCM is generated on a GPIO transition. (c.f. 34xx
> + * NDA TRM 25.5.3.1)
> + *
> + * The normal values will be restored upon ->runtime_resume()
> + * by writing back the values saved in bank->context.
> + */
> +static void __maybe_unused
> +omap2_gpio_enable_level_quirk(struct gpio_bank *bank)
> +{
> + u32 wake_low, wake_hi;
> +
> + /* Enable additional edge detection for level gpios for idle */
> + wake_low = bank->context.leveldetect0 & bank->context.wake_en;
> + if (wake_low)
> + writel_relaxed(wake_low | bank->context.fallingdetect,
> + bank->base + bank->regs->fallingdetect);
> +
> + wake_hi = bank->context.leveldetect1 & bank->context.wake_en;
> + if (wake_hi)
> + writel_relaxed(wake_hi | bank->context.risingdetect,
> + bank->base + bank->regs->risingdetect);
> +}
> +
> +static void __maybe_unused
> +omap2_gpio_disable_level_quirk(struct gpio_bank *bank)
> +{
> + /* Disable edge detection for level gpios after idle */
> + writel_relaxed(bank->context.fallingdetect,
> + bank->base + bank->regs->fallingdetect);
> + writel_relaxed(bank->context.risingdetect,
> + bank->base + bank->regs->risingdetect);
> +}
> +
> +/*
> + * On omap4 and later SoC variants a level interrupt with wkup_en
> + * enabled blocks the GPIO functional clock from idling until the GPIO
> + * instance has been reset. To avoid that, we must set wkup_en only for
> + * idle for level interrupts, and clear level registers for the duration
> + * of idle. The level interrupts will be still there on wakeup by their
> + * nature.
> + */
> +static void __maybe_unused
> +omap4_gpio_enable_level_quirk(struct gpio_bank *bank)
> +{
> + /* Update wake register for idle, edge bits might be already set */
> + writel_relaxed(bank->context.wake_en,
> + bank->base + bank->regs->wkup_en);
> +
> + /* Clear level registers for idle */
> + writel_relaxed(0, bank->base + bank->regs->leveldetect0);
> + writel_relaxed(0, bank->base + bank->regs->leveldetect1);
> +}
> +
> +static void __maybe_unused
> +omap4_gpio_disable_level_quirk(struct gpio_bank *bank)
> +{
> + /* Restore level registers after idle */
> + writel_relaxed(bank->context.leveldetect0,
> + bank->base + bank->regs->leveldetect0);
> + writel_relaxed(bank->context.leveldetect1,
> + bank->base + bank->regs->leveldetect1);
> +
> + /* Clear saved wkup_en for level, it will be set for next idle again */
> + bank->context.wake_en &= ~(bank->context.leveldetect0 |
> + bank->context.leveldetect1);
> +
> + /* Update wake with only edge configuration */
> + writel_relaxed(bank->context.wake_en,
> + bank->base + bank->regs->wkup_en);
> +}
> +
> /*---------------------------------------------------------------------*/
>
> static int omap_mpuio_suspend_noirq(struct device *dev)
> @@ -1270,6 +1366,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
> bank->chip.parent = dev;
> bank->chip.owner = THIS_MODULE;
> bank->dbck_flag = pdata->dbck_flag;
> + bank->quirks = pdata->quirks;
> bank->stride = pdata->bank_stride;
> bank->width = pdata->bank_width;
> bank->is_mpuio = pdata->is_mpuio;
> @@ -1278,6 +1375,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
> #ifdef CONFIG_OF_GPIO
> bank->chip.of_node = of_node_get(node);
> #endif
> +
> if (node) {
> if (!of_property_read_bool(node, "ti,gpio-always-on"))
> bank->loses_context = true;
> @@ -1298,6 +1396,18 @@ static int omap_gpio_probe(struct platform_device *pdev)
> omap_set_gpio_dataout_mask_multiple;
> }
>
> + if (bank->quirks & OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN) {
> + bank->funcs.idle_enable_level_quirk =
> + omap4_gpio_enable_level_quirk;
> + bank->funcs.idle_disable_level_quirk =
> + omap4_gpio_disable_level_quirk;
> + } else {
> + bank->funcs.idle_enable_level_quirk =
> + omap2_gpio_enable_level_quirk;
> + bank->funcs.idle_disable_level_quirk =
> + omap2_gpio_disable_level_quirk;
> + }
> +
> raw_spin_lock_init(&bank->lock);
> raw_spin_lock_init(&bank->wa_lock);
>
> @@ -1372,29 +1482,11 @@ static int omap_gpio_runtime_suspend(struct device *dev)
> struct gpio_bank *bank = platform_get_drvdata(pdev);
> u32 l1 = 0, l2 = 0;
> unsigned long flags;
> - u32 wake_low, wake_hi;
>
> raw_spin_lock_irqsave(&bank->lock, flags);
>
> - /*
> - * Only edges can generate a wakeup event to the PRCM.
> - *
> - * Therefore, ensure any wake-up capable GPIOs have
> - * edge-detection enabled before going idle to ensure a wakeup
> - * to the PRCM is generated on a GPIO transition. (c.f. 34xx
> - * NDA TRM 25.5.3.1)
> - *
> - * The normal values will be restored upon ->runtime_resume()
> - * by writing back the values saved in bank->context.
> - */
> - wake_low = bank->context.leveldetect0 & bank->context.wake_en;
> - if (wake_low)
> - writel_relaxed(wake_low | bank->context.fallingdetect,
> - bank->base + bank->regs->fallingdetect);
> - wake_hi = bank->context.leveldetect1 & bank->context.wake_en;
> - if (wake_hi)
> - writel_relaxed(wake_hi | bank->context.risingdetect,
> - bank->base + bank->regs->risingdetect);
> + if (bank->funcs.idle_enable_level_quirk)
> + bank->funcs.idle_enable_level_quirk(bank);
>
> if (!bank->enabled_non_wakeup_gpios)
> goto update_gpio_context_count;
> @@ -1459,16 +1551,8 @@ static int omap_gpio_runtime_resume(struct device *dev)
>
> omap_gpio_dbck_enable(bank);
>
> - /*
> - * In ->runtime_suspend(), level-triggered, wakeup-enabled
> - * GPIOs were set to edge trigger also in order to be able to
> - * generate a PRCM wakeup. Here we restore the
> - * pre-runtime_suspend() values for edge triggering.
> - */
> - writel_relaxed(bank->context.fallingdetect,
> - bank->base + bank->regs->fallingdetect);
> - writel_relaxed(bank->context.risingdetect,
> - bank->base + bank->regs->risingdetect);
> + if (bank->funcs.idle_disable_level_quirk)
> + bank->funcs.idle_disable_level_quirk(bank);
>
> if (bank->loses_context) {
> if (!bank->get_context_loss_count) {
> @@ -1706,6 +1790,7 @@ static const struct omap_gpio_platform_data omap4_pdata = {
> .regs = &omap4_gpio_regs,
> .bank_width = 32,
> .dbck_flag = true,
> + .quirks = OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN,
> };
>
> static const struct of_device_id omap_gpio_match[] = {
> diff --git a/include/linux/platform_data/gpio-omap.h b/include/linux/platform_data/gpio-omap.h
> --- a/include/linux/platform_data/gpio-omap.h
> +++ b/include/linux/platform_data/gpio-omap.h
> @@ -197,6 +197,8 @@ struct omap_gpio_platform_data {
> bool is_mpuio; /* whether the bank is of type MPUIO */
> u32 non_wakeup_gpios;
>
> + u32 quirks; /* Version specific quirks mask */
> +
> struct omap_gpio_reg_offs *regs;
>
> /* Return context loss count due to PM states changing */
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/3] gpio: omap: Remove custom PM calls and use cpu_pm instead
2018-09-20 19:35 ` Tony Lindgren
@ 2018-09-20 19:35 ` Tony Lindgren
-1 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-20 19:35 UTC (permalink / raw)
To: Linus Walleij, Alexandre Courbot
Cc: Grygorii Strashko, Aaro Koskinen, Keerthy, Tero Kristo,
linux-gpio, Ladislav Michl, linux-omap, linux-arm-kernel
For a long time the gpio-omap custom PM calls have been annoying me so
let's replace them with cpu_pm instead. This will enable GPIO PM for
deeper idle states on omap4. And we can handle GPIO PM for omap2/3/4
in the same way.
Note that with this patch we are also slightly changing GPIO PM to be
less aggressive for omap3 and only will idle GPIO when PER context
may be lost.
For omap2, we don't need to save context and don't want to remove any
triggering so let's add a quirk flag for that.
Let's do this all in a single patch to avoid a situation where old
custom calls still are used with new code.
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/mach-omap2/pm24xx.c | 7 +-
arch/arm/mach-omap2/pm34xx.c | 14 +-
drivers/gpio/gpio-omap.c | 178 ++++++++++++++----------
include/linux/platform_data/gpio-omap.h | 13 --
4 files changed, 116 insertions(+), 96 deletions(-)
diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
--- a/arch/arm/mach-omap2/pm24xx.c
+++ b/arch/arm/mach-omap2/pm24xx.c
@@ -18,6 +18,7 @@
* published by the Free Software Foundation.
*/
+#include <linux/cpu_pm.h>
#include <linux/suspend.h>
#include <linux/sched.h>
#include <linux/proc_fs.h>
@@ -29,8 +30,6 @@
#include <linux/clk-provider.h>
#include <linux/irq.h>
#include <linux/time.h>
-#include <linux/gpio.h>
-#include <linux/platform_data/gpio-omap.h>
#include <asm/fncpy.h>
@@ -87,7 +86,7 @@ static int omap2_enter_full_retention(void)
l = omap_ctrl_readl(OMAP2_CONTROL_DEVCONF0) | OMAP24XX_USBSTANDBYCTRL;
omap_ctrl_writel(l, OMAP2_CONTROL_DEVCONF0);
- omap2_gpio_prepare_for_idle(0);
+ cpu_cluster_pm_enter();
/* One last check for pending IRQs to avoid extra latency due
* to sleeping unnecessarily. */
@@ -100,7 +99,7 @@ static int omap2_enter_full_retention(void)
OMAP_SDRC_REGADDR(SDRC_POWER));
no_sleep:
- omap2_gpio_resume_after_idle();
+ cpu_cluster_pm_exit();
clk_enable(osc_ck);
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -18,19 +18,18 @@
* published by the Free Software Foundation.
*/
+#include <linux/cpu_pm.h>
#include <linux/pm.h>
#include <linux/suspend.h>
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/list.h>
#include <linux/err.h>
-#include <linux/gpio.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/omap-dma.h>
#include <linux/omap-gpmc.h>
-#include <linux/platform_data/gpio-omap.h>
#include <trace/events/power.h>
@@ -197,7 +196,6 @@ void omap_sram_idle(void)
int mpu_next_state = PWRDM_POWER_ON;
int per_next_state = PWRDM_POWER_ON;
int core_next_state = PWRDM_POWER_ON;
- int per_going_off;
u32 sdrc_pwr = 0;
mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm);
@@ -227,10 +225,8 @@ void omap_sram_idle(void)
pwrdm_pre_transition(NULL);
/* PER */
- if (per_next_state < PWRDM_POWER_ON) {
- per_going_off = (per_next_state == PWRDM_POWER_OFF) ? 1 : 0;
- omap2_gpio_prepare_for_idle(per_going_off);
- }
+ if (per_next_state == PWRDM_POWER_OFF)
+ cpu_cluster_pm_enter();
/* CORE */
if (core_next_state < PWRDM_POWER_ON) {
@@ -295,8 +291,8 @@ void omap_sram_idle(void)
pwrdm_post_transition(NULL);
/* PER */
- if (per_next_state < PWRDM_POWER_ON)
- omap2_gpio_resume_after_idle();
+ if (per_next_state == PWRDM_POWER_OFF)
+ cpu_cluster_pm_exit();
}
static void omap3_pm_idle(void)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -19,6 +19,7 @@
#include <linux/err.h>
#include <linux/clk.h>
#include <linux/io.h>
+#include <linux/cpu_pm.h>
#include <linux/device.h>
#include <linux/pm_runtime.h>
#include <linux/pm.h>
@@ -28,9 +29,9 @@
#include <linux/bitops.h>
#include <linux/platform_data/gpio-omap.h>
-#define OFF_MODE 1
#define OMAP4_GPIO_DEBOUNCINGTIME_MASK 0xFF
+#define OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER BIT(2)
#define OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN BIT(1)
static LIST_HEAD(omap_gpio_list);
@@ -72,6 +73,8 @@ struct gpio_bank {
raw_spinlock_t wa_lock;
struct gpio_chip chip;
struct clk *dbck;
+ struct notifier_block nb;
+ unsigned int is_suspended:1;
u32 mod_usage;
u32 irq_usage;
u32 dbck_enable_mask;
@@ -83,7 +86,6 @@ struct gpio_bank {
int stride;
u32 width;
int context_loss_count;
- int power_mode;
bool workaround_enabled;
u32 quirks;
@@ -1314,6 +1316,38 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
return ret;
}
+static void omap_gpio_idle(struct gpio_bank *bank, bool may_lose_context);
+static void omap_gpio_unidle(struct gpio_bank *bank);
+
+static int gpio_omap_cpu_notifier(struct notifier_block *nb,
+ unsigned long cmd, void *v)
+{
+ struct gpio_bank *bank;
+ struct device *dev;
+ unsigned long flags;
+
+ bank = container_of(nb, struct gpio_bank, nb);
+ dev = bank->chip.parent;
+
+ raw_spin_lock_irqsave(&bank->lock, flags);
+ switch (cmd) {
+ case CPU_CLUSTER_PM_ENTER:
+ if (bank->is_suspended)
+ break;
+ omap_gpio_idle(bank, true);
+ break;
+ case CPU_CLUSTER_PM_ENTER_FAILED:
+ case CPU_CLUSTER_PM_EXIT:
+ if (bank->is_suspended)
+ break;
+ omap_gpio_unidle(bank);
+ break;
+ }
+ raw_spin_unlock_irqrestore(&bank->lock, flags);
+
+ return NOTIFY_OK;
+}
+
static const struct of_device_id omap_gpio_match[];
static int omap_gpio_probe(struct platform_device *pdev)
@@ -1401,7 +1435,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
omap4_gpio_enable_level_quirk;
bank->funcs.idle_disable_level_quirk =
omap4_gpio_disable_level_quirk;
- } else {
+ } else if (bank->quirks & OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER) {
bank->funcs.idle_enable_level_quirk =
omap2_gpio_enable_level_quirk;
bank->funcs.idle_disable_level_quirk =
@@ -1451,6 +1485,12 @@ static int omap_gpio_probe(struct platform_device *pdev)
omap_gpio_show_rev(bank);
+ if (bank->funcs.idle_enable_level_quirk &&
+ bank->funcs.idle_disable_level_quirk) {
+ bank->nb.notifier_call = gpio_omap_cpu_notifier;
+ cpu_pm_register_notifier(&bank->nb);
+ }
+
pm_runtime_put(dev);
list_add_tail(&bank->node, &omap_gpio_list);
@@ -1462,6 +1502,8 @@ static int omap_gpio_remove(struct platform_device *pdev)
{
struct gpio_bank *bank = platform_get_drvdata(pdev);
+ if (bank->nb.notifier_call)
+ cpu_pm_unregister_notifier(&bank->nb);
list_del(&bank->node);
gpiochip_remove(&bank->chip);
pm_runtime_disable(&pdev->dev);
@@ -1471,19 +1513,12 @@ static int omap_gpio_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_ARCH_OMAP2PLUS
-
-#if defined(CONFIG_PM)
static void omap_gpio_restore_context(struct gpio_bank *bank);
-static int omap_gpio_runtime_suspend(struct device *dev)
+static void omap_gpio_idle(struct gpio_bank *bank, bool may_lose_context)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct gpio_bank *bank = platform_get_drvdata(pdev);
+ struct device *dev = bank->chip.parent;
u32 l1 = 0, l2 = 0;
- unsigned long flags;
-
- raw_spin_lock_irqsave(&bank->lock, flags);
if (bank->funcs.idle_enable_level_quirk)
bank->funcs.idle_enable_level_quirk(bank);
@@ -1491,10 +1526,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
if (!bank->enabled_non_wakeup_gpios)
goto update_gpio_context_count;
- if (bank->power_mode != OFF_MODE) {
- bank->power_mode = 0;
+ if (!may_lose_context)
goto update_gpio_context_count;
- }
+
/*
* If going to OFF, remove triggering for all
* non-wakeup GPIOs. Otherwise spurious IRQs will be
@@ -1519,23 +1553,16 @@ static int omap_gpio_runtime_suspend(struct device *dev)
bank->get_context_loss_count(dev);
omap_gpio_dbck_disable(bank);
- raw_spin_unlock_irqrestore(&bank->lock, flags);
-
- return 0;
}
static void omap_gpio_init_context(struct gpio_bank *p);
-static int omap_gpio_runtime_resume(struct device *dev)
+static void omap_gpio_unidle(struct gpio_bank *bank)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct gpio_bank *bank = platform_get_drvdata(pdev);
+ struct device *dev = bank->chip.parent;
u32 l = 0, gen, gen0, gen1;
- unsigned long flags;
int c;
- raw_spin_lock_irqsave(&bank->lock, flags);
-
/*
* On the first resume during the probe, the context has not
* been initialised and so initialise it now. Also initialise
@@ -1562,16 +1589,13 @@ static int omap_gpio_runtime_resume(struct device *dev)
if (c != bank->context_loss_count) {
omap_gpio_restore_context(bank);
} else {
- raw_spin_unlock_irqrestore(&bank->lock, flags);
- return 0;
+ return;
}
}
}
- if (!bank->workaround_enabled) {
- raw_spin_unlock_irqrestore(&bank->lock, flags);
- return 0;
- }
+ if (!bank->workaround_enabled)
+ return;
l = readl_relaxed(bank->base + bank->regs->datain);
@@ -1624,41 +1648,8 @@ static int omap_gpio_runtime_resume(struct device *dev)
}
bank->workaround_enabled = false;
- raw_spin_unlock_irqrestore(&bank->lock, flags);
-
- return 0;
}
-#endif /* CONFIG_PM */
-
-#if IS_BUILTIN(CONFIG_GPIO_OMAP)
-void omap2_gpio_prepare_for_idle(int pwr_mode)
-{
- struct gpio_bank *bank;
- list_for_each_entry(bank, &omap_gpio_list, node) {
- if (!BANK_USED(bank) || !bank->loses_context)
- continue;
-
- bank->power_mode = pwr_mode;
-
- pm_runtime_put_sync_suspend(bank->chip.parent);
- }
-}
-
-void omap2_gpio_resume_after_idle(void)
-{
- struct gpio_bank *bank;
-
- list_for_each_entry(bank, &omap_gpio_list, node) {
- if (!BANK_USED(bank) || !bank->loses_context)
- continue;
-
- pm_runtime_get_sync(bank->chip.parent);
- }
-}
-#endif
-
-#if defined(CONFIG_PM)
static void omap_gpio_init_context(struct gpio_bank *p)
{
struct omap_gpio_reg_offs *regs = p->regs;
@@ -1715,17 +1706,57 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
writel_relaxed(bank->context.irqenable2,
bank->base + bank->regs->irqenable2);
}
-#endif /* CONFIG_PM */
-#else
-#define omap_gpio_runtime_suspend NULL
-#define omap_gpio_runtime_resume NULL
-static inline void omap_gpio_init_context(struct gpio_bank *p) {}
-#endif
+static int __maybe_unused omap_gpio_runtime_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct gpio_bank *bank = platform_get_drvdata(pdev);
+ unsigned long flags;
+ int error = 0;
+
+ raw_spin_lock_irqsave(&bank->lock, flags);
+ /* Must be idled only by CPU_CLUSTER_PM_ENTER? */
+ if (bank->irq_usage) {
+ error = -EBUSY;
+ goto unlock;
+ }
+ omap_gpio_idle(bank, true);
+ bank->is_suspended = true;
+unlock:
+ raw_spin_unlock_irqrestore(&bank->lock, flags);
+
+ return error;
+}
+
+static int __maybe_unused omap_gpio_runtime_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct gpio_bank *bank = platform_get_drvdata(pdev);
+ unsigned long flags;
+ int error = 0;
+
+ raw_spin_lock_irqsave(&bank->lock, flags);
+ /* Must be unidled only by CPU_CLUSTER_PM_ENTER? */
+ if (bank->irq_usage) {
+ error = -EBUSY;
+ goto unlock;
+ }
+ omap_gpio_unidle(bank);
+ bank->is_suspended = false;
+unlock:
+ raw_spin_unlock_irqrestore(&bank->lock, flags);
+
+ return error;
+}
+
+#ifdef CONFIG_ARCH_OMAP2PLUS
static const struct dev_pm_ops gpio_pm_ops = {
SET_RUNTIME_PM_OPS(omap_gpio_runtime_suspend, omap_gpio_runtime_resume,
NULL)
};
+#else
+static const struct dev_pm_ops gpio_pm_ops;
+#endif /* CONFIG_ARCH_OMAP2PLUS */
#if defined(CONFIG_OF)
static struct omap_gpio_reg_offs omap2_gpio_regs = {
@@ -1774,6 +1805,11 @@ static struct omap_gpio_reg_offs omap4_gpio_regs = {
.fallingdetect = OMAP4_GPIO_FALLINGDETECT,
};
+/*
+ * Note that omap2 does not currently support idle modes with context loss so
+ * no need to add OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER quirk flag to save
+ * and restore context.
+ */
static const struct omap_gpio_platform_data omap2_pdata = {
.regs = &omap2_gpio_regs,
.bank_width = 32,
@@ -1784,13 +1820,15 @@ static const struct omap_gpio_platform_data omap3_pdata = {
.regs = &omap2_gpio_regs,
.bank_width = 32,
.dbck_flag = true,
+ .quirks = OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER,
};
static const struct omap_gpio_platform_data omap4_pdata = {
.regs = &omap4_gpio_regs,
.bank_width = 32,
.dbck_flag = true,
- .quirks = OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN,
+ .quirks = OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER |
+ OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN,
};
static const struct of_device_id omap_gpio_match[] = {
diff --git a/include/linux/platform_data/gpio-omap.h b/include/linux/platform_data/gpio-omap.h
--- a/include/linux/platform_data/gpio-omap.h
+++ b/include/linux/platform_data/gpio-omap.h
@@ -205,17 +205,4 @@ struct omap_gpio_platform_data {
int (*get_context_loss_count)(struct device *dev);
};
-#if IS_BUILTIN(CONFIG_GPIO_OMAP)
-extern void omap2_gpio_prepare_for_idle(int off_mode);
-extern void omap2_gpio_resume_after_idle(void);
-#else
-static inline void omap2_gpio_prepare_for_idle(int off_mode)
-{
-}
-
-static inline void omap2_gpio_resume_after_idle(void)
-{
-}
-#endif
-
#endif
--
2.19.0
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 2/3] gpio: omap: Remove custom PM calls and use cpu_pm instead
@ 2018-09-20 19:35 ` Tony Lindgren
0 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-20 19:35 UTC (permalink / raw)
To: linux-arm-kernel
For a long time the gpio-omap custom PM calls have been annoying me so
let's replace them with cpu_pm instead. This will enable GPIO PM for
deeper idle states on omap4. And we can handle GPIO PM for omap2/3/4
in the same way.
Note that with this patch we are also slightly changing GPIO PM to be
less aggressive for omap3 and only will idle GPIO when PER context
may be lost.
For omap2, we don't need to save context and don't want to remove any
triggering so let's add a quirk flag for that.
Let's do this all in a single patch to avoid a situation where old
custom calls still are used with new code.
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/mach-omap2/pm24xx.c | 7 +-
arch/arm/mach-omap2/pm34xx.c | 14 +-
drivers/gpio/gpio-omap.c | 178 ++++++++++++++----------
include/linux/platform_data/gpio-omap.h | 13 --
4 files changed, 116 insertions(+), 96 deletions(-)
diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
--- a/arch/arm/mach-omap2/pm24xx.c
+++ b/arch/arm/mach-omap2/pm24xx.c
@@ -18,6 +18,7 @@
* published by the Free Software Foundation.
*/
+#include <linux/cpu_pm.h>
#include <linux/suspend.h>
#include <linux/sched.h>
#include <linux/proc_fs.h>
@@ -29,8 +30,6 @@
#include <linux/clk-provider.h>
#include <linux/irq.h>
#include <linux/time.h>
-#include <linux/gpio.h>
-#include <linux/platform_data/gpio-omap.h>
#include <asm/fncpy.h>
@@ -87,7 +86,7 @@ static int omap2_enter_full_retention(void)
l = omap_ctrl_readl(OMAP2_CONTROL_DEVCONF0) | OMAP24XX_USBSTANDBYCTRL;
omap_ctrl_writel(l, OMAP2_CONTROL_DEVCONF0);
- omap2_gpio_prepare_for_idle(0);
+ cpu_cluster_pm_enter();
/* One last check for pending IRQs to avoid extra latency due
* to sleeping unnecessarily. */
@@ -100,7 +99,7 @@ static int omap2_enter_full_retention(void)
OMAP_SDRC_REGADDR(SDRC_POWER));
no_sleep:
- omap2_gpio_resume_after_idle();
+ cpu_cluster_pm_exit();
clk_enable(osc_ck);
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -18,19 +18,18 @@
* published by the Free Software Foundation.
*/
+#include <linux/cpu_pm.h>
#include <linux/pm.h>
#include <linux/suspend.h>
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/list.h>
#include <linux/err.h>
-#include <linux/gpio.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/omap-dma.h>
#include <linux/omap-gpmc.h>
-#include <linux/platform_data/gpio-omap.h>
#include <trace/events/power.h>
@@ -197,7 +196,6 @@ void omap_sram_idle(void)
int mpu_next_state = PWRDM_POWER_ON;
int per_next_state = PWRDM_POWER_ON;
int core_next_state = PWRDM_POWER_ON;
- int per_going_off;
u32 sdrc_pwr = 0;
mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm);
@@ -227,10 +225,8 @@ void omap_sram_idle(void)
pwrdm_pre_transition(NULL);
/* PER */
- if (per_next_state < PWRDM_POWER_ON) {
- per_going_off = (per_next_state == PWRDM_POWER_OFF) ? 1 : 0;
- omap2_gpio_prepare_for_idle(per_going_off);
- }
+ if (per_next_state == PWRDM_POWER_OFF)
+ cpu_cluster_pm_enter();
/* CORE */
if (core_next_state < PWRDM_POWER_ON) {
@@ -295,8 +291,8 @@ void omap_sram_idle(void)
pwrdm_post_transition(NULL);
/* PER */
- if (per_next_state < PWRDM_POWER_ON)
- omap2_gpio_resume_after_idle();
+ if (per_next_state == PWRDM_POWER_OFF)
+ cpu_cluster_pm_exit();
}
static void omap3_pm_idle(void)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -19,6 +19,7 @@
#include <linux/err.h>
#include <linux/clk.h>
#include <linux/io.h>
+#include <linux/cpu_pm.h>
#include <linux/device.h>
#include <linux/pm_runtime.h>
#include <linux/pm.h>
@@ -28,9 +29,9 @@
#include <linux/bitops.h>
#include <linux/platform_data/gpio-omap.h>
-#define OFF_MODE 1
#define OMAP4_GPIO_DEBOUNCINGTIME_MASK 0xFF
+#define OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER BIT(2)
#define OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN BIT(1)
static LIST_HEAD(omap_gpio_list);
@@ -72,6 +73,8 @@ struct gpio_bank {
raw_spinlock_t wa_lock;
struct gpio_chip chip;
struct clk *dbck;
+ struct notifier_block nb;
+ unsigned int is_suspended:1;
u32 mod_usage;
u32 irq_usage;
u32 dbck_enable_mask;
@@ -83,7 +86,6 @@ struct gpio_bank {
int stride;
u32 width;
int context_loss_count;
- int power_mode;
bool workaround_enabled;
u32 quirks;
@@ -1314,6 +1316,38 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
return ret;
}
+static void omap_gpio_idle(struct gpio_bank *bank, bool may_lose_context);
+static void omap_gpio_unidle(struct gpio_bank *bank);
+
+static int gpio_omap_cpu_notifier(struct notifier_block *nb,
+ unsigned long cmd, void *v)
+{
+ struct gpio_bank *bank;
+ struct device *dev;
+ unsigned long flags;
+
+ bank = container_of(nb, struct gpio_bank, nb);
+ dev = bank->chip.parent;
+
+ raw_spin_lock_irqsave(&bank->lock, flags);
+ switch (cmd) {
+ case CPU_CLUSTER_PM_ENTER:
+ if (bank->is_suspended)
+ break;
+ omap_gpio_idle(bank, true);
+ break;
+ case CPU_CLUSTER_PM_ENTER_FAILED:
+ case CPU_CLUSTER_PM_EXIT:
+ if (bank->is_suspended)
+ break;
+ omap_gpio_unidle(bank);
+ break;
+ }
+ raw_spin_unlock_irqrestore(&bank->lock, flags);
+
+ return NOTIFY_OK;
+}
+
static const struct of_device_id omap_gpio_match[];
static int omap_gpio_probe(struct platform_device *pdev)
@@ -1401,7 +1435,7 @@ static int omap_gpio_probe(struct platform_device *pdev)
omap4_gpio_enable_level_quirk;
bank->funcs.idle_disable_level_quirk =
omap4_gpio_disable_level_quirk;
- } else {
+ } else if (bank->quirks & OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER) {
bank->funcs.idle_enable_level_quirk =
omap2_gpio_enable_level_quirk;
bank->funcs.idle_disable_level_quirk =
@@ -1451,6 +1485,12 @@ static int omap_gpio_probe(struct platform_device *pdev)
omap_gpio_show_rev(bank);
+ if (bank->funcs.idle_enable_level_quirk &&
+ bank->funcs.idle_disable_level_quirk) {
+ bank->nb.notifier_call = gpio_omap_cpu_notifier;
+ cpu_pm_register_notifier(&bank->nb);
+ }
+
pm_runtime_put(dev);
list_add_tail(&bank->node, &omap_gpio_list);
@@ -1462,6 +1502,8 @@ static int omap_gpio_remove(struct platform_device *pdev)
{
struct gpio_bank *bank = platform_get_drvdata(pdev);
+ if (bank->nb.notifier_call)
+ cpu_pm_unregister_notifier(&bank->nb);
list_del(&bank->node);
gpiochip_remove(&bank->chip);
pm_runtime_disable(&pdev->dev);
@@ -1471,19 +1513,12 @@ static int omap_gpio_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_ARCH_OMAP2PLUS
-
-#if defined(CONFIG_PM)
static void omap_gpio_restore_context(struct gpio_bank *bank);
-static int omap_gpio_runtime_suspend(struct device *dev)
+static void omap_gpio_idle(struct gpio_bank *bank, bool may_lose_context)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct gpio_bank *bank = platform_get_drvdata(pdev);
+ struct device *dev = bank->chip.parent;
u32 l1 = 0, l2 = 0;
- unsigned long flags;
-
- raw_spin_lock_irqsave(&bank->lock, flags);
if (bank->funcs.idle_enable_level_quirk)
bank->funcs.idle_enable_level_quirk(bank);
@@ -1491,10 +1526,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
if (!bank->enabled_non_wakeup_gpios)
goto update_gpio_context_count;
- if (bank->power_mode != OFF_MODE) {
- bank->power_mode = 0;
+ if (!may_lose_context)
goto update_gpio_context_count;
- }
+
/*
* If going to OFF, remove triggering for all
* non-wakeup GPIOs. Otherwise spurious IRQs will be
@@ -1519,23 +1553,16 @@ static int omap_gpio_runtime_suspend(struct device *dev)
bank->get_context_loss_count(dev);
omap_gpio_dbck_disable(bank);
- raw_spin_unlock_irqrestore(&bank->lock, flags);
-
- return 0;
}
static void omap_gpio_init_context(struct gpio_bank *p);
-static int omap_gpio_runtime_resume(struct device *dev)
+static void omap_gpio_unidle(struct gpio_bank *bank)
{
- struct platform_device *pdev = to_platform_device(dev);
- struct gpio_bank *bank = platform_get_drvdata(pdev);
+ struct device *dev = bank->chip.parent;
u32 l = 0, gen, gen0, gen1;
- unsigned long flags;
int c;
- raw_spin_lock_irqsave(&bank->lock, flags);
-
/*
* On the first resume during the probe, the context has not
* been initialised and so initialise it now. Also initialise
@@ -1562,16 +1589,13 @@ static int omap_gpio_runtime_resume(struct device *dev)
if (c != bank->context_loss_count) {
omap_gpio_restore_context(bank);
} else {
- raw_spin_unlock_irqrestore(&bank->lock, flags);
- return 0;
+ return;
}
}
}
- if (!bank->workaround_enabled) {
- raw_spin_unlock_irqrestore(&bank->lock, flags);
- return 0;
- }
+ if (!bank->workaround_enabled)
+ return;
l = readl_relaxed(bank->base + bank->regs->datain);
@@ -1624,41 +1648,8 @@ static int omap_gpio_runtime_resume(struct device *dev)
}
bank->workaround_enabled = false;
- raw_spin_unlock_irqrestore(&bank->lock, flags);
-
- return 0;
}
-#endif /* CONFIG_PM */
-
-#if IS_BUILTIN(CONFIG_GPIO_OMAP)
-void omap2_gpio_prepare_for_idle(int pwr_mode)
-{
- struct gpio_bank *bank;
- list_for_each_entry(bank, &omap_gpio_list, node) {
- if (!BANK_USED(bank) || !bank->loses_context)
- continue;
-
- bank->power_mode = pwr_mode;
-
- pm_runtime_put_sync_suspend(bank->chip.parent);
- }
-}
-
-void omap2_gpio_resume_after_idle(void)
-{
- struct gpio_bank *bank;
-
- list_for_each_entry(bank, &omap_gpio_list, node) {
- if (!BANK_USED(bank) || !bank->loses_context)
- continue;
-
- pm_runtime_get_sync(bank->chip.parent);
- }
-}
-#endif
-
-#if defined(CONFIG_PM)
static void omap_gpio_init_context(struct gpio_bank *p)
{
struct omap_gpio_reg_offs *regs = p->regs;
@@ -1715,17 +1706,57 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
writel_relaxed(bank->context.irqenable2,
bank->base + bank->regs->irqenable2);
}
-#endif /* CONFIG_PM */
-#else
-#define omap_gpio_runtime_suspend NULL
-#define omap_gpio_runtime_resume NULL
-static inline void omap_gpio_init_context(struct gpio_bank *p) {}
-#endif
+static int __maybe_unused omap_gpio_runtime_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct gpio_bank *bank = platform_get_drvdata(pdev);
+ unsigned long flags;
+ int error = 0;
+
+ raw_spin_lock_irqsave(&bank->lock, flags);
+ /* Must be idled only by CPU_CLUSTER_PM_ENTER? */
+ if (bank->irq_usage) {
+ error = -EBUSY;
+ goto unlock;
+ }
+ omap_gpio_idle(bank, true);
+ bank->is_suspended = true;
+unlock:
+ raw_spin_unlock_irqrestore(&bank->lock, flags);
+
+ return error;
+}
+
+static int __maybe_unused omap_gpio_runtime_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct gpio_bank *bank = platform_get_drvdata(pdev);
+ unsigned long flags;
+ int error = 0;
+
+ raw_spin_lock_irqsave(&bank->lock, flags);
+ /* Must be unidled only by CPU_CLUSTER_PM_ENTER? */
+ if (bank->irq_usage) {
+ error = -EBUSY;
+ goto unlock;
+ }
+ omap_gpio_unidle(bank);
+ bank->is_suspended = false;
+unlock:
+ raw_spin_unlock_irqrestore(&bank->lock, flags);
+
+ return error;
+}
+
+#ifdef CONFIG_ARCH_OMAP2PLUS
static const struct dev_pm_ops gpio_pm_ops = {
SET_RUNTIME_PM_OPS(omap_gpio_runtime_suspend, omap_gpio_runtime_resume,
NULL)
};
+#else
+static const struct dev_pm_ops gpio_pm_ops;
+#endif /* CONFIG_ARCH_OMAP2PLUS */
#if defined(CONFIG_OF)
static struct omap_gpio_reg_offs omap2_gpio_regs = {
@@ -1774,6 +1805,11 @@ static struct omap_gpio_reg_offs omap4_gpio_regs = {
.fallingdetect = OMAP4_GPIO_FALLINGDETECT,
};
+/*
+ * Note that omap2 does not currently support idle modes with context loss so
+ * no need to add OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER quirk flag to save
+ * and restore context.
+ */
static const struct omap_gpio_platform_data omap2_pdata = {
.regs = &omap2_gpio_regs,
.bank_width = 32,
@@ -1784,13 +1820,15 @@ static const struct omap_gpio_platform_data omap3_pdata = {
.regs = &omap2_gpio_regs,
.bank_width = 32,
.dbck_flag = true,
+ .quirks = OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER,
};
static const struct omap_gpio_platform_data omap4_pdata = {
.regs = &omap4_gpio_regs,
.bank_width = 32,
.dbck_flag = true,
- .quirks = OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN,
+ .quirks = OMAP_GPIO_QUIRK_IDLE_REMOVE_TRIGGER |
+ OMAP_GPIO_QUIRK_DEFERRED_WKUP_EN,
};
static const struct of_device_id omap_gpio_match[] = {
diff --git a/include/linux/platform_data/gpio-omap.h b/include/linux/platform_data/gpio-omap.h
--- a/include/linux/platform_data/gpio-omap.h
+++ b/include/linux/platform_data/gpio-omap.h
@@ -205,17 +205,4 @@ struct omap_gpio_platform_data {
int (*get_context_loss_count)(struct device *dev);
};
-#if IS_BUILTIN(CONFIG_GPIO_OMAP)
-extern void omap2_gpio_prepare_for_idle(int off_mode);
-extern void omap2_gpio_resume_after_idle(void);
-#else
-static inline void omap2_gpio_prepare_for_idle(int off_mode)
-{
-}
-
-static inline void omap2_gpio_resume_after_idle(void)
-{
-}
-#endif
-
#endif
--
2.19.0
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/3] gpio: omap: Remove custom PM calls and use cpu_pm instead
2018-09-20 19:35 ` Tony Lindgren
@ 2018-09-20 23:01 ` Grygorii Strashko
-1 siblings, 0 replies; 22+ messages in thread
From: Grygorii Strashko @ 2018-09-20 23:01 UTC (permalink / raw)
To: Tony Lindgren, Linus Walleij, Alexandre Courbot
Cc: Ladislav Michl, Aaro Koskinen, Keerthy, Tero Kristo, linux-gpio,
linux-omap, linux-arm-kernel
On 09/20/2018 02:35 PM, Tony Lindgren wrote:
> For a long time the gpio-omap custom PM calls have been annoying me so
> let's replace them with cpu_pm instead. This will enable GPIO PM for
> deeper idle states on omap4. And we can handle GPIO PM for omap2/3/4
> in the same way.
>
> Note that with this patch we are also slightly changing GPIO PM to be
> less aggressive for omap3 and only will idle GPIO when PER context
> may be lost.
>
> For omap2, we don't need to save context and don't want to remove any
> triggering so let's add a quirk flag for that.
>
> Let's do this all in a single patch to avoid a situation where old
> custom calls still are used with new code.
>
> Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Ladislav Michl <ladis@linux-mips.org>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> arch/arm/mach-omap2/pm24xx.c | 7 +-
> arch/arm/mach-omap2/pm34xx.c | 14 +-
> drivers/gpio/gpio-omap.c | 178 ++++++++++++++----------
> include/linux/platform_data/gpio-omap.h | 13 --
> 4 files changed, 116 insertions(+), 96 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
> --- a/arch/arm/mach-omap2/pm24xx.c
> +++ b/arch/arm/mach-omap2/pm24xx.c
> @@ -18,6 +18,7 @@
> * published by the Free Software Foundation.
> */
>
[...]
>
> +static int __maybe_unused omap_gpio_runtime_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct gpio_bank *bank = platform_get_drvdata(pdev);
> + unsigned long flags;
> + int error = 0;
> +
> + raw_spin_lock_irqsave(&bank->lock, flags);
> + /* Must be idled only by CPU_CLUSTER_PM_ENTER? */
> + if (bank->irq_usage) {
> + error = -EBUSY;
Sry, I didn't get how will it work with suspend to ram?
omap_device will call this handler and, seems, abort suspend always
if gpio bank has gpios requested as IRQ. Am I missing smth?
> + goto unlock;
> + }
> + omap_gpio_idle(bank, true);
> + bank->is_suspended = true;
> +unlock:
> + raw_spin_unlock_irqrestore(&bank->lock, flags);
> +
> + return error;
> +}
> +
> +static int __maybe_unused omap_gpio_runtime_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct gpio_bank *bank = platform_get_drvdata(pdev);
> + unsigned long flags;
> + int error = 0;
> +
> + raw_spin_lock_irqsave(&bank->lock, flags);
> + /* Must be unidled only by CPU_CLUSTER_PM_ENTER? */
> + if (bank->irq_usage) {
> + error = -EBUSY;
> + goto unlock;
> + }
> + omap_gpio_unidle(bank);
> + bank->is_suspended = false;
> +unlock:
> + raw_spin_unlock_irqrestore(&bank->lock, flags);
> +
> + return error;
> +}
> +
> +#ifdef CONFIG_ARCH_OMAP2PLUS
> static const struct dev_pm_ops gpio_pm_ops = {
> SET_RUNTIME_PM_OPS(omap_gpio_runtime_suspend, omap_gpio_runtime_resume,
> NULL)
> };
> +#else
> +static const struct dev_pm_ops gpio_pm_ops;
> +#endif /* CONFIG_ARCH_OMAP2PLUS */
>
> #if defined(CONFIG_OF)
> static struct omap_gpio_reg_offs omap2_gpio_regs = {
> @@ -1774,6 +1805,11 @@ static struct omap_gpio_reg_offs omap4_gpio_regs = {
> .fallingdetect = OMAP4_GPIO_FALLINGDETECT,
> };
>
[...]
--
regards,
-grygorii
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 2/3] gpio: omap: Remove custom PM calls and use cpu_pm instead
@ 2018-09-20 23:01 ` Grygorii Strashko
0 siblings, 0 replies; 22+ messages in thread
From: Grygorii Strashko @ 2018-09-20 23:01 UTC (permalink / raw)
To: linux-arm-kernel
On 09/20/2018 02:35 PM, Tony Lindgren wrote:
> For a long time the gpio-omap custom PM calls have been annoying me so
> let's replace them with cpu_pm instead. This will enable GPIO PM for
> deeper idle states on omap4. And we can handle GPIO PM for omap2/3/4
> in the same way.
>
> Note that with this patch we are also slightly changing GPIO PM to be
> less aggressive for omap3 and only will idle GPIO when PER context
> may be lost.
>
> For omap2, we don't need to save context and don't want to remove any
> triggering so let's add a quirk flag for that.
>
> Let's do this all in a single patch to avoid a situation where old
> custom calls still are used with new code.
>
> Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Ladislav Michl <ladis@linux-mips.org>
> Cc: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> arch/arm/mach-omap2/pm24xx.c | 7 +-
> arch/arm/mach-omap2/pm34xx.c | 14 +-
> drivers/gpio/gpio-omap.c | 178 ++++++++++++++----------
> include/linux/platform_data/gpio-omap.h | 13 --
> 4 files changed, 116 insertions(+), 96 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
> --- a/arch/arm/mach-omap2/pm24xx.c
> +++ b/arch/arm/mach-omap2/pm24xx.c
> @@ -18,6 +18,7 @@
> * published by the Free Software Foundation.
> */
>
[...]
>
> +static int __maybe_unused omap_gpio_runtime_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct gpio_bank *bank = platform_get_drvdata(pdev);
> + unsigned long flags;
> + int error = 0;
> +
> + raw_spin_lock_irqsave(&bank->lock, flags);
> + /* Must be idled only by CPU_CLUSTER_PM_ENTER? */
> + if (bank->irq_usage) {
> + error = -EBUSY;
Sry, I didn't get how will it work with suspend to ram?
omap_device will call this handler and, seems, abort suspend always
if gpio bank has gpios requested as IRQ. Am I missing smth?
> + goto unlock;
> + }
> + omap_gpio_idle(bank, true);
> + bank->is_suspended = true;
> +unlock:
> + raw_spin_unlock_irqrestore(&bank->lock, flags);
> +
> + return error;
> +}
> +
> +static int __maybe_unused omap_gpio_runtime_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct gpio_bank *bank = platform_get_drvdata(pdev);
> + unsigned long flags;
> + int error = 0;
> +
> + raw_spin_lock_irqsave(&bank->lock, flags);
> + /* Must be unidled only by CPU_CLUSTER_PM_ENTER? */
> + if (bank->irq_usage) {
> + error = -EBUSY;
> + goto unlock;
> + }
> + omap_gpio_unidle(bank);
> + bank->is_suspended = false;
> +unlock:
> + raw_spin_unlock_irqrestore(&bank->lock, flags);
> +
> + return error;
> +}
> +
> +#ifdef CONFIG_ARCH_OMAP2PLUS
> static const struct dev_pm_ops gpio_pm_ops = {
> SET_RUNTIME_PM_OPS(omap_gpio_runtime_suspend, omap_gpio_runtime_resume,
> NULL)
> };
> +#else
> +static const struct dev_pm_ops gpio_pm_ops;
> +#endif /* CONFIG_ARCH_OMAP2PLUS */
>
> #if defined(CONFIG_OF)
> static struct omap_gpio_reg_offs omap2_gpio_regs = {
> @@ -1774,6 +1805,11 @@ static struct omap_gpio_reg_offs omap4_gpio_regs = {
> .fallingdetect = OMAP4_GPIO_FALLINGDETECT,
> };
>
[...]
--
regards,
-grygorii
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/3] gpio: omap: Remove custom PM calls and use cpu_pm instead
2018-09-20 23:01 ` Grygorii Strashko
@ 2018-09-20 23:47 ` Tony Lindgren
-1 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-20 23:47 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Alexandre Courbot, Ladislav Michl, Aaro Koskinen, Keerthy,
Linus Walleij, Tero Kristo, linux-gpio, linux-omap,
linux-arm-kernel
* Grygorii Strashko <grygorii.strashko@ti.com> [180920 23:05]:
> On 09/20/2018 02:35 PM, Tony Lindgren wrote:
> > +static int __maybe_unused omap_gpio_runtime_suspend(struct device *dev)
> > +{
> > + struct platform_device *pdev = to_platform_device(dev);
> > + struct gpio_bank *bank = platform_get_drvdata(pdev);
> > + unsigned long flags;
> > + int error = 0;
> > +
> > + raw_spin_lock_irqsave(&bank->lock, flags);
> > + /* Must be idled only by CPU_CLUSTER_PM_ENTER? */
> > + if (bank->irq_usage) {
> > + error = -EBUSY;
>
> Sry, I didn't get how will it work with suspend to ram?
> omap_device will call this handler and, seems, abort suspend always
> if gpio bank has gpios requested as IRQ. Am I missing smth?
Heh yeah we have _od_suspend_noirq() call pm_generic_suspend_noirq()
which will succeed. But note how after that omap_device_idle() gets
called only if pm_generic_runtime_suspend() == 0 and we still allow
supend with no error.
So for gpio interrupts there's nothing that needs to be done at that
point and the cpu_pm notifier will take care of things just before
suspend.
Do you have any better ideas?
Regards,
Tony
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 2/3] gpio: omap: Remove custom PM calls and use cpu_pm instead
@ 2018-09-20 23:47 ` Tony Lindgren
0 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-20 23:47 UTC (permalink / raw)
To: linux-arm-kernel
* Grygorii Strashko <grygorii.strashko@ti.com> [180920 23:05]:
> On 09/20/2018 02:35 PM, Tony Lindgren wrote:
> > +static int __maybe_unused omap_gpio_runtime_suspend(struct device *dev)
> > +{
> > + struct platform_device *pdev = to_platform_device(dev);
> > + struct gpio_bank *bank = platform_get_drvdata(pdev);
> > + unsigned long flags;
> > + int error = 0;
> > +
> > + raw_spin_lock_irqsave(&bank->lock, flags);
> > + /* Must be idled only by CPU_CLUSTER_PM_ENTER? */
> > + if (bank->irq_usage) {
> > + error = -EBUSY;
>
> Sry, I didn't get how will it work with suspend to ram?
> omap_device will call this handler and, seems, abort suspend always
> if gpio bank has gpios requested as IRQ. Am I missing smth?
Heh yeah we have _od_suspend_noirq() call pm_generic_suspend_noirq()
which will succeed. But note how after that omap_device_idle() gets
called only if pm_generic_runtime_suspend() == 0 and we still allow
supend with no error.
So for gpio interrupts there's nothing that needs to be done at that
point and the cpu_pm notifier will take care of things just before
suspend.
Do you have any better ideas?
Regards,
Tony
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/3] gpio: omap: Remove custom PM calls and use cpu_pm instead
2018-09-20 23:47 ` Tony Lindgren
@ 2018-09-21 0:58 ` Grygorii Strashko
-1 siblings, 0 replies; 22+ messages in thread
From: Grygorii Strashko @ 2018-09-21 0:58 UTC (permalink / raw)
To: Tony Lindgren
Cc: Alexandre Courbot, Ladislav Michl, Aaro Koskinen, Keerthy,
Linus Walleij, Tero Kristo, linux-gpio, linux-omap,
linux-arm-kernel
On 09/20/2018 06:47 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [180920 23:05]:
>> On 09/20/2018 02:35 PM, Tony Lindgren wrote:
>>> +static int __maybe_unused omap_gpio_runtime_suspend(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct gpio_bank *bank = platform_get_drvdata(pdev);
>>> + unsigned long flags;
>>> + int error = 0;
>>> +
>>> + raw_spin_lock_irqsave(&bank->lock, flags);
>>> + /* Must be idled only by CPU_CLUSTER_PM_ENTER? */
>>> + if (bank->irq_usage) {
>>> + error = -EBUSY;
>>
>> Sry, I didn't get how will it work with suspend to ram?
>> omap_device will call this handler and, seems, abort suspend always
>> if gpio bank has gpios requested as IRQ. Am I missing smth?
>
> Heh yeah we have _od_suspend_noirq() call pm_generic_suspend_noirq()
> which will succeed. But note how after that omap_device_idle() gets
> called only if pm_generic_runtime_suspend() == 0 and we still allow
> supend with no error.
Ah, sry, You are right - ret not updated in this case
>
> So for gpio interrupts there's nothing that needs to be done at that
> point and the cpu_pm notifier will take care of things just before
> suspend.
So, just to summarize:
- gpio requested (incl irq) - is_suspended = true: no actions
- no gpio irqs requested - is_suspended = false: idle->omap_gpio_idle/unidle
suspend->omap_gpio_runtime_suspend/resume
- gpio irqs requested - is_suspended = false: idle->omap_gpio_idle/unidle
suspend->omap_gpio_idle/unidle
>
> Do you have any better ideas?
No. It's looks good - thanks, but i'd like to try it, so will get back.
--
regards,
-grygorii
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 2/3] gpio: omap: Remove custom PM calls and use cpu_pm instead
@ 2018-09-21 0:58 ` Grygorii Strashko
0 siblings, 0 replies; 22+ messages in thread
From: Grygorii Strashko @ 2018-09-21 0:58 UTC (permalink / raw)
To: linux-arm-kernel
On 09/20/2018 06:47 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [180920 23:05]:
>> On 09/20/2018 02:35 PM, Tony Lindgren wrote:
>>> +static int __maybe_unused omap_gpio_runtime_suspend(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct gpio_bank *bank = platform_get_drvdata(pdev);
>>> + unsigned long flags;
>>> + int error = 0;
>>> +
>>> + raw_spin_lock_irqsave(&bank->lock, flags);
>>> + /* Must be idled only by CPU_CLUSTER_PM_ENTER? */
>>> + if (bank->irq_usage) {
>>> + error = -EBUSY;
>>
>> Sry, I didn't get how will it work with suspend to ram?
>> omap_device will call this handler and, seems, abort suspend always
>> if gpio bank has gpios requested as IRQ. Am I missing smth?
>
> Heh yeah we have _od_suspend_noirq() call pm_generic_suspend_noirq()
> which will succeed. But note how after that omap_device_idle() gets
> called only if pm_generic_runtime_suspend() == 0 and we still allow
> supend with no error.
Ah, sry, You are right - ret not updated in this case
>
> So for gpio interrupts there's nothing that needs to be done at that
> point and the cpu_pm notifier will take care of things just before
> suspend.
So, just to summarize:
- gpio requested (incl irq) - is_suspended = true: no actions
- no gpio irqs requested - is_suspended = false: idle->omap_gpio_idle/unidle
suspend->omap_gpio_runtime_suspend/resume
- gpio irqs requested - is_suspended = false: idle->omap_gpio_idle/unidle
suspend->omap_gpio_idle/unidle
>
> Do you have any better ideas?
No. It's looks good - thanks, but i'd like to try it, so will get back.
--
regards,
-grygorii
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/3] gpio: omap: Get rid of pm_runtime_irq_safe()
2018-09-20 19:35 ` Tony Lindgren
@ 2018-09-20 19:35 ` Tony Lindgren
-1 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-20 19:35 UTC (permalink / raw)
To: Linus Walleij, Alexandre Courbot
Cc: Grygorii Strashko, Aaro Koskinen, Keerthy, Tero Kristo,
linux-gpio, Ladislav Michl, linux-omap, linux-arm-kernel
If a gpio instance has any GPIO bits requested we do a pm_runtime_get()
on the device. Now with cpu_pm handling the deeper SoC idle state quirks,
let's just remove pm_runtime_irq_safe() call and add a warning in case we
ever happen to encounter it.
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/gpio/gpio-omap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -763,7 +763,9 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank)
if (WARN_ON(!isr_reg))
goto exit;
- pm_runtime_get_sync(bank->chip.parent);
+ if (WARN_ONCE(!pm_runtime_active(bank->chip.parent),
+ "gpio irq%i while runtime suspended?\n", irq))
+ return IRQ_NONE;
while (1) {
raw_spin_lock_irqsave(&bank->lock, lock_flags);
@@ -814,7 +816,6 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank)
}
}
exit:
- pm_runtime_put(bank->chip.parent);
return IRQ_HANDLED;
}
@@ -1466,7 +1467,6 @@ static int omap_gpio_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bank);
pm_runtime_enable(dev);
- pm_runtime_irq_safe(dev);
pm_runtime_get_sync(dev);
if (bank->is_mpuio)
--
2.19.0
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 3/3] gpio: omap: Get rid of pm_runtime_irq_safe()
@ 2018-09-20 19:35 ` Tony Lindgren
0 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-20 19:35 UTC (permalink / raw)
To: linux-arm-kernel
If a gpio instance has any GPIO bits requested we do a pm_runtime_get()
on the device. Now with cpu_pm handling the deeper SoC idle state quirks,
let's just remove pm_runtime_irq_safe() call and add a warning in case we
ever happen to encounter it.
Cc: Aaro Koskinen <aaro.koskinen@iki.fi>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/gpio/gpio-omap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -763,7 +763,9 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank)
if (WARN_ON(!isr_reg))
goto exit;
- pm_runtime_get_sync(bank->chip.parent);
+ if (WARN_ONCE(!pm_runtime_active(bank->chip.parent),
+ "gpio irq%i while runtime suspended?\n", irq))
+ return IRQ_NONE;
while (1) {
raw_spin_lock_irqsave(&bank->lock, lock_flags);
@@ -814,7 +816,6 @@ static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank)
}
}
exit:
- pm_runtime_put(bank->chip.parent);
return IRQ_HANDLED;
}
@@ -1466,7 +1467,6 @@ static int omap_gpio_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bank);
pm_runtime_enable(dev);
- pm_runtime_irq_safe(dev);
pm_runtime_get_sync(dev);
if (bank->is_mpuio)
--
2.19.0
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCHv2 0/3] omap gpio add level idle, cpu_pm and drop runtime_irq_safe
2018-09-20 19:35 ` Tony Lindgren
@ 2018-09-22 3:12 ` Grygorii Strashko
-1 siblings, 0 replies; 22+ messages in thread
From: Grygorii Strashko @ 2018-09-22 3:12 UTC (permalink / raw)
To: Tony Lindgren, Linus Walleij, Alexandre Courbot
Cc: linux-gpio, linux-omap, linux-arm-kernel
On 09/20/2018 02:35 PM, Tony Lindgren wrote:
> Hi,
>
> Here's v2 of the patches to add support for omap4 and later level interrupt
> idle support, get rid of the custom pm calls, and finally after these we can
> drop pm_runtime_irq_safe().
>
> Linus, assuming people are happy with these, I'd appreciate an immutable branch
> with these in case we run into merge conflicts for arch/arm/mach-omap2 changes.
>
> Regards,
>
> Tony
>
>
> Changes since v1:
> - Fix issue noted in v1 set and update comments accordingly
>
> - Don't call pm_runtime from cpu_pm to avoid further dependencies to
> pm_runtime_irq_safe() as discussed offline with Grygorii
>
> - Add a patch to drop pm_runtime_irq_safe() as it's no longer needed
>
> Tony Lindgren (3):
> gpio: omap: Add level wakeup handling for omap4 based SoCs
> gpio: omap: Remove custom PM calls and use cpu_pm instead
> gpio: omap: Get rid of pm_runtime_irq_safe()
>
> arch/arm/mach-omap2/pm24xx.c | 7 +-
> arch/arm/mach-omap2/pm34xx.c | 14 +-
> drivers/gpio/gpio-omap.c | 331 ++++++++++++++++--------
> include/linux/platform_data/gpio-omap.h | 15 +-
> 4 files changed, 237 insertions(+), 130 deletions(-)
>
Thank you
Acked-by: Grygorii Strashko <grygorii.strashko@ti.com>
--
regards,
-grygorii
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCHv2 0/3] omap gpio add level idle, cpu_pm and drop runtime_irq_safe
@ 2018-09-22 3:12 ` Grygorii Strashko
0 siblings, 0 replies; 22+ messages in thread
From: Grygorii Strashko @ 2018-09-22 3:12 UTC (permalink / raw)
To: linux-arm-kernel
On 09/20/2018 02:35 PM, Tony Lindgren wrote:
> Hi,
>
> Here's v2 of the patches to add support for omap4 and later level interrupt
> idle support, get rid of the custom pm calls, and finally after these we can
> drop pm_runtime_irq_safe().
>
> Linus, assuming people are happy with these, I'd appreciate an immutable branch
> with these in case we run into merge conflicts for arch/arm/mach-omap2 changes.
>
> Regards,
>
> Tony
>
>
> Changes since v1:
> - Fix issue noted in v1 set and update comments accordingly
>
> - Don't call pm_runtime from cpu_pm to avoid further dependencies to
> pm_runtime_irq_safe() as discussed offline with Grygorii
>
> - Add a patch to drop pm_runtime_irq_safe() as it's no longer needed
>
> Tony Lindgren (3):
> gpio: omap: Add level wakeup handling for omap4 based SoCs
> gpio: omap: Remove custom PM calls and use cpu_pm instead
> gpio: omap: Get rid of pm_runtime_irq_safe()
>
> arch/arm/mach-omap2/pm24xx.c | 7 +-
> arch/arm/mach-omap2/pm34xx.c | 14 +-
> drivers/gpio/gpio-omap.c | 331 ++++++++++++++++--------
> include/linux/platform_data/gpio-omap.h | 15 +-
> 4 files changed, 237 insertions(+), 130 deletions(-)
>
Thank you
Acked-by: Grygorii Strashko <grygorii.strashko@ti.com>
--
regards,
-grygorii
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCHv2 0/3] omap gpio add level idle, cpu_pm and drop runtime_irq_safe
2018-09-20 19:35 ` Tony Lindgren
@ 2018-09-25 6:53 ` Linus Walleij
-1 siblings, 0 replies; 22+ messages in thread
From: Linus Walleij @ 2018-09-25 6:53 UTC (permalink / raw)
To: ext Tony Lindgren
Cc: Alexandre Courbot, Linux-OMAP, Linux ARM, open list:GPIO SUBSYSTEM
On Thu, Sep 20, 2018 at 9:35 PM Tony Lindgren <tony@atomide.com> wrote:
> Linus, assuming people are happy with these, I'd appreciate an immutable branch
> with these in case we run into merge conflicts for arch/arm/mach-omap2 changes.
I created an immutable branch named "ib-omap" yesterday:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-omap
It passed build tests this morning so I merged this into my "devel" branch
for next.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCHv2 0/3] omap gpio add level idle, cpu_pm and drop runtime_irq_safe
@ 2018-09-25 6:53 ` Linus Walleij
0 siblings, 0 replies; 22+ messages in thread
From: Linus Walleij @ 2018-09-25 6:53 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 20, 2018 at 9:35 PM Tony Lindgren <tony@atomide.com> wrote:
> Linus, assuming people are happy with these, I'd appreciate an immutable branch
> with these in case we run into merge conflicts for arch/arm/mach-omap2 changes.
I created an immutable branch named "ib-omap" yesterday:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-omap
It passed build tests this morning so I merged this into my "devel" branch
for next.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCHv2 0/3] omap gpio add level idle, cpu_pm and drop runtime_irq_safe
2018-09-25 6:53 ` Linus Walleij
@ 2018-09-25 14:35 ` Tony Lindgren
-1 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2018-09-25 14:35 UTC (permalink / raw)
To: Linus Walleij
Cc: Alexandre Courbot, Linux-OMAP, Linux ARM, open list:GPIO SUBSYSTEM
* Linus Walleij <linus.walleij@linaro.org> [180925 06:57]:
> On Thu, Sep 20, 2018 at 9:35 PM Tony Lindgren <tony@atomide.com> wrote:
>
> > Linus, assuming people are happy with these, I'd appreciate an immutable branch
> > with these in case we run into merge conflicts for arch/arm/mach-omap2 changes.
>
> I created an immutable branch named "ib-omap" yesterday:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-omap
>
> It passed build tests this morning so I merged this into my "devel" branch
> for next.
OK thanks!
Tony
^ permalink raw reply [flat|nested] 22+ messages in thread