Re: [PATCH v5 5/8] can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk

[Vincent Mailhol <mailhol@kernel.org>]

On 09/06/2026 at 16:29, Ciprian Costea wrote:
> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> 
> Introduce FLEXCAN_QUIRK_IRQ_BERR quirk to handle hardware integration
> where the FlexCAN module has a dedicated interrupt line for signaling
> bus errors and device state changes.
> 
> This adds the flexcan_irq_esr() handler which composes
> flexcan_do_state() and flexcan_do_berr() to handle platforms where
> these events share a single IRQ line.
> 
> Also extend flexcan_chip_interrupts_enable() to disable/enable the
> new IRQ line during IMASK register writes.
> 
> 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>
> Reviewed-and-tested-by: Haibo Chen <haibo.chen@nxp.com>
> Tested-by: Enric Balletbo i Serra <eballetb@redhat.com>

Reviewed-by: Vincent Mailhol <mailhol@kernel.org>

> ---
>  drivers/net/can/flexcan/flexcan-core.c | 54 +++++++++++++++++++++-----
>  drivers/net/can/flexcan/flexcan.h      |  2 +
>  2 files changed, 47 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 0ed838f0719a..adf3af57fb0a 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -1300,6 +1300,22 @@ static irqreturn_t flexcan_irq_boff(int irq, void *dev_id)
>  	return handled;
>  }
>  
> +/* Combined bus error and state change IRQ handler */
> +static irqreturn_t flexcan_irq_esr(int irq, void *dev_id)
> +{
> +	struct net_device *dev = dev_id;
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +	irqreturn_t handled;
> +
> +	handled = flexcan_do_state(dev);
> +	handled |= flexcan_do_berr(dev);
> +
> +	if (handled)
> +		can_rx_offload_irq_finish(&priv->offload);
> +
> +	return handled;
> +}
> +
>  static void flexcan_set_bittiming_ctrl(const struct net_device *dev)
>  {
>  	const struct flexcan_priv *priv = netdev_priv(dev);
> @@ -1540,10 +1556,10 @@ static void flexcan_chip_interrupts_enable(const struct net_device *dev)
>  	u64 reg_imask;
>  
>  	disable_irq(dev->irq);
> -	if (quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
> +	if (quirks & FLEXCAN_QUIRK_NR_IRQ_3)
>  		disable_irq(priv->irq_boff);
> +	if (quirks & (FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_IRQ_BERR))
>  		disable_irq(priv->irq_err);
> -	}
>  	if (quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
>  		disable_irq(priv->irq_secondary_mb);
>  
> @@ -1554,10 +1570,10 @@ static void flexcan_chip_interrupts_enable(const struct net_device *dev)
>  	enable_irq(dev->irq);
>  	if (quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
>  		enable_irq(priv->irq_secondary_mb);
> -	if (quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
> -		enable_irq(priv->irq_boff);
> +	if (quirks & (FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_IRQ_BERR))
>  		enable_irq(priv->irq_err);
> -	}
> +	if (quirks & FLEXCAN_QUIRK_NR_IRQ_3)
> +		enable_irq(priv->irq_boff);
>  }
>  
>  static void flexcan_chip_interrupts_disable(const struct net_device *dev)
> @@ -1881,7 +1897,8 @@ static int flexcan_open(struct net_device *dev)
>  
>  	can_rx_offload_enable(&priv->offload);
>  
> -	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3)
> +	if (priv->devtype_data.quirks &
> +			(FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_IRQ_BERR))
>  		err = request_irq(dev->irq, flexcan_irq_mb,
>  				  IRQF_SHARED, dev->name, dev);
>  	else
> @@ -1902,6 +1919,13 @@ static int flexcan_open(struct net_device *dev)
>  			goto out_free_irq_boff;
>  	}
>  
> +	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_IRQ_BERR) {
> +		err = request_irq(priv->irq_err,
> +				  flexcan_irq_esr, IRQF_SHARED, dev->name, dev);
> +		if (err)
> +			goto out_free_irq_boff;
> +	}
> +
>  	if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ) {
>  		err = request_irq(priv->irq_secondary_mb,
>  				  flexcan_irq_mb, IRQF_SHARED, dev->name, dev);
> @@ -1916,7 +1940,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_IRQ_BERR | 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)
> @@ -1948,10 +1973,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_IRQ_BERR | 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);
> @@ -2338,12 +2365,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_IRQ_BERR) {
> +		priv->irq_err = platform_get_irq_byname(pdev, "berr");
> +		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;
>  		}
> +

Nitpick: you shouldn't have unrelated changes, like this newline
addition, in you patches.

@Marc, do you mind removing this while applying?

>  		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..bbb1a8dd4777 100644
> --- a/drivers/net/can/flexcan/flexcan.h
> +++ b/drivers/net/can/flexcan/flexcan.h
> @@ -74,6 +74,8 @@
>   * both need to have an interrupt handler registered.
>   */
>  #define FLEXCAN_QUIRK_SECONDARY_MB_IRQ	BIT(18)
> +/* Setup dedicated bus error and state change IRQ */
> +#define FLEXCAN_QUIRK_IRQ_BERR	BIT(19)
>  
>  struct flexcan_devtype_data {
>  	u32 quirks;		/* quirks needed for different IP cores */


Yours sincerely,
Vincent Mailhol