Linux GPIO subsystem development
 help / color / mirror / Atom feed
* [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable
@ 2026-04-30  5:54 Sneh Mankad
  2026-04-30  6:45 ` Maulik Shah (mkshah)
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sneh Mankad @ 2026-04-30  5:54 UTC (permalink / raw)
  To: Bjorn Andersson, Linus Walleij, Neil Armstrong,
	Krzysztof Kozlowski
  Cc: linux-arm-msm, linux-gpio, linux-kernel, stable, Sneh Mankad

The wakeup enable bit needs to be set irrespective of the SoC using PDC or
MPM as wakeup capable irqchip to allow the GPIO interrupts to be forwarded
to parent irqchip.

This is set only for PDC irqchip using additional check skip_wake_irqs
making it impossible for MPM irqchip to detect the GPIO interrupt during
SoC low power mode since for MPM irqchip the skip_wake_irqs is always
false.

Remove skip_wake_irqs condition when setting wakeup enable bit to allow
forwarding GPIO interrupts for SoCs using MPM irqchip too.

Fixes: 76b446f5b86e ("pinctrl: qcom: handle intr_target_reg wakeup_present/enable bits")
Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 45b3a2763eb85405fecdd4770ba3d4ab684563f0..96df8eb8f5d3f3bcfe165ac02a07414e491f1178 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -1247,7 +1247,7 @@ static int msm_gpio_irq_reqres(struct irq_data *d)
 	 * While the name implies only the wakeup event, it's also required for
 	 * the interrupt event.
 	 */
-	if (test_bit(d->hwirq, pctrl->skip_wake_irqs) && g->intr_wakeup_present_bit) {
+	if (g->intr_wakeup_present_bit) {
 		u32 intr_cfg;
 
 		raw_spin_lock_irqsave(&pctrl->lock, flags);
@@ -1275,7 +1275,7 @@ static void msm_gpio_irq_relres(struct irq_data *d)
 	unsigned long flags;
 
 	/* Disable the wakeup_enable bit if it has been set in msm_gpio_irq_reqres() */
-	if (test_bit(d->hwirq, pctrl->skip_wake_irqs) && g->intr_wakeup_present_bit) {
+	if (g->intr_wakeup_present_bit) {
 		u32 intr_cfg;
 
 		raw_spin_lock_irqsave(&pctrl->lock, flags);

---
base-commit: b4e07588e743c989499ca24d49e752c074924a9a
change-id: 20260430-enable_wakeup_capable_gpios-cb9439ae8772

Best regards,
-- 
Sneh Mankad <sneh.mankad@oss.qualcomm.com>


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable
  2026-04-30  5:54 [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable Sneh Mankad
@ 2026-04-30  6:45 ` Maulik Shah (mkshah)
  2026-05-05 10:28 ` Linus Walleij
  2026-05-05 10:43 ` Konrad Dybcio
  2 siblings, 0 replies; 6+ messages in thread
From: Maulik Shah (mkshah) @ 2026-04-30  6:45 UTC (permalink / raw)
  To: Sneh Mankad, Bjorn Andersson, Linus Walleij, Neil Armstrong,
	Krzysztof Kozlowski
  Cc: linux-arm-msm, linux-gpio, linux-kernel, stable



On 4/30/2026 11:24 AM, Sneh Mankad wrote:
> The wakeup enable bit needs to be set irrespective of the SoC using PDC or
> MPM as wakeup capable irqchip to allow the GPIO interrupts to be forwarded
> to parent irqchip.
> 
> This is set only for PDC irqchip using additional check skip_wake_irqs
> making it impossible for MPM irqchip to detect the GPIO interrupt during
> SoC low power mode since for MPM irqchip the skip_wake_irqs is always
> false.
> 
> Remove skip_wake_irqs condition when setting wakeup enable bit to allow
> forwarding GPIO interrupts for SoCs using MPM irqchip too.
> 
> Fixes: 76b446f5b86e ("pinctrl: qcom: handle intr_target_reg wakeup_present/enable bits")
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> ---
>  drivers/pinctrl/qcom/pinctrl-msm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index 45b3a2763eb85405fecdd4770ba3d4ab684563f0..96df8eb8f5d3f3bcfe165ac02a07414e491f1178 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -1247,7 +1247,7 @@ static int msm_gpio_irq_reqres(struct irq_data *d)
>  	 * While the name implies only the wakeup event, it's also required for
>  	 * the interrupt event.
>  	 */


Pasting full comment from driver, since this is not visible in the diff.

       /*
         * If the wakeup_enable bit is present and marked as available for the
         * requested GPIO, it should be enabled when the GPIO is marked as
         * wake irq in order to allow the interrupt event to be transfered to
         * the PDC HW.
         * While the name implies only the wakeup event, it's also required for
         * the interrupt event.
         */

Can you update in the above comment also to mention both PDC and MPM HW.
While touching this comment, please also correct spelling typo for transfered.

"transferred to the PDC/MPM HW."

Post this update,

Reviewed-by: Maulik Shah <maulik.shah@oss.qualcomm.com>

Thanks,
Maulik

> -	if (test_bit(d->hwirq, pctrl->skip_wake_irqs) && g->intr_wakeup_present_bit) {
> +	if (g->intr_wakeup_present_bit) {
>  		u32 intr_cfg;
>  
>  		raw_spin_lock_irqsave(&pctrl->lock, flags);
> @@ -1275,7 +1275,7 @@ static void msm_gpio_irq_relres(struct irq_data *d)
>  	unsigned long flags;
>  
>  	/* Disable the wakeup_enable bit if it has been set in msm_gpio_irq_reqres() */
> -	if (test_bit(d->hwirq, pctrl->skip_wake_irqs) && g->intr_wakeup_present_bit) {
> +	if (g->intr_wakeup_present_bit) {
>  		u32 intr_cfg;
>  
>  		raw_spin_lock_irqsave(&pctrl->lock, flags);
> 
> ---
> base-commit: b4e07588e743c989499ca24d49e752c074924a9a
> change-id: 20260430-enable_wakeup_capable_gpios-cb9439ae8772
> 
> Best regards,


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable
  2026-04-30  5:54 [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable Sneh Mankad
  2026-04-30  6:45 ` Maulik Shah (mkshah)
@ 2026-05-05 10:28 ` Linus Walleij
  2026-05-07  4:09   ` Sneh Mankad
  2026-05-05 10:43 ` Konrad Dybcio
  2 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2026-05-05 10:28 UTC (permalink / raw)
  To: Sneh Mankad
  Cc: Bjorn Andersson, Neil Armstrong, Krzysztof Kozlowski,
	linux-arm-msm, linux-gpio, linux-kernel, stable

On Thu, Apr 30, 2026 at 7:54 AM Sneh Mankad
<sneh.mankad@oss.qualcomm.com> wrote:

> The wakeup enable bit needs to be set irrespective of the SoC using PDC or
> MPM as wakeup capable irqchip to allow the GPIO interrupts to be forwarded
> to parent irqchip.
>
> This is set only for PDC irqchip using additional check skip_wake_irqs
> making it impossible for MPM irqchip to detect the GPIO interrupt during
> SoC low power mode since for MPM irqchip the skip_wake_irqs is always
> false.
>
> Remove skip_wake_irqs condition when setting wakeup enable bit to allow
> forwarding GPIO interrupts for SoCs using MPM irqchip too.
>
> Fixes: 76b446f5b86e ("pinctrl: qcom: handle intr_target_reg wakeup_present/enable bits")
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>

Good work here, also super-dangerous so if some more Qualcomm
engineers could pitch in on this patch it'd be great.

Is this an urgent (-rc) or nonurgent (next) fix?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable
  2026-04-30  5:54 [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable Sneh Mankad
  2026-04-30  6:45 ` Maulik Shah (mkshah)
  2026-05-05 10:28 ` Linus Walleij
@ 2026-05-05 10:43 ` Konrad Dybcio
  2026-05-07  4:07   ` Sneh Mankad
  2 siblings, 1 reply; 6+ messages in thread
From: Konrad Dybcio @ 2026-05-05 10:43 UTC (permalink / raw)
  To: Sneh Mankad, Bjorn Andersson, Linus Walleij, Neil Armstrong,
	Krzysztof Kozlowski
  Cc: linux-arm-msm, linux-gpio, linux-kernel, stable

On 4/30/26 7:54 AM, Sneh Mankad wrote:
> The wakeup enable bit needs to be set irrespective of the SoC using PDC or
> MPM as wakeup capable irqchip to allow the GPIO interrupts to be forwarded
> to parent irqchip.
> 
> This is set only for PDC irqchip using additional check skip_wake_irqs
> making it impossible for MPM irqchip to detect the GPIO interrupt during
> SoC low power mode since for MPM irqchip the skip_wake_irqs is always
> false.

This describes the impact, but not the reason why this happens. Please
expand on that.

Konrad

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable
  2026-05-05 10:43 ` Konrad Dybcio
@ 2026-05-07  4:07   ` Sneh Mankad
  0 siblings, 0 replies; 6+ messages in thread
From: Sneh Mankad @ 2026-05-07  4:07 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Linus Walleij, Neil Armstrong,
	Krzysztof Kozlowski
  Cc: linux-arm-msm, linux-gpio, linux-kernel, stable


On 05-May-26 4:13 PM, Konrad Dybcio wrote:
> On 4/30/26 7:54 AM, Sneh Mankad wrote:
>> The wakeup enable bit needs to be set irrespective of the SoC using PDC or
>> MPM as wakeup capable irqchip to allow the GPIO interrupts to be forwarded
>> to parent irqchip.
>>
>> This is set only for PDC irqchip using additional check skip_wake_irqs
>> making it impossible for MPM irqchip to detect the GPIO interrupt during
>> SoC low power mode since for MPM irqchip the skip_wake_irqs is always
>> false.
> This describes the impact, but not the reason why this happens. Please
> expand on that.
>
> Konrad

Ok Konrad, I will post v3 with proper reasoning mentioned.

Thanks,

Sneh


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable
  2026-05-05 10:28 ` Linus Walleij
@ 2026-05-07  4:09   ` Sneh Mankad
  0 siblings, 0 replies; 6+ messages in thread
From: Sneh Mankad @ 2026-05-07  4:09 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Bjorn Andersson, Neil Armstrong, Krzysztof Kozlowski,
	linux-arm-msm, linux-gpio, linux-kernel, stable

On 05-May-26 3:58 PM, Linus Walleij wrote:

> On Thu, Apr 30, 2026 at 7:54 AM Sneh Mankad
> <sneh.mankad@oss.qualcomm.com> wrote:
>
>> The wakeup enable bit needs to be set irrespective of the SoC using PDC or
>> MPM as wakeup capable irqchip to allow the GPIO interrupts to be forwarded
>> to parent irqchip.
>>
>> This is set only for PDC irqchip using additional check skip_wake_irqs
>> making it impossible for MPM irqchip to detect the GPIO interrupt during
>> SoC low power mode since for MPM irqchip the skip_wake_irqs is always
>> false.
>>
>> Remove skip_wake_irqs condition when setting wakeup enable bit to allow
>> forwarding GPIO interrupts for SoCs using MPM irqchip too.
>>
>> Fixes: 76b446f5b86e ("pinctrl: qcom: handle intr_target_reg wakeup_present/enable bits")
>> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Good work here, also super-dangerous so if some more Qualcomm
> engineers could pitch in on this patch it'd be great.
>
> Is this an urgent (-rc) or nonurgent (next) fix?
>
> Yours,
> Linus Walleij

Its fine if it gets picked in the next release.

Thanks

Sneh


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-05-07  4:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30  5:54 [PATCH] pinctrl: qcom: Unconditionally mark gpio as wakeup enable Sneh Mankad
2026-04-30  6:45 ` Maulik Shah (mkshah)
2026-05-05 10:28 ` Linus Walleij
2026-05-07  4:09   ` Sneh Mankad
2026-05-05 10:43 ` Konrad Dybcio
2026-05-07  4:07   ` Sneh Mankad

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox