[PATCH v3 3/3] pmdomain: ti_sci: handle wake IRQs for IO daisy chain wakeups

Kevin Hilman posted 3 patches 4 months, 2 weeks ago
There is a newer version of this series
[PATCH v3 3/3] pmdomain: ti_sci: handle wake IRQs for IO daisy chain wakeups
Posted by Kevin Hilman 4 months, 2 weeks ago
When a device supports IO daisy-chain wakeups, it uses a dedicated
wake IRQ.  Devices with IO daisy-chain wakeups enabled should not set
wakeup constraints since these can happen even from deep power states,
so should not prevent the DM from picking deep power states.

Wake IRQs are set with dev_pm_set_wake_irq() or
dev_pm_set_dedicated_wake_irq().  The latter is used by the serial
driver used on K3 platforms (drivers/tty/serial/8250/8250_omap.c)
when the interrupts-extended property is used to describe the
dedicated wakeup interrupt.

Detect these wake IRQs in the suspend path, and if set, skip sending
constraint.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/pmdomain/ti/ti_sci_pm_domains.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
index 1ab1e46924ab..747a7a33c0a9 100644
--- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
+++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
@@ -82,6 +82,13 @@ static inline void ti_sci_pd_set_wkup_constraint(struct device *dev)
 	int ret;
 
 	if (device_may_wakeup(dev)) {
+		/*
+		 * If device can wakeup using IO daisy chain wakeups,
+		 * we do not want to set a constraint.
+		 */
+		if (dev->power.wakeirq)
+			dev_dbg(dev, "%s: has wake IRQ, not setting constraints\n", __func__);
+
 		ret = ti_sci->ops.pm_ops.set_device_constraint(ti_sci, pd->idx,
 							       TISCI_MSG_CONSTRAINT_SET);
 		if (!ret)

-- 
2.46.0
Re: [PATCH v3 3/3] pmdomain: ti_sci: handle wake IRQs for IO daisy chain wakeups
Posted by Ulf Hansson 4 months, 2 weeks ago
On Fri, 6 Sept 2024 at 00:03, Kevin Hilman <khilman@baylibre.com> wrote:
>
> When a device supports IO daisy-chain wakeups, it uses a dedicated
> wake IRQ.  Devices with IO daisy-chain wakeups enabled should not set
> wakeup constraints since these can happen even from deep power states,
> so should not prevent the DM from picking deep power states.
>
> Wake IRQs are set with dev_pm_set_wake_irq() or
> dev_pm_set_dedicated_wake_irq().  The latter is used by the serial
> driver used on K3 platforms (drivers/tty/serial/8250/8250_omap.c)
> when the interrupts-extended property is used to describe the
> dedicated wakeup interrupt.
>
> Detect these wake IRQs in the suspend path, and if set, skip sending
> constraint.
>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  drivers/pmdomain/ti/ti_sci_pm_domains.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> index 1ab1e46924ab..747a7a33c0a9 100644
> --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
> +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> @@ -82,6 +82,13 @@ static inline void ti_sci_pd_set_wkup_constraint(struct device *dev)
>         int ret;
>
>         if (device_may_wakeup(dev)) {
> +               /*
> +                * If device can wakeup using IO daisy chain wakeups,
> +                * we do not want to set a constraint.
> +                */
> +               if (dev->power.wakeirq)
> +                       dev_dbg(dev, "%s: has wake IRQ, not setting constraints\n", __func__);

return; ?

> +
>                 ret = ti_sci->ops.pm_ops.set_device_constraint(ti_sci, pd->idx,
>                                                                TISCI_MSG_CONSTRAINT_SET);
>                 if (!ret)
>
> --
> 2.46.0
>

Kind regards
Uffe
Re: [PATCH v3 3/3] pmdomain: ti_sci: handle wake IRQs for IO daisy chain wakeups
Posted by Kevin Hilman 4 months, 2 weeks ago
Ulf Hansson <ulf.hansson@linaro.org> writes:

> On Fri, 6 Sept 2024 at 00:03, Kevin Hilman <khilman@baylibre.com> wrote:
>>
>> When a device supports IO daisy-chain wakeups, it uses a dedicated
>> wake IRQ.  Devices with IO daisy-chain wakeups enabled should not set
>> wakeup constraints since these can happen even from deep power states,
>> so should not prevent the DM from picking deep power states.
>>
>> Wake IRQs are set with dev_pm_set_wake_irq() or
>> dev_pm_set_dedicated_wake_irq().  The latter is used by the serial
>> driver used on K3 platforms (drivers/tty/serial/8250/8250_omap.c)
>> when the interrupts-extended property is used to describe the
>> dedicated wakeup interrupt.
>>
>> Detect these wake IRQs in the suspend path, and if set, skip sending
>> constraint.
>>
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> ---
>>  drivers/pmdomain/ti/ti_sci_pm_domains.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> index 1ab1e46924ab..747a7a33c0a9 100644
>> --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> @@ -82,6 +82,13 @@ static inline void ti_sci_pd_set_wkup_constraint(struct device *dev)
>>         int ret;
>>
>>         if (device_may_wakeup(dev)) {
>> +               /*
>> +                * If device can wakeup using IO daisy chain wakeups,
>> +                * we do not want to set a constraint.
>> +                */
>> +               if (dev->power.wakeirq)
>> +                       dev_dbg(dev, "%s: has wake IRQ, not setting constraints\n", __func__);
>
> return; ?
>

Oops, I meant to remove the "false" return when changing from bool to
void, but mistakenly removed the whole line.

d'oh!, good catch.

Thanks.

Kevin