Re: [PATCH v4 2/2] can: m_can: add ti,am62-m_can compatible with out-of-band wakeup support

[Markus Schneider-Pargmann]

On Wed Sep 2, 2026 at 11:03 PM CEST, Kendall Willis wrote:
> In TI AM62X, AM62A, and AM62P SoCs, the M_CAN controller is a part of
> the MCU power domain which is OFF in the deepest low power states.
> However, the m_can pins are able to wakeup the system from states where
> the MCU domain is OFF through its out-of-band wakeup functionality.

There are m_can devices in the main domain as well. Does this apply to
these as well?

>
> Introduce the ti,am62-m_can compatible with match data to identify the
> out-of-band wakeup capability for TI AM62 SoCs. During suspend, set the
> out-of-band wakeup flag when the device is configured as a wakeup
> source, the "wakeup" pinctrl state is present, and the match data
> indicates out-of-band wakeup support.
>
> Signed-off-by: Kendall Willis <k-willis@xxxxxx>
> ---
>  drivers/net/can/m_can/m_can.c          | 7 ++++++-
>  drivers/net/can/m_can/m_can.h          | 4 ++++
>  drivers/net/can/m_can/m_can_platform.c | 5 +++++
>  3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 16f80607e150e2ca2950616ff2c39b7aa13710b4..0788816d881ef8ba84ad5a84a6d2d3e60517ea7a 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -2598,6 +2598,7 @@ int m_can_class_suspend(struct device *dev)
>  {
>  	struct m_can_classdev *cdev = dev_get_drvdata(dev);
>  	struct net_device *ndev = cdev->net;
> +	const struct m_can_pdata *pdata = of_device_get_match_data(dev);

The core m_can driver works for all m_can forms, tcan, platform and pci.
I think it would be cleaner if you parse this in the platform driver and
set it in m_can_classdev instead. Otherwise of_match_data has to be the
same for all drivers which is not obvious when you are editing tcan for
example.

>  	int ret = 0;
>  
>  	if (netif_running(ndev)) {
> @@ -2622,8 +2623,12 @@ int m_can_class_suspend(struct device *dev)
>  		cdev->can.state = CAN_STATE_SLEEPING;
>  	}
>  
> -	if (!m_can_class_wakeup_pinctrl_enabled(cdev))
> +	if (m_can_class_wakeup_pinctrl_enabled(cdev)) {
> +		if (pdata && pdata->out_band_wakeup)
> +			device_set_out_band_wakeup(dev);
> +	} else {
>  		pinctrl_pm_select_sleep_state(dev);
> +	}

I just noticed these are called even if ret is not 0, so suspend failed.
This shouldn't happen, but not a problem of your patch. But you could
add a fix for that.

>  
>  	return ret;
>  }
> diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
> index 4743342b2fba3823819b9e2bc5b085cc0fe6d1e2..486d808bc677482acf446b1450c3a9b6b7405178 100644
> --- a/drivers/net/can/m_can/m_can.h
> +++ b/drivers/net/can/m_can/m_can.h
> @@ -134,6 +134,10 @@ struct m_can_classdev {
>  	struct pinctrl_state *pinctrl_state_wakeup;
>  };
>  
> +struct m_can_pdata {
> +	bool out_band_wakeup;
> +};
> +
>  struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, int sizeof_priv);
>  void m_can_class_free_dev(struct net_device *net);
>  int m_can_class_register(struct m_can_classdev *cdev);
> diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
> index 56da411878af0185ff9cc512e325137428a68255..ae2d89b65fe8da65b1ca73cbf80cd8ee8483e2e8 100644
> --- a/drivers/net/can/m_can/m_can_platform.c
> +++ b/drivers/net/can/m_can/m_can_platform.c
> @@ -218,8 +218,13 @@ static const struct dev_pm_ops m_can_pmops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(m_can_suspend, m_can_resume)
>  };
>  
> +static const struct m_can_pdata m_can_plat_am62 = {

I don't like the pdata name here, this suggests to be platform data, but
this is of match data instead. Maybe of_data?

Best
Markus

Attachment: signature.asc
Description: PGP signature