Re: [PATCH v4 01/10] gpio: mxc: fix race between chained IRQ handler install and probe completion

[Peng Fan <peng.fan@oss.nxp.com>]

Hi Bartosz,
On Thu, Oct 08, 2026 at 09:44:40AM +0000, Bartosz Golaszewski wrote:
>On Wed, 7 Oct 2026 12:57:13 +0200, sashiko-bot@kernel.org said:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] gpio: mxc: data race on mxc_gpio_ports list during concurrent probe and shared IRQ
>> --
>>
>> commit bd719d33a26c1f816073bbdb04f543bc3f5ed730
>> Author: Peng Fan <peng.fan@nxp.com>
>>
>> gpio: mxc: fix race between chained IRQ handler install and probe completion
>>
>> mxc_update_irq_chained_handler() is called before the IRQ domain, the
>> generic IRQ chip, and the port list entry are set up. If an interrupt
>> arrives in that window, it leads to a crash or missing port. This commit
>> moves the handler installation after all dependencies are ready.
>>
>>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>>> index 7e2690d92df6f..e05f276a50e8d 100644
>>> --- a/drivers/gpio/gpio-mxc.c
>>> +++ b/drivers/gpio/gpio-mxc.c
>>
>> [ ... ]
>>
>>> @@ -525,6 +523,8 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>>
>>>  	list_add_tail(&port->node, &mxc_gpio_ports);
>>>
>>
>> [Severity: High]
>> For fsl,imx21-gpio devices, all ports share a single parent IRQ. After the
>> first port probes, the shared chained handler (mx2_gpio_irq_handler)
>> becomes active.
>>
>> When a subsequent port is probed, list_add_tail() mutates the global
>> mxc_gpio_ports list without any spinlocks or RCU primitives.
>>
>
>I think we should switch to list_add_tail_rcu() and list_for_each_entry_rcu()
>first across the driver.

The sashiko report is false alarm per my understanding. imx21 only has
one arm32 uniprocessor. Another CPU does not exist.

Even the single-core IRQ preemption case is safe.
The only real concern on uniprocessor is a hardirq preempting list_add_tail()
mid-update. But list_add_tail() in the kernel uses
WRITE_ONCE(prev->next, new) as the final store, with a compiler barrier
ensuring new->next and new->prev are written first:
// __list_add():
next->prev = new;           // (1) backward link
new->next = next;           // (2) new's forward link → head
new->prev = prev;           // (3) new's backward link → old tail
WRITE_ONCE(prev->next, new); // (4) linearization point — makes new visible
mx2_gpio_irq_handler does forward-only traversal
(list_for_each_entry follows ->next). If the IRQ fires before step 4, the new
entry isn't visible - traversal sees the old list. If it fires after step 4,
new->next already points to head, so traversal terminates correctly.
Both cases are safe.

Thanks,
Peng

>
>Bart
>