Re: [PATCH v3 1/4] pmdomain: core: Introduce device_set/get_out_band_wakeup()

[Dhruva Gole <d-gole@ti.com>]

Hi Peng,

On Sep 18, 2025 at 21:12:30 +0800, Peng Fan wrote:
> Hi Dhruva,
> 
> On Thu, Sep 18, 2025 at 03:29:50PM +0530, Dhruva Gole wrote:
> >On Sep 02, 2025 at 11:33:00 +0800, Peng Fan wrote:
> >> For some cases, a device could still wakeup the system even if its power
> >> domain is in off state, because the device's wakeup hardware logic is
> >> in an always-on domain.
> >> 
> >> To support this case, introduce device_set/get_out_band_wakeup() to
> >> allow device drivers to control the behaviour in genpd for a device
> >> that is attached to it.
> >> 
> >
> >Thinking more into it, to me it seems like if the intent here is to only
> >allow the device drivers to figure out whether they should be or not be
> >executing the suspend/resume_noirqs then that can still be checked by
> >wisely using the device set_wakeup APIs in the driver itself.
> >
> >Not sure why this patch should be necessary for a
> >driver to execute the suspend_noirq or not. That decision can very well
> >be taken inside the driver's suspend resume_noirq hooks based on wakeup
> >capability and wake_enabled statuses.
> 
> I should join today's SCMI meeting, but something else caught me (:

It's alright, maybe see you in the next one ;)

> 
> Thanks for looking into this. 
> 
> In genpd_suspend_finish, genpd_sync_power_off will be called if
> "(device_awake_path(dev) && genpd_is_active_wakeup(genpd))" is false.
> So if the device is enabled wakeup, the genpd will not be turned off because
> the check return true.

Umm I think this device_awake_path stuff is only going to be true when
someone calls device_set_wakeup_path, I don't think it is going to
return true for a wakeup_capable device. I know all these "wakeup"
terminology and APIs have become all too confusing :( , so maybe Ulf can
correct me.
I maybe misremembering, but I have seen in some cases where a driver may
have marked itself wakeup_capable but the suspend hooks still do get
called... So your concern about genpd_sync_power_off not being called
due to wakeup capable device driver may not be valid... Again please
feel to correct me if I am wrong.

Did you also look at the wake IRQ stuff I mentioned?
In the path you're talking about it just checks
device_awake_path(dev) && genpd_is_active_wakeup(genpd)
However if the device irq is just marked as a wake IRQ, I don't think
that is checked anywhere in this path. So definitely if the IRQ of your
device is set as a wake IRQ, it will still get suspended and resumed as
usual and that's what you want right?


> 
> But to i.MX, if the device is configured as wakeup source, we still need to
> power off the power domain, because the device has out-of-band wakeup logic.
> 
> This patch is to make sure the power domain could be powered off in
> suspend flow and powered up in resume flow.
> 
> Thanks,
> Peng
> 
> >
> >Just a pseudo code:
> >```
> >driver_suspend_noirq () {
> >	if (device_may_wakeup()) {
> >		// do the sequence where the power domain might get turned off
> >		// but like you say device can do some out band wakeup
> >		return XXX;
> >	}
> >	// regular suspend sequence here... maybe inband wakeup config / clk
> >	// disable etc...
> >}
> > ```
> >
> >And something similar in resume_noirq?
> >
> >Just need to make sure that the probe func does the
> >device_set_wakeup_enable or capable stuff correctly as per your H/w and
> >wakeup requirements...
> >
> >
> >-- 
> >Best regards,
> >Dhruva Gole
> >Texas Instruments Incorporated

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated