Re: [PATCH 2/5] can: flexcan: add FLEXCAN_QUIRK_NR_IRQ_2 quirk for two interrupt lines

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

On 3/18/2026 12:42 PM, Marc Kleine-Budde wrote:
> On 18.03.2026 10:22:12, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>
>> Introduce FLEXCAN_QUIRK_NR_IRQ_2 quirk to handle hardware integration
> 
> I understand, you followed the pattern and introduced
> FLEXCAN_QUIRK_NR_IRQ_2.
> 
> I think it would be better to describe the actual reason why this IP
> integration uses 2 IRQs: it has a dedicated combined bus error and state
> change IRQ. What about: FLEXCAN_QUIRK_IRQ_BERR, this would match the
> interrupt name from the bindings.
> 

Good suggestion, I will rename the quirk in V2.

> In a separate patch, we could migrate the s32g compatible SoCs to the
> new quirk. But that's not subject to this patch series.
>

True. There is also 'mcf5441x' SoC which uses 'FLEXCAN_QUIRK_NR_IRQ_3' 
quirk and needs to be considered, besides s32g.

>> where the FlexCAN module has two separate interrupt lines:
>> - one for signaling error and bus state changes
> 
> Usually it's "bus error" and "state changes", as the errors happen
> visible for everyone on the bus, while the state change is local to the
> controller (every controller has it's own state).
> 
>> - one for mailboxes 0-127
>>
>> This is required for NXP S32N79 SoC support.
>>
>> Co-developed-by: Larisa Grigore <larisa.grigore@nxp.com>
>> Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>> ---
>>   drivers/net/can/flexcan/flexcan-core.c | 21 ++++++++++++++++++---
>>   drivers/net/can/flexcan/flexcan.h      | 16 +++++++++-------
>>   2 files changed, 27 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
>> index f5d22c61503f..a6b15496853c 100644
>> --- a/drivers/net/can/flexcan/flexcan-core.c
>> +++ b/drivers/net/can/flexcan/flexcan-core.c
>> @@ -1770,7 +1770,10 @@ static int flexcan_open(struct net_device *dev)
>>   				  flexcan_irq, IRQF_SHARED, dev->name, dev);
>>   		if (err)
>>   			goto out_free_irq;
>> +	}
>>
>> +	if (priv->devtype_data.quirks &
>> +			(FLEXCAN_QUIRK_NR_IRQ_2 | FLEXCAN_QUIRK_NR_IRQ_3)) {
>>   		err = request_irq(priv->irq_err,
>>   				  flexcan_irq, IRQF_SHARED, dev->name, dev);
>>   		if (err)
>> @@ -1791,7 +1794,8 @@ static int flexcan_open(struct net_device *dev)
>>   	return 0;
>>
>>    out_free_irq_err:
>> -	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3)
>> +	if (priv->devtype_data.quirks &
>> +			(FLEXCAN_QUIRK_NR_IRQ_2 | FLEXCAN_QUIRK_NR_IRQ_3))
>>   		free_irq(priv->irq_err, dev);
>>    out_free_irq_boff:
>>   	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3)
>> @@ -1823,10 +1827,12 @@ static int flexcan_close(struct net_device *dev)
>>   	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
>>   		free_irq(priv->irq_secondary_mb, dev);
>>
>> -	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
>> +	if (priv->devtype_data.quirks &
>> +			(FLEXCAN_QUIRK_NR_IRQ_2 | FLEXCAN_QUIRK_NR_IRQ_3))
>>   		free_irq(priv->irq_err, dev);
>> +
>> +	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3)
>>   		free_irq(priv->irq_boff, dev);
>> -	}
>>
>>   	free_irq(dev->irq, dev);
>>   	can_rx_offload_disable(&priv->offload);
>> @@ -2213,12 +2219,21 @@ static int flexcan_probe(struct platform_device *pdev)
>>   	if (transceiver)
>>   		priv->can.bitrate_max = transceiver->attrs.max_link_rate;
>>
>> +	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_2) {
>> +		priv->irq_err = platform_get_irq(pdev, 1);
> 
> Please use platform_get_irq_byname()
> 

Makes sense, I will update in V2.

>> +		if (priv->irq_err < 0) {
>> +			err = priv->irq_err;
>> +			goto failed_platform_get_irq;
>> +		}
>> +	}
>> +
>>   	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
>>   		priv->irq_boff = platform_get_irq(pdev, 1);
>>   		if (priv->irq_boff < 0) {
>>   			err = priv->irq_boff;
>>   			goto failed_platform_get_irq;
>>   		}
>> +
>>   		priv->irq_err = platform_get_irq(pdev, 2);
>>   		if (priv->irq_err < 0) {
>>   			err = priv->irq_err;
>> diff --git a/drivers/net/can/flexcan/flexcan.h b/drivers/net/can/flexcan/flexcan.h
>> index 16692a2502eb..f05036ca54f5 100644
>> --- a/drivers/net/can/flexcan/flexcan.h
>> +++ b/drivers/net/can/flexcan/flexcan.h
>> @@ -58,22 +58,24 @@
>>   #define FLEXCAN_QUIRK_SUPPORT_ECC BIT(10)
>>   /* Setup stop mode with SCU firmware to support wakeup */
>>   #define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW BIT(11)
>> +/* Setup 2 separate interrupts, main and err */
>> +#define FLEXCAN_QUIRK_NR_IRQ_2 BIT(12)
>>   /* Setup 3 separate interrupts, main, boff and err */
>> -#define FLEXCAN_QUIRK_NR_IRQ_3 BIT(12)
>> +#define FLEXCAN_QUIRK_NR_IRQ_3 BIT(13)
>>   /* Setup 16 mailboxes */
>> -#define FLEXCAN_QUIRK_NR_MB_16 BIT(13)
>> +#define FLEXCAN_QUIRK_NR_MB_16 BIT(14)
>>   /* Device supports RX via mailboxes */
>> -#define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX BIT(14)
>> +#define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX BIT(15)
>>   /* Device supports RTR reception via mailboxes */
>> -#define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR BIT(15)
>> +#define FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR BIT(16)
>>   /* Device supports RX via FIFO */
>> -#define FLEXCAN_QUIRK_SUPPORT_RX_FIFO BIT(16)
>> +#define FLEXCAN_QUIRK_SUPPORT_RX_FIFO BIT(17)
>>   /* Setup stop mode with ATF SCMI protocol to support wakeup */
>> -#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI BIT(17)
>> +#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI BIT(18)
>>   /* Device has two separate interrupt lines for two mailbox ranges, which
>>    * both need to have an interrupt handler registered.
>>    */
>> -#define FLEXCAN_QUIRK_SECONDARY_MB_IRQ	BIT(18)
>> +#define FLEXCAN_QUIRK_SECONDARY_MB_IRQ	BIT(19)
> 
> As I want to change the quirks anyways, just add new quirk at the end.
> 

Sure. They do need some sort of ordering.
I will add the new quirk at the end in V2.

Best Regards,
Ciprian

> regards,
> Marc
>