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@xxxxxx>
> Cc: Aaro Koskinen <aaro.koskinen@xxxxxx>
> Cc: Grygorii Strashko <grygorii.strashko@xxxxxx>
> Cc: Keerthy <j-keerthy@xxxxxx>
> Cc: Ladislav Michl <ladis@xxxxxxxxxxxxxx>
> Cc: Tero Kristo <t-kristo@xxxxxx>
> Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx>
> ---
> 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 */
>
