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@xxxxxxxxxxxx>
> ---

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@xxxxxx>

>  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@xxxxxxxxxxxx>
>