[PATCH] pmdomain: ti_sci: Handle wakeup constraint if device has pinctrl wakeup state

Kendall Willis posted 1 patch 4 weeks ago
There is a newer version of this series
drivers/pmdomain/ti/ti_sci_pm_domains.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
[PATCH] pmdomain: ti_sci: Handle wakeup constraint if device has pinctrl wakeup state
Posted by Kendall Willis 4 weeks ago
In TI K3 SoCs the PM co-processor (device manager or DM) will decide
which low power state to suspend into based off of constraints given by
Linux. If a device is marked as a wakeup source in Linux, Linux will add
a constraint that the wakeup source has to be on. The DM will enter the
deepest low power state based off of the constraint.

In cases like UARTs, IO daisy-chaining can be used to wakeup the system,
however if the UART is marked as a wakeup source, the system is not able
to enter any low power mode.

IO daisy-chain wakeup can use the pinctrl wakeup state instead of using
wake IRQs. For example, the serial driver on K3 platforms uses a wakeup
pinctrl state to be able to resume from suspend.

Devices that are marked as a wakeup source and use pinctrl wakeup state
should not set wakeup constraints since these can happen even from deepest
low power state, so the DM should not be prevented from picking deep power
states.

Detect the pinctrl wakeup state in the suspend path, and if it exists,
skip sending the constraint.

Signed-off-by: Kendall Willis <k-willis@ti.com>
---
This series is intended to be implemented along with the following
series:

1. "pmdomain: ti_sci: Handle wakeup constraint if device has pinctrl
   wakeup state": (this patch) skips setting constraints for wakeup
   sources that use pinctrl state 'wakeup'.

2. "serial: 8250: omap: Add wakeup support": Implements wakeup from
   the UARTs for TI K3 SoCs

3. "arm64: dts: ti: k3-am62: Support Main UART wakeup": Implements the
   functionality to wakeup the system from the Main UART

Testing
-------
Tested on a SK-AM62B-P1 board with all series and dependencies
implemented. Suspend/resume verified with the Main UART wakeup source
by entering a keypress on the console.

---
 drivers/pmdomain/ti/ti_sci_pm_domains.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
index 82df7e44250bb..884905fd0686c 100644
--- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
+++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
@@ -10,6 +10,7 @@
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_qos.h>
@@ -84,9 +85,24 @@ static inline void ti_sci_pd_set_wkup_constraint(struct device *dev)
 	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
 	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
 	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
