[PATCH V2 0/4] ACPI/ARM: AMBA bus ACPI module

[PATCH V2 0/4] ACPI/ARM: AMBA bus ACPI module

[Brandon Anderson]

This patch series requires the fixed-clock patches from Hanjun Guo.

Provides an ACPI module to probe ARM AMBA devices that have a
hardware ID, which is used with struct amba_device to match up
the appropriate driver. This currently uses the _DSM table for
key/value pairs, which may need to be changed to use _PRP depending
on the results of separate discussions.

See example of the required DSDT definition format below.

V2:
  Minor changes to address comments about #ifdef format and _DSM table tests.

Brandon Anderson (4):
  early fixed-clock
  AMBA bus ACPI implementation
  Add ACPI to AMBA SPI driver
  remove unneeded sections of DTS definition

 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 |    8 +
 drivers/acpi/acpi_platform.c                      |    2 +
 drivers/amba/Makefile                             |    2 +-
 drivers/amba/acpi.c                               |  338 +++++++++++++++++++++
 drivers/clk/clk-fixed-rate.c                      |    2 +-
 drivers/spi/spi-pl022.c                           |   53 ++++
 include/linux/amba/acpi.h                         |   29 ++
 9 files changed, 443 insertions(+), 5 deletions(-)
 create mode 100644 drivers/amba/acpi.c
 create mode 100644 include/linux/amba/acpi.h

-- 
1.7.9.5

-----------

diff --git a/platforms/rtsm_ve-aemv8a.acpi/dsdt.asl b/platforms/rtsm_ve-aemv8a.acpi/dsdt.asl
index 0bcc94d..64e84a4 100644
--- a/platforms/rtsm_ve-aemv8a.acpi/dsdt.asl
+++ b/platforms/rtsm_ve-aemv8a.acpi/dsdt.asl
@@ -252,6 +252,33 @@ DefinitionBlock (
 			}
 		}
 
+		Device (CLK0) {
+			Name (_HID, "LINA0008")
+			Name (_UID, 0)
+
+			Method (FREQ, 0x0, NotSerialized) {
+				Return (24000000)
+			}
+		}
+
+		Device (CLK1) {
+			Name (_HID, "LINA0008")
+			Name (_UID, 1)
+
+			Method (FREQ, 0x0, NotSerialized) {
+				Return (1000000)
+			}
+		}
+
+		Device (CLK2) {
+			Name (_HID, "LINA0008")
+			Name (_UID, 2)
+
+			Method (FREQ, 0x0, NotSerialized) {
+				Return (32768)
+			}
+		}
+
 		Device (PMU0) {
 			Name (_HID, "LINA0007")
 			Name (_UID, 0)
@@ -263,5 +290,229 @@ DefinitionBlock (
 				Return (RBUF)
 			}
 		}
+
+		Method (DTGP, 5, NotSerialized)
+		{
+			If (LEqual (Arg0, Buffer (0x10)
+					{
+					/* UUID:	a706b112-bf0b-48d2-9fa3-95591a3c4c06 */
+					  /* 0000 */	0xa7, 0x06, 0xb1, 0x12, 0xbf, 0x0b, 0x48, 0xd2,
+					  /* 0008 */	0x9f, 0xa3, 0x95, 0x59, 0x1a, 0x3c, 0x4c, 0x06
+					}))
+			{
+				If (LEqual (Arg1, 0x01))
+				{
+					If (LEqual (Arg2, 0x00))
+					{
+						Store (Buffer (0x01)
+						{
+							0x03
+						}, Arg4)
+						Return (0x01)
+					}
+
+					If (LEqual (Arg2, 0x01))
+					{
+						Return (0x01)
+					}
+				}
+			}
+
+			Store (Buffer (0x01)
+			{
+				0x00
+			}, Arg4)
+			Return (0x00)
+		}
+
+		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 (2)
+				{
+					"clock-name", "apb_pclk \\_SB.CLK0",
+				}, Local0)
+
+				DTGP (Arg0, Arg1, Arg2, Arg3, RefOf (Local0))
+
+				Return (Local0)
+			}
+
+			Device (SCT0) {
+				Name (_ADR, 0x1c020000)						/* SYSCTL */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c020000, 0x00001000)
+					})
+					Return (RBUF)
+				}
+				Method(_DSM, 4, Serialized) {
+					Store (Package (4)
+					{
+						"clock-name", "refclk \\_SB.CLK2",
+						"clock-name", "timclk \\_SB.CLK1",
+					}, Local0)
+
+					DTGP (Arg0, Arg1, Arg2, Arg3, RefOf (Local0))
+
+					Return (Local0)
+				}
+			}
+
+			Device (KMI0) {
+				Name (_ADR, 0x1c060000)
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c060000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {44}
+					})
+					Return (RBUF)
+				}
+				Method(_DSM, 4, Serialized) {
+					Store (Package (2)
+					{
+						"clock-name", "KMIREFCLK \\_SB.CLK0",
+					}, Local0)
+
+					DTGP (Arg0, Arg1, Arg2, Arg3, RefOf (Local0))
+
+					Return (Local0)
+				}
+			}
+
+			Device (KMI1) {
+				Name (_ADR, 0x1c070000)
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c070000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {45}
+					})
+					Return (RBUF)
+				}
+				Method(_DSM, 4, NotSerialized) {
+					Store (Package (2)
+					{
+						"clock-name", "KMIREFCLK \\_SB.CLK0",
+					}, Local0)
+
+					DTGP (Arg0, Arg1, Arg2, Arg3, RefOf (Local0))
+
+					Return (Local0)
+				}
+			}
+
+			Device (SER0) {
+				Name (_ADR, 0x1c090000)                         /* UART0 */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c090000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {37}
+					})
+					Return (RBUF)
+				}
+			}
+
+			Device (SER1) {
+				Name (_ADR, 0x1c0a0000)                         /* UART1 */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c0a0000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {38}
+					})
+					Return (RBUF)
+				}
+			}
+			Device (SER2) {
+				Name (_ADR, 0x1c0b0000)                         /* UART2 */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c0b0000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {39}
+					})
+					Return (RBUF)
+				}
+			}
+
+			Device (SER3) {
+				Name (_ADR, 0x1c0c0000)                         /* UART3 */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c0c0000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {40}
+					})
+					Return (RBUF)
+				}
+			}
+
+			Device (AAC0) {
+				Name (_ADR, 0x1c040000)						/* AACI */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c040000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {43}
+					})
+					Return (RBUF)
+				}
+			}
+
+			Device (WDT0) {
+				Name (_ADR, 0x1c0f0000)						/* WDT */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c0f0000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {32}
+					})
+					Return (RBUF)
+				}
+			}
+
+			Device (TIM0) {
+				Name (_ADR, 0x1c110000)						/* TIMER01 */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c110000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {34}
+					})
+					Return (RBUF)
+				}
+			}
+
+			Device (TIM2) {
+				Name (_ADR, 0x1c120000)						/* TIMER23 */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c120000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {35}
+					})
+					Return (RBUF)
+				}
+			}
+
+			Device (RTC0) {
+				Name (_ADR, 0x1c170000)						/* RTC */
+				Method (_CRS, 0x0, Serialized) {
+					Name (RBUF, ResourceTemplate () {
+						Memory32Fixed (ReadWrite, 0x1c170000, 0x00001000)
+						Interrupt (ResourceConsumer, Edge, ActiveBoth,
+								Exclusive, , , ) {36}
+					})
+					Return (RBUF)
+				}
+			}
+		}
 	}
 }

