Re: [PATCH v4 2/3] power: sequencing: qcom-wcn: support SW_CTRL wake IRQ

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] power: sequencing: missing wake IRQ trigger type in
  pwrseq_qcom_wcn_probe()
- [Low] power: sequencing: qcom-wcn: direct inclusion of
  linux/pm_wakeup.h
--

commit 832aa68ed4662747e202e91c3f57f19829c22253
Author: Linghui Wu <linghui.wu@oss.qualcomm.com>

power: sequencing: qcom-wcn: support SW_CTRL wake IRQ

This commit adds support for requesting the SW_CTRL GPIO and registering it
as a dedicated wake IRQ when the WCN PMU is marked as a wakeup source.

> diff --git a/drivers/power/sequencing/pwrseq-qcom-wcn.c b/drivers/power/sequencing/pwrseq-qcom-wcn.c
> index e51b655add01c..26b436df9b93e 100644
> --- a/drivers/power/sequencing/pwrseq-qcom-wcn.c
> +++ b/drivers/power/sequencing/pwrseq-qcom-wcn.c
> @@ -11,6 +11,8 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_wakeirq.h>
> +#include <linux/pm_wakeup.h>

[Severity: Low]
include/linux/pm_wakeup.h forbids direct inclusion (#error "Please do not
include this file directly.") and is already included via <linux/device.h>.

[ ... ]

> @@ -525,6 +534,33 @@ static int pwrseq_qcom_wcn_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(ctx->bt_gpio),
>  				     "Failed to get the Bluetooth enable GPIO\n");
>  
> +	if (device_property_read_bool(dev, "wakeup-source")) {
> +		swctrl_gpio = devm_gpiod_get(dev, "swctrl", GPIOD_IN);
> +		if (IS_ERR(swctrl_gpio))
> +			return dev_err_probe(dev, PTR_ERR(swctrl_gpio),
> +					     "Failed to get the SW_CTRL GPIO\n");
> +
> +		wake_irq = gpiod_to_irq(swctrl_gpio);
> +		if (wake_irq < 0)
> +			return dev_err_probe(dev, wake_irq,
> +					     "Failed to get the SW_CTRL wake IRQ\n");
> +
> +		ret = devm_device_init_wakeup(dev);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to enable wakeup\n");
> +
> +		ret = dev_pm_set_dedicated_wake_irq(dev, wake_irq);

[Severity: High]
Does wake_irq ever get an interrupt trigger type configured?

gpiod_to_irq() maps the GPIO line with IRQ_TYPE_NONE, and
dev_pm_set_dedicated_wake_irq() calls request_threaded_irq() with only
IRQF_ONESHOT | IRQF_NO_AUTOEN.  Because neither supplies trigger flags,
__setup_irq() skips __irq_set_trigger().  On Qualcomm TLMM GPIOs,
msm_gpio_init() initializes the IRQ with IRQ_TYPE_NONE and handle_bad_irq,
relying on .irq_set_type to program hardware trigger registers and install
the real flow handler, so SW_CTRL wake interrupts will either fail to fire
or hit handle_bad_irq() instead of waking the system.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112072009.3815276-1-linghui.wu@oss.qualcomm.com?part=2