linux-kernel - Re: [PATCH v3 1/4] pmdomain: core: Introduce device_set/get_out_band

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250918134039.zkpeqsbf6m2ymxvt@lcpd911>
Date: Thu, 18 Sep 2025 19:10:40 +0530
From: Dhruva Gole <d-gole@...com>
To: Peng Fan <peng.fan@....nxp.com>
CC: Peng Fan <peng.fan@....com>, Ulf Hansson <ulf.hansson@...aro.org>,
        "Rafael
 J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
        Pavel Machek
	<pavel@...nel.org>, Peter Chen <peter.chen@...nel.org>,
        Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer
	<s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Xu Yang <xu.yang_2@....com>, <linux-pm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <imx@...ts.linux.dev>, <arm-scmi@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 1/4] pmdomain: core: Introduce
 device_set/get_out_band_wakeup()

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