From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Nowicki Subject: Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support Date: Thu, 29 Jan 2015 17:06:25 +0100 Message-ID: <54CA5A81.6000500@linaro.org> References: <1421247905-3749-1-git-send-email-hanjun.guo@linaro.org> <1421247905-3749-14-git-send-email-hanjun.guo@linaro.org> <54B8F2E3.80306@arm.com> <54B92245.6080306@arm.com> <20150129152922.GG8951@e104818-lin.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150129152922.GG8951@e104818-lin.cambridge.arm.com> Sender: linux-kernel-owner@vger.kernel.org To: Catalin Marinas , Grant Likely Cc: Marc Zyngier , "hanjun.guo@linaro.org" , "Rafael J. Wysocki" , Olof Johansson , Arnd Bergmann , Mark Rutland , Will Deacon , Lorenzo Pieralisi , "graeme.gregory@linaro.org" , Sudeep Holla , "jcm@redhat.com" , Jason Cooper , Bjorn Helgaas , Mark Brown , Rob Herring , Robert Richter , Randy Dunlap , Charles Garcia-Tobin , "phoenix.liyi@huawei.com" , Timur Tabi , "suravee.suthikulpanit@amd.com" , wangyijing@huawei.com List-Id: linux-acpi@vger.kernel.org On 29.01.2015 16:29, Catalin Marinas wrote: > On Tue, Jan 27, 2015 at 04:12:08PM +0000, Grant Likely wrote: >> On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier wrote: >>>>>> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) >>>>>> void __init init_IRQ(void) >>>>>> { >>>>>> irqchip_init(); >>>>>> + >>>>>> + if (!handle_arch_irq) >>>>>> + acpi_gic_init(); >>>>>> + >>>>> >>>>> Why isn't this called from irqchip_init? It would seem like the logical >>>>> spot to probe an interrupt controller. >>>> >>>> What has been done here isn't an unusual choice. We've got stuff all >>>> over the kernel that may or may not be implemented depending on what >>>> the architecture supports. If the function call is renamed to >>>> acpi_init_irq(), are you content? >>> >>> My (full) suggestion was to do it like we've done it for DT, and I don't >>> think I varied much from this point of view. Yes, calling it >>> acpi_irq_init() would be a good start, and having the ACPI-compatible >>> irqchips to be self-probable even better. >>> >>> >>> >>> Hell, if nobody beats me to it, maybe I'll just write that code, with >>> proper entry points in the various GIC drivers. Yes, this is >>> infrastructure. Maybe it is grossly overdesigned. But I really spend too >>> much time dealing with the crap that people are happy to pile on top of >>> the GIC code to be madly enthusiastic about the general "good enough" >>> attitude. >>> >>> >>> >>> Or to put it in a slightly more diplomatic way: If ACPI is to be our >>> future, can we please make the future look a bit better? >> >> Hi Marc, >> >> As per our off-list discussion, I completely agree. We don't want to >> be adding hack upon hack, and I will be first in line to NAK any >> patches taking that approach. However, for this initial series, it >> only supports exactly one GIC that can be set up by ACPI. Can we agree >> to leave it as is in this series, with the agreement that it will be >> replaced for v2m and v3 support with a proper pluggable initializer? > > Can we at least call it acpi_init_irq() and avoid #including > gic-specific header files? IOW hide the apci_gic_init() behind some > generically named macro until the full solution is in place. > Yes, we will move away gic specific bits from here. Tomasz From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932160AbbA2QGe (ORCPT ); Thu, 29 Jan 2015 11:06:34 -0500 Received: from mail-wi0-f180.google.com ([209.85.212.180]:61259 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755860AbbA2QGc (ORCPT ); Thu, 29 Jan 2015 11:06:32 -0500 Message-ID: <54CA5A81.6000500@linaro.org> Date: Thu, 29 Jan 2015 17:06:25 +0100 From: Tomasz Nowicki User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Catalin Marinas , Grant Likely CC: Marc Zyngier , "hanjun.guo@linaro.org" , "Rafael J. Wysocki" , Olof Johansson , Arnd Bergmann , Mark Rutland , Will Deacon , Lorenzo Pieralisi , "graeme.gregory@linaro.org" , Sudeep Holla , "jcm@redhat.com" , Jason Cooper , Bjorn Helgaas , Mark Brown , Rob Herring , Robert Richter , Randy Dunlap , Charles Garcia-Tobin , "phoenix.liyi@huawei.com" , Timur Tabi , "suravee.suthikulpanit@amd.com" , "wangyijing@huawei.com" , "linux-acpi@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linaro-acpi@lists.linaro.org" Subject: Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support References: <1421247905-3749-1-git-send-email-hanjun.guo@linaro.org> <1421247905-3749-14-git-send-email-hanjun.guo@linaro.org> <54B8F2E3.80306@arm.com> <54B92245.6080306@arm.com> <20150129152922.GG8951@e104818-lin.cambridge.arm.com> In-Reply-To: <20150129152922.GG8951@e104818-lin.cambridge.arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29.01.2015 16:29, Catalin Marinas wrote: > On Tue, Jan 27, 2015 at 04:12:08PM +0000, Grant Likely wrote: >> On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier wrote: >>>>>> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) >>>>>> void __init init_IRQ(void) >>>>>> { >>>>>> irqchip_init(); >>>>>> + >>>>>> + if (!handle_arch_irq) >>>>>> + acpi_gic_init(); >>>>>> + >>>>> >>>>> Why isn't this called from irqchip_init? It would seem like the logical >>>>> spot to probe an interrupt controller. >>>> >>>> What has been done here isn't an unusual choice. We've got stuff all >>>> over the kernel that may or may not be implemented depending on what >>>> the architecture supports. If the function call is renamed to >>>> acpi_init_irq(), are you content? >>> >>> My (full) suggestion was to do it like we've done it for DT, and I don't >>> think I varied much from this point of view. Yes, calling it >>> acpi_irq_init() would be a good start, and having the ACPI-compatible >>> irqchips to be self-probable even better. >>> >>> >>> >>> Hell, if nobody beats me to it, maybe I'll just write that code, with >>> proper entry points in the various GIC drivers. Yes, this is >>> infrastructure. Maybe it is grossly overdesigned. But I really spend too >>> much time dealing with the crap that people are happy to pile on top of >>> the GIC code to be madly enthusiastic about the general "good enough" >>> attitude. >>> >>> >>> >>> Or to put it in a slightly more diplomatic way: If ACPI is to be our >>> future, can we please make the future look a bit better? >> >> Hi Marc, >> >> As per our off-list discussion, I completely agree. We don't want to >> be adding hack upon hack, and I will be first in line to NAK any >> patches taking that approach. However, for this initial series, it >> only supports exactly one GIC that can be set up by ACPI. Can we agree >> to leave it as is in this series, with the agreement that it will be >> replaced for v2m and v3 support with a proper pluggable initializer? > > Can we at least call it acpi_init_irq() and avoid #including > gic-specific header files? IOW hide the apci_gic_init() behind some > generically named macro until the full solution is in place. > Yes, we will move away gic specific bits from here. Tomasz From mboxrd@z Thu Jan 1 00:00:00 1970 From: tomasz.nowicki@linaro.org (Tomasz Nowicki) Date: Thu, 29 Jan 2015 17:06:25 +0100 Subject: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support In-Reply-To: <20150129152922.GG8951@e104818-lin.cambridge.arm.com> References: <1421247905-3749-1-git-send-email-hanjun.guo@linaro.org> <1421247905-3749-14-git-send-email-hanjun.guo@linaro.org> <54B8F2E3.80306@arm.com> <54B92245.6080306@arm.com> <20150129152922.GG8951@e104818-lin.cambridge.arm.com> Message-ID: <54CA5A81.6000500@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 29.01.2015 16:29, Catalin Marinas wrote: > On Tue, Jan 27, 2015 at 04:12:08PM +0000, Grant Likely wrote: >> On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier wrote: >>>>>> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) >>>>>> void __init init_IRQ(void) >>>>>> { >>>>>> irqchip_init(); >>>>>> + >>>>>> + if (!handle_arch_irq) >>>>>> + acpi_gic_init(); >>>>>> + >>>>> >>>>> Why isn't this called from irqchip_init? It would seem like the logical >>>>> spot to probe an interrupt controller. >>>> >>>> What has been done here isn't an unusual choice. We've got stuff all >>>> over the kernel that may or may not be implemented depending on what >>>> the architecture supports. If the function call is renamed to >>>> acpi_init_irq(), are you content? >>> >>> My (full) suggestion was to do it like we've done it for DT, and I don't >>> think I varied much from this point of view. Yes, calling it >>> acpi_irq_init() would be a good start, and having the ACPI-compatible >>> irqchips to be self-probable even better. >>> >>> >>> >>> Hell, if nobody beats me to it, maybe I'll just write that code, with >>> proper entry points in the various GIC drivers. Yes, this is >>> infrastructure. Maybe it is grossly overdesigned. But I really spend too >>> much time dealing with the crap that people are happy to pile on top of >>> the GIC code to be madly enthusiastic about the general "good enough" >>> attitude. >>> >>> >>> >>> Or to put it in a slightly more diplomatic way: If ACPI is to be our >>> future, can we please make the future look a bit better? >> >> Hi Marc, >> >> As per our off-list discussion, I completely agree. We don't want to >> be adding hack upon hack, and I will be first in line to NAK any >> patches taking that approach. However, for this initial series, it >> only supports exactly one GIC that can be set up by ACPI. Can we agree >> to leave it as is in this series, with the agreement that it will be >> replaced for v2m and v3 support with a proper pluggable initializer? > > Can we at least call it acpi_init_irq() and avoid #including > gic-specific header files? IOW hide the apci_gic_init() behind some > generically named macro until the full solution is in place. > Yes, we will move away gic specific bits from here. Tomasz