[PATCH v2] pinctrl: qcom: Unconditionally mark gpio as wakeup enable

Sneh Mankad posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
drivers/pinctrl/qcom/pinctrl-msm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH v2] pinctrl: qcom: Unconditionally mark gpio as wakeup enable
Posted by Sneh Mankad 1 month, 2 weeks ago
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>
Reviewed-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
Changes in v2:
- Modified comment to specify MPM HW as well.
- Spelling correction.
- Link to v1: https://lore.kernel.org/r/20260430-enable_wakeup_capable_gpios-v1-1-5de39bf06094@oss.qualcomm.com
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 45b3a2763eb85405fecdd4770ba3d4ab684563f0..6a24f9b5e4a979528ba6b5b87fd297c2783ec765 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -1242,12 +1242,12 @@ static int msm_gpio_irq_reqres(struct irq_data *d)
 	/*
 	 * 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.
+	 * wake irq in order to allow the interrupt event to be transferred to
+	 * the PDC/MPM HW.
 	 * 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>
Re: [PATCH v2] pinctrl: qcom: Unconditionally mark gpio as wakeup enable
Posted by Konrad Dybcio 1 month ago
On 4/30/26 11:20 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>
> Reviewed-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
> Changes in v2:
> - Modified comment to specify MPM HW as well.

$ b4 diff 20260430-enable_wakeup_capable_gpios-v2-1-8c26ac795318@oss.qualcomm.com

[...]


      ## drivers/pinctrl/qcom/pinctrl-msm.c ##
     @@ drivers/pinctrl/qcom/pinctrl-msm.c: static int msm_gpio_irq_reqres(struct irq_data *d)
    +   /*
    +    * 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.
    ++   * wake irq in order to allow the interrupt event to be transferred to
    ++   * the PDC/MPM HW.
         * While the name implies only the wakeup event, it's also required for
         * the interrupt event.
         */

This is not what I asked for.

Instead, please focus on explaining what skip_wake_irqs is, perhaps under
what conditions it is set, and how that differs for PDC vs MPM

Konrad
Re: [PATCH v2] pinctrl: qcom: Unconditionally mark gpio as wakeup enable
Posted by Sneh Mankad 4 hours ago

On 15-May-26 5:02 PM, Konrad Dybcio wrote:
> On 4/30/26 11:20 AM, Sneh Mankad wrote:

[...]

> 
> $ b4 diff 20260430-enable_wakeup_capable_gpios-v2-1-8c26ac795318@oss.qualcomm.com
> 
> [...]
> 
> 
>       ## drivers/pinctrl/qcom/pinctrl-msm.c ##
>      @@ drivers/pinctrl/qcom/pinctrl-msm.c: static int msm_gpio_irq_reqres(struct irq_data *d)
>     +   /*
>     +    * 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.
>     ++   * wake irq in order to allow the interrupt event to be transferred to
>     ++   * the PDC/MPM HW.
>          * While the name implies only the wakeup event, it's also required for
>          * the interrupt event.
>          */
> 
> This is not what I asked for.
> 
> Instead, please focus on explaining what skip_wake_irqs is, perhaps under
> what conditions it is set, and how that differs for PDC vs MPM
> 

I raised v2 in reply to Maulik's comment. By that time I had not seen your reply, apologies for that.
Will raise v3 with added description.

Thanks,
Sneh