| Message ID | 1416437493-25588-2-git-send-email-dianders@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 19, 2014 at 11:51 PM, Doug Anderson <dianders@chromium.org> wrote: > The rockchip pinctrl driver was using irq_gc_set_wake() as its > implementation of irq_set_wake() but was totally ignoring everything > that irq_gc_set_wake() did (which is to upkeep gc->wake_active). > > Let's fix that by setting gc->wake_active as GPIO_INTEN at suspend > time and restoring GPIO_INTEN at resume time. > > NOTE a few quirks when thinking about this patch: > - Rockchip pinctrl hardware supports both "disable/enable" and > "mask/unmask". Right now we only use "disable/enable" and present > those to Linux as "mask/unmask". This should be OK because > enable/disable is optional and Linux will implement it in terms of > mask/unmask. At the moment we always tell hardware all interrupts > are unmasked (the boot default). > - At suspend time Linux tries to call "disable" on all interrupts and > also enables wakeup on all wakeup interrupts. One would think that > since "disable" is implemented as "mask" when "disable" isn't > provided and that since we were ignoring gc->wake_active that > nothing would have woken us up. That's not the case since Linux > "optimizes" things and just leaves interrutps unmasked, assuming it > could mask them later when they go off. That meant that at suspend > time all interrupts were actually being left enabled. > > With this patch random non-wakeup interrupts no longer wake the system > up. Wakeup interrupts still wake the system up. > > Signed-off-by: Doug Anderson <dianders@chromium.org> > Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Reviewed-by: Heiko Stuebner <heiko@sntech.de> > --- > Changes in v2: None Patch applied. Yours, Linus Walleij
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c index ba74f0a..e91e845 100644 --- a/drivers/pinctrl/pinctrl-rockchip.c +++ b/drivers/pinctrl/pinctrl-rockchip.c @@ -89,6 +89,7 @@ struct rockchip_iomux { * @reg_pull: optional separate register for additional pull settings * @clk: clock of the gpio bank * @irq: interrupt of the gpio bank + * @saved_enables: Saved content of GPIO_INTEN at suspend time. * @pin_base: first pin number * @nr_pins: number of pins in this bank * @name: name of the bank @@ -107,6 +108,7 @@ struct rockchip_pin_bank { struct regmap *regmap_pull; struct clk *clk; int irq; + u32 saved_enables; u32 pin_base; u8 nr_pins; char *name; @@ -1543,6 +1545,23 @@ static int rockchip_irq_set_type(struct irq_data *d, unsigned int type) return 0; } +static void rockchip_irq_suspend(struct irq_data *d) +{ + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); + struct rockchip_pin_bank *bank = gc->private; + + bank->saved_enables = irq_reg_readl(gc, GPIO_INTEN); + irq_reg_writel(gc, gc->wake_active, GPIO_INTEN); +} + +static void rockchip_irq_resume(struct irq_data *d) +{ + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); + struct rockchip_pin_bank *bank = gc->private; + + irq_reg_writel(gc, bank->saved_enables, GPIO_INTEN); +} + static int rockchip_interrupts_register(struct platform_device *pdev, struct rockchip_pinctrl *info) { @@ -1587,6 +1606,8 @@ static int rockchip_interrupts_register(struct platform_device *pdev, gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit; gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit; gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake; + gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend; + gc->chip_types[0].chip.irq_resume = rockchip_irq_resume; gc->chip_types[0].chip.irq_set_type = rockchip_irq_set_type; gc->wake_enabled = IRQ_MSK(bank->nr_pins);