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

[Kendall Willis]

Hi Markus,
On 11:51-20261001, Markus Schneider-Pargmann wrote:
> On Thu Sep 24, 2026 at 11:54 PM CEST, Kendall Willis wrote:
> > In TI K3 AM62 SoCs, the M_CAN controllers are in power domains which are
> > OFF in the deepest low power states. However, the m_can pins are able to
> > wakeup the system from states where the power domain is OFF through its
> > out-of-band wakeup functionality.
> >
> > Introduce the ti,am62-mcan 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          | 10 ++++++++--
> >  drivers/net/can/m_can/m_can.h          |  6 ++++++
> >  drivers/net/can/m_can/m_can_platform.c |  9 +++++++++
> >  3 files changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index 91a0c5eca260c19fa97c6f5724341cad0c1c95b8..815a11531ff47b8460aa3efa3075804b4f769848 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -2628,8 +2628,14 @@ int m_can_class_suspend(struct device *dev)
> >  		cdev->can.state = CAN_STATE_SLEEPING;
> >  	}
> >  
> > -	if (!m_can_class_wakeup_pinctrl_enabled(cdev))
> > -		pinctrl_pm_select_sleep_state(dev);
> > +	if (!ret) {
> 
> I guess this !ret is from an earlier iteration? Code at this point is
> only executed if everything was successful so far. So I think you can
> drop this outer if.
> 

You're right this is redundant. Thanks for pointing it out, will fix in
next version.

> > +		if (m_can_class_wakeup_pinctrl_enabled(cdev)) {
> > +			if (cdev->out_band_wakeup)
> > +				device_set_out_band_wakeup(dev);
> > +		} else {
> > +			pinctrl_pm_select_sleep_state(dev);
> > +		}
> > +	}
> >  
> >  	return 0;
> >  
> > diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
> > index 4743342b2fba3823819b9e2bc5b085cc0fe6d1e2..da64e6bbcef2a721a330e67fd852ca4a703c5794 100644
> > --- a/drivers/net/can/m_can/m_can.h
> > +++ b/drivers/net/can/m_can/m_can.h
> > @@ -132,6 +132,12 @@ struct m_can_classdev {
> >  
> >  	struct pinctrl *pinctrl;
> >  	struct pinctrl_state *pinctrl_state_wakeup;
> > +
> > +	bool out_band_wakeup;
> > +};
> > +
> > +struct m_can_of_data {
> > +	bool out_band_wakeup;
> 
> Can you please move struct m_can_of_data to the top of m_can_platform.c
> to keep it local to where it is used?
> 

Will fix it in the next version.

Thanks,
Kendall

> The rest looks good to me.
> 
> Best
> Markus
> 
> >  };
> >  
> >  struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, int sizeof_priv);
> > diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
> > index 51dd41dbf2c9a971295994946e190ec0187addd5..dbd59730c0bc16f519cb2c40bcd8093ca1bcb369 100644
> > --- a/drivers/net/can/m_can/m_can_platform.c
> > +++ b/drivers/net/can/m_can/m_can_platform.c
> > @@ -83,6 +83,7 @@ static int m_can_plat_probe(struct platform_device *pdev)
> >  	void __iomem *addr;
> >  	void __iomem *mram_addr;
> >  	struct phy *transceiver;
> > +	const struct m_can_of_data *of_data;
> >  	int irq = 0, ret = 0;
> >  
> >  	mcan_class = m_can_class_allocate_dev(&pdev->dev,
> > @@ -148,6 +149,9 @@ static int m_can_plat_probe(struct platform_device *pdev)
> >  
> >  	mcan_class->is_peripheral = false;
> >  
> > +	of_data = of_device_get_match_data(&pdev->dev);
> > +	mcan_class->out_band_wakeup = of_data && of_data->out_band_wakeup;
> > +
> >  	platform_set_drvdata(pdev, mcan_class);
> >  
> >  	pm_runtime_enable(mcan_class->dev);
> > @@ -218,8 +222,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_of_data m_can_plat_am62 = {
> > +	.out_band_wakeup = true,
> > +};
> > +
> >  static const struct of_device_id m_can_of_table[] = {
> >  	{ .compatible = "bosch,m_can", .data = NULL },
> > +	{ .compatible = "ti,am62-mcan", .data = &m_can_plat_am62 },
> >  	{ /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, m_can_of_table);
> 
>