On 28.08.2015 15:25, Marc Zyngier wrote:
On 17/08/15 14:23, Hanjun Guo wrote:
From: Tomasz Nowicki tn@semihalf.com
From the top level prospective this self-probe infrastructure works in the similar way as OF, however there are some internal differences.
For DT, the init fn is called once it finds compatible strings, but for ACPI there are no such thing. Instead, we initialize irqchips using static MADT table. Doing that, we need to relay on subtables presence which define details to corresponding IRQ controller. Also, for some subtable we can do additional matching, like for GIC distributor version. See patch for more detailed information.
This mechanism can also be used for clock declare and may also works on x86 for some table parsing too.
Signed-off-by: Tomasz Nowicki tn@semihalf.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/Makefile | 2 + drivers/acpi/irq.c | 110 ++++++++++++++++++++++++++++++++++++++ include/asm-generic/vmlinux.lds.h | 13 +++++ include/linux/acpi.h | 18 +++++++ include/linux/irqchip.h | 13 +++++ include/linux/mod_devicetable.h | 9 ++++ 6 files changed, 165 insertions(+) create mode 100644 drivers/acpi/irq.c
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 8321430..599f1df 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -27,6 +27,8 @@ acpi-$(CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT) += sleep.o acpi-y += device_pm.o acpi-$(CONFIG_ACPI_SLEEP) += proc.o
+# IRQ controller probe +acpi-y += irq.o
# # ACPI Bus and Device Drivers diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c new file mode 100644 index 0000000..4216b34 --- /dev/null +++ b/drivers/acpi/irq.c @@ -0,0 +1,110 @@ +/*
- Copyright (C) 2015, Linaro Ltd.
- Author: Tomasz Nowicki tomasz.nowicki@linaro.org
- Author: Hanjun Guo hanjun.guo@linaro.org
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program. If not, see http://www.gnu.org/licenses/.
- */
+#include <linux/acpi.h> +#include <linux/init.h>
+/*
- This special acpi_table_id is the sentinel at the end of the
- acpi_table_id[] array of all irqchips. It is automatically placed at
- the end of the array by the linker, thanks to being part of a
- special section.
- */
+static const struct acpi_table_id +irqchip_acpi_match_end __used __section(__irqchip_acpi_table_end); +extern struct acpi_table_id __irqchip_acpi_table[]; +static struct acpi_table_id *iterator;
+static int __init +acpi_match_gic_redist(struct acpi_subtable_header *header,
const unsigned long end)+{
- return 0;
+}
+static bool __init +acpi_gic_redist_is_present(void) +{
- int count;
- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
acpi_match_gic_redist, 0);- return count > 0;
+}
I was really dreading to review this series, and I was right... What are these two functions doing here? This file is supposed to be irqchip agnostic and only provide a generic framework for ACPI-based ipchips.
So NAK. You can do some filtering on a per irqchip basis so that we don't have to pollute the firmware abstraction with properties that only matter to a given irqchip.
+static int __init +acpi_match_madt_subtable(struct acpi_subtable_header *header,
const unsigned long end)+{
- struct acpi_madt_generic_distributor *dist;
- u8 gic_version = ACPI_MADT_GIC_VERSION_NONE;
- /* Found appropriated subtable, now try to do additional matching */
- switch (header->type) {
- case ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR:
dist = (struct acpi_madt_generic_distributor *)header;gic_version = dist->version;/** This is for backward compatibility with ACPI 5.1,* which has no gic_version field.*/if (gic_version == ACPI_MADT_GIC_VERSION_NONE) {/* It is GICv3/v4 if redistributor is present */if (acpi_gic_redist_is_present())gic_version = ACPI_MADT_GIC_VERSION_V3;elsegic_version = ACPI_MADT_GIC_VERSION_V2;return 0;Seriously? Can you explain the exact effect of that code? You can't possibly have tested it.
Apologies for this silly code. We tested it, but obviously not with ACPI 5.1.
Anyway I agree with your comments and your clean up looks much better.
Tomasz