[PATCH V2 1/4] ACPI/ARM: Load fixed-clk module early

[Brandon Anderson]

An AMBA device will be probed once two things have occured: 1) ACPI has
registered a device, and 2) the driver has registered itself as an AMBA
driver. Since several drivers register themselves very early on, the
probe will happen as soon as ACPI registers the device. If a device
depends on a clock, the clock must be probed before ACPI registers the
device. This means that the clock definition must be encountered first in the
DSDT file, and the clock driver must already be loaded when this happens.

There are other potential solutions, with this solution registering the clock
driver very early in the boot process to avoid changes to other drivers.


Signed-off-by: Brandon Anderson <brandon.anderson@amd.com>
---
 drivers/clk/clk-fixed-rate.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index 226cefb..778291e 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -174,5 +174,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
-- 
1.7.9.5

[PATCH V2 2/4] ACPI/ARM: Add AMBA bus ACPI module

[Brandon Anderson]

This AMBA bus ACPI module provides a generic handler for compatible
devices. It depends on a DSDT definition as provided in the related
'0/4' email.

It uses the same common code as the device tree method to probe for a hardware
ID and match up a driver for each device.


Signed-off-by: Brandon Anderson <brandon.anderson@amd.com>
---
 drivers/acpi/acpi_platform.c |    2 +
 drivers/amba/Makefile        |    2 +-
 drivers/amba/acpi.c          |  338 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/amba/acpi.h    |   29 ++++
 4 files changed, 370 insertions(+), 1 deletion(-)
 create mode 100644 drivers/amba/acpi.c
 create mode 100644 include/linux/amba/acpi.h

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..7b33535
--- /dev/null
+++ b/drivers/amba/acpi.c
@@ -0,0 +1,338 @@
+/*
+ * 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>
+#include <linux/amba/acpi.h>
+
+struct acpi_amba_bus_info {
+	struct platform_device *pdev;
+	struct clk *clk;
+	char *clk_name;
+};
+
+/* UUID: a706b112-bf0b-48d2-9fa3-95591a3c4c06 (randomly generated) */
+static const char acpi_amba_dsm_uuid[] = {
+	0xa7, 0x06, 0xb1, 0x12, 0xbf, 0x0b, 0x48, 0xd2,
+	0x9f, 0xa3, 0x95, 0x59, 0x1a, 0x3c, 0x4c, 0x06
+};
+
+/* acpi_amba_dsm_lookup()
+ *
+ * Helper to parse through ACPI _DSM object for a device. Each entry
+ * has three fields.
+ */
+int acpi_amba_dsm_lookup(acpi_handle handle,
+		const char *tag, int index,
+		struct acpi_amba_dsm_entry *entry)
+{
+	acpi_status status;
+	struct acpi_object_list input;
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object params[4];
+	union acpi_object *obj;
+	int len, match_count, i;
+
+	/* invalidate output in case there's no entry to supply */
+	entry->key = NULL;
+	entry->value = NULL;
+
+	input.count = 4;
+	params[0].type = ACPI_TYPE_BUFFER;		/* UUID */
+	params[0].buffer.length = sizeof(acpi_amba_dsm_uuid);
+	params[0].buffer.pointer = (char *)acpi_amba_dsm_uuid;
+	params[1].type = ACPI_TYPE_INTEGER;		/* Revision */
+	params[1].integer.value = 1;
+	params[2].type = ACPI_TYPE_INTEGER;		/* Function # */
+	params[2].integer.value = 1;
+	params[3].type = ACPI_TYPE_PACKAGE;		/* Arguments */
+	params[3].package.count = 0;
+	params[3].package.elements = NULL;
+	input.pointer = params;
+
+	status = acpi_evaluate_object_typed(handle, "_DSM",
+			&input, &output, ACPI_TYPE_PACKAGE);
+	if (ACPI_FAILURE(status)) {
+		if (status != AE_NOT_FOUND)
+			pr_err("failed to get _DSM package for this device\n");
+		return -ENOENT;
+	}
+
+	obj = (union acpi_object *)output.pointer;
+
+	/* parse 2 objects per entry */
+	match_count = 0;
+	for (i = 0; (i + 2) <= obj->package.count; i += 2) {
+		/* key must be a string */
+		len = obj->package.elements[i].string.length;
+		if (len <= 0)
+			continue;
+
+		/* check to see if this is the entry to return */
+		if (strncmp(tag, obj->package.elements[i].string.pointer,
+				len) != 0 ||
+				match_count < index) {
+			match_count++;
+			continue;
+		}
+
+		/* copy the key */
+		entry->key = kmalloc(len + 1, GFP_KERNEL);
+		strncpy(entry->key,
+				obj->package.elements[i].string.pointer,
+				len);
+		entry->key[len] = '\0';
+
+		/* value is a string with space-delimited fields if necessary */
+		len = obj->package.elements[i + 1].string.length;
+		if (len > 0) {
+			entry->value = kmalloc(len + 1, GFP_KERNEL);
+			strncpy(entry->value,
+				obj->package.elements[i+1].string.pointer,
+				len);
+			entry->value[len] = '\0';
+		}
+
+		break;
+	}
+
+	kfree(output.pointer);
+
+	if (entry->key == NULL)
+		return -ENOENT;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_amba_dsm_lookup);
+
+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("failed to map memory resource\n");
+		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;
+	case ACPI_RESOURCE_TYPE_END_TAG:
+		/* ignore the end tag */
+		break;
+	default:
+		/* log an error, but proceed with driver probe */
+		pr_err("unhandled acpi resource type= %d\n",
+				ares->type);
+		break;
+	}
+
+	return 1; /* Tell ACPI core that this resource has been handled */
+}
+
+static struct clk *acpi_amba_get_clk(acpi_handle handle, int index,
+		char **clk_name)
+{
+	struct acpi_amba_dsm_entry entry;
+	acpi_handle clk_handle;
+	struct acpi_device *adev, *clk_adev;
+	char *clk_path;
+	struct clk *clk = NULL;
+	int len;
+
+	if (acpi_bus_get_device(handle, &adev)) {
+		pr_err("cannot get device from handle\n");
+		return NULL;
+	}
+
+	/* key=value format for clocks is:
+	 *	"clock-name"="apb_pclk \\_SB.CLK0"
+	 */
+	if (acpi_amba_dsm_lookup(handle, "clock-name", index, &entry) != 0)
+		return NULL;
+
+	/* look under the clock device for the clock that matches the entry */
+	*clk_name = NULL;
+	len = strcspn(entry.value, " ");
+	if (len > 0 && (len + 1) < strlen(entry.value)) {
+		clk_path = entry.value + len + 1;
+		*clk_name = kmalloc(len + 1, GFP_KERNEL);
+		strncpy(*clk_name, entry.value, len);
+		(*clk_name)[len] = '\0';
+		if (ACPI_FAILURE(
+			acpi_get_handle(NULL, clk_path, &clk_handle)) == 0 &&
+			acpi_bus_get_device(clk_handle, &clk_adev) == 0)
+			clk = clk_get_sys(dev_name(&clk_adev->dev), *clk_name);
+	} else
+		pr_err("Invalid clock-name value format '%s' for %s\n",
+				entry.value, dev_name(&adev->dev));
+
+	kfree(entry.key);
+	kfree(entry.value);
+
+	return clk;
+}
+
+static void acpi_amba_register_clocks(struct acpi_device *adev,
+		struct clk *default_clk, const char *default_clk_name)
+{
+	struct clk *clk;
+	int i;
+	char *clk_name;
+
+	if (default_clk) {
+		/* for amba_get_enable_pclk() ... */
+		clk_register_clkdev(default_clk, default_clk_name,
+				dev_name(&adev->dev));
+		/* for devm_clk_get() ... */
+		clk_register_clkdev(default_clk, NULL, dev_name(&adev->dev));
+	}
+
+	for (i = 0;; i++) {
+		clk_name = NULL;
+		clk = acpi_amba_get_clk(ACPI_HANDLE(&adev->dev), i, &clk_name);
+		if (!clk)
+			break;
+
+		clk_register_clkdev(clk, clk_name, dev_name(&adev->dev));
+
+		kfree(clk_name);
+	}
+}
+
+/* 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 acpi_amba_bus_info *bus_info = data;
+	struct platform_device *bus_pdev = bus_info->pdev;
+
+	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 = &bus_pdev->dev;
+	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);
+	ACPI_HANDLE_SET(&adev->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_clocks(adev, bus_info->clk, bus_info->clk_name);
+
+	/* 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_OK; /* don't prevent other devices from being probed */
+}
+
+static int acpi_amba_bus_probe(struct platform_device *pdev)
+{
+	struct acpi_amba_bus_info bus_info;
+	bus_info.clk_name = NULL;
+
+	/* see if there's a top-level clock to use as default for sub-devices */
+	bus_info.clk = acpi_amba_get_clk(ACPI_HANDLE(&pdev->dev), 0,
+			&bus_info.clk_name);
+
+	/* probe each device */
+	bus_info.pdev = pdev;
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_HANDLE(&pdev->dev), 1,
+			acpi_amba_add_device, NULL, &bus_info, NULL);
+
+	kfree(bus_info.clk_name);
+
+	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 */
diff --git a/include/linux/amba/acpi.h b/include/linux/amba/acpi.h
new file mode 100644
index 0000000..40a72b2
--- /dev/null
+++ b/include/linux/amba/acpi.h
@@ -0,0 +1,29 @@
+/*
+ * AMBA bus ACPI helpers
+ *
+ * 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.
+ */
+#ifndef __ARM_AMBA_ACPI_H__
+#define __ARM_AMBA_ACPI_H__
+
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+
+struct acpi_amba_dsm_entry {
+	char *key;
+	char *value;
+};
+
+int acpi_amba_dsm_lookup(acpi_handle handle,
+		const char *tag, int index,
+		struct acpi_amba_dsm_entry *entry);
+
+#endif
+
+#endif
-- 
1.7.9.5

