Re: [PATCH RESEND v4 6/8] can: flexcan: add NXP S32N79 SoC support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Wakeup functionality is broken for multi-IRQ FlexCAN platforms (like S32N79) because `enable_irq_wake()` is called on the mailbox interrupt (`dev->irq`) instead of the state/wakeup interrupt (`priv->irq_err`).
--

commit 39bd2be7b5ab93fd5fb7064b8e7d3eac344ed38f
Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>

can: flexcan: add NXP S32N79 SoC support

This patch introduces support for the NXP S32N79 SoC in the FlexCAN driver.
It adds a new device data structure specifying necessary quirks, including
multiple interrupt handling, and registers the new compatible string.

> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -397,6 +397,15 @@ static const struct flexcan_devtype_data nxp_s32g2_devtype_data = {
>  		FLEXCAN_QUIRK_SECONDARY_MB_IRQ,
>  };
>  
> +static const struct flexcan_devtype_data nxp_s32n_devtype_data = {
> +	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> +		FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_RX_MAILBOX |
> +		FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SUPPORT_FD |
> +		FLEXCAN_QUIRK_SUPPORT_ECC | FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
> +		FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR |
> +		FLEXCAN_QUIRK_IRQ_BERR,

[Severity: High]
This isn't a bug introduced by this patch, but will wakeup functionality be
broken for this new platform since it uses multiple interrupts?

In flexcan_suspend() and flexcan_resume(), the driver hardcodes
enable_irq_wake(dev->irq) and disable_irq_wake(dev->irq):

flexcan_suspend() {
	...
	if (device_may_wakeup(device)) {
		enable_irq_wake(dev->irq);
	...
}

For multi-IRQ platforms using FLEXCAN_QUIRK_IRQ_BERR, dev->irq is dedicated
to mailboxes, while state changes and wakeups (FLEXCAN_ESR_WAK_INT) are
routed to priv->irq_err.

Because enable_irq_wake() is never called for priv->irq_err, will the
interrupt controller mask the actual wakeup interrupt during suspend,
preventing the CAN controller from waking up the CPU?

> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603071342.641874-1-ciprianmarian.costea@oss.nxp.com?part=6