All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Revert "irqdomain: Don't set type when mapping an IRQ"
@ 2016-08-06  8:18 Linus Walleij
  2016-08-06 15:13 ` Marc Zyngier
  2016-08-06 23:01 ` kbuild test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Linus Walleij @ 2016-08-06  8:18 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, John Stultz
  Cc: linux-kernel, Linus Walleij, Jon Hunter, Björn Andersson,
	Stephen Boyd, Abhijeet Dharmapurikar

This reverts commit 1e2a7d78499ec8859d2b469051b7b80bad3b08aa.

When using the APQ8060 Dragonboard I have lost all interrupts from
the PMIC after this commit: power button, keypad, RTC alarm and
all GPIOs. Reverting the commit solves the issue.

The affected irqchip driver is drivers/mfd/pm8921-core.c

I cannot immediately see what the problem is, so if you have a
better solution than just reverting the patch, please suggest.

Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Björn Andersson <bjorn.andersson@linaro.org>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I am pretty sure that this is the same bug that John Stultz is
seeing on the Nexus 7, John: please confirm.
---
 include/linux/irqdomain.h |  3 ---
 kernel/irq/irqdomain.c    | 23 +++++------------------
 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index ffb84604c1de..1aee0fbe900e 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -455,9 +455,6 @@ static inline int irq_domain_alloc_irqs(struct irq_domain *domain,
 	return -1;
 }
 
-static inline void irq_domain_free_irqs(unsigned int virq,
-					unsigned int nr_irqs) { }
-
 static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
 {
 	return false;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 4752b43662e0..9ba6a6ec4408 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -567,7 +567,6 @@ static void of_phandle_args_to_fwspec(struct of_phandle_args *irq_data,
 unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 {
 	struct irq_domain *domain;
-	struct irq_data *irq_data;
 	irq_hw_number_t hwirq;
 	unsigned int type = IRQ_TYPE_NONE;
 	int virq;
@@ -615,11 +614,7 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 		 * it now and return the interrupt number.
 		 */
 		if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
-			irq_data = irq_get_irq_data(virq);
-			if (!irq_data)
-				return 0;
-
-			irqd_set_trigger_type(irq_data, type);
+			irq_set_irq_type(virq, type);
 			return virq;
 		}
 
@@ -639,18 +634,10 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 			return virq;
 	}
 
-	irq_data = irq_get_irq_data(virq);
-	if (!irq_data) {
-		if (irq_domain_is_hierarchy(domain))
-			irq_domain_free_irqs(virq, 1);
-		else
-			irq_dispose_mapping(virq);
-		return 0;
-	}
-
-	/* Store trigger type */
-	irqd_set_trigger_type(irq_data, type);
-
+	/* Set type if specified and different than the current one */
+	if (type != IRQ_TYPE_NONE &&
+	    type != irq_get_trigger_type(virq))
+		irq_set_irq_type(virq, type);
 	return virq;
 }
 EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);
-- 
2.7.4

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

* Re: [PATCH] Revert "irqdomain: Don't set type when mapping an IRQ"
  2016-08-06  8:18 [PATCH] Revert "irqdomain: Don't set type when mapping an IRQ" Linus Walleij
