From: Brandon Anderson brandon.anderson@amd.com
This is a proposal for ACPI driver probing of the ARM AMBA devices currently listed under ‘iofpga’ in the RTSM dts file. The main addition is drivers/amba/acpi.c which fits the role of an AMBA bus ‘connector resource’ for ACPI using struct amba_device.
I have not yet figured out the implementation details regarding clocks (handled in the dts file with clock-names). However, I have included a proposed ASL format for clock information in the DSDT example below. With the last patch that hacks the clock info, this prototype will run on the Foundation and RTSM models.
These patches require Hanjun’s fixed-clock patches to be applied first on top of a Linaro ACPI kernel: https://git.linaro.org/gitweb?p=arm/acpi/leg-kernel.git%3Ba=summary
Please comment on both the concept and the implementation.
Brandon Anderson (3): Remove UART and KMI entries from DTS file Prototype of AMBA bus 'connector resource' for ACPI Hack clock names to get prototype running
arch/arm64/boot/dts/foundation-v8-acpi.dts | 10 +- arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts | 4 + arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi | 4 + drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 172 +++++++++++++++++++++ drivers/clk/clk-fixed-rate.c | 15 +- 7 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 drivers/amba/acpi.c
From: Brandon Anderson brandon.anderson@amd.com
Remove DTS definition of UART and KMI so that ACPI definition in DSDT can take care of driver probing for these devices.
Signed-off-by: Brandon Anderson brandon.anderson@amd.com --- arch/arm64/boot/dts/foundation-v8-acpi.dts | 10 +++++++--- arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts | 4 ++++ arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi | 4 ++++ 3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/foundation-v8-acpi.dts b/arch/arm64/boot/dts/foundation-v8-acpi.dts index 7f57c53..f0671eb 100644 --- a/arch/arm64/boot/dts/foundation-v8-acpi.dts +++ b/arch/arm64/boot/dts/foundation-v8-acpi.dts @@ -15,12 +15,16 @@
chosen { };
+ /* + * Removed for ACPI + * aliases { serial0 = &v2m_serial0; serial1 = &v2m_serial1; serial2 = &v2m_serial2; serial3 = &v2m_serial3; }; + */
cpus { #address-cells = <2>; @@ -196,6 +200,9 @@ reg = <0x010000 0x1000>; };
+ /* + * Removed for ACPI + * v2m_serial0: uart@090000 { compatible = "arm,pl011", "arm,primecell"; reg = <0x090000 0x1000>; @@ -227,9 +234,6 @@ clocks = <&v2m_clk24mhz>, <&v2m_clk24mhz>; clock-names = "uartclk", "apb_pclk"; }; - /* - * Removed for ACPI - * virtio_block@0130000 { compatible = "virtio,mmio"; reg = <0x130000 0x1000>; diff --git a/arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts b/arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts index 7f348eb..0953fd0 100644 --- a/arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts +++ b/arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts @@ -20,12 +20,16 @@
chosen { };
+ /* + * Removed for ACPI + * aliases { serial0 = &v2m_serial0; serial1 = &v2m_serial1; serial2 = &v2m_serial2; serial3 = &v2m_serial3; }; + */
cpus { #address-cells = <2>; diff --git a/arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi b/arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi index bbaaa8e..ba4e364 100644 --- a/arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi +++ b/arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi @@ -100,6 +100,9 @@ clock-names = "mclk", "apb_pclk"; };
+ /* + * Removed for ACPI + * kmi@060000 { compatible = "arm,pl050", "arm,primecell"; reg = <0x060000 0x1000>; @@ -147,6 +150,7 @@ clocks = <&v2m_clk24mhz>, <&v2m_clk24mhz>; clock-names = "uartclk", "apb_pclk"; }; + */
wdt@0f0000 { compatible = "arm,sp805", "arm,primecell";
On 2013-10-22 8:33, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
Remove DTS definition of UART and KMI so that ACPI definition in DSDT can take care of driver probing for these devices.
Signed-off-by: Brandon Anderson brandon.anderson@amd.com
arch/arm64/boot/dts/foundation-v8-acpi.dts | 10 +++++++--- arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts | 4 ++++ arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi | 4 ++++ 3 files changed, 15 insertions(+), 3 deletions(-)
I think this patch should be the last one, because this patch will stop the UART working and break the git bisect.
Thanks Hanjun
From: Brandon Anderson brandon.anderson@amd.com
Add AMBA bus 'connector resource' module to call amba_device_add() for each device under the top-level 'AMBA0000' entry. Clock handling is yet to be implemented.
Signed-off-by: Brandon Anderson brandon.anderson@amd.com --- drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 162 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 drivers/amba/acpi.c
diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 37b8af8..fdfa990 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -36,6 +36,8 @@ static const struct acpi_device_id acpi_platform_device_ids[] = { { "LINA0007" }, /* armv8 pmu */ { "LINA0008" }, /* Fixed clock */
+ { "AMBA0000" }, + { } };
diff --git a/drivers/amba/Makefile b/drivers/amba/Makefile index 66e81c2..6d088e7 100644 --- a/drivers/amba/Makefile +++ b/drivers/amba/Makefile @@ -1,2 +1,2 @@ -obj-$(CONFIG_ARM_AMBA) += bus.o +obj-$(CONFIG_ARM_AMBA) += bus.o acpi.o obj-$(CONFIG_TEGRA_AHB) += tegra-ahb.o diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c new file mode 100644 index 0000000..04efcbd --- /dev/null +++ b/drivers/amba/acpi.c @@ -0,0 +1,162 @@ +/* + * AMBA Connector Resource for ACPI + * + * Copyright (C) 2013 Advanced Micro Devices, Inc. + * + * Author: Brandon Anderson brandon.anderson@amd.com + * + * 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. + */ + +#ifdef CONFIG_ACPI + +#include <linux/module.h> +#include <linux/amba/bus.h> +#include <linux/platform_device.h> +#include <linux/acpi.h> +#include <linux/clkdev.h> + +static int acpi_amba_add_resource(struct acpi_resource *ares, void *data) +{ + struct amba_device *dev = data; + struct resource r; + int irq_idx; + + switch (ares->type) + { + case ACPI_RESOURCE_TYPE_FIXED_MEMORY32: + if (!acpi_dev_resource_memory(ares, &dev->res)) { + pr_err("%s: failed to map memory resource\n", __func__); + } + break; + case ACPI_RESOURCE_TYPE_EXTENDED_IRQ: + for (irq_idx = 0; irq_idx < AMBA_NR_IRQS; irq_idx++) { + if (acpi_dev_resource_interrupt(ares, irq_idx, &r)) { + dev->irq[irq_idx] = r.start; + } else { + break; + } + } + + break; + default: + pr_debug("%s: unhandled acpi resource type= %d\n", __func__, + ares->type); + break; + } + + return 1; /* Tell ACPI core to skip this resource */ +} + +static void acpi_amba_register_clk(struct acpi_device *adev) +{ + /* TODO: Retrieve clock details from ACPI and register the appropriate + clock under this device for amba subsystem and drivers to reference */ +} + +/* acpi_amba_add_device() + * + * ACPI equivalent to of_amba_device_create() + */ +static acpi_status acpi_amba_add_device(acpi_handle handle, u32 lvl_not_used, + void *data, void **not_used) +{ + struct list_head resource_list; + struct acpi_device *adev; + struct amba_device *dev; + int ret; + struct platform_device *pdata = data; + + if (acpi_bus_get_device(handle, &adev)) { + pr_err("%s: acpi_bus_get_device failed\n", __func__); + return AE_OK; + } + + pr_debug("Creating amba device %s\n", dev_name(&adev->dev)); + + dev = amba_device_alloc(NULL, 0, 0); + if (!dev) { + pr_err("%s(): amba_device_alloc() failed for %s\n", + __func__, dev_name(&adev->dev)); + return AE_CTRL_TERMINATE; + } + + /* setup generic device info */ + dev->dev.coherent_dma_mask = ~0; + dev->dev.parent = pdata->dev.parent; + //dev->dev.platform_data = platform_data; //FIXME: is this needed by any drivers? + dev_set_name(&dev->dev, "%s", dev_name(&adev->dev)); + + /* setup amba-specific device info */ + dev->dma_mask = ~0; + + ACPI_HANDLE_SET(&dev->dev, handle); + + INIT_LIST_HEAD(&resource_list); + acpi_dev_get_resources(adev, &resource_list, + acpi_amba_add_resource, dev); + acpi_dev_free_resource_list(&resource_list); + + /* Add clocks */ + acpi_amba_register_clk(adev); + + /* Read AMBA hardware ID and add device to system. If a driver matching + * hardware ID has already been registered, bind this device to it. + * Otherwise, the platform subsystem will match up the hardware ID when + * the matching driver is registered. + */ + ret = amba_device_add(dev, &iomem_resource); + if (ret) { + pr_err("%s(): amba_device_add() failed (%d) for %s\n", + __func__, ret, dev_name(&adev->dev)); + goto err_free; + } + + return AE_OK; + +err_free: + amba_device_put(dev); + return AE_CTRL_TERMINATE; +} + +static int acpi_amba_bus_probe(struct platform_device *pdev) +{ + // see if there's a top-level clock to use as default for sub-devices + //TODO + + // probe each device + acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_HANDLE(&pdev->dev), 1, + acpi_amba_add_device, NULL, pdev, NULL); + + return 0; +} + +static const struct acpi_device_id amba_bus_acpi_match[] = { + { "AMBA0000", 0 }, + { }, +}; + +static struct platform_driver amba_bus_acpi_driver = { + .driver = { + .name = "amba-acpi", + .owner = THIS_MODULE, + .acpi_match_table = ACPI_PTR(amba_bus_acpi_match), + }, + .probe = acpi_amba_bus_probe, +}; + +static int __init acpi_amba_bus_init(void) +{ + return platform_driver_register(&amba_bus_acpi_driver); +} + +postcore_initcall(acpi_amba_bus_init); + +MODULE_AUTHOR("Brandon Anderson brandon.anderson@amd.com"); +MODULE_DESCRIPTION("ACPI Connector Resource for AMBA bus"); +MODULE_LICENSE("GPLv2"); +MODULE_ALIAS("platform:amba-acpi"); + +#endif //CONFIG_ACPI
On 2013-10-22 8:33, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
Add AMBA bus 'connector resource' module to call amba_device_add() for each device under the top-level 'AMBA0000' entry. Clock handling is yet to be implemented.
Signed-off-by: Brandon Anderson brandon.anderson@amd.com
drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 162 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 drivers/amba/acpi.c
diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 37b8af8..fdfa990 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -36,6 +36,8 @@ static const struct acpi_device_id acpi_platform_device_ids[] = { { "LINA0007" }, /* armv8 pmu */ { "LINA0008" }, /* Fixed clock */
- { "AMBA0000" },
- { }
}; diff --git a/drivers/amba/Makefile b/drivers/amba/Makefile index 66e81c2..6d088e7 100644 --- a/drivers/amba/Makefile +++ b/drivers/amba/Makefile @@ -1,2 +1,2 @@ -obj-$(CONFIG_ARM_AMBA) += bus.o +obj-$(CONFIG_ARM_AMBA) += bus.o acpi.o obj-$(CONFIG_TEGRA_AHB) += tegra-ahb.o diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c new file mode 100644 index 0000000..04efcbd --- /dev/null +++ b/drivers/amba/acpi.c @@ -0,0 +1,162 @@ +/*
- AMBA Connector Resource for ACPI
- Copyright (C) 2013 Advanced Micro Devices, Inc.
- Author: Brandon Anderson brandon.anderson@amd.com
- 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.
- */
+#ifdef CONFIG_ACPI
+#include <linux/module.h> +#include <linux/amba/bus.h> +#include <linux/platform_device.h> +#include <linux/acpi.h> +#include <linux/clkdev.h>
+static int acpi_amba_add_resource(struct acpi_resource *ares, void *data) +{
- struct amba_device *dev = data;
- struct resource r;
- int irq_idx;
- switch (ares->type)
- {
case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:if (!acpi_dev_resource_memory(ares, &dev->res)) {pr_err("%s: failed to map memory resource\n", __func__);}break;case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:for (irq_idx = 0; irq_idx < AMBA_NR_IRQS; irq_idx++) {if (acpi_dev_resource_interrupt(ares, irq_idx, &r)) {dev->irq[irq_idx] = r.start;} else {break;}}break;default:pr_debug("%s: unhandled acpi resource type= %d\n", __func__,ares->type);break;- }
- return 1; /* Tell ACPI core to skip this resource */
+}
+static void acpi_amba_register_clk(struct acpi_device *adev) +{
- /* TODO: Retrieve clock details from ACPI and register the appropriate
clock under this device for amba subsystem and drivers to reference */+}
+/* acpi_amba_add_device()
- ACPI equivalent to of_amba_device_create()
- */
+static acpi_status acpi_amba_add_device(acpi_handle handle, u32 lvl_not_used,
void *data, void **not_used)+{
- struct list_head resource_list;
- struct acpi_device *adev;
- struct amba_device *dev;
- int ret;
- struct platform_device *pdata = data;
- if (acpi_bus_get_device(handle, &adev)) {
pr_err("%s: acpi_bus_get_device failed\n", __func__);return AE_OK;- }
- pr_debug("Creating amba device %s\n", dev_name(&adev->dev));
- dev = amba_device_alloc(NULL, 0, 0);
- if (!dev) {
pr_err("%s(): amba_device_alloc() failed for %s\n",__func__, dev_name(&adev->dev));return AE_CTRL_TERMINATE;- }
- /* setup generic device info */
- dev->dev.coherent_dma_mask = ~0;
- dev->dev.parent = pdata->dev.parent;
- //dev->dev.platform_data = platform_data; //FIXME: is this needed by any drivers?
- dev_set_name(&dev->dev, "%s", dev_name(&adev->dev));
- /* setup amba-specific device info */
- dev->dma_mask = ~0;
- ACPI_HANDLE_SET(&dev->dev, handle);
- INIT_LIST_HEAD(&resource_list);
- acpi_dev_get_resources(adev, &resource_list,
acpi_amba_add_resource, dev);- acpi_dev_free_resource_list(&resource_list);
- /* Add clocks */
- acpi_amba_register_clk(adev);
- /* Read AMBA hardware ID and add device to system. If a driver matching
* hardware ID has already been registered, bind this device to it.* Otherwise, the platform subsystem will match up the hardware ID when* the matching driver is registered.- */
- ret = amba_device_add(dev, &iomem_resource);
- if (ret) {
pr_err("%s(): amba_device_add() failed (%d) for %s\n",__func__, ret, dev_name(&adev->dev));goto err_free;- }
- return AE_OK;
+err_free:
- amba_device_put(dev);
- return AE_CTRL_TERMINATE;
+}
+static int acpi_amba_bus_probe(struct platform_device *pdev) +{
- // see if there's a top-level clock to use as default for sub-devices
- //TODO
Hmm, how about add _DSM under each device object which need the clock? I mean some thing like (not the real ASL code): Device (SER0) { .... Method(_DSM, 4, NotSerialized) { clock path = "\_SB.SMB.CLK0" } } Then we can get the clock ACPI handle from the path, then ACPI dev, if we we attach the pointer of clock struct to that ACPI handle, we can directly get the clock struct.
\_SB.SMB.CLK0 -> clock ACPI handle -> clock ACPI dev -> get the clk data
What do you think?
Thanks Hanjun
On Wed, Oct 23, 2013 at 07:52:30PM +0800, Hanjun Guo wrote:
On 2013-10-22 8:33, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
Add AMBA bus 'connector resource' module to call amba_device_add() for each device under the top-level 'AMBA0000' entry. Clock handling is yet to be implemented.
Signed-off-by: Brandon Anderson brandon.anderson@amd.com
drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 162 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 drivers/amba/acpi.c
diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 37b8af8..fdfa990 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -36,6 +36,8 @@ static const struct acpi_device_id acpi_platform_device_ids[] = { { "LINA0007" }, /* armv8 pmu */ { "LINA0008" }, /* Fixed clock */
- { "AMBA0000" },
- { }
}; diff --git a/drivers/amba/Makefile b/drivers/amba/Makefile index 66e81c2..6d088e7 100644 --- a/drivers/amba/Makefile +++ b/drivers/amba/Makefile @@ -1,2 +1,2 @@ -obj-$(CONFIG_ARM_AMBA) += bus.o +obj-$(CONFIG_ARM_AMBA) += bus.o acpi.o obj-$(CONFIG_TEGRA_AHB) += tegra-ahb.o diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c new file mode 100644 index 0000000..04efcbd --- /dev/null +++ b/drivers/amba/acpi.c @@ -0,0 +1,162 @@ +/*
- AMBA Connector Resource for ACPI
- Copyright (C) 2013 Advanced Micro Devices, Inc.
- Author: Brandon Anderson brandon.anderson@amd.com
- 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.
- */
+#ifdef CONFIG_ACPI
+#include <linux/module.h> +#include <linux/amba/bus.h> +#include <linux/platform_device.h> +#include <linux/acpi.h> +#include <linux/clkdev.h>
+static int acpi_amba_add_resource(struct acpi_resource *ares, void *data) +{
- struct amba_device *dev = data;
- struct resource r;
- int irq_idx;
- switch (ares->type)
- {
case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:if (!acpi_dev_resource_memory(ares, &dev->res)) {pr_err("%s: failed to map memory resource\n", __func__);}break;case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:for (irq_idx = 0; irq_idx < AMBA_NR_IRQS; irq_idx++) {if (acpi_dev_resource_interrupt(ares, irq_idx, &r)) {dev->irq[irq_idx] = r.start;} else {break;}}break;default:pr_debug("%s: unhandled acpi resource type= %d\n", __func__,ares->type);break;- }
- return 1; /* Tell ACPI core to skip this resource */
+}
+static void acpi_amba_register_clk(struct acpi_device *adev) +{
- /* TODO: Retrieve clock details from ACPI and register the appropriate
clock under this device for amba subsystem and drivers to reference */+}
+/* acpi_amba_add_device()
- ACPI equivalent to of_amba_device_create()
- */
+static acpi_status acpi_amba_add_device(acpi_handle handle, u32 lvl_not_used,
void *data, void **not_used)+{
- struct list_head resource_list;
- struct acpi_device *adev;
- struct amba_device *dev;
- int ret;
- struct platform_device *pdata = data;
- if (acpi_bus_get_device(handle, &adev)) {
pr_err("%s: acpi_bus_get_device failed\n", __func__);return AE_OK;- }
- pr_debug("Creating amba device %s\n", dev_name(&adev->dev));
- dev = amba_device_alloc(NULL, 0, 0);
- if (!dev) {
pr_err("%s(): amba_device_alloc() failed for %s\n",__func__, dev_name(&adev->dev));return AE_CTRL_TERMINATE;- }
- /* setup generic device info */
- dev->dev.coherent_dma_mask = ~0;
- dev->dev.parent = pdata->dev.parent;
- //dev->dev.platform_data = platform_data; //FIXME: is this needed by any drivers?
- dev_set_name(&dev->dev, "%s", dev_name(&adev->dev));
- /* setup amba-specific device info */
- dev->dma_mask = ~0;
- ACPI_HANDLE_SET(&dev->dev, handle);
- INIT_LIST_HEAD(&resource_list);
- acpi_dev_get_resources(adev, &resource_list,
acpi_amba_add_resource, dev);- acpi_dev_free_resource_list(&resource_list);
- /* Add clocks */
- acpi_amba_register_clk(adev);
- /* Read AMBA hardware ID and add device to system. If a driver matching
* hardware ID has already been registered, bind this device to it.* Otherwise, the platform subsystem will match up the hardware ID when* the matching driver is registered.- */
- ret = amba_device_add(dev, &iomem_resource);
- if (ret) {
pr_err("%s(): amba_device_add() failed (%d) for %s\n",__func__, ret, dev_name(&adev->dev));goto err_free;- }
- return AE_OK;
+err_free:
- amba_device_put(dev);
- return AE_CTRL_TERMINATE;
+}
+static int acpi_amba_bus_probe(struct platform_device *pdev) +{
- // see if there's a top-level clock to use as default for sub-devices
- //TODO
Hmm, how about add _DSM under each device object which need the clock? I mean some thing like (not the real ASL code): Device (SER0) { .... Method(_DSM, 4, NotSerialized) { clock path = "\_SB.SMB.CLK0" } } Then we can get the clock ACPI handle from the path, then ACPI dev, if we we attach the pointer of clock struct to that ACPI handle, we can directly get the clock struct.
\_SB.SMB.CLK0 -> clock ACPI handle -> clock ACPI dev -> get the clk data
What do you think?
This is certainly the form we should be using based on ACPI vs DT discussion at kernel plumbers conference.
Graeme
Thank you Hanjun and Graeme for reviewing the clock definition.
Some devices have multiple clocks and they need to be labeled, so I propose the following format:
Method(_DSM, 4, NotSerialized) { Store (Package (6) { "clock-name", "KMIREFCLK", "\_SB_.CLK0", }, Local0) Return (Local0) }
Other custom parameters are required (for MMCI driver in particular), and they will fit into this format as well:
Method(_DSM, 4, NotSerialized) { Store (Package (9) { "clock-name", "mclk", "\_SB_.CLK0", "max-frequency", 12000000, "", "vmmc-supply", "", "", }, Local0)
Return (Local0) }
Three fields must be specified for each entry (each line above is an entry). Is this acceptable?
Brandon
-----Original Message----- From: Graeme Gregory [mailto:graeme.gregory@linaro.org] Sent: Wednesday, October 23, 2013 7:50 AM To: Hanjun Guo Cc: Anderson, Brandon; linaro-acpi@lists.linaro.org; linux-acpi@vger.kernel.org; lenb@kernel.org; rjw@sisk.pl; naresh.bhat@linaro.org; Suthikulpanit, Suravee Subject: Re: [RFC PATCH 2/3] ACPI: Prototype of AMBA bus 'connector resource'
On Wed, Oct 23, 2013 at 07:52:30PM +0800, Hanjun Guo wrote:
On 2013-10-22 8:33, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
Add AMBA bus 'connector resource' module to call amba_device_add() for each device under the top-level 'AMBA0000' entry. Clock handling is yet to be implemented.
Signed-off-by: Brandon Anderson brandon.anderson@amd.com
drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 162 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 drivers/amba/acpi.c
diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 37b8af8..fdfa990 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -36,6 +36,8 @@ static const struct acpi_device_id acpi_platform_device_ids[] = { { "LINA0007" }, /* armv8 pmu */ { "LINA0008" }, /* Fixed clock */
- { "AMBA0000" },
- { }
}; diff --git a/drivers/amba/Makefile b/drivers/amba/Makefile index 66e81c2..6d088e7 100644 --- a/drivers/amba/Makefile +++ b/drivers/amba/Makefile @@ -1,2 +1,2 @@ -obj-$(CONFIG_ARM_AMBA) += bus.o +obj-$(CONFIG_ARM_AMBA) += bus.o acpi.o obj-$(CONFIG_TEGRA_AHB) += tegra-ahb.o diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c new file mode 100644 index 0000000..04efcbd --- /dev/null +++ b/drivers/amba/acpi.c @@ -0,0 +1,162 @@ +/*
- AMBA Connector Resource for ACPI
- Copyright (C) 2013 Advanced Micro Devices, Inc.
- Author: Brandon Anderson brandon.anderson@amd.com
- 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.
- */
+#ifdef CONFIG_ACPI
+#include <linux/module.h> +#include <linux/amba/bus.h> +#include <linux/platform_device.h> +#include <linux/acpi.h> +#include <linux/clkdev.h>
+static int acpi_amba_add_resource(struct acpi_resource *ares, void +*data) {
- struct amba_device *dev = data;
- struct resource r;
- int irq_idx;
- switch (ares->type)
- {
case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:if (!acpi_dev_resource_memory(ares, &dev->res)) {pr_err("%s: failed to map memory resource\n", __func__);}break;case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:for (irq_idx = 0; irq_idx < AMBA_NR_IRQS; irq_idx++) {if (acpi_dev_resource_interrupt(ares, irq_idx, &r)) {dev->irq[irq_idx] = r.start;} else {break;}}break;default:pr_debug("%s: unhandled acpi resource type= %d\n", __func__,ares->type);break;- }
- return 1; /* Tell ACPI core to skip this resource */ }
+static void acpi_amba_register_clk(struct acpi_device *adev) {
- /* TODO: Retrieve clock details from ACPI and register the appropriate
clock under this device for amba subsystem and drivers to+reference */ }
+/* acpi_amba_add_device()
- ACPI equivalent to of_amba_device_create() */ static
+acpi_status acpi_amba_add_device(acpi_handle handle, u32 lvl_not_used,
void *data, void **not_used)+{
- struct list_head resource_list;
- struct acpi_device *adev;
- struct amba_device *dev;
- int ret;
- struct platform_device *pdata = data;
- if (acpi_bus_get_device(handle, &adev)) {
pr_err("%s: acpi_bus_get_device failed\n", __func__);return AE_OK;- }
- pr_debug("Creating amba device %s\n", dev_name(&adev->dev));
- dev = amba_device_alloc(NULL, 0, 0);
- if (!dev) {
pr_err("%s(): amba_device_alloc() failed for %s\n",__func__, dev_name(&adev->dev));return AE_CTRL_TERMINATE;- }
- /* setup generic device info */
- dev->dev.coherent_dma_mask = ~0;
- dev->dev.parent = pdata->dev.parent;
- //dev->dev.platform_data = platform_data; //FIXME: is this needed by any drivers?
- dev_set_name(&dev->dev, "%s", dev_name(&adev->dev));
- /* setup amba-specific device info */
- dev->dma_mask = ~0;
- ACPI_HANDLE_SET(&dev->dev, handle);
- INIT_LIST_HEAD(&resource_list);
- acpi_dev_get_resources(adev, &resource_list,
acpi_amba_add_resource, dev);- acpi_dev_free_resource_list(&resource_list);
- /* Add clocks */
- acpi_amba_register_clk(adev);
- /* Read AMBA hardware ID and add device to system. If a driver matching
* hardware ID has already been registered, bind this device to it.* Otherwise, the platform subsystem will match up the hardware ID when* the matching driver is registered.- */
- ret = amba_device_add(dev, &iomem_resource);
- if (ret) {
pr_err("%s(): amba_device_add() failed (%d) for %s\n",__func__, ret, dev_name(&adev->dev));goto err_free;- }
- return AE_OK;
+err_free:
- amba_device_put(dev);
- return AE_CTRL_TERMINATE;
+}
+static int acpi_amba_bus_probe(struct platform_device *pdev) {
- // see if there's a top-level clock to use as default for sub-devices
- //TODO
Hmm, how about add _DSM under each device object which need the clock? I mean some thing like (not the real ASL code): Device (SER0) { .... Method(_DSM, 4, NotSerialized) { clock path = "\_SB.SMB.CLK0" } } Then we can get the clock ACPI handle from the path, then ACPI dev, if we we attach the pointer of clock struct to that ACPI handle, we can directly get the clock struct.
\_SB.SMB.CLK0 -> clock ACPI handle -> clock ACPI dev -> get the clk data
What do you think?
This is certainly the form we should be using based on ACPI vs DT discussion at kernel plumbers conference.
Graeme
The decision coming out of the Kernel Plumbers is that these should be key=value types.
Certain characters like " " and ";" are not allowed in ACPI namespace so they can be used as seperators.
Method(_DSM, 4, NotSerialized) { Store (Package (6) { "clock-name", "KMIREFCLK \_SB_.CLK0", }, Local0) Return (Local0) }
And of course the _DSM has to check for the GUID of the key/value pairs data type before returning the package. It does not do it unconditionally.
Thanks
Graeme
On 23 October 2013 16:32, Anderson, Brandon Brandon.Anderson@amd.comwrote:
Thank you Hanjun and Graeme for reviewing the clock definition.
Some devices have multiple clocks and they need to be labeled, so I propose the following format:
Method(_DSM, 4, NotSerialized) { Store (Package (6) { "clock-name", "KMIREFCLK", "\\_SB_.CLK0", }, Local0) Return (Local0) }Other custom parameters are required (for MMCI driver in particular), and they will fit into this format as well:
Method(_DSM, 4, NotSerialized) { Store (Package (9) { "clock-name", "mclk", "\\_SB_.CLK0", "max-frequency", 12000000, "", "vmmc-supply", "", "", }, Local0) Return (Local0) }Three fields must be specified for each entry (each line above is an entry). Is this acceptable?
Brandon
-----Original Message----- From: Graeme Gregory [mailto:graeme.gregory@linaro.org] Sent: Wednesday, October 23, 2013 7:50 AM To: Hanjun Guo Cc: Anderson, Brandon; linaro-acpi@lists.linaro.org; linux-acpi@vger.kernel.org; lenb@kernel.org; rjw@sisk.pl; naresh.bhat@linaro.org; Suthikulpanit, Suravee Subject: Re: [RFC PATCH 2/3] ACPI: Prototype of AMBA bus 'connector resource'
On Wed, Oct 23, 2013 at 07:52:30PM +0800, Hanjun Guo wrote:
On 2013-10-22 8:33, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
Add AMBA bus 'connector resource' module to call amba_device_add() for
each device under the top-level 'AMBA0000' entry. Clock handling is yet to be implemented.
Signed-off-by: Brandon Anderson brandon.anderson@amd.com
drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 162
++++++++++++++++++++++++++++++++++++++++++
3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 drivers/amba/acpi.c
diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 37b8af8..fdfa990 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -36,6 +36,8 @@ static const struct acpi_device_id
acpi_platform_device_ids[] = {
{ "LINA0007" }, /* armv8 pmu */ { "LINA0008" }, /* Fixed clock */
- { "AMBA0000" },
- { }
};
diff --git a/drivers/amba/Makefile b/drivers/amba/Makefile index 66e81c2..6d088e7 100644 --- a/drivers/amba/Makefile +++ b/drivers/amba/Makefile @@ -1,2 +1,2 @@ -obj-$(CONFIG_ARM_AMBA) += bus.o +obj-$(CONFIG_ARM_AMBA) += bus.o acpi.o obj-$(CONFIG_TEGRA_AHB) += tegra-ahb.o diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c new file mode 100644 index 0000000..04efcbd --- /dev/null +++ b/drivers/amba/acpi.c @@ -0,0 +1,162 @@ +/*
- AMBA Connector Resource for ACPI
- Copyright (C) 2013 Advanced Micro Devices, Inc.
- Author: Brandon Anderson brandon.anderson@amd.com
- 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.
- */
+#ifdef CONFIG_ACPI
+#include <linux/module.h> +#include <linux/amba/bus.h> +#include <linux/platform_device.h> +#include <linux/acpi.h> +#include <linux/clkdev.h>
+static int acpi_amba_add_resource(struct acpi_resource *ares, void +*data) {
- struct amba_device *dev = data;
- struct resource r;
- int irq_idx;
- switch (ares->type)
- {
case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:if (!acpi_dev_resource_memory(ares, &dev->res)) {pr_err("%s: failed to map memoryresource\n", __func__);
}break;case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:for (irq_idx = 0; irq_idx < AMBA_NR_IRQS;irq_idx++) {
if (acpi_dev_resource_interrupt(ares,irq_idx, &r)) {
dev->irq[irq_idx] = r.start;} else {break;}}break;default:pr_debug("%s: unhandled acpi resource type= %d\n",__func__,
ares->type);break;- }
- return 1; /* Tell ACPI core to skip this resource */ }
+static void acpi_amba_register_clk(struct acpi_device *adev) {
- /* TODO: Retrieve clock details from ACPI and register the
appropriate
clock under this device for amba subsystem and drivers to+reference */ }
+/* acpi_amba_add_device()
- ACPI equivalent to of_amba_device_create() */ static
+acpi_status acpi_amba_add_device(acpi_handle handle, u32 lvl_not_used,
void *data, void **not_used)+{
- struct list_head resource_list;
- struct acpi_device *adev;
- struct amba_device *dev;
- int ret;
- struct platform_device *pdata = data;
- if (acpi_bus_get_device(handle, &adev)) {
pr_err("%s: acpi_bus_get_device failed\n", __func__);return AE_OK;- }
- pr_debug("Creating amba device %s\n", dev_name(&adev->dev));
- dev = amba_device_alloc(NULL, 0, 0);
- if (!dev) {
pr_err("%s(): amba_device_alloc() failed for %s\n",__func__, dev_name(&adev->dev));return AE_CTRL_TERMINATE;- }
- /* setup generic device info */
- dev->dev.coherent_dma_mask = ~0;
- dev->dev.parent = pdata->dev.parent;
- //dev->dev.platform_data = platform_data; //FIXME: is this needed
by any drivers?
- dev_set_name(&dev->dev, "%s", dev_name(&adev->dev));
- /* setup amba-specific device info */
- dev->dma_mask = ~0;
- ACPI_HANDLE_SET(&dev->dev, handle);
- INIT_LIST_HEAD(&resource_list);
- acpi_dev_get_resources(adev, &resource_list,
acpi_amba_add_resource, dev);- acpi_dev_free_resource_list(&resource_list);
- /* Add clocks */
- acpi_amba_register_clk(adev);
- /* Read AMBA hardware ID and add device to system. If a driver
matching
hardware ID has already been registered, bind this deviceto it.
Otherwise, the platform subsystem will match up thehardware ID when
the matching driver is registered.- */
- ret = amba_device_add(dev, &iomem_resource);
- if (ret) {
pr_err("%s(): amba_device_add() failed (%d) for %s\n",__func__, ret, dev_name(&adev->dev));goto err_free;- }
- return AE_OK;
+err_free:
- amba_device_put(dev);
- return AE_CTRL_TERMINATE;
+}
+static int acpi_amba_bus_probe(struct platform_device *pdev) {
- // see if there's a top-level clock to use as default for
sub-devices
- //TODO
Hmm, how about add _DSM under each device object which need the clock? I mean some thing like (not the real ASL code): Device (SER0) { .... Method(_DSM, 4, NotSerialized) { clock path = "\_SB.SMB.CLK0" } } Then we can get the clock ACPI handle from the path, then ACPI dev, if we we attach the pointer of clock struct to that ACPI handle, we can directly get the clock struct.
\_SB.SMB.CLK0 -> clock ACPI handle -> clock ACPI dev -> get the clk data
What do you think?
This is certainly the form we should be using based on ACPI vs DT discussion at kernel plumbers conference.
Graeme
Thank you, Graeme. This is exactly the clarification that I needed.
Are there any other requirements for the GUID implementation? Is it appropriate to calculate a single GUID with uuidgen to use in both the AMBA bus ACPI module and any device drivers that need to access these key/value pairs?
Brandon
From: Graeme Gregory [mailto:graeme.gregory@linaro.org] Sent: Friday, October 25, 2013 5:37 AM To: Anderson, Brandon Cc: Hanjun Guo; linaro-acpi@lists.linaro.org; linux-acpi@vger.kernel.org; lenb@kernel.org; rjw@sisk.pl; naresh.bhat@linaro.org; Suthikulpanit, Suravee Subject: Re: [RFC PATCH 2/3] ACPI: Prototype of AMBA bus 'connector resource'
The decision coming out of the Kernel Plumbers is that these should be key=value types.
Certain characters like " " and ";" are not allowed in ACPI namespace so they can be used as seperators.
Method(_DSM, 4, NotSerialized) { Store (Package (6) { "clock-name", "KMIREFCLK \_SB_.CLK0file:///\\_SB_.CLK0", }, Local0) Return (Local0) }
And of course the _DSM has to check for the GUID of the key/value pairs data type before returning the package. It does not do it unconditionally.
Thanks
Graeme
On 23 October 2013 16:32, Anderson, Brandon <Brandon.Anderson@amd.commailto:Brandon.Anderson@amd.com> wrote: Thank you Hanjun and Graeme for reviewing the clock definition.
Some devices have multiple clocks and they need to be labeled, so I propose the following format:
Method(_DSM, 4, NotSerialized) { Store (Package (6) { "clock-name", "KMIREFCLK", "\_SB_.CLK0file:///\\_SB_.CLK0", }, Local0) Return (Local0) }
Other custom parameters are required (for MMCI driver in particular), and they will fit into this format as well:
Method(_DSM, 4, NotSerialized) { Store (Package (9) { "clock-name", "mclk", "\_SB_.CLK0file:///\\_SB_.CLK0", "max-frequency", 12000000, "", "vmmc-supply", "", "", }, Local0)
Return (Local0) }
Three fields must be specified for each entry (each line above is an entry). Is this acceptable?
Brandon
-----Original Message----- From: Graeme Gregory [mailto:graeme.gregory@linaro.orgmailto:graeme.gregory@linaro.org] Sent: Wednesday, October 23, 2013 7:50 AM To: Hanjun Guo Cc: Anderson, Brandon; linaro-acpi@lists.linaro.orgmailto:linaro-acpi@lists.linaro.org; linux-acpi@vger.kernel.orgmailto:linux-acpi@vger.kernel.org; lenb@kernel.orgmailto:lenb@kernel.org; rjw@sisk.plmailto:rjw@sisk.pl; naresh.bhat@linaro.orgmailto:naresh.bhat@linaro.org; Suthikulpanit, Suravee Subject: Re: [RFC PATCH 2/3] ACPI: Prototype of AMBA bus 'connector resource'
On Wed, Oct 23, 2013 at 07:52:30PM +0800, Hanjun Guo wrote:
On 2013-10-22 8:33, brandon.anderson@amd.commailto:brandon.anderson@amd.com wrote:
From: Brandon Anderson <brandon.anderson@amd.commailto:brandon.anderson@amd.com>
Add AMBA bus 'connector resource' module to call amba_device_add() for each device under the top-level 'AMBA0000' entry. Clock handling is yet to be implemented.
Signed-off-by: Brandon Anderson <brandon.anderson@amd.commailto:brandon.anderson@amd.com>
drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 162 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 drivers/amba/acpi.c
diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 37b8af8..fdfa990 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -36,6 +36,8 @@ static const struct acpi_device_id acpi_platform_device_ids[] = { { "LINA0007" }, /* armv8 pmu */ { "LINA0008" }, /* Fixed clock */
- { "AMBA0000" },
- { }
};
diff --git a/drivers/amba/Makefile b/drivers/amba/Makefile index 66e81c2..6d088e7 100644 --- a/drivers/amba/Makefile +++ b/drivers/amba/Makefile @@ -1,2 +1,2 @@ -obj-$(CONFIG_ARM_AMBA) += bus.o +obj-$(CONFIG_ARM_AMBA) += bus.o acpi.o obj-$(CONFIG_TEGRA_AHB) += tegra-ahb.o diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c new file mode 100644 index 0000000..04efcbd --- /dev/null +++ b/drivers/amba/acpi.c @@ -0,0 +1,162 @@ +/*
- AMBA Connector Resource for ACPI
- Copyright (C) 2013 Advanced Micro Devices, Inc.
- Author: Brandon Anderson <brandon.anderson@amd.commailto:brandon.anderson@amd.com>
- 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.
- */
+#ifdef CONFIG_ACPI
+#include <linux/module.h> +#include <linux/amba/bus.h> +#include <linux/platform_device.h> +#include <linux/acpi.h> +#include <linux/clkdev.h>
+static int acpi_amba_add_resource(struct acpi_resource *ares, void +*data) {
- struct amba_device *dev = data;
- struct resource r;
- int irq_idx;
- switch (ares->type)
- {
case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:if (!acpi_dev_resource_memory(ares, &dev->res)) {pr_err("%s: failed to map memory resource\n", __func__);}break;case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:for (irq_idx = 0; irq_idx < AMBA_NR_IRQS; irq_idx++) {if (acpi_dev_resource_interrupt(ares, irq_idx, &r)) {dev->irq[irq_idx] = r.start;} else {break;}}break;default:pr_debug("%s: unhandled acpi resource type= %d\n", __func__,ares->type);break;- }
- return 1; /* Tell ACPI core to skip this resource */ }
+static void acpi_amba_register_clk(struct acpi_device *adev) {
- /* TODO: Retrieve clock details from ACPI and register the appropriate
clock under this device for amba subsystem and drivers to+reference */ }
+/* acpi_amba_add_device()
- ACPI equivalent to of_amba_device_create() */ static
+acpi_status acpi_amba_add_device(acpi_handle handle, u32 lvl_not_used,
void *data, void **not_used)+{
- struct list_head resource_list;
- struct acpi_device *adev;
- struct amba_device *dev;
- int ret;
- struct platform_device *pdata = data;
- if (acpi_bus_get_device(handle, &adev)) {
pr_err("%s: acpi_bus_get_device failed\n", __func__);return AE_OK;- }
- pr_debug("Creating amba device %s\n", dev_name(&adev->dev));
- dev = amba_device_alloc(NULL, 0, 0);
- if (!dev) {
pr_err("%s(): amba_device_alloc() failed for %s\n",__func__, dev_name(&adev->dev));return AE_CTRL_TERMINATE;- }
- /* setup generic device info */
- dev->dev.coherent_dma_mask = ~0;
- dev->dev.parent = pdata->dev.parent;
- //dev->dev.platform_data = platform_data; //FIXME: is this needed by any drivers?
- dev_set_name(&dev->dev, "%s", dev_name(&adev->dev));
- /* setup amba-specific device info */
- dev->dma_mask = ~0;
- ACPI_HANDLE_SET(&dev->dev, handle);
- INIT_LIST_HEAD(&resource_list);
- acpi_dev_get_resources(adev, &resource_list,
acpi_amba_add_resource, dev);- acpi_dev_free_resource_list(&resource_list);
- /* Add clocks */
- acpi_amba_register_clk(adev);
- /* Read AMBA hardware ID and add device to system. If a driver matching
hardware ID has already been registered, bind this device to it.
Otherwise, the platform subsystem will match up the hardware ID when
the matching driver is registered.- */
- ret = amba_device_add(dev, &iomem_resource);
- if (ret) {
pr_err("%s(): amba_device_add() failed (%d) for %s\n",__func__, ret, dev_name(&adev->dev));goto err_free;- }
- return AE_OK;
+err_free:
- amba_device_put(dev);
- return AE_CTRL_TERMINATE;
+}
+static int acpi_amba_bus_probe(struct platform_device *pdev) {
- // see if there's a top-level clock to use as default for sub-devices
- //TODO
Hmm, how about add _DSM under each device object which need the clock? I mean some thing like (not the real ASL code): Device (SER0) { .... Method(_DSM, 4, NotSerialized) { clock path = "\_SB.SMB.CLK0file:///\\_SB.SMB.CLK0" } } Then we can get the clock ACPI handle from the path, then ACPI dev, if we we attach the pointer of clock struct to that ACPI handle, we can directly get the clock struct.
\_SB.SMB.CLK0file:///\\_SB.SMB.CLK0 -> clock ACPI handle -> clock ACPI dev -> get the clk data
What do you think?
This is certainly the form we should be using based on ACPI vs DT discussion at kernel plumbers conference.
Graeme
Hi Brandon,
I do not recall if we decided on the GUID to be used or not. I guess for now it does not matter, it can always be altered on the way upstream.
This was the kernel thread that started the discussion BTW
https://lkml.org/lkml/2013/8/20/556
Graeme
On 25 October 2013 20:14, Anderson, Brandon Brandon.Anderson@amd.comwrote:
Thank you, Graeme. This is exactly the clarification that I needed.****
Are there any other requirements for the GUID implementation? Is it appropriate to calculate a single GUID with uuidgen to use in both the AMBA bus ACPI module and any device drivers that need to access these key/value pairs?****
Brandon****
*From:* Graeme Gregory [mailto:graeme.gregory@linaro.org] *Sent:* Friday, October 25, 2013 5:37 AM *To:* Anderson, Brandon *Cc:* Hanjun Guo; linaro-acpi@lists.linaro.org; linux-acpi@vger.kernel.org; lenb@kernel.org; rjw@sisk.pl; naresh.bhat@linaro.org; Suthikulpanit, Suravee
*Subject:* Re: [RFC PATCH 2/3] ACPI: Prototype of AMBA bus 'connector resource'****
The decision coming out of the Kernel Plumbers is that these should be key=value types.****
Certain characters like " " and ";" are not allowed in ACPI namespace so they can be used as seperators.****
Method(_DSM, 4, NotSerialized) { Store (Package (6) { "clock-name", "KMIREFCLK \\_SB_.CLK0", }, Local0) Return (Local0) }****
And of course the _DSM has to check for the GUID of the key/value pairs data type before returning the package. It does not do it unconditionally.
Thanks****
Graeme****
On 23 October 2013 16:32, Anderson, Brandon Brandon.Anderson@amd.com wrote:****
Thank you Hanjun and Graeme for reviewing the clock definition.
Some devices have multiple clocks and they need to be labeled, so I propose the following format:
Method(_DSM, 4, NotSerialized) { Store (Package (6) { "clock-name", "KMIREFCLK", "\\_SB_.CLK0", }, Local0) Return (Local0) }Other custom parameters are required (for MMCI driver in particular), and they will fit into this format as well:
Method(_DSM, 4, NotSerialized) { Store (Package (9) { "clock-name", "mclk", "\\_SB_.CLK0", "max-frequency", 12000000, "", "vmmc-supply", "", "", }, Local0) Return (Local0) }Three fields must be specified for each entry (each line above is an entry). Is this acceptable?
Brandon****
-----Original Message----- From: Graeme Gregory [mailto:graeme.gregory@linaro.org] Sent: Wednesday, October 23, 2013 7:50 AM To: Hanjun Guo Cc: Anderson, Brandon; linaro-acpi@lists.linaro.org; linux-acpi@vger.kernel.org; lenb@kernel.org; rjw@sisk.pl; naresh.bhat@linaro.org; Suthikulpanit, Suravee Subject: Re: [RFC PATCH 2/3] ACPI: Prototype of AMBA bus 'connector resource'
On Wed, Oct 23, 2013 at 07:52:30PM +0800, Hanjun Guo wrote:
On 2013-10-22 8:33, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
Add AMBA bus 'connector resource' module to call amba_device_add() for
each device under the top-level 'AMBA0000' entry. Clock handling is yet to be implemented.
Signed-off-by: Brandon Anderson brandon.anderson@amd.com
drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 162
++++++++++++++++++++++++++++++++++++++++++
3 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 drivers/amba/acpi.c
diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 37b8af8..fdfa990 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -36,6 +36,8 @@ static const struct acpi_device_id
acpi_platform_device_ids[] = {
{ "LINA0007" }, /* armv8 pmu */ { "LINA0008" }, /* Fixed clock */
- { "AMBA0000" },
- { }
};
diff --git a/drivers/amba/Makefile b/drivers/amba/Makefile index 66e81c2..6d088e7 100644 --- a/drivers/amba/Makefile +++ b/drivers/amba/Makefile @@ -1,2 +1,2 @@ -obj-$(CONFIG_ARM_AMBA) += bus.o +obj-$(CONFIG_ARM_AMBA) += bus.o acpi.o obj-$(CONFIG_TEGRA_AHB) += tegra-ahb.o diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c new file mode 100644 index 0000000..04efcbd --- /dev/null +++ b/drivers/amba/acpi.c @@ -0,0 +1,162 @@ +/*
- AMBA Connector Resource for ACPI
- Copyright (C) 2013 Advanced Micro Devices, Inc.
- Author: Brandon Anderson brandon.anderson@amd.com
- 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.
- */
+#ifdef CONFIG_ACPI
+#include <linux/module.h> +#include <linux/amba/bus.h> +#include <linux/platform_device.h> +#include <linux/acpi.h> +#include <linux/clkdev.h>
+static int acpi_amba_add_resource(struct acpi_resource *ares, void***
+*data) {****
- struct amba_device *dev = data;
- struct resource r;
- int irq_idx;
- switch (ares->type)
- {
case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:if (!acpi_dev_resource_memory(ares, &dev->res)) {pr_err("%s: failed to map memoryresource\n", __func__);
}break;case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:for (irq_idx = 0; irq_idx < AMBA_NR_IRQS;irq_idx++) {
if (acpi_dev_resource_interrupt(ares,irq_idx, &r)) {
dev->irq[irq_idx] = r.start;} else {break;}}break;default:pr_debug("%s: unhandled acpi resource type= %d\n",__func__,
ares->type);break;- }
- return 1; /* Tell ACPI core to skip this resource */ }
+****
+static void acpi_amba_register_clk(struct acpi_device *adev) {****
- /* TODO: Retrieve clock details from ACPI and register the
appropriate
clock under this device for amba subsystem and drivers to****+reference */ }****
+/* acpi_amba_add_device()
- ACPI equivalent to of_amba_device_create() */ static
+acpi_status acpi_amba_add_device(acpi_handle handle, u32 lvl_not_used,
void *data, void **not_used)+{
- struct list_head resource_list;
- struct acpi_device *adev;
- struct amba_device *dev;
- int ret;
- struct platform_device *pdata = data;
- if (acpi_bus_get_device(handle, &adev)) {
pr_err("%s: acpi_bus_get_device failed\n", __func__);return AE_OK;- }
- pr_debug("Creating amba device %s\n", dev_name(&adev->dev));
- dev = amba_device_alloc(NULL, 0, 0);
- if (!dev) {
pr_err("%s(): amba_device_alloc() failed for %s\n",__func__, dev_name(&adev->dev));return AE_CTRL_TERMINATE;- }
- /* setup generic device info */
- dev->dev.coherent_dma_mask = ~0;
- dev->dev.parent = pdata->dev.parent;
- //dev->dev.platform_data = platform_data; //FIXME: is this needed
by any drivers?
- dev_set_name(&dev->dev, "%s", dev_name(&adev->dev));
- /* setup amba-specific device info */
- dev->dma_mask = ~0;
- ACPI_HANDLE_SET(&dev->dev, handle);
- INIT_LIST_HEAD(&resource_list);
- acpi_dev_get_resources(adev, &resource_list,
acpi_amba_add_resource, dev);- acpi_dev_free_resource_list(&resource_list);
- /* Add clocks */
- acpi_amba_register_clk(adev);
- /* Read AMBA hardware ID and add device to system. If a driver
matching
hardware ID has already been registered, bind this deviceto it.
Otherwise, the platform subsystem will match up thehardware ID when
the matching driver is registered.- */
- ret = amba_device_add(dev, &iomem_resource);
- if (ret) {
pr_err("%s(): amba_device_add() failed (%d) for %s\n",__func__, ret, dev_name(&adev->dev));goto err_free;- }
- return AE_OK;
+err_free:
- amba_device_put(dev);
- return AE_CTRL_TERMINATE;
+}
+static int acpi_amba_bus_probe(struct platform_device *pdev) {****
- // see if there's a top-level clock to use as default for
sub-devices
- //TODO
Hmm, how about add _DSM under each device object which need the clock? I mean some thing like (not the real ASL code): Device (SER0) { .... Method(_DSM, 4, NotSerialized) { clock path = "\_SB.SMB.CLK0" } } Then we can get the clock ACPI handle from the path, then ACPI dev, if we we attach the pointer of clock struct to that ACPI handle, we can directly get the clock struct.
\_SB.SMB.CLK0 -> clock ACPI handle -> clock ACPI dev -> get the clk data
What do you think?
This is certainly the form we should be using based on ACPI vs DT discussion at kernel plumbers conference.
Graeme
From: Brandon Anderson brandon.anderson@amd.com
!!! For prototyping use only, not intended for final implementation !!!
Manually label the first fixed-clock as 'apb_pclk', and then manually register this clock with each device being registered. The amba subsystem and drivers require this clock when probing the device. This code will not be required once ACPI clock info is integrated into drivers/amba/acpi.c
Signed-off-by: Brandon Anderson brandon.anderson@amd.com --- drivers/amba/acpi.c | 14 ++++++++++++-- drivers/clk/clk-fixed-rate.c | 15 +++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c index 04efcbd..822000e 100644 --- a/drivers/amba/acpi.c +++ b/drivers/amba/acpi.c @@ -52,8 +52,18 @@ static int acpi_amba_add_resource(struct acpi_resource *ares, void *data)
static void acpi_amba_register_clk(struct acpi_device *adev) { - /* TODO: Retrieve clock details from ACPI and register the appropriate - clock under this device for amba subsystem and drivers to reference */ + /* HACK: re-register the first clock under this device so that other + subsystems will find it */ + struct clk *clk = clk_get_sys("LINA0008:00", "apb_pclk"); + + if (!IS_ERR(clk)) { + /* for amba_get_enable_pclk() */ + clk_register_clkdev(clk, "apb_pclk", dev_name(&adev->dev)); + /* for pl011 driver call to devm_get_clk() */ + clk_register_clkdev(clk, NULL, dev_name(&adev->dev)); + } else { + pr_debug("%s: cannot look up clock\n", __func__); + } }
/* acpi_amba_add_device() diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c index 226cefb..015002a 100644 --- a/drivers/clk/clk-fixed-rate.c +++ b/drivers/clk/clk-fixed-rate.c @@ -111,6 +111,8 @@ static int fixed_clk_probe_acpi(struct platform_device *pdev) { struct clk *clk = ERR_PTR(-ENODEV); unsigned long long rate = 0; acpi_status status; + static int clk_idx = 0; + char *dt_clk_name = NULL;
/* there is a corresponding FREQ method under fixed clock object */ status = acpi_evaluate_integer(ACPI_HANDLE(&pdev->dev), "FREQ", @@ -123,11 +125,20 @@ static int fixed_clk_probe_acpi(struct platform_device *pdev) { if (IS_ERR(clk)) return -ENODEV;
+ /* HACK: label the first clock as 'apb_pclk' */ + if (clk_idx++ == 0) { + dt_clk_name = "apb_pclk"; + } + + pr_debug("%s: register '%s' as '%s'\n", __FUNCTION__, dev_name(&pdev->dev), + dt_clk_name == NULL ? "<unknown>" : dt_clk_name); + /* * if we don't register the clk here, we can't get the clk * for AMBA bus when CONFIG_OF=n */ - return clk_register_clkdev(clk, NULL, dev_name(&pdev->dev)); } + return clk_register_clkdev(clk, dt_clk_name, dev_name(&pdev->dev)); +} #else static inline int fixed_clk_probe_acpi(struct platform_device *pdev) { return -ENODEV; @@ -174,5 +185,5 @@ static int __init fixed_clk_init(void) * fixed clock will used for AMBA bus, UART and etc, so it should be * initialized early enough. */ -subsys_initcall(fixed_clk_init); +postcore_initcall(fixed_clk_init); #endif
On 2013-10-22 8:33, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
!!! For prototyping use only, not intended for final implementation !!!
Manually label the first fixed-clock as 'apb_pclk', and then manually register this clock with each device being registered. The amba subsystem and drivers require this clock when probing the device. This code will not be required once ACPI clock info is integrated into drivers/amba/acpi.c
Signed-off-by: Brandon Anderson brandon.anderson@amd.com
drivers/amba/acpi.c | 14 ++++++++++++-- drivers/clk/clk-fixed-rate.c | 15 +++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c index 04efcbd..822000e 100644 --- a/drivers/amba/acpi.c +++ b/drivers/amba/acpi.c @@ -52,8 +52,18 @@ static int acpi_amba_add_resource(struct acpi_resource *ares, void *data) static void acpi_amba_register_clk(struct acpi_device *adev) {
- /* TODO: Retrieve clock details from ACPI and register the appropriate
clock under this device for amba subsystem and drivers to reference */
- /* HACK: re-register the first clock under this device so that other
subsystems will find it */- struct clk *clk = clk_get_sys("LINA0008:00", "apb_pclk");
if we can get the clk data as I said in the former email, this HACK is not needed any more.
- if (!IS_ERR(clk)) {
/* for amba_get_enable_pclk() */clk_register_clkdev(clk, "apb_pclk", dev_name(&adev->dev));/* for pl011 driver call to devm_get_clk() */clk_register_clkdev(clk, NULL, dev_name(&adev->dev));- } else {
pr_debug("%s: cannot look up clock\n", __func__);- }
} /* acpi_amba_add_device() diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c index 226cefb..015002a 100644 --- a/drivers/clk/clk-fixed-rate.c +++ b/drivers/clk/clk-fixed-rate.c @@ -111,6 +111,8 @@ static int fixed_clk_probe_acpi(struct platform_device *pdev) { struct clk *clk = ERR_PTR(-ENODEV); unsigned long long rate = 0; acpi_status status;
- static int clk_idx = 0;
- char *dt_clk_name = NULL;
/* there is a corresponding FREQ method under fixed clock object */ status = acpi_evaluate_integer(ACPI_HANDLE(&pdev->dev), "FREQ", @@ -123,11 +125,20 @@ static int fixed_clk_probe_acpi(struct platform_device *pdev) { if (IS_ERR(clk)) return -ENODEV;
- /* HACK: label the first clock as 'apb_pclk' */
- if (clk_idx++ == 0) {
dt_clk_name = "apb_pclk";- }
and here too.
may be I missed something, your comments are welcome :)
Thanks Hanjun
Yes, these hacks will be removed when the clocks are implemented in ACPI. I provided this patch only so people could compile and run the previous patches and provide feedback.
Brandon
-----Original Message----- From: Hanjun Guo [mailto:hanjun.guo@linaro.org] Sent: Wednesday, October 23, 2013 6:58 AM To: Anderson, Brandon; linaro-acpi@lists.linaro.org Cc: naresh.bhat@linaro.org; graeme.gregory@linaro.org; Suthikulpanit, Suravee Subject: Re: [RFC PATCH 3/3] ACPI: Hack clock names to get prototype running
On 2013-10-22 8:33, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
!!! For prototyping use only, not intended for final implementation !!!
Manually label the first fixed-clock as 'apb_pclk', and then manually register this clock with each device being registered. The amba subsystem and drivers require this clock when probing the device. This code will not be required once ACPI clock info is integrated into drivers/amba/acpi.c
Signed-off-by: Brandon Anderson brandon.anderson@amd.com
drivers/amba/acpi.c | 14 ++++++++++++-- drivers/clk/clk-fixed-rate.c | 15 +++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/amba/acpi.c b/drivers/amba/acpi.c index 04efcbd..822000e 100644 --- a/drivers/amba/acpi.c +++ b/drivers/amba/acpi.c @@ -52,8 +52,18 @@ static int acpi_amba_add_resource(struct acpi_resource *ares, void *data) static void acpi_amba_register_clk(struct acpi_device *adev) {
- /* TODO: Retrieve clock details from ACPI and register the appropriate
clock under this device for amba subsystem and drivers to reference */
- /* HACK: re-register the first clock under this device so that other
subsystems will find it */- struct clk *clk = clk_get_sys("LINA0008:00", "apb_pclk");
if we can get the clk data as I said in the former email, this HACK is not needed any more.
- if (!IS_ERR(clk)) {
/* for amba_get_enable_pclk() */clk_register_clkdev(clk, "apb_pclk", dev_name(&adev->dev));/* for pl011 driver call to devm_get_clk() */clk_register_clkdev(clk, NULL, dev_name(&adev->dev));- } else {
pr_debug("%s: cannot look up clock\n", __func__);- }
} /* acpi_amba_add_device() diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c index 226cefb..015002a 100644 --- a/drivers/clk/clk-fixed-rate.c +++ b/drivers/clk/clk-fixed-rate.c @@ -111,6 +111,8 @@ static int fixed_clk_probe_acpi(struct platform_device *pdev) { struct clk *clk = ERR_PTR(-ENODEV); unsigned long long rate = 0; acpi_status status;
- static int clk_idx = 0;
- char *dt_clk_name = NULL;
/* there is a corresponding FREQ method under fixed clock object */ status = acpi_evaluate_integer(ACPI_HANDLE(&pdev->dev), "FREQ", @@ -123,11 +125,20 @@ static int fixed_clk_probe_acpi(struct platform_device *pdev) { if (IS_ERR(clk)) return -ENODEV;
- /* HACK: label the first clock as 'apb_pclk' */
- if (clk_idx++ == 0) {
dt_clk_name = "apb_pclk";- }
and here too.
may be I missed something, your comments are welcome :)
Thanks Hanjun
Hi Anderson,
The patches applied, compiled (except there was a conflict in the last patch "Hack clock names to get prototype running"). Can you please post the ASL code changes patch ? looks like ASL changes are different from what we have written originally.
-Naresh
On 22 October 2013 06:03, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
This is a proposal for ACPI driver probing of the ARM AMBA devices currently listed under ‘iofpga’ in the RTSM dts file. The main addition is drivers/amba/acpi.c which fits the role of an AMBA bus ‘connector resource’ for ACPI using struct amba_device.
I have not yet figured out the implementation details regarding clocks (handled in the dts file with clock-names). However, I have included a proposed ASL format for clock information in the DSDT example below. With the last patch that hacks the clock info, this prototype will run on the Foundation and RTSM models.
These patches require Hanjun’s fixed-clock patches to be applied first on top of a Linaro ACPI kernel: https://git.linaro.org/gitweb?p=arm/acpi/leg-kernel.git%3Ba=summary
Please comment on both the concept and the implementation.
Brandon Anderson (3): Remove UART and KMI entries from DTS file Prototype of AMBA bus 'connector resource' for ACPI Hack clock names to get prototype running
arch/arm64/boot/dts/foundation-v8-acpi.dts | 10 +- arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts | 4 + arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi | 4 + drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 172 +++++++++++++++++++++ drivers/clk/clk-fixed-rate.c | 15 +- 7 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 drivers/amba/acpi.c
-- 1.7.9.5
Device (AMBA) { Name (_HID, "AMBA0000") /* the parallel to "arm,primecell" in DTS */ Name (_UID, 0) /* Define 'apb_pclk' as a default clock source since it is common with devices below */ Method(_DSM, 4, NotSerialized) { Store (Package (3) { "clock-name", "apb_pclk", "\\_SB_.CLK0", }, Local0) Return (Local0) } Device (KMI0) { Name (_ADR,0x1c060000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c060000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {44} }) Return (RBUF) } } Device (KMI1) { Name (_ADR,0x1c070000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c070000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {45} }) Return (RBUF) } } Device (SER0) { Name (_ADR,0x1c090000) // UART0 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c090000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {37} }) Return (RBUF) } } Device (SER1) { Name (_ADR,0x1c0a0000) // UART1 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0a0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {38} }) Return (RBUF) } } Device (SER2) { Name (_ADR,0x1c0b0000) // UART2 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0b0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {39} }) Return (RBUF) } } Device (SER3) { Name (_ADR,0x1c0c0000) // UART3 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0c0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {40} }) Return (RBUF) } } }
Hi Naresh,
Isn't the ASL in the first post of the series?
Graeme
On Tue, Oct 22, 2013 at 01:50:32PM +0530, Naresh Bhat wrote:
Hi Anderson,
The patches applied, compiled (except there was a conflict in the last patch "Hack clock names to get prototype running"). Can you please post the ASL code changes patch ? looks like ASL changes are different from what we have written originally.
-Naresh
On 22 October 2013 06:03, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
This is a proposal for ACPI driver probing of the ARM AMBA devices currently listed under ‘iofpga’ in the RTSM dts file. The main addition is drivers/amba/acpi.c which fits the role of an AMBA bus ‘connector resource’ for ACPI using struct amba_device.
I have not yet figured out the implementation details regarding clocks (handled in the dts file with clock-names). However, I have included a proposed ASL format for clock information in the DSDT example below. With the last patch that hacks the clock info, this prototype will run on the Foundation and RTSM models.
These patches require Hanjun’s fixed-clock patches to be applied first on top of a Linaro ACPI kernel: https://git.linaro.org/gitweb?p=arm/acpi/leg-kernel.git%3Ba=summary
Please comment on both the concept and the implementation.
Brandon Anderson (3): Remove UART and KMI entries from DTS file Prototype of AMBA bus 'connector resource' for ACPI Hack clock names to get prototype running
arch/arm64/boot/dts/foundation-v8-acpi.dts | 10 +- arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts | 4 + arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi | 4 + drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 172 +++++++++++++++++++++ drivers/clk/clk-fixed-rate.c | 15 +- 7 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 drivers/amba/acpi.c
-- 1.7.9.5
Device (AMBA) { Name (_HID, "AMBA0000") /* the parallel to "arm,primecell" in DTS */ Name (_UID, 0) /* Define 'apb_pclk' as a default clock source since it is common with devices below */ Method(_DSM, 4, NotSerialized) { Store (Package (3) { "clock-name", "apb_pclk", "\\_SB_.CLK0", }, Local0) Return (Local0) } Device (KMI0) { Name (_ADR,0x1c060000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c060000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {44} }) Return (RBUF) } } Device (KMI1) { Name (_ADR,0x1c070000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c070000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {45} }) Return (RBUF) } } Device (SER0) { Name (_ADR,0x1c090000) // UART0 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c090000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {37} }) Return (RBUF) } } Device (SER1) { Name (_ADR,0x1c0a0000) // UART1 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0a0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {38} }) Return (RBUF) } } Device (SER2) { Name (_ADR,0x1c0b0000) // UART2 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0b0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {39} }) Return (RBUF) } } Device (SER3) { Name (_ADR,0x1c0c0000) // UART3 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0c0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {40} }) Return (RBUF) } } }
Hi Graeme,
I have't seen any. Can you please forward me if you have got in your inbox.
-Naresh
On 22 October 2013 14:11, Graeme Gregory graeme.gregory@linaro.org wrote:
Hi Naresh,
Isn't the ASL in the first post of the series?
Graeme
On Tue, Oct 22, 2013 at 01:50:32PM +0530, Naresh Bhat wrote:
Hi Anderson,
The patches applied, compiled (except there was a conflict in the last patch "Hack clock names to get prototype running"). Can you please post the ASL code changes patch ? looks like ASL changes are different from what we have written originally.
-Naresh
On 22 October 2013 06:03, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
This is a proposal for ACPI driver probing of the ARM AMBA devices currently listed under ‘iofpga’ in the RTSM dts file. The main addition is drivers/amba/acpi.c which fits the role of an AMBA bus ‘connector resource’ for ACPI using struct amba_device.
I have not yet figured out the implementation details regarding clocks (handled in the dts file with clock-names). However, I have included a proposed ASL format for clock information in the DSDT example below. With the last patch that hacks the clock info, this prototype will run on the Foundation and RTSM models.
These patches require Hanjun’s fixed-clock patches to be applied first on top of a Linaro ACPI kernel: https://git.linaro.org/gitweb?p=arm/acpi/leg-kernel.git%3Ba=summary
Please comment on both the concept and the implementation.
Brandon Anderson (3): Remove UART and KMI entries from DTS file Prototype of AMBA bus 'connector resource' for ACPI Hack clock names to get prototype running
arch/arm64/boot/dts/foundation-v8-acpi.dts | 10 +- arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts | 4 + arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi | 4 + drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 172 +++++++++++++++++++++ drivers/clk/clk-fixed-rate.c | 15 +- 7 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 drivers/amba/acpi.c
-- 1.7.9.5
Device (AMBA) { Name (_HID, "AMBA0000") /* the parallel to "arm,primecell" in DTS */ Name (_UID, 0) /* Define 'apb_pclk' as a default clock source since it is common with devices below */ Method(_DSM, 4, NotSerialized) { Store (Package (3) { "clock-name", "apb_pclk", "\\_SB_.CLK0", }, Local0) Return (Local0) } Device (KMI0) { Name (_ADR,0x1c060000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c060000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {44} }) Return (RBUF) } } Device (KMI1) { Name (_ADR,0x1c070000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c070000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {45} }) Return (RBUF) } } Device (SER0) { Name (_ADR,0x1c090000) // UART0 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c090000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {37} }) Return (RBUF) } } Device (SER1) { Name (_ADR,0x1c0a0000) // UART1 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0a0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {38} }) Return (RBUF) } } Device (SER2) { Name (_ADR,0x1c0b0000) // UART2 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0b0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {39} }) Return (RBUF) } } Device (SER3) { Name (_ADR,0x1c0c0000) // UART3 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0c0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {40} }) Return (RBUF) } } }
Hi Brandon,
Haven't had time for a deep review yet, but I am liking the direction of these patches.
Thanks
Graeme
On Mon, Oct 21, 2013 at 07:33:46PM -0500, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
This is a proposal for ACPI driver probing of the ARM AMBA devices currently listed under ‘iofpga’ in the RTSM dts file. The main addition is drivers/amba/acpi.c which fits the role of an AMBA bus ‘connector resource’ for ACPI using struct amba_device.
I have not yet figured out the implementation details regarding clocks (handled in the dts file with clock-names). However, I have included a proposed ASL format for clock information in the DSDT example below. With the last patch that hacks the clock info, this prototype will run on the Foundation and RTSM models.
These patches require Hanjun’s fixed-clock patches to be applied first on top of a Linaro ACPI kernel: https://git.linaro.org/gitweb?p=arm/acpi/leg-kernel.git%3Ba=summary
Please comment on both the concept and the implementation.
Brandon Anderson (3): Remove UART and KMI entries from DTS file Prototype of AMBA bus 'connector resource' for ACPI Hack clock names to get prototype running
arch/arm64/boot/dts/foundation-v8-acpi.dts | 10 +- arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts | 4 + arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi | 4 + drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 172 +++++++++++++++++++++ drivers/clk/clk-fixed-rate.c | 15 +- 7 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 drivers/amba/acpi.c
-- 1.7.9.5
Device (AMBA) { Name (_HID, "AMBA0000") /* the parallel to "arm,primecell" in DTS */ Name (_UID, 0) /* Define 'apb_pclk' as a default clock source since it is common with devices below */ Method(_DSM, 4, NotSerialized) { Store (Package (3) { "clock-name", "apb_pclk", "\\_SB_.CLK0", }, Local0) Return (Local0) } Device (KMI0) { Name (_ADR,0x1c060000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c060000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {44} }) Return (RBUF) } } Device (KMI1) { Name (_ADR,0x1c070000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c070000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {45} }) Return (RBUF) } } Device (SER0) { Name (_ADR,0x1c090000) // UART0 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c090000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {37} }) Return (RBUF) } } Device (SER1) { Name (_ADR,0x1c0a0000) // UART1 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0a0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {38} }) Return (RBUF) } } Device (SER2) { Name (_ADR,0x1c0b0000) // UART2 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0b0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {39} }) Return (RBUF) } } Device (SER3) { Name (_ADR,0x1c0c0000) // UART3 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0c0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {40} }) Return (RBUF) } } }
Is the proposed ASL format appropriate? I recognize that the ASL definitions for these devices must be standardized across all ARM ACPI platform definitions if we want the software to work consistently, so this is an important aspect to agree upon.
How about contrasting it with an alternative where each device has _HID of "AMBA0000" and there's no top-level 'Device (AMBA)'? This would mean that all the devices would be labeled by ACPI subsystem as "AMBA0000:NN" instead of "device:NN". However, with this solution there would be no easy way to define a default clock. See example below.
Brandon
---
Alternative ASL #2:
Device (KMI0) { Name (_HID, "AMBA0000") Name (_UID, 0) Name (_ADR,0x1c060000) Method(_CRS, ... ) { ... } Method(_DSM, 4, NotSerialized) { Store (Package (6) { "clock-name", "KMIREFCLK", "\_SB_.CLK0", "clock-name", "apb_pclk", "\_SB_.CLK0", }, Local0) Return (Local0) } } ...
Device (SER0) { Name (_HID, "AMBA0000") Name (_UID, 2) Name (_ADR,0x1c090000) Method(_CRS, ... ) { ... } Method(_DSM, 4, NotSerialized) { Store (Package (3) { "clock-name", "apb_pclk", "\_SB_.CLK0", }, Local0) Return (Local0) } }
-----Original Message----- From: brandon.anderson@amd.com [mailto:brandon.anderson@amd.com] Sent: Monday, October 21, 2013 7:34 PM To: linaro-acpi@lists.linaro.org; linux-acpi@vger.kernel.org Cc: lenb@kernel.org; rjw@sisk.pl; naresh.bhat@linaro.org; hanjun.guo@linaro.org; graeme.gregory@linaro.org; Suthikulpanit, Suravee; Anderson, Brandon Subject: [RFC PATCH 0/3] ACPI: ARM AMBA bus connector resource
From: Brandon Anderson brandon.anderson@amd.com
This is a proposal for ACPI driver probing of the ARM AMBA devices currently listed under ‘iofpga’ in the RTSM dts file. The main addition is drivers/amba/acpi.c which fits the role of an AMBA bus ‘connector resource’ for ACPI using struct amba_device.
I have not yet figured out the implementation details regarding clocks (handled in the dts file with clock-names). However, I have included a proposed ASL format for clock information in the DSDT example below. With the last patch that hacks the clock info, this prototype will run on the Foundation and RTSM models.
These patches require Hanjun’s fixed-clock patches to be applied first on top of a Linaro ACPI kernel: https://git.linaro.org/gitweb?p=arm/acpi/leg-kernel.git%3Ba=summary
Please comment on both the concept and the implementation.
Brandon Anderson (3): Remove UART and KMI entries from DTS file Prototype of AMBA bus 'connector resource' for ACPI Hack clock names to get prototype running
arch/arm64/boot/dts/foundation-v8-acpi.dts | 10 +- arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts | 4 + arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi | 4 + drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 172 +++++++++++++++++++++ drivers/clk/clk-fixed-rate.c | 15 +- 7 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 drivers/amba/acpi.c
-- 1.7.9.5
---
Device (AMBA) { Name (_HID, "AMBA0000") /* the parallel to "arm,primecell" in DTS */ Name (_UID, 0)
/* Define 'apb_pclk' as a default clock source since it is common with devices below */ Method(_DSM, 4, NotSerialized) { Store (Package (3) { "clock-name", "apb_pclk", "\_SB_.CLK0", }, Local0)
Return (Local0) }
Device (KMI0) { Name (_ADR,0x1c060000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c060000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {44} }) Return (RBUF) } }
Device (KMI1) { Name (_ADR,0x1c070000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c070000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {45} }) Return (RBUF) } }
Device (SER0) { Name (_ADR,0x1c090000) // UART0 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c090000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {37} }) Return (RBUF) } }
Device (SER1) { Name (_ADR,0x1c0a0000) // UART1 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0a0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {38} }) Return (RBUF) } } Device (SER2) { Name (_ADR,0x1c0b0000) // UART2 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0b0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {39} }) Return (RBUF) } }
Device (SER3) { Name (_ADR,0x1c0c0000) // UART3 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0c0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {40} }) Return (RBUF) } } }
On 2013-10-23 8:13, Anderson, Brandon wrote:
Is the proposed ASL format appropriate? I recognize that the ASL
It is ok for me. it represents the device topology very clear also.
definitions for these devices must be standardized across all ARM ACPI platform definitions if we want the software to work consistently, so this is an important aspect to agree upon.
How about contrasting it with an alternative where each device has _HID of "AMBA0000" and there's no top-level 'Device (AMBA)'? This would mean that all the devices would be labeled by ACPI subsystem as "AMBA0000:NN" instead of "device:NN". However, with this solution there would be no easy way to define a default clock. See example below.
I think a top-level 'Device (AMBA)' is needed. there is a good example for PCI host bridge, its HID is PNP0A08 or PNP0A03, and there are a top level device for PCI host bridge and child devices with different HIDs.
if there's no top-level 'Device (AMBA)', AMBA0000:NN will confuse people if there are multi AMBAs in the system, people will think that AMBA0000:01 is the second AMBA bus in the system but not some devices under AMBA bus.
Thanks Hanjun
Thank you for confirming the original proposal for the ASL format , Hanjun.
I will refine this patchset, implement the clock definition, and then resubmit to the list for review.
Brandon
-----Original Message----- From: Hanjun Guo [mailto:hanjun.guo@linaro.org] Sent: Wednesday, October 23, 2013 2:38 AM To: Anderson, Brandon; linaro-acpi@lists.linaro.org; linux-acpi@vger.kernel.org Cc: lenb@kernel.org; rjw@sisk.pl; naresh.bhat@linaro.org; graeme.gregory@linaro.org; Suthikulpanit, Suravee Subject: Re: [RFC PATCH 0/3] ACPI: ARM AMBA bus connector resource
On 2013-10-23 8:13, Anderson, Brandon wrote:
Is the proposed ASL format appropriate? I recognize that the ASL
It is ok for me. it represents the device topology very clear also.
definitions for these devices must be standardized across all ARM ACPI platform definitions if we want the software to work consistently, so this is an important aspect to agree upon.
How about contrasting it with an alternative where each device has _HID of "AMBA0000" and there's no top-level 'Device (AMBA)'? This would mean that all the devices would be labeled by ACPI subsystem as "AMBA0000:NN" instead of "device:NN". However, with this solution there would be no easy way to define a default clock. See example below.
I think a top-level 'Device (AMBA)' is needed. there is a good example for PCI host bridge, its HID is PNP0A08 or PNP0A03, and there are a top level device for PCI host bridge and child devices with different HIDs.
if there's no top-level 'Device (AMBA)', AMBA0000:NN will confuse people if there are multi AMBAs in the system, people will think that AMBA0000:01 is the second AMBA bus in the system but not some devices under AMBA bus.
Thanks Hanjun
Thanks. I have applied on top of linaro acpi kernel and verified it on the foundation-v8 model.
Acked-by: naresh.bhat@linaro.org
On 22 October 2013 06:03, brandon.anderson@amd.com wrote:
From: Brandon Anderson brandon.anderson@amd.com
This is a proposal for ACPI driver probing of the ARM AMBA devices currently listed under ‘iofpga’ in the RTSM dts file. The main addition is drivers/amba/acpi.c which fits the role of an AMBA bus ‘connector resource’ for ACPI using struct amba_device.
I have not yet figured out the implementation details regarding clocks (handled in the dts file with clock-names). However, I have included a proposed ASL format for clock information in the DSDT example below. With the last patch that hacks the clock info, this prototype will run on the Foundation and RTSM models.
These patches require Hanjun’s fixed-clock patches to be applied first on top of a Linaro ACPI kernel: https://git.linaro.org/gitweb?p=arm/acpi/leg-kernel.git%3Ba=summary
Please comment on both the concept and the implementation.
Brandon Anderson (3): Remove UART and KMI entries from DTS file Prototype of AMBA bus 'connector resource' for ACPI Hack clock names to get prototype running
arch/arm64/boot/dts/foundation-v8-acpi.dts | 10 +- arch/arm64/boot/dts/rtsm_ve-aemv8a-acpi.dts | 4 + arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi | 4 + drivers/acpi/acpi_platform.c | 2 + drivers/amba/Makefile | 2 +- drivers/amba/acpi.c | 172 +++++++++++++++++++++ drivers/clk/clk-fixed-rate.c | 15 +- 7 files changed, 203 insertions(+), 6 deletions(-) create mode 100644 drivers/amba/acpi.c
-- 1.7.9.5
Device (AMBA) { Name (_HID, "AMBA0000") /* the parallel to "arm,primecell" in DTS */ Name (_UID, 0) /* Define 'apb_pclk' as a default clock source since it is common with devices below */ Method(_DSM, 4, NotSerialized) { Store (Package (3) { "clock-name", "apb_pclk", "\\_SB_.CLK0", }, Local0) Return (Local0) } Device (KMI0) { Name (_ADR,0x1c060000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c060000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {44} }) Return (RBUF) } } Device (KMI1) { Name (_ADR,0x1c070000) Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c070000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {45} }) Return (RBUF) } } Device (SER0) { Name (_ADR,0x1c090000) // UART0 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c090000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {37} }) Return (RBUF) } } Device (SER1) { Name (_ADR,0x1c0a0000) // UART1 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0a0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {38} }) Return (RBUF) } } Device (SER2) { Name (_ADR,0x1c0b0000) // UART2 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0b0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {39} }) Return (RBUF) } } Device (SER3) { Name (_ADR,0x1c0c0000) // UART3 Method (_CRS, 0x0, Serialized) { Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x1c0c0000, 0x00010000) Interrupt (ResourceConsumer, Edge, ActiveBoth, Exclusive, , , ) {40} }) Return (RBUF) } } }
-
Anderson, Brandon -
brandon.anderson＠amd.com
-
Graeme Gregory -
Hanjun Guo -
Naresh Bhat