LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/2] irqdomain: Export irq_domain_disconnect_hierarchy()
@ 2021-08-17 10:19 Maulik Shah
  2021-08-17 10:19 ` [PATCH 2/2] irqchip: qcom-pdc: Disconnect domain hierarchy for GPIO_NO_WAKE_IRQs Maulik Shah
  0 siblings, 1 reply; 4+ messages in thread
From: Maulik Shah @ 2021-08-17 10:19 UTC (permalink / raw)
  To: maz, tglx
  Cc: linux-kernel, linux-arm-msm, linux-gpio, bjorn.andersson,
	linus.walleij, tkjos, lsrao, Maulik Shah

Export irq_domain_disconnect_hierarchy() so irqchip module drivers
can use it.

Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
---
 kernel/irq/irqdomain.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 0eee481..19e83e9 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1216,6 +1216,7 @@ int irq_domain_disconnect_hierarchy(struct irq_domain *domain,
 	irqd->chip = ERR_PTR(-ENOTCONN);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(irq_domain_disconnect_hierarchy);
 
 static int irq_domain_trim_hierarchy(unsigned int virq)
 {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


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

* [PATCH 2/2] irqchip: qcom-pdc: Disconnect domain hierarchy for GPIO_NO_WAKE_IRQs
  2021-08-17 10:19 [PATCH 1/2] irqdomain: Export irq_domain_disconnect_hierarchy() Maulik Shah
@ 2021-08-17 10:19 ` Maulik Shah
  2021-08-18  9:31   ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Maulik Shah @ 2021-08-17 10:19 UTC (permalink / raw)
  To: maz, tglx
  Cc: linux-kernel, linux-arm-msm, linux-gpio, bjorn.andersson,
	linus.walleij, tkjos, lsrao, Maulik Shah

gpio_to_irq() reports error at irq_domain_trim_hierarchy() for non wakeup
capable GPIOs that do not have dedicated interrupt at GIC.

Since PDC irqchip do not allocate irq at parent GIC domain for such GPIOs
indicate same by using irq_domain_disconnect_hierarchy().

Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
---
 drivers/irqchip/qcom-pdc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
index 32d5920..0ba0461 100644
--- a/drivers/irqchip/qcom-pdc.c
+++ b/drivers/irqchip/qcom-pdc.c
@@ -324,8 +324,11 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
 	if (ret)
 		return ret;
 
-	if (hwirq == GPIO_NO_WAKE_IRQ)
+	if (hwirq == GPIO_NO_WAKE_IRQ) {
+		if (domain->parent)
+			irq_domain_disconnect_hierarchy(domain->parent, virq);
 		return 0;
+	}
 
 	parent_hwirq = get_parent_hwirq(hwirq);
 	if (parent_hwirq == PDC_NO_PARENT_IRQ)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


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

* Re: [PATCH 2/2] irqchip: qcom-pdc: Disconnect domain hierarchy for GPIO_NO_WAKE_IRQs
  2021-08-17 10:19 ` [PATCH 2/2] irqchip: qcom-pdc: Disconnect domain hierarchy for GPIO_NO_WAKE_IRQs Maulik Shah
@ 2021-08-18  9:31   ` Marc Zyngier
  2021-08-19 11:35     ` Maulik Shah
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2021-08-18  9:31 UTC (permalink / raw)
  To: Maulik Shah
  Cc: tglx, linux-kernel, linux-arm-msm, linux-gpio, bjorn.andersson,
	linus.walleij, tkjos, lsrao

Hi Maulik,

In the future, please always add a cover-letter email if sending a
series that has more than a single patch. This considerably helps the
tracking, and gives you an opportunity to explain what you are doing.

On Tue, 17 Aug 2021 11:19:06 +0100,
Maulik Shah <mkshah@codeaurora.org> wrote:
> 
> gpio_to_irq() reports error at irq_domain_trim_hierarchy() for non wakeup
> capable GPIOs that do not have dedicated interrupt at GIC.
> 
> Since PDC irqchip do not allocate irq at parent GIC domain for such GPIOs
> indicate same by using irq_domain_disconnect_hierarchy().
> 
> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
> ---
>  drivers/irqchip/qcom-pdc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
> index 32d5920..0ba0461 100644
> --- a/drivers/irqchip/qcom-pdc.c
> +++ b/drivers/irqchip/qcom-pdc.c
> @@ -324,8 +324,11 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
>  	if (ret)
>  		return ret;
>  
> -	if (hwirq == GPIO_NO_WAKE_IRQ)
> +	if (hwirq == GPIO_NO_WAKE_IRQ) {
> +		if (domain->parent)
> +			irq_domain_disconnect_hierarchy(domain->parent, virq);
>  		return 0;
> +	}
>  
>  	parent_hwirq = get_parent_hwirq(hwirq);
>  	if (parent_hwirq == PDC_NO_PARENT_IRQ)

It feels like you are papering over the core of the problem, which is
that most of the GPIO_NO_WAKE_IRQ stuff should simply go away now that
we have a way to drop parts of the hierarchy.

I had a go at that a few months back, but never had the opportunity to
actually test the resulting code[1]. Could you please give it a go and
let me know what breaks?

Thanks,

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/qcom-pdc-nowake&id=331b2ba388a4a79b5c40b8addf56cbe35099a410

-- 
Without deviation from the norm, progress is not possible.

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

* Re: [PATCH 2/2] irqchip: qcom-pdc: Disconnect domain hierarchy for GPIO_NO_WAKE_IRQs
  2021-08-18  9:31   ` Marc Zyngier