@ 2016-08-06 15:13 ` Marc Zyngier
  2016-08-06 23:01 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: Marc Zyngier @ 2016-08-06 15:13 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Thomas Gleixner, John Stultz, linux-kernel, Jon Hunter,
	Björn Andersson, Stephen Boyd, Abhijeet Dharmapurikar

On Sat,  6 Aug 2016 10:18:03 +0200
Linus Walleij <linus.walleij@linaro.org> wrote:

> This reverts commit 1e2a7d78499ec8859d2b469051b7b80bad3b08aa.
> 
> When using the APQ8060 Dragonboard I have lost all interrupts from
> the PMIC after this commit: power button, keypad, RTC alarm and
> all GPIOs. Reverting the commit solves the issue.
> 
> The affected irqchip driver is drivers/mfd/pm8921-core.c
> 
> I cannot immediately see what the problem is, so if you have a
> better solution than just reverting the patch, please suggest.
> 
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Björn Andersson <bjorn.andersson@linaro.org>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I am pretty sure that this is the same bug that John Stultz is
> seeing on the Nexus 7, John: please confirm.

Hi Linus,

Before blindly reverting this patch (which itself is going to cause
other things to break), I'd like to understand the failure mode.

Any chance you could instrument pm8xxx_irq_set_type() and see if we get
called (and which which parameters)? A call stack would be ideal. I'm
normally expecting __irq_set_trigger() to be called from
manage.c::__setup_irq(), but your failure mode would tend to indicate
that it is not going that way. Is there any irq line sharing going on
by any chance?

Also, John has now isolated the failure to a much simpler piece of code,
so I'd hope to narrow it down pretty quickly (see the other thread).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

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

* Re: [PATCH] Revert "irqdomain: Don't set type when mapping an IRQ"
  2016-08-06  8:18 [PATCH] Revert "irqdomain: Don't set type when mapping an IRQ" Linus Walleij
  2016-08-06 15:13 ` Marc Zyngier
@ 2016-08-06 23:01 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2016-08-06 23:01 UTC (permalink / raw)
  To: Linus Walleij
  Cc: kbuild-all, Thomas Gleixner, Marc Zyngier, John Stultz,
	linux-kernel, Linus Walleij, Jon Hunter, Björn Andersson,
	Stephen Boyd, Abhijeet Dharmapurikar

[-- Attachment #1: Type: text/plain, Size: 2151 bytes --]

Hi Linus,

[auto build test ERROR on tip/irq/core]
[also build test ERROR on next-20160805]
[cannot apply to v4.7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Linus-Walleij/Revert-irqdomain-Don-t-set-type-when-mapping-an-IRQ/20160807-064011
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   kernel/irq/irqdomain.c: In function 'irq_dispose_mapping':
>> kernel/irq/irqdomain.c:671:3: error: implicit declaration of function 'irq_domain_free_irqs' [-Werror=implicit-function-declaration]
      irq_domain_free_irqs(virq, 1);
      ^
   cc1: some warnings being treated as errors

vim +/irq_domain_free_irqs +671 kernel/irq/irqdomain.c

cc79ca691c Grant Likely 2012-02-16  665  
68700650e7 Grant Likely 2012-02-14  666  	domain = irq_data->domain;
68700650e7 Grant Likely 2012-02-14  667  	if (WARN_ON(domain == NULL))
cc79ca691c Grant Likely 2012-02-16  668  		return;
cc79ca691c Grant Likely 2012-02-16  669  
d16dcd3d18 Jon Hunter   2016-06-21  670  	if (irq_domain_is_hierarchy(domain)) {
d16dcd3d18 Jon Hunter   2016-06-21 @671  		irq_domain_free_irqs(virq, 1);
d16dcd3d18 Jon Hunter   2016-06-21  672  	} else {
ddaf144c61 Grant Likely 2013-06-10  673  		irq_domain_disassociate(domain, virq);
cc79ca691c Grant Likely 2012-02-16  674  		irq_free_desc(virq);

:::::: The code at line 671 was first introduced by commit
:::::: d16dcd3d18759eb955e0325572d07457f93494f5 irqdomain: Fix disposal of mappings for interrupt hierarchies

:::::: TO: Jon Hunter <jonathanh@nvidia.com>
:::::: CC: Thomas Gleixner <tglx@linutronix.de>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 46451 bytes --]

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

end of thread, other threads:[~2016-08-06 22:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-06  8:18 [PATCH] Revert "irqdomain: Don't set type when mapping an IRQ" Linus Walleij
2016-08-06 15:13 ` Marc Zyngier
2016-08-06 23:01 ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.