| Message ID | 20220724171057.18549-2-marex@denx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/2] gpio: mxc: Protect GPIO irqchip RMW with bgpio spinlock | expand |
On Sun, Jul 24, 2022 at 7:21 PM Marek Vasut <marex@denx.de> wrote: > > Always configure GPIO pins which are used as interrupt source as INPUTs. > In case the default pin configuration is OUTPUT, or the prior stage does > configure the pins as OUTPUT, then Linux will not reconfigure the pin as > INPUT and no interrupts are received. > > Always configure interrupt source GPIO pin as input to fix the above case. the interrupt ... I'm wondering if it's configured as output (by who?) shouldn't you issue a warning?
On 7/25/22 22:36, Andy Shevchenko wrote: > On Sun, Jul 24, 2022 at 7:21 PM Marek Vasut <marex@denx.de> wrote: >> >> Always configure GPIO pins which are used as interrupt source as INPUTs. >> In case the default pin configuration is OUTPUT, or the prior stage does >> configure the pins as OUTPUT, then Linux will not reconfigure the pin as >> INPUT and no interrupts are received. >> >> Always configure interrupt source GPIO pin as input to fix the above case. > > the interrupt > > ... > > I'm wondering if it's configured as output (by who?) shouldn't you > issue a warning? Probably not, I can think of valid use-case where the pin can be configured as output by the prior stage e.g. to operate an LED, but then reconfigured as input by Linux to e.g. sample a button. There is hardware which shares button and LED on the same GPIO line to my knowledge.
On Mon, Jul 25, 2022 at 10:36 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Sun, Jul 24, 2022 at 7:21 PM Marek Vasut <marex@denx.de> wrote: > > > > Always configure GPIO pins which are used as interrupt source as INPUTs. > > In case the default pin configuration is OUTPUT, or the prior stage does > > configure the pins as OUTPUT, then Linux will not reconfigure the pin as > > INPUT and no interrupts are received. > > > > Always configure interrupt source GPIO pin as input to fix the above case. > > the interrupt > > ... > > I'm wondering if it's configured as output (by who?) shouldn't you > issue a warning? That could be the power-on default or boot loader that put it into output mode I think, so it can be something outside of the kernel and outside of our control. We just adjust the reality to the map and go on :P Yours, Linus Walleij
diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c index 74a50139c9202..945e8a2efb896 100644 --- a/drivers/gpio/gpio-mxc.c +++ b/drivers/gpio/gpio-mxc.c @@ -148,7 +148,7 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mxc_gpio_port *port = gc->private; unsigned long flags; - u32 bit, val; + u32 bit, val, dir; u32 gpio_idx = d->hwirq; int edge; void __iomem *reg = port->base; @@ -207,6 +207,10 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type) writel(1 << gpio_idx, port->base + GPIO_ISR); + dir = readl(port->base + GPIO_GDIR); + dir &= ~BIT(gpio_idx); + writel(dir, port->base + GPIO_GDIR); + spin_unlock_irqrestore(&port->gc.bgpio_lock, flags); return 0;
Always configure GPIO pins which are used as interrupt source as INPUTs. In case the default pin configuration is OUTPUT, or the prior stage does configure the pins as OUTPUT, then Linux will not reconfigure the pin as INPUT and no interrupts are received. Always configure interrupt source GPIO pin as input to fix the above case. Fixes: 07bd1a6cc7cbb ("MXC arch: Add gpio support for the whole platform") Signed-off-by: Marek Vasut <marex@denx.de> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Loic Poulain <loic.poulain@linaro.org> Cc: Marc Zyngier <maz@kernel.org> Cc: NXP Linux Team <linux-imx@nxp.com> Cc: Peng Fan <peng.fan@nxp.com> Cc: Shawn Guo <shawnguo@kernel.org> --- V2: Actually update and clear bits in GDIR register V3: Rebase on top of new patch 1/2, expand CC list, add Fixes tag --- drivers/gpio/gpio-mxc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)