On 05/04/2015 07:04 AM, fu.wei@linaro.org wrote:
From: Fu Wei <fu.wei@linaro.org>
Signed-off-by: Fu Wei <fu.wei@linaro.org>
Like others have said, you need to fix your patch descriptions.
Also, this patch should be 3/6. That is, before patch #4.
Blank line before the 'return'
---
arch/arm64/kernel/acpi.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 154 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 8b83955..833d21e 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -23,6 +23,7 @@
#include <linux/irqdomain.h>
#include <linux/memblock.h>
#include <linux/of_fdt.h>
+#include <linux/platform_device.h>
#include <linux/smp.h>
#include <asm/cputype.h>
@@ -343,3 +344,156 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size);
}
+
+static void __init *acpi_gtdt_subtable(struct acpi_table_gtdt *gtdt)
+{
+ if (!gtdt->platform_timer_count)
+ return NULL;
+
+ return ((void *)gtdt + (gtdt->platform_timer_offset));
+}
+
+static int __init acpi_gtdt_set_resource_irq_flags(u32 timer_flags)
+{
+ int flags;
+
+ switch (timer_flags &
+ (ACPI_GTDT_INTERRUPT_MODE | ACPI_GTDT_INTERRUPT_POLARITY)) {
+ case (ACPI_GTDT_INTERRUPT_MODE | ACPI_GTDT_INTERRUPT_POLARITY):
+ flags = IORESOURCE_IRQ_LOWEDGE | IORESOURCE_IRQ;
+ pr_debug("GTDT: sbsa-gwdt irq flags is low edge");
+ break;
+ case ACPI_GTDT_INTERRUPT_MODE:
+ flags = IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ;
+ pr_debug("GTDT: sbsa-gwdt irq flags is high edge");
+ break;
+ case ACPI_GTDT_INTERRUPT_POLARITY:
+ flags = IORESOURCE_IRQ_LOWLEVEL | IORESOURCE_IRQ;
+ pr_debug("GTDT: sbsa-gwdt irq flags is low level");
+ break;
+ default:
+ flags = IORESOURCE_IRQ_HIGHLEVEL | IORESOURCE_IRQ;
+ pr_debug("GTDT: sbsa-gwdt irq flags is high level");
+ }
+ pr_debug("GTDT: sbsa-gwdt irq flags:%d\n", flags);
+ return flags;
How about using acpi_register_gsi()? That way, you won't need to export map_generic_timer_interrupt().
+}
+
+static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_header *table,
+ int index)
+{
+ struct acpi_gtdt_watchdog *watchdog;
+ int irq, gsi;
+ int err;
+ u32 flags;
+ void *rf_base_phy, *cf_base_phy;
+ struct platform_device *pdev;
+ struct resource *res;
+
+ watchdog = container_of(table, struct acpi_gtdt_watchdog, header);
+
+ /* get SBSA Generic Watchdog device information from GTDT */
+ rf_base_phy = (void *)watchdog->refresh_frame_address;
+ cf_base_phy = (void *)watchdog->control_frame_address;
+ irq = watchdog->timer_interrupt;
+ flags = watchdog->timer_flags;
+
+ pr_info("GTDT: found a device @ 0x%p/0x%p irq:%d flags:%x\n",
+ rf_base_phy, cf_base_phy, irq, flags);
+ if (!rf_base_phy || !cf_base_phy || !irq) {
+ pr_err("GTDT: fail to get the device info.\n");
+ return -EINVAL;
+ }
+
+ gsi = map_generic_timer_interrupt(irq, flags);
Just merge the contents of acpi_gtdt_subtable() into here. No one else will call acpi_gtdt_subtable().
+
+ pdev = platform_device_alloc("sbsa-gwdt", index);
+ if (!pdev)
+ return -ENOMEM;
+
+ res = kcalloc(3, sizeof(*res), GFP_KERNEL);
+ if (!res)
+ goto err_free_device;
+
+ res[0].start = (resource_size_t)rf_base_phy;
+ res[0].end = (resource_size_t)(rf_base_phy + SZ_64K - 1);
+ res[0].name = "refresh_frame";
+ res[0].flags = IORESOURCE_MEM;
+
+ res[1].start = (resource_size_t)cf_base_phy;
+ res[1].end = (resource_size_t)(cf_base_phy + SZ_64K - 1);
+ res[1].name = "control_frame";
+ res[1].flags = IORESOURCE_MEM;
+
+ res[2].start = res[2].end = gsi;
+ res[2].name = "ws0";
+ res[2].flags = acpi_gtdt_set_resource_irq_flags(flags);
+
+ err = platform_device_add_resources(pdev, res, 3);
+ if (err)
+ goto err_free_res;
+
+ err = platform_device_add(pdev);
+ if (err)
+ goto err_free_res;
+
+ return 0;
+
+err_free_res:
+ kfree(res);
+
+err_free_device:
+ platform_device_put(pdev);
+ return err;
+
+}
+
+/* Initialize SBSA generic Watchdog platform device info from GTDT */
+static int __init acpi_gtdt_sbsa_gwdt_init(struct acpi_table_header *table)
+{
+ struct acpi_table_gtdt *gtdt;
+ int i, timer_count, gwdt_count = 0;
+ struct acpi_gtdt_header *header;
+ void __iomem *gtdt_subtable;
+
+ if (table->revision < 2) {
+ pr_warn("GTDT: Revision:%d doesn't support SBSA GWDT.\n",
+ table->revision);
+ return -ENODEV;
+ }
+
+ gtdt = container_of(table, struct acpi_table_gtdt, header);
+
+ timer_count = gtdt->platform_timer_count;
+
+ gtdt_subtable = acpi_gtdt_subtable(gtdt);
+ if (!gtdt_subtable) {
+ pr_warn("GTDT: No Platform Timer structures!\n");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < timer_count; i++) {
+ header = (struct acpi_gtdt_header *)gtdt_subtable;
+ if (header->type == ACPI_GTDT_TYPE_WATCHDOG) {
+ if (acpi_gtdt_import_sbsa_gwdt(header, i))
+ pr_err("GTDT: adding sbsa-gwdt%d "
+ "platform device fail! subtable: %d\n",
Please no exclamation marks in messages. Try to make your messages like normal English. How about this:
pr_err("GTDT: failed trying to create platform sbsa-gwdt%d\n", gwdt_count);
However, I'm not crazy about gwdt_count because you never pass that value to acpi_gtdt_import_sbsa_gwdt(). Instead you pass 'i', which is different value.
+ gwdt_count, i);
+ gwdt_count++;
+ }
+ gtdt_subtable += header->length;
You should add a safety check on the value of gtdt_subtable to make sure it doesn't go past the end of the buffer. timer_count might be corrupted.
+ }
+
+ return 0;
+}
+
+/* Initialize the SBSA Generic Watchdog presented in GTDT */
+static int __init acpi_gtdt_init(void)
+{
+ if (acpi_disabled)
+ return 0;
+
+ return acpi_table_parse(ACPI_SIG_GTDT, acpi_gtdt_sbsa_gwdt_init);
+}
I think instead of creating this function, you should move it into arch_timer_acpi_init().
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.