[PATCH V2 3/4] ACPI/ARM: Add ACPI to AMBA SPI driver

[Brandon Anderson]

Neither Foundation nor RTSM have a SPI device, but here are the necessary driver
changes as an example of how to use acpi_amba_dsm_lookup() to get non-standard
parameters from ACPI.

This was tested by wiring up a SPI device into an RTSM Fast Model.


Signed-off-by: Brandon Anderson <brandon.anderson@amd.com>
---
 drivers/spi/spi-pl022.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 9c511a9..1d0a8ec 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -43,6 +43,7 @@
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/amba/acpi.h>
 
 /*
  * This macro is used to define some register default values.
@@ -2069,6 +2070,55 @@ pl022_platform_data_dt_get(struct device *dev)
 	return pd;
 }
 
+#ifdef CONFIG_ACPI
+static struct pl022_ssp_controller *
+acpi_pl022_get_platform_data(struct device *dev)
+{
+	struct pl022_ssp_controller *pd, *ret;
+	struct acpi_amba_dsm_entry entry;
+
+	pd = devm_kzalloc(dev, sizeof(struct pl022_ssp_controller), GFP_KERNEL);
+	if (!pd) {
+		dev_err(dev, "cannot allocate platform data memory\n");
+		return NULL;
+	}
+	ret = pd;
+
+	pd->bus_id = -1;
+	pd->enable_dma = 1;
+	if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "num-cs", 0, &entry) == 0) {
+		if (kstrtou8(entry.value, 0, &pd->num_chipselect) != 0) {
+			dev_err(dev, "invalid 'num-cs' in ACPI definition\n");
+			ret = NULL;
+		}
+		kfree(entry.key);
+		kfree(entry.value);
+	}
+	if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev),
+			"autosuspend-delay", 0, &entry) == 0) {
+		if (kstrtoint(entry.value, 0, &pd->autosuspend_delay) != 0) {
+			dev_err(dev, "invalid 'autosuspend-delay' in ACPI definition\n");
+			ret = NULL;
+		}
+		kfree(entry.key);
+		kfree(entry.value);
+	}
+	if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "rt", 0, &entry) == 0) {
+		pd->rt = (entry.value && strcmp(entry.value, "1") == 0);
+		kfree(entry.key);
+		kfree(entry.value);
+	}
+
+	return ret;
+}
+#else
+static struct pl022_ssp_controller *
+acpi_pl022_get_platform_data(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int pl022_probe(struct amba_device *adev, const struct amba_id *id)
 {
 	struct device *dev = &adev->dev;
@@ -2081,6 +2131,9 @@ static int pl022_probe(struct amba_device *adev, const struct amba_id *id)
 
 	dev_info(&adev->dev,
 		 "ARM PL022 driver, device ID: 0x%08x\n", adev->periphid);
+	if (!platform_info && ACPI_HANDLE(dev))
+		platform_info = acpi_pl022_get_platform_data(dev);
+	else
 	if (!platform_info && IS_ENABLED(CONFIG_OF))
 		platform_info = pl022_platform_data_dt_get(dev);
 
-- 
1.7.9.5

[PATCH V2 4/4] ACPI/ARM: Remove sections of DTS definition

[Brandon Anderson]

Use ACPI definition instead of DTS definition for devices that are supported
by AMBA bus ACPI module. The replacement ACPI definition was provided in the
related '0/4' email.

Note that MMCI is compatible with AMBA bus ACPI module, but is not ported
due to complex relations with sysreg and voltage regulator.


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 |    8 ++++++++
 3 files changed, 19 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..f20af14 100644
--- a/arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi
+++ b/arch/arm64/boot/dts/rtsm_ve-motherboard-acpi.dtsi
@@ -71,6 +71,9 @@
 				#gpio-cells = <2>;
 			};
 
+			/*
+			 * Removed for ACPI
+			 *
 			v2m_sysctl: sysctl@020000 {
 				compatible = "arm,sp810", "arm,primecell";
 				reg = <0x020000 0x1000>;
@@ -87,6 +90,7 @@
 				clocks = <&v2m_clk24mhz>;
 				clock-names = "apb_pclk";
 			};
+			*/
 
 			mmci@050000 {
 				compatible = "arm,pl180", "arm,primecell";
@@ -100,6 +104,9 @@
 				clock-names = "mclk", "apb_pclk";
 			};
 