+	struct pinctrl *pinctrl = devm_pinctrl_get(dev);
+	struct pinctrl_state *pinctrl_state_wakeup;
 	int ret;
 
 	if (device_may_wakeup(dev)) {
+		/*
+		 * If device can wakeup using pinctrl wakeup state,
+		 * we do not want to set a constraint
+		 */
+		if (!IS_ERR_OR_NULL(pinctrl)) {
+			pinctrl_state_wakeup = pinctrl_lookup_state(pinctrl, "wakeup");
+			if (!IS_ERR_OR_NULL(pinctrl_state_wakeup)) {
+				dev_dbg(dev, "%s: has wake pinctrl wakeup state, not setting " \
+						"constraints\n", __func__);
+				return;
+			}
+		}
+
 		/*
 		 * If device can wakeup using IO daisy chain wakeups,
 		 * we do not want to set a constraint.

base-commit: 4ac65880ebca1b68495bd8704263b26c050ac010
-- 
2.34.1
Re: [PATCH] pmdomain: ti_sci: Handle wakeup constraint if device has pinctrl wakeup state
Posted by Dhruva Gole 3 weeks, 6 days ago
On Sep 04, 2025 at 16:16:07 -0500, Kendall Willis wrote:
> In TI K3 SoCs the PM co-processor (device manager or DM) will decide
> which low power state to suspend into based off of constraints given by
> Linux. If a device is marked as a wakeup source in Linux, Linux will add
> a constraint that the wakeup source has to be on. The DM will enter the
> deepest low power state based off of the constraint.
> 
> In cases like UARTs, IO daisy-chaining can be used to wakeup the system,
> however if the UART is marked as a wakeup source, the system is not able
> to enter any low power mode.
> 
> IO daisy-chain wakeup can use the pinctrl wakeup state instead of using
> wake IRQs. For example, the serial driver on K3 platforms uses a wakeup

I think this statement will only make sense to someone who's worked on
io daisychain and wake IRQs on our SoCs.
I don't think it's coming out very clearly why wake IRQs are needed for
IO Daisychain in the first place.

You could probably just reference commit b06bc47279919 ("pmdomain:
ti_sci: handle wake IRQs for IO daisy chain wakeups") from Kevin where
we explain whats io daisychain and need for wake IRQ. Talk in this patch
with relation to that patch - what you're doing that the other one
missed.

> pinctrl state to be able to resume from suspend.

Perhaps I am missing the order in which the patches are being applied.
But if it's like the 1 -> 2 -> 3 order you mentioned below then this
reference to serial drv won't really make sense right?

If we're going in the order of applying the series that you
specified, there's no guarantee that the UART patch makes it in
along with this patch, so I'd say let's skip mentioning the serial
driver to avoid sending people in search for how the serial driver is
doing this atall.

In Kevin's earlier commit it made sense, because he was talking about
daisychaining, here we're not. So the 8250 driver doesn't actually have
the changes we're talking about in this patch.

> 
> Devices that are marked as a wakeup source and use pinctrl wakeup state
> should not set wakeup constraints since these can happen even from deepest
> low power state, so the DM should not be prevented from picking deep power
> states.
> 
> Detect the pinctrl wakeup state in the suspend path, and if it exists,
> skip sending the constraint.
> 
> Signed-off-by: Kendall Willis <k-willis@ti.com>
> ---
> This series is intended to be implemented along with the following
> series:
> 
> 1. "pmdomain: ti_sci: Handle wakeup constraint if device has pinctrl
>    wakeup state": (this patch) skips setting constraints for wakeup
>    sources that use pinctrl state 'wakeup'.
> 
> 2. "serial: 8250: omap: Add wakeup support": Implements wakeup from
>    the UARTs for TI K3 SoCs
> 
> 3. "arm64: dts: ti: k3-am62: Support Main UART wakeup": Implements the
>    functionality to wakeup the system from the Main UART
> 
> Testing
> -------
> Tested on a SK-AM62B-P1 board with all series and dependencies
> implemented. Suspend/resume verified with the Main UART wakeup source
> by entering a keypress on the console.
> 
> ---
>  drivers/pmdomain/ti/ti_sci_pm_domains.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> index 82df7e44250bb..884905fd0686c 100644
> --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
> +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> @@ -10,6 +10,7 @@
>  #include <linux/err.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
>  #include <linux/pm_qos.h>
> @@ -84,9 +85,24 @@ static inline void ti_sci_pd_set_wkup_constraint(struct device *dev)
>  	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>  	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
>  	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
> +	struct pinctrl *pinctrl = devm_pinctrl_get(dev);
> +	struct pinctrl_state *pinctrl_state_wakeup;
>  	int ret;
>  
>  	if (device_may_wakeup(dev)) {
> +		/*
> +		 * If device can wakeup using pinctrl wakeup state,
> +		 * we do not want to set a constraint
> +		 */
> +		if (!IS_ERR_OR_NULL(pinctrl)) {
> +			pinctrl_state_wakeup = pinctrl_lookup_state(pinctrl, "wakeup");
> +			if (!IS_ERR_OR_NULL(pinctrl_state_wakeup)) {
> +				dev_dbg(dev, "%s: has wake pinctrl wakeup state, not setting " \
> +						"constraints\n", __func__);
> +				return;
> +			}
> +		}
> +

Not much objections to the code itself, as it makes sense that we want
to ignore the constraint for wakeup pinctrl similar to how we did for the
wake IRQ one.

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated
Re: [PATCH] pmdomain: ti_sci: Handle wakeup constraint if device has pinctrl wakeup state
Posted by Kendall Willis 3 weeks, 3 days ago
Hi Dhruva,

On 9/5/25 13:39, Dhruva Gole wrote:
> On Sep 04, 2025 at 16:16:07 -0500, Kendall Willis wrote:
>> In TI K3 SoCs the PM co-processor (device manager or DM) will decide
>> which low power state to suspend into based off of constraints given by
>> Linux. If a device is marked as a wakeup source in Linux, Linux will add
>> a constraint that the wakeup source has to be on. The DM will enter the
>> deepest low power state based off of the constraint.
>>
>> In cases like UARTs, IO daisy-chaining can be used to wakeup the system,
>> however if the UART is marked as a wakeup source, the system is not able
>> to enter any low power mode.
>>
>> IO daisy-chain wakeup can use the pinctrl wakeup state instead of using
>> wake IRQs. For example, the serial driver on K3 platforms uses a wakeup
> 
> I think this statement will only make sense to someone who's worked on
> io daisychain and wake IRQs on our SoCs.
> I don't think it's coming out very clearly why wake IRQs are needed for
> IO Daisychain in the first place.
> 
> You could probably just reference commit b06bc47279919 ("pmdomain:
> ti_sci: handle wake IRQs for IO daisy chain wakeups") from Kevin where
> we explain whats io daisychain and need for wake IRQ. Talk in this patch
> with relation to that patch - what you're doing that the other one
> missed.
> 
>> pinctrl state to be able to resume from suspend.
> 
> Perhaps I am missing the order in which the patches are being applied.
> But if it's like the 1 -> 2 -> 3 order you mentioned below then this
> reference to serial drv won't really make sense right?
> 
> If we're going in the order of applying the series that you
> specified, there's no guarantee that the UART patch makes it in
> along with this patch, so I'd say let's skip mentioning the serial
> driver to avoid sending people in search for how the serial driver is
> doing this atall.
> 
> In Kevin's earlier commit it made sense, because he was talking about
> daisychaining, here we're not. So the 8250 driver doesn't actually have
> the changes we're talking about in this patch.

In v2, I will add references to commit b06bc47279919 ("pmdomain: ti_sci: 
handle wake IRQs for IO daisy chain wakeups") and I will remove the 
reference to the UART driver.

Thanks,
Kendall

> 
>>
>> Devices that are marked as a wakeup source and use pinctrl wakeup state
>> should not set wakeup constraints since these can happen even from deepest
>> low power state, so the DM should not be prevented from picking deep power
>> states.
>>
>> Detect the pinctrl wakeup state in the suspend path, and if it exists,
>> skip sending the constraint.
>>
>> Signed-off-by: Kendall Willis <k-willis@ti.com>
>> ---
>> This series is intended to be implemented along with the following
>> series:
>>
>> 1. "pmdomain: ti_sci: Handle wakeup constraint if device has pinctrl
>>     wakeup state": (this patch) skips setting constraints for wakeup
>>     sources that use pinctrl state 'wakeup'.
>>
>> 2. "serial: 8250: omap: Add wakeup support": Implements wakeup from
>>     the UARTs for TI K3 SoCs
>>
>> 3. "arm64: dts: ti: k3-am62: Support Main UART wakeup": Implements the
>>     functionality to wakeup the system from the Main UART
>>
>> Testing
>> -------
>> Tested on a SK-AM62B-P1 board with all series and dependencies
>> implemented. Suspend/resume verified with the Main UART wakeup source
>> by entering a keypress on the console.
>>
>> ---
>>   drivers/pmdomain/ti/ti_sci_pm_domains.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> index 82df7e44250bb..884905fd0686c 100644
>> --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/err.h>
>>   #include <linux/module.h>
>>   #include <linux/of.h>
>> +#include <linux/pinctrl/consumer.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/pm_domain.h>
>>   #include <linux/pm_qos.h>
>> @@ -84,9 +85,24 @@ static inline void ti_sci_pd_set_wkup_constraint(struct device *dev)
>>   	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>>   	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
>>   	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
>> +	struct pinctrl *pinctrl = devm_pinctrl_get(dev);
>> +	struct pinctrl_state *pinctrl_state_wakeup;
>>   	int ret;
>>   
>>   	if (device_may_wakeup(dev)) {
>> +		/*
>> +		 * If device can wakeup using pinctrl wakeup state,
>> +		 * we do not want to set a constraint
>> +		 */
>> +		if (!IS_ERR_OR_NULL(pinctrl)) {
>> +			pinctrl_state_wakeup = pinctrl_lookup_state(pinctrl, "wakeup");
>> +			if (!IS_ERR_OR_NULL(pinctrl_state_wakeup)) {
>> +				dev_dbg(dev, "%s: has wake pinctrl wakeup state, not setting " \
>> +						"constraints\n", __func__);
>> +				return;
>> +			}
>> +		}
>> +
> 
> Not much objections to the code itself, as it makes sense that we want
> to ignore the constraint for wakeup pinctrl similar to how we did for the
> wake IRQ one.
>