Re: [PATCH v7 6/6] 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] Incorrect interrupt line enabled for system wakeup in flexcan_suspend() on multi-IRQ platforms.
--

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

can: flexcan: add NXP S32N79 SoC support

This commit adds device data and compatible strings for the NXP S32N79 SoC
to the flexcan driver.

> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 601d88bfcfeb2..0533cdf969b2c 100644
> --- 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 is a pre-existing issue, but adding FLEXCAN_QUIRK_IRQ_BERR for the new
S32N79 SoC exposes an existing problem with system wakeup.

On multi-IRQ platforms that use FLEXCAN_QUIRK_IRQ_BERR or
FLEXCAN_QUIRK_NR_IRQ_3, dev->irq is dedicated exclusively to mailbox events.
The FlexCAN Wake-Up Interrupt (FLEXCAN_ESR_WAK_INT) triggers a state change
event, which is routed to the dedicated state change interrupt line
(priv->irq_err or priv->irq_boff).

However, when the system goes to sleep with Wake-on-CAN enabled, does
flexcan_suspend() enable the wrong interrupt line for wakeup?

drivers/net/can/flexcan/flexcan-core.c:flexcan_suspend() {
    ...
    if (device_may_wakeup(device)) {
        ...
        enable_irq_wake(dev->irq);
        ...
    }
    ...
}

Since enable_irq_wake() configures the irqchip to listen to the mailbox
interrupt instead of the state change interrupt, will the system fail to wake
up from suspend on CAN network activity?

The same incorrectly assigned wakeup interrupt is subsequently disabled in
flexcan_resume() by calling disable_irq_wake(dev->irq).

> +};
> +
>  static const struct can_bittiming_const flexcan_bittiming_const = {

[ ... ]

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