+			/*
+			 * Removed for ACPI
+			 *
 			kmi@060000 {
 				compatible = "arm,pl050", "arm,primecell";
 				reg = <0x060000 0x1000>;
@@ -179,6 +186,7 @@
 				clocks = <&v2m_clk24mhz>;
 				clock-names = "apb_pclk";
 			};
+			*/
 
 			clcd@1f0000 {
 				compatible = "arm,pl111", "arm,primecell";
-- 
1.7.9.5

Re: [PATCH V2 1/4] ACPI/ARM: Load fixed-clk module early

[Olof Johansson]

On Fri, Nov 22, 2013 at 10:12 AM, Brandon Anderson
<brandon.anderson@amd.com> wrote:
> An AMBA device will be probed once two things have occured: 1) ACPI has
> registered a device, and 2) the driver has registered itself as an AMBA
> driver. Since several drivers register themselves very early on, the
> probe will happen as soon as ACPI registers the device. If a device
> depends on a clock, the clock must be probed before ACPI registers the
> device. This means that the clock definition must be encountered first in the
> DSDT file, and the clock driver must already be loaded when this happens.
>
> There are other potential solutions, with this solution registering the clock
> driver very early in the boot process to avoid changes to other drivers.
>
>
> Signed-off-by: Brandon Anderson <brandon.anderson@amd.com>

You guys really need to start posting these patches to a wider
audience than just linux-acpi. At least cc the clock maintainer on
clock patches, and since these are ARM related, the linux-arm-kernel
list should be cc:d too.


-Olof

Re: [PATCH V2 0/4] ACPI/ARM: AMBA bus ACPI module

[Olof Johansson]

Hi,

On Fri, Nov 22, 2013 at 10:12 AM, Brandon Anderson
<brandon.anderson@amd.com> wrote:
> This patch series requires the fixed-clock patches from Hanjun Guo.

I don't see them posted anywhere? Please don't post patches that don't
apply to mainline (or to publicly posted dependencies).

>  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 |    8 +

These files don't exist in mainline, nor in linux-next.


-Olof

RE: [PATCH V2 0/4] ACPI/ARM: AMBA bus ACPI module

[Anderson, Brandon]

Olof,
I apologize for posting confusing patches to the linux-acpi list.

These patches are on top of the Linaro ACPI work, http://git.linaro.org/git-ro/arm/acpi/leg-kernel.git

Brandon


-----Original Message-----
From: Olof Johansson [mailto:olof@lixom.net] 
Sent: Friday, November 22, 2013 1:04 PM
To: Anderson, Brandon
Cc: Russell King - ARM Linux; Rafael J. Wysocki; linaro-acpi@lists.linaro.org; linux-acpi@vger.kernel.org
Subject: Re: [PATCH V2 0/4] ACPI/ARM: AMBA bus ACPI module

Hi,

On Fri, Nov 22, 2013 at 10:12 AM, Brandon Anderson <brandon.anderson@amd.com> wrote:
> This patch series requires the fixed-clock patches from Hanjun Guo.

I don't see them posted anywhere? Please don't post patches that don't apply to mainline (or to publicly posted dependencies).

>  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 |    8 +

These files don't exist in mainline, nor in linux-next.


-Olof

Re: [Linaro-acpi] [PATCH V2 4/4] ACPI/ARM: Remove sections of DTS definition

[Grant Likely]

On Fri, Nov 22, 2013 at 6:12 PM, Brandon Anderson
<brandon.anderson@amd.com> wrote:
> Use ACPI definition instead of DTS definition for devices that are supported
> by AMBA bus ACPI module. The replacement ACPI definition was provided in the
> related '0/4' email.
>
> Note that MMCI is compatible with AMBA bus ACPI module, but is not ported
> due to complex relations with sysreg and voltage regulator.
>
>
> 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 |    8 ++++++++
>  3 files changed, 19 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;
>         };
> +       */

There's no point in commenting the sections out. If you're removing
them, remove the outright.

g.

Re: [Linaro-acpi] [PATCH V2 3/4] ACPI/ARM: Add ACPI to AMBA SPI driver

[Grant Likely]

On Fri, Nov 22, 2013 at 6:12 PM, Brandon Anderson
<brandon.anderson@amd.com> wrote:
> Neither Foundation nor RTSM have a SPI device, but here are the necessary driver
> changes as an example of how to use acpi_amba_dsm_lookup() to get non-standard
> parameters from ACPI.
>
> This was tested by wiring up a SPI device into an RTSM Fast Model.
>
>
> Signed-off-by: Brandon Anderson <brandon.anderson@amd.com>
> ---
>  drivers/spi/spi-pl022.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
> index 9c511a9..1d0a8ec 100644
> --- a/drivers/spi/spi-pl022.c
> +++ b/drivers/spi/spi-pl022.c
> @@ -43,6 +43,7 @@
>  #include <linux/gpio.h>
>  #include <linux/of_gpio.h>
>  #include <linux/pinctrl/consumer.h>
> +#include <linux/amba/acpi.h>
>
>  /*
>   * This macro is used to define some register default values.
> @@ -2069,6 +2070,55 @@ pl022_platform_data_dt_get(struct device *dev)
>         return pd;
>  }
>
> +#ifdef CONFIG_ACPI
> +static struct pl022_ssp_controller *
> +acpi_pl022_get_platform_data(struct device *dev)
> +{
> +       struct pl022_ssp_controller *pd, *ret;
> +       struct acpi_amba_dsm_entry entry;
> +
> +       pd = devm_kzalloc(dev, sizeof(struct pl022_ssp_controller), GFP_KERNEL);
> +       if (!pd) {
> +               dev_err(dev, "cannot allocate platform data memory\n");
> +               return NULL;
> +       }
> +       ret = pd;
> +
> +       pd->bus_id = -1;
> +       pd->enable_dma = 1;
> +       if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "num-cs", 0, &entry) == 0) {
> +               if (kstrtou8(entry.value, 0, &pd->num_chipselect) != 0) {
> +                       dev_err(dev, "invalid 'num-cs' in ACPI definition\n");
> +                       ret = NULL;
> +               }
> +               kfree(entry.key);
> +               kfree(entry.value);
> +       }
> +       if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev),
> +                       "autosuspend-delay", 0, &entry) == 0) {
> +               if (kstrtoint(entry.value, 0, &pd->autosuspend_delay) != 0) {
> +                       dev_err(dev, "invalid 'autosuspend-delay' in ACPI definition\n");
> +                       ret = NULL;
> +               }
> +               kfree(entry.key);
> +               kfree(entry.value);
> +       }
> +       if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "rt", 0, &entry) == 0) {
> +               pd->rt = (entry.value && strcmp(entry.value, "1") == 0);
> +               kfree(entry.key);
> +               kfree(entry.value);
> +       }
> +
> +       return ret;
> +}

As discussed in the ACPI vs DT thread. The kinds of lookups here are
identical to the DT property lookups except that the function name is
different and the property name is needlessly different. That's
madness. In all of the trivial cases the DT and ACPI lookup code
should be identical. We need a property value lookup function that
both DT and ACPI can use.

g.

RE: [Linaro-acpi] [PATCH V2 4/4] ACPI/ARM: Remove sections of DTS definition

[Anderson, Brandon]

-----Original Message-----
From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of Grant Likely
Sent: Friday, November 22, 2013 1:48 PM
To: Anderson, Brandon
Cc: Russell King - ARM Linux; Rafael J. Wysocki; linaro-acpi; ACPI Devel Mailing List
Subject: Re: [Linaro-acpi] [PATCH V2 4/4] ACPI/ARM: Remove sections of DTS definition

On Fri, Nov 22, 2013 at 6:12 PM, Brandon Anderson <brandon.anderson@amd.com> wrote:
> Use ACPI definition instead of DTS definition for devices that are 
> supported by AMBA bus ACPI module. The replacement ACPI definition was 
> provided in the related '0/4' email.
>
> Note that MMCI is compatible with AMBA bus ACPI module, but is not 
> ported due to complex relations with sysreg and voltage regulator.
>
>
> 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 |    8 ++++++++
>  3 files changed, 19 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;
>         };
> +       */

There's no point in commenting the sections out. If you're removing them, remove the outright.

g.

Acknowledged.

Re: [Linaro-acpi] [PATCH V2 3/4] ACPI/ARM: Add ACPI to AMBA SPI driver

[Al Stone]

On 11/22/2013 12:52 PM, Grant Likely wrote:
> On Fri, Nov 22, 2013 at 6:12 PM, Brandon Anderson
> <brandon.anderson@amd.com> wrote:
>> Neither Foundation nor RTSM have a SPI device, but here are the necessary driver
>> changes as an example of how to use acpi_amba_dsm_lookup() to get non-standard
>> parameters from ACPI.
>>
>> This was tested by wiring up a SPI device into an RTSM Fast Model.
>>
>>
>> Signed-off-by: Brandon Anderson <brandon.anderson@amd.com>
>> ---
>>   drivers/spi/spi-pl022.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>>
>> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
>> index 9c511a9..1d0a8ec 100644
>> --- a/drivers/spi/spi-pl022.c
>> +++ b/drivers/spi/spi-pl022.c
>> @@ -43,6 +43,7 @@
>>   #include <linux/gpio.h>
>>   #include <linux/of_gpio.h>
>>   #include <linux/pinctrl/consumer.h>
>> +#include <linux/amba/acpi.h>
>>
>>   /*
>>    * This macro is used to define some register default values.
>> @@ -2069,6 +2070,55 @@ pl022_platform_data_dt_get(struct device *dev)
>>          return pd;
>>   }
>>
>> +#ifdef CONFIG_ACPI
>> +static struct pl022_ssp_controller *
>> +acpi_pl022_get_platform_data(struct device *dev)
>> +{
>> +       struct pl022_ssp_controller *pd, *ret;
>> +       struct acpi_amba_dsm_entry entry;
>> +
>> +       pd = devm_kzalloc(dev, sizeof(struct pl022_ssp_controller), GFP_KERNEL);
>> +       if (!pd) {
>> +               dev_err(dev, "cannot allocate platform data memory\n");
>> +               return NULL;
>> +       }
>> +       ret = pd;
>> +
>> +       pd->bus_id = -1;
>> +       pd->enable_dma = 1;
>> +       if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "num-cs", 0, &entry) == 0) {
>> +               if (kstrtou8(entry.value, 0, &pd->num_chipselect) != 0) {
>> +                       dev_err(dev, "invalid 'num-cs' in ACPI definition\n");
>> +                       ret = NULL;
>> +               }
>> +               kfree(entry.key);
>> +               kfree(entry.value);
>> +       }
>> +       if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev),
>> +                       "autosuspend-delay", 0, &entry) == 0) {
>> +               if (kstrtoint(entry.value, 0, &pd->autosuspend_delay) != 0) {
>> +                       dev_err(dev, "invalid 'autosuspend-delay' in ACPI definition\n");
>> +                       ret = NULL;
>> +               }
>> +               kfree(entry.key);
>> +               kfree(entry.value);
>> +       }
>> +       if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "rt", 0, &entry) == 0) {
>> +               pd->rt = (entry.value && strcmp(entry.value, "1") == 0);
>> +               kfree(entry.key);
>> +               kfree(entry.value);
>> +       }
>> +
>> +       return ret;
>> +}
>
> As discussed in the ACPI vs DT thread. The kinds of lookups here are
> identical to the DT property lookups except that the function name is
> different and the property name is needlessly different. That's
> madness. In all of the trivial cases the DT and ACPI lookup code
> should be identical. We need a property value lookup function that
> both DT and ACPI can use.
>
> g.

Agreed.  I'm in the process of prototyping this code and my plan
is to have something available for comment the week after next
(unless I get _really_ bored Thanksgiving weekend...).

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone@linaro.org
-----------------------------------

RE: [Linaro-acpi] [PATCH V2 3/4] ACPI/ARM: Add ACPI to AMBA SPI driver

[Anderson, Brandon]

-----Original Message-----
From: Al Stone [mailto:al.stone@linaro.org] 
Sent: Friday, November 22, 2013 2:13 PM
To: Grant Likely; Anderson, Brandon
Cc: Rafael J. Wysocki; Russell King - ARM Linux; ACPI Devel Mailing List; linaro-acpi
Subject: Re: [Linaro-acpi] [PATCH V2 3/4] ACPI/ARM: Add ACPI to AMBA SPI driver

On 11/22/2013 12:52 PM, Grant Likely wrote:
> On Fri, Nov 22, 2013 at 6:12 PM, Brandon Anderson 
> <brandon.anderson@amd.com> wrote:
>> Neither Foundation nor RTSM have a SPI device, but here are the 
>> necessary driver changes as an example of how to use 
>> acpi_amba_dsm_lookup() to get non-standard parameters from ACPI.
>>
>> This was tested by wiring up a SPI device into an RTSM Fast Model.
>>
>>
>> Signed-off-by: Brandon Anderson <brandon.anderson@amd.com>
>> ---
>>   drivers/spi/spi-pl022.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>>
>> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c index 
>> 9c511a9..1d0a8ec 100644
>> --- a/drivers/spi/spi-pl022.c
>> +++ b/drivers/spi/spi-pl022.c
>> @@ -43,6 +43,7 @@
>>   #include <linux/gpio.h>
>>   #include <linux/of_gpio.h>
>>   #include <linux/pinctrl/consumer.h>
>> +#include <linux/amba/acpi.h>
>>
>>   /*
>>    * This macro is used to define some register default values.
>> @@ -2069,6 +2070,55 @@ pl022_platform_data_dt_get(struct device *dev)
>>          return pd;
>>   }
>>
>> +#ifdef CONFIG_ACPI
>> +static struct pl022_ssp_controller * 
>> +acpi_pl022_get_platform_data(struct device *dev) {
>> +       struct pl022_ssp_controller *pd, *ret;
>> +       struct acpi_amba_dsm_entry entry;
>> +
>> +       pd = devm_kzalloc(dev, sizeof(struct pl022_ssp_controller), GFP_KERNEL);
>> +       if (!pd) {
>> +               dev_err(dev, "cannot allocate platform data memory\n");
>> +               return NULL;
>> +       }
>> +       ret = pd;
>> +
>> +       pd->bus_id = -1;
>> +       pd->enable_dma = 1;
>> +       if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "num-cs", 0, &entry) == 0) {
>> +               if (kstrtou8(entry.value, 0, &pd->num_chipselect) != 0) {
>> +                       dev_err(dev, "invalid 'num-cs' in ACPI definition\n");
>> +                       ret = NULL;
>> +               }
>> +               kfree(entry.key);
>> +               kfree(entry.value);
>> +       }
>> +       if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev),
>> +                       "autosuspend-delay", 0, &entry) == 0) {
>> +               if (kstrtoint(entry.value, 0, &pd->autosuspend_delay) != 0) {
>> +                       dev_err(dev, "invalid 'autosuspend-delay' in ACPI definition\n");
>> +                       ret = NULL;
>> +               }
>> +               kfree(entry.key);
>> +               kfree(entry.value);
>> +       }
>> +       if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "rt", 0, &entry) == 0) {
>> +               pd->rt = (entry.value && strcmp(entry.value, "1") == 0);
>> +               kfree(entry.key);
>> +               kfree(entry.value);
>> +       }
>> +
>> +       return ret;
>> +}
>
> As discussed in the ACPI vs DT thread. The kinds of lookups here are 
> identical to the DT property lookups except that the function name is 
> different and the property name is needlessly different. That's 
> madness. In all of the trivial cases the DT and ACPI lookup code 
> should be identical. We need a property value lookup function that 
> both DT and ACPI can use.
>
> g.

Agreed.  I'm in the process of prototyping this code and my plan is to have something available for comment the week after next (unless I get _really_ bored Thanksgiving weekend...).

Great, Al! 

I agree, and Russell King has made the same comment. This patch in the series can be dropped with no ill effect, as it is an example only. Or modified to use a common lookup function once it is available.

--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone@linaro.org
-----------------------------------

Re: [PATCH V2 2/4] ACPI/ARM: Add AMBA bus ACPI module

[Matthew Garrett]

On Fri, Nov 22, 2013 at 12:12:42PM -0600, Brandon Anderson wrote:

> +	status = acpi_evaluate_object_typed(handle, "_DSM",
> +			&input, &output, ACPI_TYPE_PACKAGE);

We've got enough uses of _DSM that it'd be nice to have a generic helper 
function for setting them up. Not that this is a blocker.

> +	/* parse 2 objects per entry */

