Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933007AbbFWRid (ORCPT ); Tue, 23 Jun 2015 13:38:33 -0400 Received: from foss.arm.com ([217.140.101.70]:52002 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753479AbbFWRi0 (ORCPT ); Tue, 23 Jun 2015 13:38:26 -0400 Date: Tue, 23 Jun 2015 18:38:45 +0100 From: Lorenzo Pieralisi To: Hanjun Guo Cc: Marc Zyngier , Jason Cooper , Will Deacon , Catalin Marinas , "Rafael J. Wysocki" , Thomas Gleixner , Jiang Liu , Arnd Bergmann , Tomasz Nowicki , "grant.likely@linaro.org" , Olof Johansson , Wei Huang , "linux-arm-kernel@lists.infradead.org" , "linux-acpi@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linaro-acpi@lists.linaro.org" Subject: Re: [PATCH v2 6/9] irqchip / gic: Add stacked irqdomain support for ACPI based GICv2 init Message-ID: <20150623173845.GC31466@red-moon> References: <1434703572-26221-1-git-send-email-hanjun.guo@linaro.org> <1434703572-26221-7-git-send-email-hanjun.guo@linaro.org> <20150622172014.GB26129@red-moon> <5589772A.8070708@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5589772A.8070708@linaro.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2729 Lines: 82 On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote: [...] > >> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > >> index 8fc67bc..d1b2131 100644 > >> --- a/drivers/irqchip/irq-gic.c > >> +++ b/drivers/irqchip/irq-gic.c > >> @@ -851,15 +851,22 @@ static struct notifier_block gic_cpu_notifier = { > >> static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, > >> unsigned int nr_irqs, void *arg) > >> { > >> - int i, ret; > >> + int i; > >> irq_hw_number_t hwirq; > >> - unsigned int type = IRQ_TYPE_NONE; > >> - struct of_phandle_args *irq_data = arg; > >> > >> - ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args, > >> - irq_data->args_count, &hwirq, &type); > >> - if (ret) > >> - return ret; > >> + if (domain->of_node) { /* DT case */ > >> + int ret; > >> + unsigned int type = IRQ_TYPE_NONE; > >> + struct of_phandle_args *irq_data = arg; > >> + > >> + ret = gic_irq_domain_xlate(domain, irq_data->np, > >> + irq_data->args, > >> + irq_data->args_count, &hwirq, &type); > >> + if (ret) > >> + return ret; > >> + } else { /* ACPI case */ > >> + hwirq = (irq_hw_number_t)*(u32 *)arg; > >> + } > > > > If domain->of_node is NULL and system booted with DT the code above > > does not fail (and if it fails almost certainly that won't be graceful) > > but it should. > > how about the following logic? > > if (!domain->of_node && acpi_disabled) > return -ENODEV; > else if (domain->of_node) > dt related code; > else > ACPI related code; Code is not checking the node at present so: if (acpi_disabled) dt code; else ACPI code; would do, but that's a nit. > >> for (i = 0; i < nr_irqs; i++) > >> gic_irq_domain_map(domain, virq + i, hwirq + i); > >> @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > >> gic_irqs = 1020; > >> gic->gic_irqs = gic_irqs; > >> > >> - if (node) { /* DT case */ > >> + if (node || !acpi_disabled) { /* DT or ACPI case */ > >> gic->domain = irq_domain_add_linear(node, gic_irqs, > >> &gic_irq_domain_hierarchy_ops, > >> gic); I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI (for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here. Lorenzo -- 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/