Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161628AbaKNRgD (ORCPT ); Fri, 14 Nov 2014 12:36:03 -0500 Received: from service87.mimecast.com ([91.220.42.44]:42549 "EHLO service87.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161180AbaKNRgB convert rfc822-to-8bit (ORCPT ); Fri, 14 Nov 2014 12:36:01 -0500 Message-ID: <54663D7C.7020104@arm.com> Date: Fri, 14 Nov 2014 17:35:56 +0000 From: Marc Zyngier User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130330 Thunderbird/17.0.5 MIME-Version: 1.0 To: Jiang Liu CC: Thomas Gleixner , LKML , Bjorn Helgaas , "grant.likely@linaro.org" , Yingjoe Chen , Yijing Wang Subject: Re: [patch 07/16] genirq: Introduce helper irq_domain_set_info() to reduce duplicated code References: <20141112133941.647950773@linutronix.de> <20141112134120.393705922@linutronix.de> <54662062.8030807@arm.com> <546622A1.1030308@linux.intel.com> In-Reply-To: <546622A1.1030308@linux.intel.com> X-Enigmail-Version: 1.4.6 X-OriginalArrivalTime: 14 Nov 2014 17:35:57.0660 (UTC) FILETIME=[730131C0:01D00031] X-MC-Unique: 114111417355901701 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14/11/14 15:41, Jiang Liu wrote: > On 2014/11/14 23:31, Marc Zyngier wrote: >> On 12/11/14 13:43, Thomas Gleixner wrote: >>> From: Jiang Liu >>> >>> Signed-off-by: Jiang Liu >>> Cc: Bjorn Helgaas >>> Cc: Grant Likely >>> Cc: Marc Zyngier >>> Cc: Yingjoe Chen >>> Cc: Yijing Wang >>> Signed-off-by: Thomas Gleixner >>> --- >>> include/linux/irqdomain.h | 5 +++++ >>> kernel/irq/irqdomain.c | 10 ++++++++++ >>> 2 files changed, 15 insertions(+) >>> >>> Index: tip/include/linux/irqdomain.h >>> =================================================================== >>> --- tip.orig/include/linux/irqdomain.h >>> +++ tip/include/linux/irqdomain.h >>> @@ -33,6 +33,7 @@ >>> #define _LINUX_IRQDOMAIN_H >>> >>> #include >>> +#include >>> #include >>> >>> struct device_node; >>> @@ -263,6 +264,10 @@ extern int irq_domain_set_hwirq_and_chip >>> irq_hw_number_t hwirq, >>> struct irq_chip *chip, >>> void *chip_data); >>> +extern void irq_domain_set_info(struct irq_domain *domain, unsigned int virq, >>> + irq_hw_number_t hwirq, struct irq_chip *chip, >>> + void *chip_data, irq_flow_handler_t handler, >>> + void *handler_data, const char *handler_name); >>> extern void irq_domain_reset_irq_data(struct irq_data *irq_data); >>> extern void irq_domain_free_irqs_common(struct irq_domain *domain, >>> int virq, int nr_irqs); >>> Index: tip/kernel/irq/irqdomain.c >>> =================================================================== >>> --- tip.orig/kernel/irq/irqdomain.c >>> +++ tip/kernel/irq/irqdomain.c >>> @@ -882,6 +882,16 @@ int irq_domain_set_hwirq_and_chip(struct >>> return 0; >>> } >>> >>> +void irq_domain_set_info(struct irq_domain *domain, unsigned int virq, >>> + irq_hw_number_t hwirq, struct irq_chip *chip, >>> + void *chip_data, irq_flow_handler_t handler, >>> + void *handler_data, const char *handler_name) >>> +{ >>> + irq_domain_set_hwirq_and_chip(domain, virq, hwirq, chip, chip_data); >>> + __irq_set_handler(virq, handler, 0, handler_name); >>> + irq_set_handler_data(virq, handler_data); >>> +} >>> + >> >> We still have the issue that, depending on where in the stack this is >> called, this will succeed or fail: If this is called from the inner >> irqchip, __irq_set_handler() will fail, as it will look at the outer >> domain as the (desc->irq_data.chip == &no_irq_chip) test fails (we >> haven't set the top level yet). >> >> I have this very imperfect workaround in my tree: >> >> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c >> index d028b34..91e6515 100644 >> --- a/kernel/irq/chip.c >> +++ b/kernel/irq/chip.c >> @@ -731,7 +731,16 @@ __irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained, >> if (!handle) { >> handle = handle_bad_irq; >> } else { >> - if (WARN_ON(desc->irq_data.chip == &no_irq_chip)) >> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY >> + struct irq_data *irq_data = &desc->irq_data; >> + while (irq_data) { >> + if (irq_data->chip != &no_irq_chip) >> + break; >> + irq_data = irq_data->parent_data; >> + } >> +#endif >> + >> + if (WARN_ON(!irq_data || irq_data->chip == &no_irq_chip)) >> goto out; >> } >> >> Which translate into: If there is at least one irqchip in the domain, >> it will probably sort itself out. Not ideal. Any real solution to >> this problem? >> >> GICv2 faces this exact problem, as some of its interrupts are used >> directly, and some others are used through the MSI domain. In the >> GIC driver, it is almost impossible to find out... > Hi Marc, > I prefer the above solution to relax the warning conditions. > Changing the calling order in irq_domain_ops->alloc() looks a little > strange, and other interrupt drivers may still run into the same issue. OK. Where do we from from there? Do you want a proper patch, or will you fold this into the existing code? Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/