But if we're expecting anyone to use _DSM for DT properties, this really 
should be a generic function. Don't make it AMBA specific.

> +	/* key=value format for clocks is:
> +	 *	"clock-name"="apb_pclk \\_SB.CLK0"
> +	 */
> +	if (acpi_amba_dsm_lookup(handle, "clock-name", index, &entry) != 0)
> +		return NULL;

Why use DT properties for this? If you're just pointing to something 
else in the ACPI namespace then add a method that returns the object.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH V2 3/4] ACPI/ARM: Add ACPI to AMBA SPI driver

[Russell King - ARM Linux]

On Fri, Nov 22, 2013 at 12:12:43PM -0600, Brandon Anderson wrote:
> +#ifdef CONFIG_ACPI
> +static struct pl022_ssp_controller *
> +acpi_pl022_get_platform_data(struct device *dev)
> +{
> +	struct pl022_ssp_controller *pd, *ret;
> +	struct acpi_amba_dsm_entry entry;
> +
> +	pd = devm_kzalloc(dev, sizeof(struct pl022_ssp_controller), GFP_KERNEL);
> +	if (!pd) {
> +		dev_err(dev, "cannot allocate platform data memory\n");
> +		return NULL;
> +	}
> +	ret = pd;
> +
> +	pd->bus_id = -1;
> +	pd->enable_dma = 1;
> +	if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "num-cs", 0, &entry) == 0) {
> +		if (kstrtou8(entry.value, 0, &pd->num_chipselect) != 0) {
> +			dev_err(dev, "invalid 'num-cs' in ACPI definition\n");
> +			ret = NULL;
> +		}
> +		kfree(entry.key);
> +		kfree(entry.value);
> +	}
> +	if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev),
> +			"autosuspend-delay", 0, &entry) == 0) {
> +		if (kstrtoint(entry.value, 0, &pd->autosuspend_delay) != 0) {
> +			dev_err(dev, "invalid 'autosuspend-delay' in ACPI definition\n");
> +			ret = NULL;
> +		}
> +		kfree(entry.key);
> +		kfree(entry.value);
> +	}
> +	if (acpi_amba_dsm_lookup(ACPI_HANDLE(dev), "rt", 0, &entry) == 0) {
> +		pd->rt = (entry.value && strcmp(entry.value, "1") == 0);
> +		kfree(entry.key);
> +		kfree(entry.value);
> +	}
> +
> +	return ret;
> +}

I thought I'd already commented about this, and said if the properties
are the same as the DT stuff, this should be done in a common way
rather than modifing every driver.

For stuff like the above, it's really quite simple.

Rename the of_property_read_*() function to firmware_read_*() function
and make it take a struct device.

Then you can hide the of_property_read_*() if there's an OF node present
inside the firmware_read_*() function.  If not, and it has an ACPI handle,
then you can read it from ACPI.

That means the only modification that drivers see is a function rename
and a _slight_ change of argument, which is a lot saner for driver authors
to review rather than having to review an entire set of new parsing code.

Re: [PATCH V2 0/4] ACPI/ARM: AMBA bus ACPI module

[Rafael J. Wysocki]

On Friday, November 22, 2013 07:44:53 PM Anderson, Brandon wrote:
> Olof,
> I apologize for posting confusing patches to the linux-acpi list.
> 
> These patches are on top of the Linaro ACPI work, http://git.linaro.org/git-ro/arm/acpi/leg-kernel.git

So here's a rule.  I'm not going to look into any Linaro repositories, so
please don't expect me to do that.  If you want me to look at any code outside
of the mainline kernel, please post it here.

Thanks,
Rafael

Re: [PATCH V2 0/4] ACPI/ARM: AMBA bus ACPI module

[Olof Johansson]

On Fri, Nov 22, 2013 at 11:44 AM, Anderson, Brandon
<Brandon.Anderson@amd.com> wrote:
> Olof,
> I apologize for posting confusing patches to the linux-acpi list.
>
> These patches are on top of the Linaro ACPI work, http://git.linaro.org/git-ro/arm/acpi/leg-kernel.git

Hi Brandon,

First, please don't top-post replies to emails on kernel mailing
lists, we usually do in-line replies.

Posting patches that only apply to your forked/private kernel to a
community-wide list is not appropriate. Upstream mailing lists should
only see code targeted for upstream.


-Olof