On 17.07.2024 02:03:35, Bough Chen wrote:
> > On 16.07.2024 10:40:31, Frank Li wrote:
> > > > > @@ -2330,9 +2366,12 @@ static int __maybe_unused
> > flexcan_noirq_resume(struct device *device)
> > > > > if (netif_running(dev)) {
> > > > > int err;
> > > > >
> > > > > - err = pm_runtime_force_resume(device);
> > > > > - if (err)
> > > > > - return err;
> > > > > + if (!(device_may_wakeup(device) &&
> > > > ^^^^^^^^^^^^^^^^^^^^^^^^
> > > >
> > > > Where does this come from?
> > >
> > > include/linux/pm_wakeup.h
> > >
> > > static inline bool device_may_wakeup(struct device *dev)
> > > {
> > > return dev->power.can_wakeup && !!dev->power.wakeup;
> > > }
> >
> > Sorry for the confusion. I wanted to point out, that the original driver doesn't
> > have the check to device_may_wakeup(). Why was this added?
>
> Here add this to make sure for CAN with flag
> FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI and really used as wakeup source,
> do not need to call pm_runtime_force_resume(), keep it align with what
> we do in flexcan_noirq_suspend.
> As the comment in flexcan_noirq_suspend, CAN with flag
> FLEXCAN_QUIRK_SETUP_STOP_MODE_SCMI, when used as wakeup source, need
> to keep CAN clock on when system suspend, let ATF part logic works,
> detail steps please refer to this patch commit log. Whether gate off
> the CAN clock or not depends on the Hardware design. So for this case,
> in flexcan_noirq_suspend, directly return0, do not call the
> pm_runtime_force_suspend(), then in flexcan_noirq_resume(), use the
> same logic to skip the pm_runtime_force_resume().
Please change the control flow, so that flexcan_noirq_suspend() and
flexcan_noirq_resume() look symmetrical.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
