Re: [PATCH v5 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] Missing `enable_irq_wake()` for the actual device state change/wakeup interrupt on multi-IRQ platforms.
--

commit 92f9d2a83610f1e8d4d83febb9e00ed2aea10a29
Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>

can: flexcan: add NXP S32N79 SoC support

Adds device data and compatible string for NXP S32N79 SoC. The FlexCAN IP
integration on this SoC uses two interrupts, separating mailboxes from bus
errors and device state changes.

> 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
[ ... ]
> @@ -403,6 +403,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 this is a pre-existing issue
with platforms that use FLEXCAN_QUIRK_IRQ_BERR.

Will this platform fail to wake up from sleep upon CAN activity?

In flexcan_suspend() and flexcan_resume(), the code configures wakeup using
the primary interrupt:

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

However, on multi-IRQ platforms using FLEXCAN_QUIRK_IRQ_BERR, the device
state change and wakeup interrupt is handled by flexcan_irq_esr() and routed
to priv->irq_err (or priv->irq_boff). Since dev->irq is strictly dedicated
to mailboxes on these platforms and ignores device state changes, the
correct interrupt is never configured as a system wakeup source.

Should the suspend and resume routines be updated to call enable_irq_wake()
and disable_irq_wake() on priv->irq_err for platforms with this quirk?

> +};

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