From 68bda47c57c9d671820672badc1cb62211ec4700 Mon Sep 17 00:00:00 2001 From: Doug Anderson Date: Wed, 19 Nov 2014 14:51:32 -0800 Subject: pinctrl: rockchip: Handle wakeup pins 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 Reviewed-by: Dmitry Torokhov Reviewed-by: Heiko Stuebner Signed-off-by: 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); -- cgit v0.10.2 From f2dd028c2632d107c26b1daed543d9efd4f0decd Mon Sep 17 00:00:00 2001 From: Doug Anderson Date: Wed, 19 Nov 2014 14:51:33 -0800 Subject: pinctrl: rockchip: Fix enable/disable/mask/unmask The Rockchip pinctrl driver was only implementing the "mask" and "unmask" operations though the hardware actually has two distinct things: enable/disable and mask/unmask. It was implementing the "mask" operations as a hardware enable/disable and always leaving all interrupts unmasked. I believe that the old system had some downsides, specifically: - (Untested) if an interrupt went off while interrupts were "masked" it would be lost. Now it will be kept track of. - If someone wanted to change an interrupt back into a GPIO (is such a thing sensible?) by calling irq_disable() it wouldn't actually take effect. That's because Linux does some extra optimizations when there's no true "disable" function: it does a lazy mask. Let's actually implement enable/disable/mask/unmask properly. Signed-off-by: Doug Anderson Reviewed-by: Dmitry Torokhov Reviewed-by: Heiko Stuebner Signed-off-by: Linus Walleij diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c index e91e845..3c22dbe 100644 --- a/drivers/pinctrl/pinctrl-rockchip.c +++ b/drivers/pinctrl/pinctrl-rockchip.c @@ -1562,6 +1562,34 @@ static void rockchip_irq_resume(struct irq_data *d) irq_reg_writel(gc, bank->saved_enables, GPIO_INTEN); } +static void rockchip_irq_disable(struct irq_data *d) +{ + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); + u32 val; + + irq_gc_lock(gc); + + val = irq_reg_readl(gc, GPIO_INTEN); + val &= ~d->mask; + irq_reg_writel(gc, val, GPIO_INTEN); + + irq_gc_unlock(gc); +} + +static void rockchip_irq_enable(struct irq_data *d) +{ + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); + u32 val; + + irq_gc_lock(gc); + + val = irq_reg_readl(gc, GPIO_INTEN); + val |= d->mask; + irq_reg_writel(gc, val, GPIO_INTEN); + + irq_gc_unlock(gc); +} + static int rockchip_interrupts_register(struct platform_device *pdev, struct rockchip_pinctrl *info) { @@ -1600,11 +1628,13 @@ static int rockchip_interrupts_register(struct platform_device *pdev, gc = irq_get_domain_generic_chip(bank->domain, 0); gc->reg_base = bank->reg_base; gc->private = bank; - gc->chip_types[0].regs.mask = GPIO_INTEN; + gc->chip_types[0].regs.mask = GPIO_INTMASK; gc->chip_types[0].regs.ack = GPIO_PORTS_EOI; gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit; - 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_mask = irq_gc_mask_set_bit; + gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit; + gc->chip_types[0].chip.irq_enable = rockchip_irq_enable; + gc->chip_types[0].chip.irq_disable = rockchip_irq_disable; 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; -- cgit v0.10.2 From 96d16c3088a924a44175d2923cc0eafa0d8bbecc Mon Sep 17 00:00:00 2001 From: Francesco VIRLINZI Date: Mon, 5 Jan 2015 11:04:13 +0100 Subject: pinctrl: st: avoid multiple mutex lock Using the sysfs inteface to inspect the pins configuration the system can walk around a path which acquires the same mutex twice. On STiH407 platform, for example : cat /sys/kernel/debug/pinctrl/920f080.pin-controller-front0/pinconf-pins hangs the kernel and never returns. With this patch the mutex is temporary freed. Signed-off-by: Francesco Virlinzi Signed-off-by: Patrice Chotard Signed-off-by: Linus Walleij diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c index 7c9d513..87570e6 100644 --- a/drivers/pinctrl/pinctrl-st.c +++ b/drivers/pinctrl/pinctrl-st.c @@ -1012,8 +1012,10 @@ static void st_pinconf_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s, unsigned pin_id) { unsigned long config; - st_pinconf_get(pctldev, pin_id, &config); + mutex_unlock(&pctldev->mutex); + st_pinconf_get(pctldev, pin_id, &config); + mutex_lock(&pctldev->mutex); seq_printf(s, "[OE:%ld,PU:%ld,OD:%ld]\n" "\t\t[retime:%ld,invclk:%ld,clknotdat:%ld," "de:%ld,rt-clk:%ld,rt-delay:%ld]", -- cgit v0.10.2 From fce7fcc785998d91a4e2293ec6c0fa7a4677999b Mon Sep 17 00:00:00 2001 From: Patrice CHOTARD Date: Mon, 5 Jan 2015 11:04:14 +0100 Subject: pinctrl: st: Add irq_disable hook to st_gpio_irqchip Currently disable_irq() doesn't work for pinctrl-st driver, due to missing irq_disable hook in the driver. disable_irq() is required only for level-triggered interrupts, which is not the case normally. Signed-off-by: Pankaj Dev Signed-off-by: Patrice Chotard Signed-off-by: Linus Walleij diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c index 87570e6..9e5ec00 100644 --- a/drivers/pinctrl/pinctrl-st.c +++ b/drivers/pinctrl/pinctrl-st.c @@ -1445,6 +1445,7 @@ static struct gpio_chip st_gpio_template = { static struct irq_chip st_gpio_irqchip = { .name = "GPIO", + .irq_disable = st_gpio_irq_mask, .irq_mask = st_gpio_irq_mask, .irq_unmask = st_gpio_irq_unmask, .irq_set_type = st_gpio_irq_set_type, -- cgit v0.10.2