@ 2021-08-19 11:35     ` Maulik Shah
  0 siblings, 0 replies; 4+ messages in thread
From: Maulik Shah @ 2021-08-19 11:35 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: tglx, linux-kernel, linux-arm-msm, linux-gpio, bjorn.andersson,
	linus.walleij, tkjos, lsrao

Hi Marc,

On 8/18/2021 3:01 PM, Marc Zyngier wrote:
> Hi Maulik,
>
> In the future, please always add a cover-letter email if sending a
> series that has more than a single patch. This considerably helps the
> tracking, and gives you an opportunity to explain what you are doing.
sure. i included same in v2 now.
>
> On Tue, 17 Aug 2021 11:19:06 +0100,
> Maulik Shah <mkshah@codeaurora.org> wrote:
>> gpio_to_irq() reports error at irq_domain_trim_hierarchy() for non wakeup
>> capable GPIOs that do not have dedicated interrupt at GIC.
>>
>> Since PDC irqchip do not allocate irq at parent GIC domain for such GPIOs
>> indicate same by using irq_domain_disconnect_hierarchy().
>>
>> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
>> ---
>>   drivers/irqchip/qcom-pdc.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c
>> index 32d5920..0ba0461 100644
>> --- a/drivers/irqchip/qcom-pdc.c
>> +++ b/drivers/irqchip/qcom-pdc.c
>> @@ -324,8 +324,11 @@ static int qcom_pdc_gpio_alloc(struct irq_domain *domain, unsigned int virq,
>>   	if (ret)
>>   		return ret;
>>   
>> -	if (hwirq == GPIO_NO_WAKE_IRQ)
>> +	if (hwirq == GPIO_NO_WAKE_IRQ) {
>> +		if (domain->parent)
>> +			irq_domain_disconnect_hierarchy(domain->parent, virq);
>>   		return 0;
>> +	}
>>   
>>   	parent_hwirq = get_parent_hwirq(hwirq);
>>   	if (parent_hwirq == PDC_NO_PARENT_IRQ)
> It feels like you are papering over the core of the problem, which is
> that most of the GPIO_NO_WAKE_IRQ stuff should simply go away now that
> we have a way to drop parts of the hierarchy.
ok makes sense to disconnect from PDC domain itself instead only 
disconnecting for parent GIC domain.
>
> I had a go at that a few months back, but never had the opportunity to
> actually test the resulting code[1]. Could you please give it a go and
> let me know what breaks?
Thanks for the patch [1]. i tested and found below issues.

1.
For GPIO_NO_WAKE_IRQ case, The patch disconnects hierarchy for current 
domain (PDC)
However for parent domain (GIC) its don't call disconnect.
This leads to irq_domain_trim_hierarchy() still complain the error at 
parent domain.
To fix this, whenever irqchip disconnects hierarchy at its domain, it 
has to disconnect for all its parent domains too.

something like this works in qcom_pdc_gpio_alloc()

         if (hwirq == GPIO_NO_WAKE_IRQ) {
-               if (domain->parent) {
- irq_domain_disconnect_hierarchy(domain->parent, virq);

+               for (parent = domain; parent; parent = parent->parent) {
+                       ret = irq_domain_disconnect_hierarchy(parent, virq);
+                       if (ret)
+                               return ret;
                 }
                 return 0;
         }

2.  irq_domain_trim_hierarchy() has two issues.

     The first is tail is moving along with irqd of domain.
     so trimming do not start at correct parent domain.
     tails has to be initialized only once, starting from which we want 
to trim all the parent domains hierarchy.

     The second is the below check is not proper to find valid irq chip.
     say for both (PDC and GIC) domains the irqd->chip is set to -ENOTCONN.
     then irqd->chip check will still pass for domain (even if its 
-ENOTCONN) and if tail is set it will false complain.

     /* Can't have a valid irqchip after a trim marker */
-               if (irqd->chip && tail)
+               if (!IS_ERR(irqd->chip) && tail) {


I have picked up your change in v2 and added above mentioned issue fixes.
please take a look on v2.

Thanks,
Maulik
>
> Thanks,
>
> 	M.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/qcom-pdc-nowake&id=331b2ba388a4a79b5c40b8addf56cbe35099a410
>
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


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

end of thread, other threads:[~2021-08-19 11:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-17 10:19 [PATCH 1/2] irqdomain: Export irq_domain_disconnect_hierarchy() Maulik Shah
2021-08-17 10:19 ` [PATCH 2/2] irqchip: qcom-pdc: Disconnect domain hierarchy for GPIO_NO_WAKE_IRQs Maulik Shah
2021-08-18  9:31   ` Marc Zyngier
2021-08-19 11:35     ` Maulik Shah

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).