Hi Timur,
On 14 May 2015 at 16:27, Fu Wei fu.wei@linaro.org wrote:
Hi Timur,
On 13 May 2015 at 06:44, Timur Tabi timur@codeaurora.org wrote:
On 05/12/2015 05:56 AM, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
Import SBSA Generic watchdog info in GTDT into platform device. That make ARM SBSA watchdog driver can work on the platform which boot with ACPI.
"Parse the SBSA Generic Watchdog ACPI table, and create a platform device with that information. This platform device is used by the ARM SBSA Generic Watchdog driver."
OK , thanks
Signed-off-by: Fu Wei fu.wei@linaro.org
arch/arm64/kernel/acpi.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 8b83955..90422b5 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,133 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size);}
+static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
int index)+{
struct platform_device *pdev;struct resource *res;u32 gsi, flags;int trigger, polarity;resource_size_t rf_base_phy, cf_base_phy;int err;/* Get SBSA Generic Watchdog info* from a watchdog type subtable in GTDT*/Please reformat.
/* * ..... */Fixed , thanks
rf_base_phy = (resource_size_t)wd->refresh_frame_address;cf_base_phy = (resource_size_t)wd->control_frame_address;gsi = wd->timer_interrupt;flags = wd->timer_flags;trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE: ACPI_LEVEL_SENSITIVE;polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ?ACPI_ACTIVE_LOW
: ACPI_ACTIVE_HIGH;pr_info("GTDT: found a sbsa-gwdt device @0x%llx/0x%llx gsi:%uflags:0x%x\n",
"an", not "a"
thanks
rf_base_phy, cf_base_phy, gsi, flags);if (!(rf_base_phy && cf_base_phy && gsi)) {pr_err("GTDT: failed geting the device info.\n");return -EINVAL;}pdev = platform_device_alloc("sbsa-gwdt", index);if (!pdev)return -ENOMEM;res = kcalloc(3, sizeof(*res), GFP_KERNEL);if (!res) {err = -ENOMEM;goto err_free_device;}res[0].start = rf_base_phy;res[0].end = rf_base_phy + SZ_4K - 1;res[0].name = "refresh";res[0].flags = IORESOURCE_MEM;res[1].start = cf_base_phy;res[1].end = cf_base_phy + SZ_4K - 1;res[1].name = "control";res[1].flags = IORESOURCE_MEM;res[2].start = acpi_register_gsi(NULL, gsi, trigger, polarity);What happened to moving the acpi_register_gsi() call into the driver, so that res[2].start contains the physical IRQ?
it just do the mapping, config gic with flags, return irq number. and in FDT (OF) driver, they do the same thing , so it makes sense to put the acpi_register_gsi() here.
that make driver get the same kind of value. is that right? let me know if i misunderstand some thing.
res[2].end = res[2].start;res[2].name = "ws0";res[2].flags = IORESOURCE_IRQ;err = platform_device_add_resources(pdev, res, 3);if (err)goto err_free_res;FYI, platform_device_add_resources() uses kmemdup(), so you don't need to use kcalloc() to allocate the res[] array. Instead, you can do this:
struct resource res[3];I have tried , but fail, not sure why , but once I use kcalloc() again, this func works.
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);If you call acpi_register_gsi() in this function, then you need to call acpi_unregister_gsi() on the ex
yes, you are right, fixed
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;struct acpi_gtdt_header *header;void *gtdt_subtable;int i, gwdt_index;int ret = 0;if (table->revision < 2) {I wonder if this should be != 2. Do you think that this code would work with version 3 without change?
that may work or may not. but we can fix this if we get revision 3 or higher.
pr_info("GTDT: Revision:%d doesn't support PlatformTimers.\n",
table->revision);return 0;}gtdt = container_of(table, struct acpi_table_gtdt, header);if (!gtdt->platform_timer_count) {pr_info("GTDT: No Platform Timer structures.\n");return 0;}gtdt_subtable = (void *)gtdt + gtdt->platform_timer_offset;for (i = 0, gwdt_index = 0; i < gtdt->platform_timer_count; i++) {if (gtdt_subtable > (void *)table + table->length) {pr_err("GTDT: subtable pointer overflows, badtable\n");
ret = -EINVAL;break;Just do "return -EINVAL" here.
yes, fixed
}header = (struct acpi_gtdt_header *)gtdt_subtable;Unnecessary typecast.
no, we need it
sorry, misunderstood this, we don't need it , Fixed it
if (header->type == ACPI_GTDT_TYPE_WATCHDOG) {How about
if (header->type != ACPI_GTDT_TYPE_WATCHDOG) continue;That would make the rest of the block format better.
no, we may reuse this func soon.
ret = acpi_gtdt_import_sbsa_gwdt((struct acpi_gtdt_watchdog*)gtdt_subtable,
Unnecessary typecast.
yes, thanks
gwdt_index);if (ret)pr_err("GTDT: failed to import subtable%d\n",
i);else++gwdt_index;gwdt_index++;same effect, but OK, we can use that .
}gtdt_subtable += header->length;}return ret;+}
+/* 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);+}
+arch_initcall(acpi_gtdt_init);
-- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
-- Best regards,
Fu Wei Software Engineer Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch Ph: +86 21 61221326(direct) Ph: +86 186 2020 4684 (mobile) Room 1512, Regus One Corporate Avenue,Level 15, One Corporate Avenue,222 Hubin Road,Huangpu District, Shanghai,China 200021