[v14,4/9] acpi/arm64: Add GTDT table parse driver

Message ID 1475086637-1914-5-git-send-email-fu.wei@linaro.org
State New
Headers show

Commit Message

Fu Wei Sept. 28, 2016, 6:17 p.m.
From: Fu Wei <fu.wei@linaro.org>


This patch adds support for parsing arch timer in GTDT,
provides some kernel APIs to parse all the PPIs and
always-on info in GTDT and export them.

By this driver, we can simplify arm_arch_timer drivers, and
separate the ACPI GTDT knowledge from it.

Signed-off-by: Fu Wei <fu.wei@linaro.org>

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

---
 arch/arm64/Kconfig          |   1 +
 drivers/acpi/arm64/Kconfig  |   3 +
 drivers/acpi/arm64/Makefile |   1 +
 drivers/acpi/arm64/gtdt.c   | 152 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/clocksource/Kconfig |   1 -
 include/linux/acpi.h        |   6 ++
 6 files changed, 163 insertions(+), 1 deletion(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mark Rutland Oct. 20, 2016, 4:37 p.m. | #1
Hi,

As a heads-up, on v4.9-rc1 I see conflicts at least against
arch/arm64/Kconfig. Luckily git am -3 seems to be able to fix that up
automatically, but this will need to be rebased before the next posting
and/or merging.

On Thu, Sep 29, 2016 at 02:17:12AM +0800, fu.wei@linaro.org wrote:
> +static int __init map_gt_gsi(u32 interrupt, u32 flags)

> +{

> +	int trigger, polarity;

> +

> +	if (!interrupt)

> +		return 0;


Urgh.

Only the secure interrupt (which we do not need) is optional in this
manner, and (hilariously), zero appears to also be a valid GSIV, per
figure 5-24 in the ACPI 6.1 spec.

So, I think that:

(a) we should not bother parsing the secure interrupt
(b) we should drop the check above
(c) we should report the spec issue to the ASWG

> +/*

> + * acpi_gtdt_c3stop - got c3stop info from GTDT

> + *

> + * Returns 1 if the timer is powered in deep idle state, 0 otherwise.

> + */

> +bool __init acpi_gtdt_c3stop(void)

> +{

> +	struct acpi_table_gtdt *gtdt = acpi_gtdt_desc.gtdt;

> +

> +	return !(gtdt->non_secure_el1_flags & ACPI_GTDT_ALWAYS_ON);

> +}


It looks like this can differ per interrupt. Shouldn't we check the
appropriate one?

> +int __init acpi_gtdt_init(struct acpi_table_header *table)

> +{

> +	void *start;

> +	struct acpi_table_gtdt *gtdt;

> +

> +	gtdt = container_of(table, struct acpi_table_gtdt, header);

> +

> +	acpi_gtdt_desc.gtdt = gtdt;

> +	acpi_gtdt_desc.gtdt_end = (void *)table + table->length;

> +

> +	if (table->revision < 2) {

> +		pr_debug("Revision:%d doesn't support Platform Timers.\n",

> +			 table->revision);

> +		return 0;

> +	}

> +

> +	if (!gtdt->platform_timer_count) {

> +		pr_debug("No Platform Timer.\n");

> +		return 0;

> +	}

> +

> +	start = (void *)gtdt + gtdt->platform_timer_offset;

> +	if (start < (void *)table + sizeof(struct acpi_table_gtdt)) {

> +		pr_err(FW_BUG "Failed to retrieve timer info from firmware: invalid data.\n");

> +		return -EINVAL;

> +	}

> +	acpi_gtdt_desc.platform_timer_start = start;

> +

> +	return gtdt->platform_timer_count;

> +}


This is never used as anything other than a status code.

Just return zero; we haven't parsed the platform timers themselves at
this point anyway.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fu Wei Oct. 26, 2016, 1:41 p.m. | #2
Hi Marc,

On 26 October 2016 at 20:11, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 26/10/16 12:10, Fu Wei wrote:

>> Hi Mark,

>>

>> On 21 October 2016 at 00:37, Mark Rutland <mark.rutland@arm.com> wrote:

>>> Hi,

>>>

>>> As a heads-up, on v4.9-rc1 I see conflicts at least against

>>> arch/arm64/Kconfig. Luckily git am -3 seems to be able to fix that up

>>> automatically, but this will need to be rebased before the next posting

>>> and/or merging.

>>>

>>> On Thu, Sep 29, 2016 at 02:17:12AM +0800, fu.wei@linaro.org wrote:

>>>> +static int __init map_gt_gsi(u32 interrupt, u32 flags)

>>>> +{

>>>> +     int trigger, polarity;

>>>> +

>>>> +     if (!interrupt)

>>>> +             return 0;

>>>

>>> Urgh.

>>>

>>> Only the secure interrupt (which we do not need) is optional in this

>>> manner, and (hilariously), zero appears to also be a valid GSIV, per

>>> figure 5-24 in the ACPI 6.1 spec.

>>>

>>> So, I think that:

>>>

>>> (a) we should not bother parsing the secure interrupt

>>

>> If I understand correctly, from this point of view, kernel don't

>> handle the secure interrupt.

>> But the current arm_arch_timer driver still enable/disable/request

>> PHYS_SECURE_PPI

>> with PHYS_NONSECURE_PPI.

>> That means we still need to parse the secure interrupt.

>> Please correct me, if I misunderstand something? :-)

>

> That's because we can use the per-cpu timer when 32bit Linux is running

> on the secure side (and we cannot distinguish between secure and

> non-secure at runtime). ACPI is 64bit only, and Linux on 64bit isn't

> supported on the secure side, so only registering the non-secure timer

> is perfectly acceptable.


Great thanks for your explanation :-)
So we just don't need to fill arch_timer_ppi[PHYS_SECURE_PPI] , just skip it.

>

> Thanks,

>

>         M.

> --

> Jazz is not dead. It just smells funny...




-- 
Best regards,

Fu Wei
Software Engineer
Red Hat
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hanjun Guo Nov. 11, 2016, 1:58 p.m. | #3
On 11/11/2016 09:46 PM, Hanjun Guo wrote:
> Hi Mark,

>

> Sorry for the late reply.

>

> On 10/21/2016 12:37 AM, Mark Rutland wrote:

>> Hi,

>>

>> As a heads-up, on v4.9-rc1 I see conflicts at least against

>> arch/arm64/Kconfig. Luckily git am -3 seems to be able to fix that up

>> automatically, but this will need to be rebased before the next posting

>> and/or merging.

>>

>> On Thu, Sep 29, 2016 at 02:17:12AM +0800, fu.wei@linaro.org wrote:

>>> +static int __init map_gt_gsi(u32 interrupt, u32 flags)

>>> +{

>>> +    int trigger, polarity;

>>> +

>>> +    if (!interrupt)

>>> +        return 0;

>>

>> Urgh.

>>

>> Only the secure interrupt (which we do not need) is optional in this

>> manner, and (hilariously), zero appears to also be a valid GSIV, per

>> figure 5-24 in the ACPI 6.1 spec.

>>

>> So, I think that:

>>

>> (a) we should not bother parsing the secure interrupt

>> (b) we should drop the check above

>> (c) we should report the spec issue to the ASWG

>

> Sorry, I willing to do that, but I need to figure out the issue here.

> What kind of issue in detail? do you mean that zero should not be valid

> for arch timer interrupts?


OK, I think you are referring to "we don't need the secure interrupt",
correct me if I'm wrong (still in jet lag...).

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch hide | download patch | download mbox

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index bc3f00f..0607728 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -2,6 +2,7 @@  config ARM64
 	def_bool y
 	select ACPI_CCA_REQUIRED if ACPI
 	select ACPI_GENERIC_GSI if ACPI
+	select ACPI_GTDT if ACPI
 	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
 	select ACPI_MCFG if ACPI
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
diff --git a/drivers/acpi/arm64/Kconfig b/drivers/acpi/arm64/Kconfig
index 4616da4..5a6f80f 100644
--- a/drivers/acpi/arm64/Kconfig
+++ b/drivers/acpi/arm64/Kconfig
@@ -4,3 +4,6 @@ 
 
 config ACPI_IORT
 	bool
+
+config ACPI_GTDT
+	bool
diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile
index 72331f2..1017def 100644
--- a/drivers/acpi/arm64/Makefile
+++ b/drivers/acpi/arm64/Makefile
@@ -1 +1,2 @@ 
 obj-$(CONFIG_ACPI_IORT) 	+= iort.o
+obj-$(CONFIG_ACPI_GTDT) 	+= gtdt.o
diff --git a/drivers/acpi/arm64/gtdt.c b/drivers/acpi/arm64/gtdt.c
new file mode 100644
index 0000000..b24844d
--- /dev/null
+++ b/drivers/acpi/arm64/gtdt.c
@@ -0,0 +1,152 @@ 
+/*
+ * ARM Specific GTDT table Support
+ *
+ * Copyright (C) 2016, Linaro Ltd.
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *         Fu Wei <fu.wei@linaro.org>
+ *         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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+
+#include <clocksource/arm_arch_timer.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "ACPI GTDT: " fmt
+
+struct acpi_gtdt_descriptor {
+	struct acpi_table_gtdt *gtdt;
+	void *platform_timer_start;
+	void *gtdt_end;
+};
+
+static struct acpi_gtdt_descriptor acpi_gtdt_desc __initdata;
+
+static inline void *next_platform_timer(void *platform_timer)
+{
+	struct acpi_gtdt_header *gh = platform_timer;
+
+	platform_timer += gh->length;
+	if (platform_timer < acpi_gtdt_desc.gtdt_end)
+		return platform_timer;
+
+	return NULL;
+}
+
+#define for_each_platform_timer(_g)				\
+	for (_g = acpi_gtdt_desc.platform_timer_start; _g;	\
+	     _g = next_platform_timer(_g))
+
+static inline bool is_timer_block(void *platform_timer)
+{
+	struct acpi_gtdt_header *gh = platform_timer;
+
+	return gh->type == ACPI_GTDT_TYPE_TIMER_BLOCK;
+}
+
+static inline bool is_watchdog(void *platform_timer)
+{
+	struct acpi_gtdt_header *gh = platform_timer;
+
+	return gh->type == ACPI_GTDT_TYPE_WATCHDOG;
+}
+
+static int __init map_gt_gsi(u32 interrupt, u32 flags)
+{
+	int trigger, polarity;
+
+	if (!interrupt)
+		return 0;
+
+	trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
+			: ACPI_LEVEL_SENSITIVE;
+
+	polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
+			: ACPI_ACTIVE_HIGH;
+
+	return acpi_register_gsi(NULL, interrupt, trigger, polarity);
+}
+
+/*
+ * Map the PPIs of per-cpu arch_timer.
+ * @type: the type of PPI
+ * Returns 0 if error.
+ */
+int __init acpi_gtdt_map_ppi(int type)
+{
+	struct acpi_table_gtdt *gtdt = acpi_gtdt_desc.gtdt;
+
+	switch (type) {
+	case PHYS_SECURE_PPI:
+		return map_gt_gsi(gtdt->secure_el1_interrupt,
+				  gtdt->secure_el1_flags);
+	case PHYS_NONSECURE_PPI:
+		return map_gt_gsi(gtdt->non_secure_el1_interrupt,
+				  gtdt->non_secure_el1_flags);
+	case VIRT_PPI:
+		return map_gt_gsi(gtdt->virtual_timer_interrupt,
+				  gtdt->virtual_timer_flags);
+
+	case HYP_PPI:
+		return map_gt_gsi(gtdt->non_secure_el2_interrupt,
+				  gtdt->non_secure_el2_flags);
+	default:
+		pr_err("Failed to map timer interrupt: invalid type.\n");
+	}
+
+	return 0;
+}
+
+/*
+ * acpi_gtdt_c3stop - got c3stop info from GTDT
+ *
+ * Returns 1 if the timer is powered in deep idle state, 0 otherwise.
+ */
+bool __init acpi_gtdt_c3stop(void)
+{
+	struct acpi_table_gtdt *gtdt = acpi_gtdt_desc.gtdt;
+
+	return !(gtdt->non_secure_el1_flags & ACPI_GTDT_ALWAYS_ON);
+}
+
+/*
+ * Get some basic info from GTDT table, and init the global variables above
+ * for all timers initialization of Generic Timer.
+ * This function does some validation on GTDT table.
+ */
+int __init acpi_gtdt_init(struct acpi_table_header *table)
+{
+	void *start;
+	struct acpi_table_gtdt *gtdt;
+
+	gtdt = container_of(table, struct acpi_table_gtdt, header);
+
+	acpi_gtdt_desc.gtdt = gtdt;
+	acpi_gtdt_desc.gtdt_end = (void *)table + table->length;
+
+	if (table->revision < 2) {
+		pr_debug("Revision:%d doesn't support Platform Timers.\n",
+			 table->revision);
+		return 0;
+	}
+
+	if (!gtdt->platform_timer_count) {
+		pr_debug("No Platform Timer.\n");
+		return 0;
+	}
+
+	start = (void *)gtdt + gtdt->platform_timer_offset;
+	if (start < (void *)table + sizeof(struct acpi_table_gtdt)) {
+		pr_err(FW_BUG "Failed to retrieve timer info from firmware: invalid data.\n");
+		return -EINVAL;
+	}
+	acpi_gtdt_desc.platform_timer_start = start;
+
+	return gtdt->platform_timer_count;
+}
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 5677886..b58d259 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -285,7 +285,6 @@  config CLKSRC_MPS2
 config ARM_ARCH_TIMER
 	bool
 	select CLKSRC_OF if OF
-	select CLKSRC_ACPI if ACPI
 
 config ARM_ARCH_TIMER_EVTSTREAM
 	bool "Enable ARM architected timer event stream generation by default"
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c5eaf2f..e2f9841 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -567,6 +567,12 @@  enum acpi_reconfig_event  {
 int acpi_reconfig_notifier_register(struct notifier_block *nb);
 int acpi_reconfig_notifier_unregister(struct notifier_block *nb);
 
+#ifdef CONFIG_ACPI_GTDT
+int acpi_gtdt_init(struct acpi_table_header *table);
+int acpi_gtdt_map_ppi(int type);
+bool acpi_gtdt_c3stop(void);
+#endif
+
 #else	/* !CONFIG_ACPI */
 
 #define acpi_disabled 1