On 7/24/22 19:50, Marc Zyngier wrote: [...]
+++ b/drivers/gpio/gpio-mxc.c @@ -147,6 +147,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 gpio_idx = d->hwirq; int edge; @@ -185,6 +186,8 @@ static int gpio_set_irq_type(struct irq_data *d, u32 type) return -EINVAL; }+ spin_lock_irqsave(&port->gc.bgpio_lock, flags);In my tree, bgpio is a raw spinlock, and has been since 3c938cc5cebcb. Now, looking a bit closer at this code, I have to withdraw my earlier comment about the lack of mutual exclusion in the existing code. All writes are of the form: writel(single_bit_mask, some_addr + MXS_{SET,CLR}); which indicates that the write side can be accessed with a hot-bit pattern, avoiding a RWM pattern and thus the need for a lock.
Only for the ISR/IMR, not for the GDIR register, that's why the locks are added only around the RMW which don't have these "hot bits".
Your second patch, however requires the lock. I'm not sure it is safe to do after the interrupt type has been configured though. You may want to refer to the TRM for this.
There is in fact another unprotected RMW in gpio_set_irq_type() , look for GPIO_EDGE_SEL, so the locks should be valid as they are now, right ?
