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

[Marc Kleine-Budde]

On 18.03.2026 10:22:12, Ciprian Costea wrote:
> From: Ciprian Marian Costea <ciprianmarian.costea@xxxxxxxxxxx>
>
> 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.

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

> 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@xxxxxxx>
> Signed-off-by: Larisa Grigore <larisa.grigore@xxxxxxx>
> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@xxxxxxxxxxx>
> ---
>  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()

> +		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.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachment: signature.asc
Description: PGP signature