public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: marc.zyngier@arm•com (Marc Zyngier)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v3 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support
Date: Thu, 11 Sep 2014 13:01:35 +0100	[thread overview]
Message-ID: <54118F1F.4050501@arm.com> (raw)
In-Reply-To: <20140911114831.E22E6C408F0@trevor.secretlab.ca>

Hi Grant,

On 11/09/14 12:48, Grant Likely wrote:
> On Tue, 02 Sep 2014 13:48:37 +0200, Tomasz Nowicki <tomasz.nowicki@linaro•org> wrote:
>> On 01.09.2014 19:35, Marc Zyngier wrote:
>>> On 01/09/14 15:57, Hanjun Guo wrote:
>>>> From: Tomasz Nowicki <tomasz.nowicki@linaro•org>
>>>>
>>>> ACPI kernel uses MADT table for proper GIC initialization. It needs to
>>>> parse GIC related subtables, collect CPU interface and distributor
>>>> addresses and call driver initialization function (which is hardware
>>>> abstraction agnostic). In a similar way, FDT initialize GICv1/2.
>>>>
>>>> NOTE: This commit allow to initialize GICv1/2 only.
>>>
>>> I cannot help but notice that there is no support for KVM here. It'd be
>>> good to add a note to that effect, so that people do not expect
>>> virtualization support to be working when booting with ACPI.
>>
>> yes, it is worth mentioning!
>>
>>>
>>>> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro•org>
>>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro•org>
>>>> ---
>>>>   arch/arm64/include/asm/acpi.h        |    2 -
>>>>   arch/arm64/kernel/acpi.c             |   23 +++++++
>>>>   arch/arm64/kernel/irq.c              |    5 ++
>>>>   drivers/irqchip/irq-gic.c            |  114 ++++++++++++++++++++++++++++++++++
>>>>   include/linux/irqchip/arm-gic-acpi.h |   33 ++++++++++
>>>>   5 files changed, 175 insertions(+), 2 deletions(-)
>>>>   create mode 100644 include/linux/irqchip/arm-gic-acpi.h
>>>>
>>>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>>>> index a867467..5d2ab63 100644
>>>> --- a/arch/arm64/include/asm/acpi.h
>>>> +++ b/arch/arm64/include/asm/acpi.h
>>>> @@ -97,8 +97,6 @@ void __init acpi_smp_init_cpus(void);
>>>>   extern int (*acpi_suspend_lowlevel)(void);
>>>>   #define acpi_wakeup_address 0
>>>>
>>>> -#define ACPI_MAX_GIC_CPU_INTERFACE_ENTRIES 65535
>>>> -
>>>>   #else
>>>>
>>>>   static inline bool acpi_psci_present(void) { return false; }
>>>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>>>> index 354b912..b3b82b0 100644
>>>> --- a/arch/arm64/kernel/acpi.c
>>>> +++ b/arch/arm64/kernel/acpi.c
>>>> @@ -23,6 +23,7 @@
>>>>   #include <linux/irqdomain.h>
>>>>   #include <linux/bootmem.h>
>>>>   #include <linux/smp.h>
>>>> +#include <linux/irqchip/arm-gic-acpi.h>
>>>>
>>>>   #include <asm/cputype.h>
>>>>   #include <asm/cpu_ops.h>
>>>> @@ -313,6 +314,28 @@ void __init acpi_boot_table_init(void)
>>>>   		pr_err("Can't find FADT or error happened during parsing FADT\n");
>>>>   }
>>>>
>>>> +void __init acpi_gic_init(void)
>>>> +{
>>>> +	struct acpi_table_header *table;
>>>> +	acpi_status status;
>>>> +	acpi_size tbl_size;
>>>> +	int err;
>>>> +
>>>> +	status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, &table, &tbl_size);
>>>> +	if (ACPI_FAILURE(status)) {
>>>> +		const char *msg = acpi_format_exception(status);
>>>> +
>>>> +		pr_err("Failed to get MADT table, %s\n", msg);
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	err = gic_v2_acpi_init(table);
>>>> +	if (err)
>>>> +		pr_err("Failed to initialize GIC IRQ controller");
>>>
>>> What will happen when you get to implement GICv3 support? Another entry
>>> like this? Why isn't this entirely contained in the GIC driver? Do I
>>> sound like a stuck record?
>>
>> There will be another call to GICv3 init:
>> [...]
>> 	err = gic_v3_acpi_init(table);
>> 	if (err)
>> 		err = gic_v2_acpi_init(table);
>> 	if (err)
>> 		pr_err("Failed to initialize GIC IRQ controller");
>> [...]
>> This is the main reason I put common code here.
>>
>>>
>>>> +
>>>> +	early_acpi_os_unmap_memory((char *)table, tbl_size);
>>>> +}
>>>> +
>>>>   /*
>>>>    * acpi_suspend_lowlevel() - save kernel state and suspend.
>>>>    *
>>>> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
>>>> index 0f08dfd..c074d60 100644
>>>> --- a/arch/arm64/kernel/irq.c
>>>> +++ b/arch/arm64/kernel/irq.c
>>>> @@ -28,6 +28,7 @@
>>>>   #include <linux/irqchip.h>
>>>>   #include <linux/seq_file.h>
>>>>   #include <linux/ratelimit.h>
>>>> +#include <linux/irqchip/arm-gic-acpi.h>
>>>>
>>>>   unsigned long irq_err_count;
>>>>
>>>> @@ -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.
>>
>> irqchip.c is OF dependent, I want to decouple these from the very 
>> beginning.
> 
> It doesn't have to be that way, but given that ARM64 is the only
> platform in the foreseeable future that will use ACPI irq setup, it
> doesn't make sense to put it in there.

I have a different perspective. There is no reason to pollute the arch
code with something that is essentially platform specific.

irqchip_init is the logical place to probe for an irqchip, and I fail to
see the point of sticking this code somewhere else. Why would ACPI be so
special that it requires additional hooks in the arch code?

Thanks,

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

  reply	other threads:[~2014-09-11 12:01 UTC|newest]

Thread overview: 117+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-01 14:57 [PATCH v3 00/17] Introduce ACPI for ARM64 based on ACPI 5.1 Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 01/17] ARM64: Move the init of cpu_logical_map(0) before unflatten_device_tree() Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 02/17] ARM64 / ACPI: Get RSDP and ACPI boot-time tables Hanjun Guo
2014-09-09 16:26   ` Catalin Marinas
2014-09-09 16:41     ` Jon Masters
2014-09-09 16:44       ` Jon Masters
2014-09-09 17:15       ` Mark Rutland
2014-09-09 17:33         ` Jon Masters
2014-09-09 17:50         ` Lorenzo Pieralisi
2014-09-09 18:05           ` Sudeep Holla
2014-09-09 19:06             ` Jon Masters
2014-09-10 11:13               ` Hanjun Guo
2014-09-10 12:33                 ` Catalin Marinas
2014-09-10 21:51                   ` Grant Likely
2014-09-11 11:01                     ` Catalin Marinas
2014-09-14 15:40                       ` Grant Likely
2014-09-14 21:59                         ` Catalin Marinas
2014-09-15  3:53                           ` Grant Likely
2014-09-16  5:29                     ` Zheng, Lv
2014-09-10 21:41                 ` Grant Likely
2014-09-09 16:54     ` Mark Rutland
2014-09-10  7:30     ` Hanjun Guo
2014-09-10 21:37     ` Grant Likely
2014-09-01 14:57 ` [PATCH v3 03/17] ARM64 / ACPI: Introduce lowlevel suspend function Hanjun Guo
2014-09-09 16:35   ` Catalin Marinas
2014-09-09 22:04     ` Graeme Gregory
2014-09-01 14:57 ` [PATCH v3 04/17] ARM64 / ACPI: Introduce early_param for "acpi" Hanjun Guo
2014-09-09 16:37   ` Catalin Marinas
2014-09-09 17:17   ` Bjorn Helgaas
2014-09-09 22:14     ` Jon Masters
2014-09-10 13:04       ` Will Deacon
2014-09-10 13:21         ` Bjorn Helgaas
2014-09-10 18:30           ` Will Deacon
2014-09-10 21:58           ` Grant Likely
2014-09-01 14:57 ` [PATCH v3 05/17] ARM64 / ACPI: If we chose to boot from acpi then disable FDT Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 06/17] ARM64 / ACPI: Make PCI optional for ACPI on ARM64 Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 07/17] ARM64 / ACPI: Parse FADT table to get PSCI flags for PSCI init Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 08/17] ACPI / table: Print GIC information when MADT is parsed Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 09/17] ARM64 / ACPI: Parse MADT for SMP initialization Hanjun Guo
2014-09-03 17:21   ` Lorenzo Pieralisi
2014-09-04 15:29     ` Hanjun Guo
2014-09-09  4:29       ` Jon Masters
2014-09-09  5:11         ` Hanjun Guo
2014-09-09  5:34           ` Jon Masters
2014-09-09 16:52       ` Lorenzo Pieralisi
2014-09-09 17:00         ` Jon Masters
2014-09-09 17:02         ` Jon Masters
2014-09-09  4:23   ` Jon Masters
2014-09-09  4:57     ` Hanjun Guo
2014-09-09  5:44       ` Jon Masters
2014-09-09 16:00         ` Hanjun Guo
2014-09-09 16:04           ` Jon Masters
2014-09-09 16:14             ` Hanjun Guo
2014-09-11 14:15             ` Will Deacon
2014-09-12 21:30               ` Jon Masters
2014-09-11 10:24   ` Grant Likely
2014-09-01 14:57 ` [PATCH v3 10/17] ACPI / processor: Make it possible to get CPU hardware ID via GICC Hanjun Guo
2014-09-03 16:27   ` Lorenzo Pieralisi
2014-09-08 13:10     ` Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 11/17] ARM64 / ACPI: Introduce ACPI_IRQ_MODEL_GIC and register device's gsi Hanjun Guo
2014-09-11 11:08   ` Grant Likely
2014-09-11 11:34     ` Grant Likely
2014-09-12  9:42     ` Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 12/17] ACPI / table: Add new function to get table entries Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support Hanjun Guo
2014-09-01 17:35   ` Marc Zyngier
2014-09-02  8:28     ` [Linaro-acpi] " Alexander Spyridakis
2014-09-02 11:48     ` Tomasz Nowicki
2014-09-02 13:02       ` Marc Zyngier
2014-09-02 15:45         ` Hanjun Guo
2014-09-02 15:59           ` Marc Zyngier
2014-09-02 16:11           ` Sudeep Holla
2014-09-03 10:30           ` Marc Zyngier
2014-09-03 11:17             ` Hanjun Guo
2014-09-04 14:03               ` Hanjun Guo
2014-09-09  6:21             ` Jon Masters
2014-09-03  9:26         ` Tomasz Nowicki
2014-09-03 14:57           ` Arnd Bergmann
2014-09-05  8:52             ` Tomasz Nowicki
2014-09-05  9:47             ` Marc Zyngier
2014-09-05 10:13               ` [Linaro-acpi] " Arnd Bergmann
2014-09-05 10:36                 ` Tomasz Nowicki
2014-09-05 10:39                 ` Marc Zyngier
2014-09-05 10:49                   ` Tomasz Nowicki
2014-09-09  6:27             ` Jon Masters
2014-09-11 13:43         ` Grant Likely
2014-09-02 16:34       ` Catalin Marinas
2014-09-11 11:48       ` Grant Likely
2014-09-11 12:01         ` Marc Zyngier [this message]
2014-09-09  6:14     ` Jon Masters
2014-09-03 18:42   ` Arnd Bergmann
2014-09-04 10:10     ` Tomasz Nowicki
2014-09-04 10:14       ` Arnd Bergmann
2014-09-04 10:39         ` Tomasz Nowicki
2014-09-09  6:35     ` Jon Masters
2014-09-01 14:57 ` [PATCH v3 14/17] ARM64 / ACPI: Parse GTDT to initialize arch timer Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 15/17] ARM64 / ACPI: Select ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64 Hanjun Guo
2014-09-01 14:57 ` [PATCH v3 16/17] ARM64 / ACPI: Enable ARM64 in Kconfig Hanjun Guo
2014-09-11 15:18   ` Lorenzo Pieralisi
2014-09-01 14:57 ` [PATCH v3 17/17] Documentation: ACPI for ARM64 Hanjun Guo
2014-09-11 13:29 ` [PATCH v3 00/17] Introduce ACPI for ARM64 based on ACPI 5.1 Grant Likely
2014-09-11 13:49   ` Will Deacon
2014-09-12 21:38     ` Jon Masters
2014-09-12 21:43       ` Jon Masters
2014-09-15  4:21     ` Grant Likely
2014-09-11 14:23   ` Rafael J. Wysocki
2014-09-11 14:04     ` Grant Likely
2014-09-11 15:37   ` Catalin Marinas
2014-09-11 15:57     ` Sudeep Holla
2014-09-11 16:06       ` Graeme Gregory
2014-09-11 16:14         ` Sudeep Holla
2014-09-15  4:31     ` Grant Likely
2014-09-15  9:15       ` Catalin Marinas
2014-09-15 22:48         ` Grant Likely
2014-09-16 10:12           ` Catalin Marinas
2014-09-11 16:05   ` Olof Johansson
2014-09-15  4:37     ` Grant Likely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54118F1F.4050501@arm.com \
    --to=marc.zyngier@arm$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox