[PATCH] can: m_can: Fix suspend deinit() error path

[PATCH] can: m_can: Fix suspend deinit() error path

[Markus Schneider-Pargmann (TI)]

When suspending mcan, deinit is called and its return value is returned,
but nothing is restored. Returning an error in the suspend function will
stop suspending and resume the system immediately. So on error the
device should be restored to its previous state.

Fixes: ad1ddb3bfb0c ("can: m_can: call deinit/init callback when going into suspend/resume")
Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
---
 drivers/net/can/m_can/m_can.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 16f80607e150..91a0c5eca260 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2612,8 +2612,14 @@ int m_can_class_suspend(struct device *dev)
 			hrtimer_cancel(&cdev->hrtimer);
 			m_can_write(cdev, M_CAN_IE, IR_RF0N);
 
-			if (cdev->ops->deinit)
+			if (cdev->ops->deinit) {
 				ret = cdev->ops->deinit(cdev);
+				if (ret) {
+					netdev_err(cdev->net, "failed to deinit device while suspending %pe\n",
+						   ERR_PTR(ret));
+					goto err_restore_interface;
+				}
+			}
 		} else {
 			m_can_stop(ndev);
 		}
@@ -2625,6 +2631,21 @@ int m_can_class_suspend(struct device *dev)
 	if (!m_can_class_wakeup_pinctrl_enabled(cdev))
 		pinctrl_pm_select_sleep_state(dev);
 
+	return 0;
+
+err_restore_interface:
+	if (netif_running(ndev)) {
+		if (cdev->pm_wake_source) {
+			/* Enable interrupts that trigger immediately if
+			 * something is there and keep the hrtimer off
+			 */
+			cdev->active_interrupts |= IR_RF0N | IR_TEFN;
+			m_can_write(cdev, M_CAN_IE, cdev->active_interrupts);
+		}
+		netif_device_attach(ndev);
+		netif_start_queue(ndev);
+	}
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(m_can_class_suspend);

---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260914-v7-3-topic-mcan-suspend-fix-fix-b943929d8fef

Best regards,
--  
Markus Schneider-Pargmann <msp@baylibre.com>

Re: [PATCH] can: m_can: Fix suspend deinit() error path

[Kendall Willis]

On 10:29-20260918, Markus Schneider-Pargmann (TI) wrote:
> When suspending mcan, deinit is called and its return value is returned,
> but nothing is restored. Returning an error in the suspend function will
> stop suspending and resume the system immediately. So on error the
> device should be restored to its previous state.
> 
> Fixes: ad1ddb3bfb0c ("can: m_can: call deinit/init callback when going into suspend/resume")
> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
> ---

Separate from this commit, but does cdev->can.state in
m_can_class_resume need to be moved to outside the cdev->pm_wake_source
conditional since in the suspend path it is only in the
netif_running(ndev) conditional?

Reviewed-by: Kendall Willis <k-willis@ti.com>

>  drivers/net/can/m_can/m_can.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 16f80607e150..91a0c5eca260 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -2612,8 +2612,14 @@ int m_can_class_suspend(struct device *dev)
>  			hrtimer_cancel(&cdev->hrtimer);
>  			m_can_write(cdev, M_CAN_IE, IR_RF0N);
>  
> -			if (cdev->ops->deinit)
> +			if (cdev->ops->deinit) {
>  				ret = cdev->ops->deinit(cdev);
> +				if (ret) {
> +					netdev_err(cdev->net, "failed to deinit device while suspending %pe\n",
> +						   ERR_PTR(ret));
> +					goto err_restore_interface;
> +				}
> +			}
>  		} else {
>  			m_can_stop(ndev);
>  		}
> @@ -2625,6 +2631,21 @@ int m_can_class_suspend(struct device *dev)
>  	if (!m_can_class_wakeup_pinctrl_enabled(cdev))
>  		pinctrl_pm_select_sleep_state(dev);
>  
> +	return 0;
> +
> +err_restore_interface:
> +	if (netif_running(ndev)) {
> +		if (cdev->pm_wake_source) {
> +			/* Enable interrupts that trigger immediately if
> +			 * something is there and keep the hrtimer off
> +			 */
> +			cdev->active_interrupts |= IR_RF0N | IR_TEFN;
> +			m_can_write(cdev, M_CAN_IE, cdev->active_interrupts);
> +		}
> +		netif_device_attach(ndev);
> +		netif_start_queue(ndev);
> +	}
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(m_can_class_suspend);
> 
> ---
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
> change-id: 20260914-v7-3-topic-mcan-suspend-fix-fix-b943929d8fef
> 
> Best regards,
> --  
> Markus Schneider-Pargmann <msp@baylibre.com>
> 

Re: [PATCH] can: m_can: Fix suspend deinit() error path

[Markus Schneider-Pargmann]

[-- Attachment #1: Type: text/plain, Size: 968 bytes --]

On Fri Sep 18, 2026 at 9:21 PM CEST, Kendall Willis wrote:
> On 10:29-20260918, Markus Schneider-Pargmann (TI) wrote:
>> When suspending mcan, deinit is called and its return value is returned,
>> but nothing is restored. Returning an error in the suspend function will
>> stop suspending and resume the system immediately. So on error the
>> device should be restored to its previous state.
>> 
>> Fixes: ad1ddb3bfb0c ("can: m_can: call deinit/init callback when going into suspend/resume")
>> Signed-off-by: Markus Schneider-Pargmann (TI) <msp@baylibre.com>
>> ---
>
> Separate from this commit, but does cdev->can.state in
> m_can_class_resume need to be moved to outside the cdev->pm_wake_source
> conditional since in the suspend path it is only in the
> netif_running(ndev) conditional?

Thanks for the review and pointing this out. But m_can_start() in the
else case sets cdev->can.state as well. So I think it should be fine.

Best
Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]