Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 0D77660A73
Date:   Thu, 31 Jan 2019 09:34:35 -0700
From:   Lina Iyer <ilina@codeaurora.org>
To:     Stephen Boyd <swboyd@chromium.org>
Cc:     evgreen@chromium.org, marc.zyngier@arm.com,
        linux-kernel@vger.kernel.org, rplsssn@codeaurora.org,
        linux-arm-msm@vger.kernel.org, thierry.reding@gmail.com,
        bjorn.andersson@linaro.org, dianders@chromium.org,
        linus.walleij@linaro.org
Subject: Re: [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip
 hierarchy
Message-ID: <20190131163435.GB6069@codeaurora.org>
References: <20190124202205.7940-1-ilina@codeaurora.org>
 <20190124202205.7940-6-ilina@codeaurora.org>
 <154888832839.169292.6913084380036629941@swboyd.mtv.corp.google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <154888832839.169292.6913084380036629941@swboyd.mtv.corp.google.com>
User-Agent: Mutt/1.11.1 (2018-12-01)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Jan 30 2019 at 15:45 -0700, Stephen Boyd wrote:
>Quoting Lina Iyer (2019-01-24 12:22:02)
>> To allow GPIOs to wakeup the system from suspend or deep idle, the
>> wakeup capable GPIOs are setup in hierarchy with interrupts from the
>> wakeup-parent irqchip.
>>
>> In older SoC's, the TLMM will handover detection to the parent irqchip
>> and in newer SoC's, the parent irqchip may also be active as well as the
>> TLMM and therefore the GPIOs need to be masked at TLMM to avoid
>> duplicate interrupts. To enable both these configurations to exist,
>> allow the parent irqchip to dictate the TLMM irqchip's behavior when
>> masking/unmasking the interrupt.
>>
>> Co-developed-by: Stephen Boyd <swboyd@chromium.org>
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>>
>> ---
>> Changes in v2:
>>         - Fix bug when unmaksing PDC interrupt
>
>What was the bug? 
The problem was an incorrect merge (possibly manual), causing the PDC to
be not used at all. This is what I had -

 static void msm_gpio_irq_mask(struct irq_data *d)                                  
 {                                                                                  
         struct gpio_chip *gc = irq_data_get_irq_chip_data(d);                      
         struct msm_pinctrl *pctrl = gpiochip_get_data(gc);                         
         const struct msm_pingroup *g;                                              
         unsigned long flags;                                                       
         u32 val;                                                                   
                                                                                    
         g = &pctrl->soc->groups[d->hwirq];                                         
                                                                                    
         if (d->parent_data)                                                        
                 irq_chip_unmask_parent(d);                                         
                                                                                    
         /* Monitored by parent wakeup controller? Keep masked */                   
         if (test_bit(d->hwirq, pctrl->wakeup_masked_irqs))                         
                 return;                                                            
                                                                                    

The above unmask_parent() call in an irq_mask() callback was the bug.

>Is that why the mask callback in this gpio chip no
>longer calls the parent irq chip? We should keep calling the parent
>irqchip from what I can tell. Otherwise, we may never mask the irq at
>the PDC and only mask it at the GPIO level, which may not even care
>about it if it's being monitored by the PDC.
>
>This causes me to get a bunch of interrupts on my touchscreen when I
>touch it once vs. only a handful (like 4) when I fix it with the below
>patch:
>
Hmm... I did not see an issue in my local testing :(
Thanks for the fix.

>Can you fold it in?
>
Yes, I will fold it in and send a rev out with the fix.

Thanks,
Lina

>diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
>index dd72ec8fb8db..9b45219893bd 100644
>--- a/drivers/pinctrl/qcom/pinctrl-msm.c
>+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
>@@ -682,6 +682,9 @@ static void msm_gpio_irq_mask(struct irq_data *d)
> 	clear_bit(d->hwirq, pctrl->enabled_irqs);
>
> 	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
>+
>+	if (d->parent_data)
>+		irq_chip_mask_parent(d);
> }
>
> static void msm_gpio_irq_unmask(struct irq_data *d)