Re: [PATCH v6 2/6] can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable()

[Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>]

On 8/24/2026 11:57 AM, Bough Chen wrote:
> On Mon, Jul 13, 2026 at 09:07:04AM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] On multi-IRQ platforms, Wake-on-CAN is broken because `flexcan_suspend` only arms the primary IRQ (`dev->irq`) for wakeup, completely ignoring the state/BOFF interrupt (`priv->irq_boff`) which actually handles the wakeup event.
>> - [High] In `flexcan_resume`, the network queue is restarted (`netif_device_attach` and `netif_start_queue`) before the hardware is initialized (`flexcan_chip_start`). This creates a race condition where a packet can be queued for transmission just before the hardware undergoes a soft reset, leading to a permanent TX stall.
>> --
>>
>> --- Patch [2]: [PATCH v6 2/6] can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable() ---
>> commit ca7ea75d45df1ecf13f11dcec0bd19377eccf3c6
>> Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>
>> can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable()
>>
>> flexcan_chip_interrupts_enable() disables only the primary IRQ line while
>> writing to the IMASK and CTRL registers. On multi-IRQ platforms, the
>> additional IRQ lines remain active so their handlers can fire while registers
>> are inconsistent. This commit disables all registered IRQ lines around the
>> IMASK/CTRL writes.
>>
>>> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
>>> index f73ff442d5304..7dde2e623defe 100644
>>> --- a/drivers/net/can/flexcan/flexcan-core.c
>>> +++ b/drivers/net/can/flexcan/flexcan-core.c
>>> @@ -1519,14 +1519,28 @@ static void flexcan_chip_interrupts_enable(const struct net_device *dev)
>>>   {
>>>   	const struct flexcan_priv *priv = netdev_priv(dev);
>>>   	struct flexcan_regs __iomem *regs = priv->regs;
>>> +	u32 quirks = priv->devtype_data.quirks;
>>>   	u64 reg_imask;
>>>   
>>>   	disable_irq(dev->irq);
>>> +	if (quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
>>> +		disable_irq(priv->irq_boff);
>>
>> [Severity: High]
>> This is a pre-existing issue, but on multi-IRQ platforms, is Wake-on-CAN
>> broken because we only arm the primary IRQ during suspend?
>>
>> In drivers/net/can/flexcan/flexcan-core.c:flexcan_suspend(), we see:
>>
>> 	if (device_may_wakeup(device)) {
>> 		enable_irq_wake(dev->irq);
>> 		err = flexcan_enter_stop_mode(priv);
>>
>> Since the hardware asserts irq_boff (which handles FLEXCAN_ESR_WAK_INT) for
>> wakeup events, doesn't irq_boff need to be armed with enable_irq_wake() as
>> well? Otherwise, the system might fail to wake from suspend since the
>> interrupt controller won't treat the asserted IRQ line as a wakeup source.
> 
> Yes, this need to be addressed.
> 
>>
>> [Severity: High]
>> This is also a pre-existing issue, but does flexcan_resume() restart the
>> network queue prematurely?
>>
>> In drivers/net/can/flexcan/flexcan-core.c:flexcan_resume(), the queue is
>> restarted before hardware initialization:
>>
>> 	if (netif_running(dev)) {
>> 		netif_device_attach(dev);
>> 		netif_start_queue(dev);
>> 		if (device_may_wakeup(device)) {
>> 			...
>> 		} else {
>> 			...
>> 			err = flexcan_chip_start(dev);
>>
>> If netif_start_queue() is called before flexcan_chip_start() reinitializes
>> the hardware, could the networking stack concurrently call ndo_start_xmit
>> (flexcan_start_xmit) and queue a packet?
>>
>> If so, would the subsequent flexcan_chip_start() soft reset wipe out the
>> freshly populated message buffers, aborting the transmission and leading to a
>> permanent TX stall since the completion interrupt will never fire?
> 
> Good catch, those two findings should be addressed in another patch set.
> 
> Regards
> Haibo Chen

Agree. Since these two findings are pre-existing I will target them in a 
new patchset separate from S32N79 Flexcan enablement.

Best Regards,
Ciprian

>>
>> [ ... ]
>>
>> -- 
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260713085306.2643794-1-ciprianmarian.costea@oss.nxp.com?part=2