All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
@ 2014-11-18  5:58 ` Ken Xue
  0 siblings, 0 replies; 15+ messages in thread
From: Ken Xue @ 2014-11-18  5:58 UTC (permalink / raw)
  To: rjw, lenb; +Cc: linux-acpi, linux-kernel, Ken Xue, Jeff Wu

This new feature is to interpret AMD specific ACPI device to platform device
such as I2C, UART found on AMD CZ and later chipsets. It is based on example
INTEL LPSS. Now, it can support AMD I2C & UART.

Signed-off-by: Ken Xue <Ken.Xue@amd.com>
Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>
---
 arch/x86/Kconfig        |  11 +++
 drivers/acpi/Makefile   |   1 +
 drivers/acpi/acpi_apd.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/internal.h |   6 ++
 drivers/acpi/scan.c     |   1 +
 5 files changed, 264 insertions(+)
 create mode 100644 drivers/acpi/acpi_apd.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ded8a67..6402c79f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -495,6 +495,17 @@ config X86_INTEL_LPSS
 	  things like clock tree (common clock framework) and pincontrol
 	  which are needed by the LPSS peripheral drivers.
 
+config X86_AMD_PLATFORM_DEVICE
+	bool "AMD ACPI2Platform devices support"
+	depends on ACPI
+	select COMMON_CLK
+	select PINCTRL
+	---help---
+	  Select to interpret AMD specific ACPI device to platform device
+	  such as I2C, UART found on AMD CARRIZO and later chipset. Selecting
+	  this option enables things like clock tree (common clock framework)
+	  and pinctrl.
+
 config IOSF_MBI
 	tristate "Intel SoC IOSF Sideband support for SoC platforms"
 	depends on PCI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index c3b2fcb..91fc1c2 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -41,6 +41,7 @@ acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o
 acpi-y				+= acpi_lpss.o
+acpi-$(CONFIG_X86_AMD_PLATFORM_DEVICE)	+= acpi_apd.o
 acpi-y				+= acpi_platform.o
 acpi-y				+= acpi_pnp.o
 acpi-y				+= int340x_thermal.o
diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
new file mode 100644
index 0000000..994b7db
--- /dev/null
+++ b/drivers/acpi/acpi_apd.c
@@ -0,0 +1,245 @@
+/*
+ * AMD ACPI support for ACPI2platform device.
+ *
+ * Copyright (c) 2014, AMD Corporation.
+ * Authors: Ken Xue <Ken.Xue@amd.com>
+ *	Jeff Wu <Jeff.Wu@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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include "internal.h"
+
+ACPI_MODULE_NAME("acpi_apd");
+struct apd_private_data;
+
+struct apd_device_desc {
+	bool clk_required;
+	bool fixed_root_clock;
+	const char *clk_name;
+	unsigned long	rate;
+	size_t prv_size_override;
+	void (*setup)(struct apd_private_data *pdata);
+};
+
+struct apd_private_data {
+	void __iomem *mmio_base;
+	resource_size_t mmio_size;
+	struct clk *clk;
+	const struct apd_device_desc *dev_desc;
+};
+
+static struct apd_device_desc amd_i2c_desc = {
+	.clk_required = true,
+	.fixed_root_clock = true,
+	.clk_name = "i2c_clk",
+	.rate = 133000000, /*(133 * 1000 * 1000)*/
+};
+
+static struct apd_device_desc amd_uart_desc = {
+	.clk_required = true,
+	.fixed_root_clock = true,
+	.clk_name = "uart_clk",
+	.rate = 48000000,
+};
+
+static const struct acpi_device_id acpi_apd_device_ids[] = {
+	/* Generic apd devices */
+	{ "AMD0010", (unsigned long)&amd_i2c_desc },
+	{ "AMD0020", (unsigned long)&amd_uart_desc },
+	{ }
+};
+
+static int is_memory(struct acpi_resource *res, void *not_used)
+{
+	struct resource r;
+
+	return !acpi_dev_resource_memory(res, &r);
+}
+
+static int register_device_clock(struct acpi_device *adev,
+				 struct apd_private_data *pdata)
+{
+	const struct apd_device_desc *dev_desc = pdata->dev_desc;
+	struct clk *clk = ERR_PTR(-ENODEV);
+
+	clk = pdata->clk;
+	if (!clk && dev_desc->fixed_root_clock) {
+		clk = clk_register_fixed_rate(&adev->dev, dev_name(&adev->dev),
+				      NULL, CLK_IS_ROOT, dev_desc->rate);
+		pdata->clk = clk;
+		clk_register_clkdev(clk, NULL, dev_name(&adev->dev));
+	}
+
+	return 0;
+}
+
+static int acpi_apd_create_device(struct acpi_device *adev,
+				   const struct acpi_device_id *id)
+{
+	struct apd_device_desc *dev_desc;
+	struct apd_private_data *pdata;
+	struct resource_list_entry *rentry;
+	struct list_head resource_list;
+	struct platform_device *pdev;
+	int ret;
+
+	dev_desc = (struct apd_device_desc *)id->driver_data;
+	if (!dev_desc) {
+		pdev = acpi_create_platform_device(adev);
+		return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1;
+	}
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&resource_list);
+	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
+	if (ret < 0)
+		goto err_out;
+
+	list_for_each_entry(rentry, &resource_list, node)
+		if (resource_type(&rentry->res) == IORESOURCE_MEM) {
+			if (dev_desc->prv_size_override)
+				pdata->mmio_size = dev_desc->prv_size_override;
+			else
+				pdata->mmio_size = resource_size(&rentry->res);
+			pdata->mmio_base = ioremap(rentry->res.start,
+						   pdata->mmio_size);
+			break;
+		}
+
+	acpi_dev_free_resource_list(&resource_list);
+
+	pdata->dev_desc = dev_desc;
+
+	if (dev_desc->clk_required) {
+		ret = register_device_clock(adev, pdata);
+		if (ret) {
+			/* Skip the device, but continue the namespace scan. */
+			ret = 0;
+			goto err_out;
+		}
+	}
+
+	/*
+	 * This works around a known issue in ACPI tables where apd devices
+	 * have _PS0 and _PS3 without _PSC (and no power resources), so
+	 * acpi_bus_init_power() will assume that the BIOS has put them into D0.
+	 */
+	ret = acpi_device_fix_up_power(adev);
+	if (ret) {
+		/* Skip the device, but continue the namespace scan. */
+		ret = 0;
+		goto err_out;
+	}
+
+	if (dev_desc->setup)
+		dev_desc->setup(pdata);
+
+	adev->driver_data = pdata;
+	pdev = acpi_create_platform_device(adev);
+	if (!IS_ERR_OR_NULL(pdev)) {
+		device_enable_async_suspend(&pdev->dev);
+		return ret;
+	}
+
+	ret = PTR_ERR(pdev);
+	adev->driver_data = NULL;
+
+ err_out:
+	kfree(pdata);
+	return ret;
+}
+
+static ssize_t apd_device_desc_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int ret;
+	struct acpi_device *adev;
+	struct apd_private_data *pdata;
+
+	ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev);
+	if (WARN_ON(ret))
+		return ret;
+
+	pdata = acpi_driver_data(adev);
+	if (WARN_ON(!pdata || !pdata->dev_desc))
+		return -ENODEV;
+
+	if (pdata->dev_desc->clk_required)
+		return sprintf(buf, "Required clk: %s %s %ld\n",
+			pdata->dev_desc->clk_name,
+			pdata->dev_desc->fixed_root_clock ?
+			"fix rate" : "no fix rate",
+			pdata->dev_desc->rate);
+	else
+		return sprintf(buf, "No need clk\n");
+}
+
+static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL);
+
+static struct attribute *apd_attrs[] = {
+	&dev_attr_device_desc.attr,
+	NULL,
+};
+
+static struct attribute_group apd_attr_group = {
+	.attrs = apd_attrs,
+	.name = "apd_ltr",
+};
+
+static int acpi_apd_platform_notify(struct notifier_block *nb,
+				     unsigned long action, void *data)
+{
+	struct platform_device *pdev = to_platform_device(data);
+	struct apd_private_data *pdata;
+	struct acpi_device *adev;
+	const struct acpi_device_id *id;
+	int ret = 0;
+
+	id = acpi_match_device(acpi_apd_device_ids, &pdev->dev);
+	if (!id || !id->driver_data)
+		return 0;
+
+	if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
+		return 0;
+
+	pdata = acpi_driver_data(adev);
+	if (!pdata || !pdata->mmio_base)
+		return 0;
+
+	if (action == BUS_NOTIFY_ADD_DEVICE)
+		ret = sysfs_create_group(&pdev->dev.kobj, &apd_attr_group);
+	else if (action == BUS_NOTIFY_DEL_DEVICE)
+		sysfs_remove_group(&pdev->dev.kobj, &apd_attr_group);
+
+	return ret;
+}
+
+static struct notifier_block acpi_apd_nb = {
+	.notifier_call = acpi_apd_platform_notify,
+};
+
+static struct acpi_scan_handler apd_handler = {
+	.ids = acpi_apd_device_ids,
+	.attach = acpi_apd_create_device,
+};
+
+void __init acpi_apd_init(void)
+{
+	bus_register_notifier(&platform_bus_type, &acpi_apd_nb);
+	acpi_scan_add_handler(&apd_handler);
+}
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 447f6d6..c8a0e8e 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void) { return; }
 #endif
 void acpi_lpss_init(void);
 
+#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
+void acpi_apd_init(void);
+#else
+static inline void acpi_apd_init(void) {}
+#endif
+
 acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
 bool acpi_queue_hotplug_work(struct work_struct *work);
 void acpi_device_hotplug(struct acpi_device *adev, u32 src);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0476e90..24fef2b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void)
 	acpi_pci_link_init();
 	acpi_processor_init();
 	acpi_lpss_init();
+	acpi_apd_init();
 	acpi_cmos_rtc_init();
 	acpi_container_init();
 	acpi_memory_hotplug_init();
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
@ 2014-11-18  5:58 ` Ken Xue
  0 siblings, 0 replies; 15+ messages in thread
From: Ken Xue @ 2014-11-18  5:58 UTC (permalink / raw)
  To: rjw, lenb; +Cc: linux-acpi, linux-kernel, Ken Xue, Jeff Wu

This new feature is to interpret AMD specific ACPI device to platform device
such as I2C, UART found on AMD CZ and later chipsets. It is based on example
INTEL LPSS. Now, it can support AMD I2C & UART.

Signed-off-by: Ken Xue <Ken.Xue@amd.com>
Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>
---
 arch/x86/Kconfig        |  11 +++
 drivers/acpi/Makefile   |   1 +
 drivers/acpi/acpi_apd.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/internal.h |   6 ++
 drivers/acpi/scan.c     |   1 +
 5 files changed, 264 insertions(+)
 create mode 100644 drivers/acpi/acpi_apd.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ded8a67..6402c79f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -495,6 +495,17 @@ config X86_INTEL_LPSS
 	  things like clock tree (common clock framework) and pincontrol
 	  which are needed by the LPSS peripheral drivers.
 
+config X86_AMD_PLATFORM_DEVICE
+	bool "AMD ACPI2Platform devices support"
+	depends on ACPI
+	select COMMON_CLK
+	select PINCTRL
+	---help---
+	  Select to interpret AMD specific ACPI device to platform device
+	  such as I2C, UART found on AMD CARRIZO and later chipset. Selecting
+	  this option enables things like clock tree (common clock framework)
+	  and pinctrl.
+
 config IOSF_MBI
 	tristate "Intel SoC IOSF Sideband support for SoC platforms"
 	depends on PCI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index c3b2fcb..91fc1c2 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -41,6 +41,7 @@ acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o
 acpi-y				+= acpi_lpss.o
+acpi-$(CONFIG_X86_AMD_PLATFORM_DEVICE)	+= acpi_apd.o
 acpi-y				+= acpi_platform.o
 acpi-y				+= acpi_pnp.o
 acpi-y				+= int340x_thermal.o
diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
new file mode 100644
index 0000000..994b7db
--- /dev/null
+++ b/drivers/acpi/acpi_apd.c
@@ -0,0 +1,245 @@
+/*
+ * AMD ACPI support for ACPI2platform device.
+ *
+ * Copyright (c) 2014, AMD Corporation.
+ * Authors: Ken Xue <Ken.Xue@amd.com>
+ *	Jeff Wu <Jeff.Wu@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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include "internal.h"
+
+ACPI_MODULE_NAME("acpi_apd");
+struct apd_private_data;
+
+struct apd_device_desc {
+	bool clk_required;
+	bool fixed_root_clock;
+	const char *clk_name;
+	unsigned long	rate;
+	size_t prv_size_override;
+	void (*setup)(struct apd_private_data *pdata);
+};
+
+struct apd_private_data {
+	void __iomem *mmio_base;
+	resource_size_t mmio_size;
+	struct clk *clk;
+	const struct apd_device_desc *dev_desc;
+};
+
+static struct apd_device_desc amd_i2c_desc = {
+	.clk_required = true,
+	.fixed_root_clock = true,
+	.clk_name = "i2c_clk",
+	.rate = 133000000, /*(133 * 1000 * 1000)*/
+};
+
+static struct apd_device_desc amd_uart_desc = {
+	.clk_required = true,
+	.fixed_root_clock = true,
+	.clk_name = "uart_clk",
+	.rate = 48000000,
+};
+
+static const struct acpi_device_id acpi_apd_device_ids[] = {
+	/* Generic apd devices */
+	{ "AMD0010", (unsigned long)&amd_i2c_desc },
+	{ "AMD0020", (unsigned long)&amd_uart_desc },
+	{ }
+};
+
+static int is_memory(struct acpi_resource *res, void *not_used)
+{
+	struct resource r;
+
+	return !acpi_dev_resource_memory(res, &r);
+}
+
+static int register_device_clock(struct acpi_device *adev,
+				 struct apd_private_data *pdata)
+{
+	const struct apd_device_desc *dev_desc = pdata->dev_desc;
+	struct clk *clk = ERR_PTR(-ENODEV);
+
+	clk = pdata->clk;
+	if (!clk && dev_desc->fixed_root_clock) {
+		clk = clk_register_fixed_rate(&adev->dev, dev_name(&adev->dev),
+				      NULL, CLK_IS_ROOT, dev_desc->rate);
+		pdata->clk = clk;
+		clk_register_clkdev(clk, NULL, dev_name(&adev->dev));
+	}
+
+	return 0;
+}
+
+static int acpi_apd_create_device(struct acpi_device *adev,
+				   const struct acpi_device_id *id)
+{
+	struct apd_device_desc *dev_desc;
+	struct apd_private_data *pdata;
+	struct resource_list_entry *rentry;
+	struct list_head resource_list;
+	struct platform_device *pdev;
+	int ret;
+
+	dev_desc = (struct apd_device_desc *)id->driver_data;
+	if (!dev_desc) {
+		pdev = acpi_create_platform_device(adev);
+		return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1;
+	}
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&resource_list);
+	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
+	if (ret < 0)
+		goto err_out;
+
+	list_for_each_entry(rentry, &resource_list, node)
+		if (resource_type(&rentry->res) == IORESOURCE_MEM) {
+			if (dev_desc->prv_size_override)
+				pdata->mmio_size = dev_desc->prv_size_override;
+			else
+				pdata->mmio_size = resource_size(&rentry->res);
+			pdata->mmio_base = ioremap(rentry->res.start,
+						   pdata->mmio_size);
+			break;
+		}
+
+	acpi_dev_free_resource_list(&resource_list);
+
+	pdata->dev_desc = dev_desc;
+
+	if (dev_desc->clk_required) {
+		ret = register_device_clock(adev, pdata);
+		if (ret) {
+			/* Skip the device, but continue the namespace scan. */
+			ret = 0;
+			goto err_out;
+		}
+	}
+
+	/*
+	 * This works around a known issue in ACPI tables where apd devices
+	 * have _PS0 and _PS3 without _PSC (and no power resources), so
+	 * acpi_bus_init_power() will assume that the BIOS has put them into D0.
+	 */
+	ret = acpi_device_fix_up_power(adev);
+	if (ret) {
+		/* Skip the device, but continue the namespace scan. */
+		ret = 0;
+		goto err_out;
+	}
+
+	if (dev_desc->setup)
+		dev_desc->setup(pdata);
+
+	adev->driver_data = pdata;
+	pdev = acpi_create_platform_device(adev);
+	if (!IS_ERR_OR_NULL(pdev)) {
+		device_enable_async_suspend(&pdev->dev);
+		return ret;
+	}
+
+	ret = PTR_ERR(pdev);
+	adev->driver_data = NULL;
+
+ err_out:
+	kfree(pdata);
+	return ret;
+}
+
+static ssize_t apd_device_desc_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int ret;
+	struct acpi_device *adev;
+	struct apd_private_data *pdata;
+
+	ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev);
+	if (WARN_ON(ret))
+		return ret;
+
+	pdata = acpi_driver_data(adev);
+	if (WARN_ON(!pdata || !pdata->dev_desc))
+		return -ENODEV;
+
+	if (pdata->dev_desc->clk_required)
+		return sprintf(buf, "Required clk: %s %s %ld\n",
+			pdata->dev_desc->clk_name,
+			pdata->dev_desc->fixed_root_clock ?
+			"fix rate" : "no fix rate",
+			pdata->dev_desc->rate);
+	else
+		return sprintf(buf, "No need clk\n");
+}
+
+static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL);
+
+static struct attribute *apd_attrs[] = {
+	&dev_attr_device_desc.attr,
+	NULL,
+};
+
+static struct attribute_group apd_attr_group = {
+	.attrs = apd_attrs,
+	.name = "apd_ltr",
+};
+
+static int acpi_apd_platform_notify(struct notifier_block *nb,
+				     unsigned long action, void *data)
+{
+	struct platform_device *pdev = to_platform_device(data);
+	struct apd_private_data *pdata;
+	struct acpi_device *adev;
+	const struct acpi_device_id *id;
+	int ret = 0;
+
+	id = acpi_match_device(acpi_apd_device_ids, &pdev->dev);
+	if (!id || !id->driver_data)
+		return 0;
+
+	if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
+		return 0;
+
+	pdata = acpi_driver_data(adev);
+	if (!pdata || !pdata->mmio_base)
+		return 0;
+
+	if (action == BUS_NOTIFY_ADD_DEVICE)
+		ret = sysfs_create_group(&pdev->dev.kobj, &apd_attr_group);
+	else if (action == BUS_NOTIFY_DEL_DEVICE)
+		sysfs_remove_group(&pdev->dev.kobj, &apd_attr_group);
+
+	return ret;
+}
+
+static struct notifier_block acpi_apd_nb = {
+	.notifier_call = acpi_apd_platform_notify,
+};
+
+static struct acpi_scan_handler apd_handler = {
+	.ids = acpi_apd_device_ids,
+	.attach = acpi_apd_create_device,
+};
+
+void __init acpi_apd_init(void)
+{
+	bus_register_notifier(&platform_bus_type, &acpi_apd_nb);
+	acpi_scan_add_handler(&apd_handler);
+}
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 447f6d6..c8a0e8e 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void) { return; }
 #endif
 void acpi_lpss_init(void);
 
+#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
+void acpi_apd_init(void);
+#else
+static inline void acpi_apd_init(void) {}
+#endif
+
 acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
 bool acpi_queue_hotplug_work(struct work_struct *work);
 void acpi_device_hotplug(struct acpi_device *adev, u32 src);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0476e90..24fef2b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void)
 	acpi_pci_link_init();
 	acpi_processor_init();
 	acpi_lpss_init();
+	acpi_apd_init();
 	acpi_cmos_rtc_init();
 	acpi_container_init();
 	acpi_memory_hotplug_init();
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* RE: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-18  5:58 ` Ken Xue
  (?)
@ 2014-11-21  5:15 ` Xue, Ken
  2014-11-21 14:58   ` Rafael J. Wysocki
  -1 siblings, 1 reply; 15+ messages in thread
From: Xue, Ken @ 2014-11-21  5:15 UTC (permalink / raw)
  To: rjw, lenb, mika.wasterberg; +Cc: linux-acpi, linux-kernel, Wu, Jeff

Hi Len,  Rafael J & Mika,
	Please help to review this patch. And tell me any of your concern. Thanks.

	I understand it is a trend that convert a ACPI device to be a platform device.
	For AMD, we try to use this patch to match this trend well. The patch based on INTEL LPSS . 
	But we cannot reuse LPSS, because of specific definition of "lpss_device_desc" for LPSS.

Regards,
Ken


-----Original Message-----
From: Ken Xue [mailto:Ken.Xue@amd.com] 
Sent: Tuesday, November 18, 2014 1:58 PM
To: rjw@rjwysocki.net; lenb@kernel.org
Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; Xue, Ken; Wu, Jeff
Subject: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.

This new feature is to interpret AMD specific ACPI device to platform device such as I2C, UART found on AMD CZ and later chipsets. It is based on example INTEL LPSS. Now, it can support AMD I2C & UART.

Signed-off-by: Ken Xue <Ken.Xue@amd.com>
Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>
---
 arch/x86/Kconfig        |  11 +++
 drivers/acpi/Makefile   |   1 +
 drivers/acpi/acpi_apd.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/internal.h |   6 ++
 drivers/acpi/scan.c     |   1 +
 5 files changed, 264 insertions(+)
 create mode 100644 drivers/acpi/acpi_apd.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ded8a67..6402c79f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -495,6 +495,17 @@ config X86_INTEL_LPSS
 	  things like clock tree (common clock framework) and pincontrol
 	  which are needed by the LPSS peripheral drivers.
 
+config X86_AMD_PLATFORM_DEVICE
+	bool "AMD ACPI2Platform devices support"
+	depends on ACPI
+	select COMMON_CLK
+	select PINCTRL
+	---help---
+	  Select to interpret AMD specific ACPI device to platform device
+	  such as I2C, UART found on AMD CARRIZO and later chipset. Selecting
+	  this option enables things like clock tree (common clock framework)
+	  and pinctrl.
+
 config IOSF_MBI
 	tristate "Intel SoC IOSF Sideband support for SoC platforms"
 	depends on PCI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index c3b2fcb..91fc1c2 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -41,6 +41,7 @@ acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o
 acpi-y				+= acpi_lpss.o
+acpi-$(CONFIG_X86_AMD_PLATFORM_DEVICE)	+= acpi_apd.o
 acpi-y				+= acpi_platform.o
 acpi-y				+= acpi_pnp.o
 acpi-y				+= int340x_thermal.o
diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c new file mode 100644 index 0000000..994b7db
--- /dev/null
+++ b/drivers/acpi/acpi_apd.c
@@ -0,0 +1,245 @@
+/*
+ * AMD ACPI support for ACPI2platform device.
+ *
+ * Copyright (c) 2014, AMD Corporation.
+ * Authors: Ken Xue <Ken.Xue@amd.com>
+ *	Jeff Wu <Jeff.Wu@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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include "internal.h"
+
+ACPI_MODULE_NAME("acpi_apd");
+struct apd_private_data;
+
+struct apd_device_desc {
+	bool clk_required;
+	bool fixed_root_clock;
+	const char *clk_name;
+	unsigned long	rate;
+	size_t prv_size_override;
+	void (*setup)(struct apd_private_data *pdata); };
+
+struct apd_private_data {
+	void __iomem *mmio_base;
+	resource_size_t mmio_size;
+	struct clk *clk;
+	const struct apd_device_desc *dev_desc; };
+
+static struct apd_device_desc amd_i2c_desc = {
+	.clk_required = true,
+	.fixed_root_clock = true,
+	.clk_name = "i2c_clk",
+	.rate = 133000000, /*(133 * 1000 * 1000)*/ };
+
+static struct apd_device_desc amd_uart_desc = {
+	.clk_required = true,
+	.fixed_root_clock = true,
+	.clk_name = "uart_clk",
+	.rate = 48000000,
+};
+
+static const struct acpi_device_id acpi_apd_device_ids[] = {
+	/* Generic apd devices */
+	{ "AMD0010", (unsigned long)&amd_i2c_desc },
+	{ "AMD0020", (unsigned long)&amd_uart_desc },
+	{ }
+};
+
+static int is_memory(struct acpi_resource *res, void *not_used) {
+	struct resource r;
+
+	return !acpi_dev_resource_memory(res, &r); }
+
+static int register_device_clock(struct acpi_device *adev,
+				 struct apd_private_data *pdata)
+{
+	const struct apd_device_desc *dev_desc = pdata->dev_desc;
+	struct clk *clk = ERR_PTR(-ENODEV);
+
+	clk = pdata->clk;
+	if (!clk && dev_desc->fixed_root_clock) {
+		clk = clk_register_fixed_rate(&adev->dev, dev_name(&adev->dev),
+				      NULL, CLK_IS_ROOT, dev_desc->rate);
+		pdata->clk = clk;
+		clk_register_clkdev(clk, NULL, dev_name(&adev->dev));
+	}
+
+	return 0;
+}
+
+static int acpi_apd_create_device(struct acpi_device *adev,
+				   const struct acpi_device_id *id) {
+	struct apd_device_desc *dev_desc;
+	struct apd_private_data *pdata;
+	struct resource_list_entry *rentry;
+	struct list_head resource_list;
+	struct platform_device *pdev;
+	int ret;
+
+	dev_desc = (struct apd_device_desc *)id->driver_data;
+	if (!dev_desc) {
+		pdev = acpi_create_platform_device(adev);
+		return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1;
+	}
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&resource_list);
+	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
+	if (ret < 0)
+		goto err_out;
+
+	list_for_each_entry(rentry, &resource_list, node)
+		if (resource_type(&rentry->res) == IORESOURCE_MEM) {
+			if (dev_desc->prv_size_override)
+				pdata->mmio_size = dev_desc->prv_size_override;
+			else
+				pdata->mmio_size = resource_size(&rentry->res);
+			pdata->mmio_base = ioremap(rentry->res.start,
+						   pdata->mmio_size);
+			break;
+		}
+
+	acpi_dev_free_resource_list(&resource_list);
+
+	pdata->dev_desc = dev_desc;
+
+	if (dev_desc->clk_required) {
+		ret = register_device_clock(adev, pdata);
+		if (ret) {
+			/* Skip the device, but continue the namespace scan. */
+			ret = 0;
+			goto err_out;
+		}
+	}
+
+	/*
+	 * This works around a known issue in ACPI tables where apd devices
+	 * have _PS0 and _PS3 without _PSC (and no power resources), so
+	 * acpi_bus_init_power() will assume that the BIOS has put them into D0.
+	 */
+	ret = acpi_device_fix_up_power(adev);
+	if (ret) {
+		/* Skip the device, but continue the namespace scan. */
+		ret = 0;
+		goto err_out;
+	}
+
+	if (dev_desc->setup)
+		dev_desc->setup(pdata);
+
+	adev->driver_data = pdata;
+	pdev = acpi_create_platform_device(adev);
+	if (!IS_ERR_OR_NULL(pdev)) {
+		device_enable_async_suspend(&pdev->dev);
+		return ret;
+	}
+
+	ret = PTR_ERR(pdev);
+	adev->driver_data = NULL;
+
+ err_out:
+	kfree(pdata);
+	return ret;
+}
+
+static ssize_t apd_device_desc_show(struct device *dev,
+				struct device_attribute *attr, char *buf) {
+	int ret;
+	struct acpi_device *adev;
+	struct apd_private_data *pdata;
+
+	ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev);
+	if (WARN_ON(ret))
+		return ret;
+
+	pdata = acpi_driver_data(adev);
+	if (WARN_ON(!pdata || !pdata->dev_desc))
+		return -ENODEV;
+
+	if (pdata->dev_desc->clk_required)
+		return sprintf(buf, "Required clk: %s %s %ld\n",
+			pdata->dev_desc->clk_name,
+			pdata->dev_desc->fixed_root_clock ?
+			"fix rate" : "no fix rate",
+			pdata->dev_desc->rate);
+	else
+		return sprintf(buf, "No need clk\n"); }
+
+static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL);
+
+static struct attribute *apd_attrs[] = {
+	&dev_attr_device_desc.attr,
+	NULL,
+};
+
+static struct attribute_group apd_attr_group = {
+	.attrs = apd_attrs,
+	.name = "apd_ltr",
+};
+
+static int acpi_apd_platform_notify(struct notifier_block *nb,
+				     unsigned long action, void *data) {
+	struct platform_device *pdev = to_platform_device(data);
+	struct apd_private_data *pdata;
+	struct acpi_device *adev;
+	const struct acpi_device_id *id;
+	int ret = 0;
+
+	id = acpi_match_device(acpi_apd_device_ids, &pdev->dev);
+	if (!id || !id->driver_data)
+		return 0;
+
+	if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
+		return 0;
+
+	pdata = acpi_driver_data(adev);
+	if (!pdata || !pdata->mmio_base)
+		return 0;
+
+	if (action == BUS_NOTIFY_ADD_DEVICE)
+		ret = sysfs_create_group(&pdev->dev.kobj, &apd_attr_group);
+	else if (action == BUS_NOTIFY_DEL_DEVICE)
+		sysfs_remove_group(&pdev->dev.kobj, &apd_attr_group);
+
+	return ret;
+}
+
+static struct notifier_block acpi_apd_nb = {
+	.notifier_call = acpi_apd_platform_notify, };
+
+static struct acpi_scan_handler apd_handler = {
+	.ids = acpi_apd_device_ids,
+	.attach = acpi_apd_create_device,
+};
+
+void __init acpi_apd_init(void)
+{
+	bus_register_notifier(&platform_bus_type, &acpi_apd_nb);
+	acpi_scan_add_handler(&apd_handler);
+}
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 447f6d6..c8a0e8e 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void) { return; }  #endif  void acpi_lpss_init(void);
 
+#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
+void acpi_apd_init(void);
+#else
+static inline void acpi_apd_init(void) {} #endif
+
 acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);  bool acpi_queue_hotplug_work(struct work_struct *work);  void acpi_device_hotplug(struct acpi_device *adev, u32 src); diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 0476e90..24fef2b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void)
 	acpi_pci_link_init();
 	acpi_processor_init();
 	acpi_lpss_init();
+	acpi_apd_init();
 	acpi_cmos_rtc_init();
 	acpi_container_init();
 	acpi_memory_hotplug_init();
--
1.9.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-21  5:15 ` Xue, Ken
@ 2014-11-21 14:58   ` Rafael J. Wysocki
  0 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2014-11-21 14:58 UTC (permalink / raw)
  To: Xue, Ken; +Cc: lenb, mika.wasterberg, linux-acpi, linux-kernel, Wu, Jeff

On Friday, November 21, 2014 05:15:46 AM Xue, Ken wrote:
> Hi Len,  Rafael J & Mika,
> 	Please help to review this patch. And tell me any of your concern. Thanks.
> 
> 	I understand it is a trend that convert a ACPI device to be a platform device.
> 	For AMD, we try to use this patch to match this trend well. The patch based on INTEL LPSS . 
> 	But we cannot reuse LPSS, because of specific definition of "lpss_device_desc" for LPSS.


You don't have to resend the patch every day for us to notice it.  We're
actually in the process of reviewing your original submission.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-24  1:15 ` Rafael J. Wysocki
@ 2014-11-24  1:02     ` Xue, Ken
  0 siblings, 0 replies; 15+ messages in thread
From: Xue, Ken @ 2014-11-24  1:02 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: lenb, linux-acpi, linux-kernel, Wu, Jeff, Mika Westerberg,
	Andriy Shevchenko


On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote:
> This new feature is to interpret AMD specific ACPI device to platform 
> device such as I2C, UART found on AMD CZ and later chipsets. It is 
> based on example INTEL LPSS. Now, it can support AMD I2C & UART.
> 
> Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>

Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead.  What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)?

Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen.

[ken] sounds fair enough.  Let me take action to merge drivers to acpi_soc.c ? or you have other plan?

[...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
@ 2014-11-24  1:02     ` Xue, Ken
  0 siblings, 0 replies; 15+ messages in thread
From: Xue, Ken @ 2014-11-24  1:02 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: lenb, linux-acpi, linux-kernel, Wu, Jeff, Mika Westerberg,
	Andriy Shevchenko

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1008 bytes --]


On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote:
> This new feature is to interpret AMD specific ACPI device to platform 
> device such as I2C, UART found on AMD CZ and later chipsets. It is 
> based on example INTEL LPSS. Now, it can support AMD I2C & UART.
> 
> Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>

Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead.  What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)?

Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen.

[ken] sounds fair enough.  Let me take action to merge drivers to acpi_soc.c ? or you have other plan?

[...]
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-18  5:58 ` Ken Xue
  (?)
  (?)
@ 2014-11-24  1:15 ` Rafael J. Wysocki
  2014-11-24  1:02     ` Xue, Ken
  -1 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2014-11-24  1:15 UTC (permalink / raw)
  To: Ken Xue
  Cc: lenb, linux-acpi, linux-kernel, Jeff Wu, Mika Westerberg,
	Andriy Shevchenko

On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote:
> This new feature is to interpret AMD specific ACPI device to platform device
> such as I2C, UART found on AMD CZ and later chipsets. It is based on example
> INTEL LPSS. Now, it can support AMD I2C & UART.
> 
> Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>

Generally speaking, this seems to duplicate much code from acpi_lpss which
should be re-used instead.  What about moving the code that will be common
between acpi_lpss and the new driver into a new file (say acpi_soc.c)?

Also, you need to avoid automatic creation of platform devices when
!X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things
will happen.


> ---
>  arch/x86/Kconfig        |  11 +++
>  drivers/acpi/Makefile   |   1 +
>  drivers/acpi/acpi_apd.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/internal.h |   6 ++
>  drivers/acpi/scan.c     |   1 +
>  5 files changed, 264 insertions(+)
>  create mode 100644 drivers/acpi/acpi_apd.c
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index ded8a67..6402c79f 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -495,6 +495,17 @@ config X86_INTEL_LPSS
>  	  things like clock tree (common clock framework) and pincontrol
>  	  which are needed by the LPSS peripheral drivers.
>  
> +config X86_AMD_PLATFORM_DEVICE
> +	bool "AMD ACPI2Platform devices support"
> +	depends on ACPI
> +	select COMMON_CLK
> +	select PINCTRL
> +	---help---
> +	  Select to interpret AMD specific ACPI device to platform device
> +	  such as I2C, UART found on AMD CARRIZO and later chipset. Selecting
> +	  this option enables things like clock tree (common clock framework)
> +	  and pinctrl.
> +
>  config IOSF_MBI
>  	tristate "Intel SoC IOSF Sideband support for SoC platforms"
>  	depends on PCI
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index c3b2fcb..91fc1c2 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -41,6 +41,7 @@ acpi-y				+= ec.o
>  acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
>  acpi-y				+= pci_root.o pci_link.o pci_irq.o
>  acpi-y				+= acpi_lpss.o
> +acpi-$(CONFIG_X86_AMD_PLATFORM_DEVICE)	+= acpi_apd.o
>  acpi-y				+= acpi_platform.o
>  acpi-y				+= acpi_pnp.o
>  acpi-y				+= int340x_thermal.o
> diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
> new file mode 100644
> index 0000000..994b7db
> --- /dev/null
> +++ b/drivers/acpi/acpi_apd.c
> @@ -0,0 +1,245 @@
> +/*
> + * AMD ACPI support for ACPI2platform device.
> + *
> + * Copyright (c) 2014, AMD Corporation.
> + * Authors: Ken Xue <Ken.Xue@amd.com>
> + *	Jeff Wu <Jeff.Wu@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.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "internal.h"
> +
> +ACPI_MODULE_NAME("acpi_apd");
> +struct apd_private_data;
> +
> +struct apd_device_desc {
> +	bool clk_required;
> +	bool fixed_root_clock;
> +	const char *clk_name;
> +	unsigned long	rate;
> +	size_t prv_size_override;
> +	void (*setup)(struct apd_private_data *pdata);
> +};
> +
> +struct apd_private_data {
> +	void __iomem *mmio_base;
> +	resource_size_t mmio_size;
> +	struct clk *clk;
> +	const struct apd_device_desc *dev_desc;
> +};
> +
> +static struct apd_device_desc amd_i2c_desc = {
> +	.clk_required = true,
> +	.fixed_root_clock = true,
> +	.clk_name = "i2c_clk",
> +	.rate = 133000000, /*(133 * 1000 * 1000)*/
> +};
> +
> +static struct apd_device_desc amd_uart_desc = {
> +	.clk_required = true,
> +	.fixed_root_clock = true,
> +	.clk_name = "uart_clk",
> +	.rate = 48000000,
> +};
> +
> +static const struct acpi_device_id acpi_apd_device_ids[] = {
> +	/* Generic apd devices */
> +	{ "AMD0010", (unsigned long)&amd_i2c_desc },
> +	{ "AMD0020", (unsigned long)&amd_uart_desc },
> +	{ }
> +};
> +
> +static int is_memory(struct acpi_resource *res, void *not_used)
> +{
> +	struct resource r;
> +
> +	return !acpi_dev_resource_memory(res, &r);
> +}
> +
> +static int register_device_clock(struct acpi_device *adev,
> +				 struct apd_private_data *pdata)
> +{
> +	const struct apd_device_desc *dev_desc = pdata->dev_desc;
> +	struct clk *clk = ERR_PTR(-ENODEV);
> +
> +	clk = pdata->clk;
> +	if (!clk && dev_desc->fixed_root_clock) {
> +		clk = clk_register_fixed_rate(&adev->dev, dev_name(&adev->dev),
> +				      NULL, CLK_IS_ROOT, dev_desc->rate);
> +		pdata->clk = clk;
> +		clk_register_clkdev(clk, NULL, dev_name(&adev->dev));
> +	}
> +
> +	return 0;
> +}
> +
> +static int acpi_apd_create_device(struct acpi_device *adev,
> +				   const struct acpi_device_id *id)
> +{
> +	struct apd_device_desc *dev_desc;
> +	struct apd_private_data *pdata;
> +	struct resource_list_entry *rentry;
> +	struct list_head resource_list;
> +	struct platform_device *pdev;
> +	int ret;
> +
> +	dev_desc = (struct apd_device_desc *)id->driver_data;
> +	if (!dev_desc) {
> +		pdev = acpi_create_platform_device(adev);
> +		return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1;
> +	}
> +
> +	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&resource_list);
> +	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
> +	if (ret < 0)
> +		goto err_out;
> +
> +	list_for_each_entry(rentry, &resource_list, node)
> +		if (resource_type(&rentry->res) == IORESOURCE_MEM) {
> +			if (dev_desc->prv_size_override)
> +				pdata->mmio_size = dev_desc->prv_size_override;
> +			else
> +				pdata->mmio_size = resource_size(&rentry->res);
> +			pdata->mmio_base = ioremap(rentry->res.start,
> +						   pdata->mmio_size);
> +			break;
> +		}
> +
> +	acpi_dev_free_resource_list(&resource_list);
> +
> +	pdata->dev_desc = dev_desc;
> +
> +	if (dev_desc->clk_required) {
> +		ret = register_device_clock(adev, pdata);
> +		if (ret) {
> +			/* Skip the device, but continue the namespace scan. */
> +			ret = 0;
> +			goto err_out;
> +		}
> +	}
> +
> +	/*
> +	 * This works around a known issue in ACPI tables where apd devices
> +	 * have _PS0 and _PS3 without _PSC (and no power resources), so
> +	 * acpi_bus_init_power() will assume that the BIOS has put them into D0.
> +	 */
> +	ret = acpi_device_fix_up_power(adev);
> +	if (ret) {
> +		/* Skip the device, but continue the namespace scan. */
> +		ret = 0;
> +		goto err_out;
> +	}
> +
> +	if (dev_desc->setup)
> +		dev_desc->setup(pdata);
> +
> +	adev->driver_data = pdata;
> +	pdev = acpi_create_platform_device(adev);
> +	if (!IS_ERR_OR_NULL(pdev)) {
> +		device_enable_async_suspend(&pdev->dev);
> +		return ret;
> +	}
> +
> +	ret = PTR_ERR(pdev);
> +	adev->driver_data = NULL;
> +
> + err_out:
> +	kfree(pdata);
> +	return ret;
> +}
> +
> +static ssize_t apd_device_desc_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	int ret;
> +	struct acpi_device *adev;
> +	struct apd_private_data *pdata;
> +
> +	ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev);
> +	if (WARN_ON(ret))
> +		return ret;
> +
> +	pdata = acpi_driver_data(adev);
> +	if (WARN_ON(!pdata || !pdata->dev_desc))
> +		return -ENODEV;
> +
> +	if (pdata->dev_desc->clk_required)
> +		return sprintf(buf, "Required clk: %s %s %ld\n",
> +			pdata->dev_desc->clk_name,
> +			pdata->dev_desc->fixed_root_clock ?
> +			"fix rate" : "no fix rate",
> +			pdata->dev_desc->rate);
> +	else
> +		return sprintf(buf, "No need clk\n");
> +}
> +
> +static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL);
> +
> +static struct attribute *apd_attrs[] = {
> +	&dev_attr_device_desc.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group apd_attr_group = {
> +	.attrs = apd_attrs,
> +	.name = "apd_ltr",
> +};
> +
> +static int acpi_apd_platform_notify(struct notifier_block *nb,
> +				     unsigned long action, void *data)
> +{
> +	struct platform_device *pdev = to_platform_device(data);
> +	struct apd_private_data *pdata;
> +	struct acpi_device *adev;
> +	const struct acpi_device_id *id;
> +	int ret = 0;
> +
> +	id = acpi_match_device(acpi_apd_device_ids, &pdev->dev);
> +	if (!id || !id->driver_data)
> +		return 0;
> +
> +	if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
> +		return 0;
> +
> +	pdata = acpi_driver_data(adev);
> +	if (!pdata || !pdata->mmio_base)
> +		return 0;
> +
> +	if (action == BUS_NOTIFY_ADD_DEVICE)
> +		ret = sysfs_create_group(&pdev->dev.kobj, &apd_attr_group);
> +	else if (action == BUS_NOTIFY_DEL_DEVICE)
> +		sysfs_remove_group(&pdev->dev.kobj, &apd_attr_group);
> +
> +	return ret;
> +}
> +
> +static struct notifier_block acpi_apd_nb = {
> +	.notifier_call = acpi_apd_platform_notify,
> +};
> +
> +static struct acpi_scan_handler apd_handler = {
> +	.ids = acpi_apd_device_ids,
> +	.attach = acpi_apd_create_device,
> +};
> +
> +void __init acpi_apd_init(void)
> +{
> +	bus_register_notifier(&platform_bus_type, &acpi_apd_nb);
> +	acpi_scan_add_handler(&apd_handler);
> +}
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 447f6d6..c8a0e8e 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void) { return; }
>  #endif
>  void acpi_lpss_init(void);
>  
> +#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
> +void acpi_apd_init(void);
> +#else
> +static inline void acpi_apd_init(void) {}
> +#endif
> +
>  acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
>  bool acpi_queue_hotplug_work(struct work_struct *work);
>  void acpi_device_hotplug(struct acpi_device *adev, u32 src);
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 0476e90..24fef2b 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void)
>  	acpi_pci_link_init();
>  	acpi_processor_init();
>  	acpi_lpss_init();
> +	acpi_apd_init();
>  	acpi_cmos_rtc_init();
>  	acpi_container_init();
>  	acpi_memory_hotplug_init();
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-24  1:02     ` Xue, Ken
  (?)
@ 2014-11-24  1:47     ` Rafael J. Wysocki
  2014-11-26 10:31         ` Ken Xue
  -1 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2014-11-24  1:47 UTC (permalink / raw)
  To: Xue, Ken
  Cc: lenb, linux-acpi, linux-kernel, Wu, Jeff, Mika Westerberg,
	Andriy Shevchenko

On Monday, November 24, 2014 01:02:30 AM Xue, Ken wrote:
> 
> On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote:
> > This new feature is to interpret AMD specific ACPI device to platform 
> > device such as I2C, UART found on AMD CZ and later chipsets. It is 
> > based on example INTEL LPSS. Now, it can support AMD I2C & UART.
> > 
> > Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> > Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>
> 
> Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead.  What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)?
> 
> Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen.
> 
> [ken] sounds fair enough.  Let me take action to merge drivers to acpi_soc.c ? or you have other plan?

I'd prefer the common code to reside in one file (or one .c file and one header
file), and the driver-specific code to stay in separate per-driver files.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-24  1:47     ` Rafael J. Wysocki
@ 2014-11-26 10:31         ` Ken Xue
  0 siblings, 0 replies; 15+ messages in thread
From: Ken Xue @ 2014-11-26 10:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: lenb, linux-acpi, linux-kernel, Wu, Jeff, Mika Westerberg,
	Andriy Shevchenko

On 一, 2014-11-24 at 02:47 +0100, Rafael J. Wysocki wrote:
> On Monday, November 24, 2014 01:02:30 AM Xue, Ken wrote:
> > 
> > On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote:
> > > This new feature is to interpret AMD specific ACPI device to platform 
> > > device such as I2C, UART found on AMD CZ and later chipsets. It is 
> > > based on example INTEL LPSS. Now, it can support AMD I2C & UART.
> > > 
> > > Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> > > Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>
> > 
> > Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead.  What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)?
> > 
> > Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen.
> > 
> > [ken] sounds fair enough.  Let me take action to merge drivers to acpi_soc.c ? or you have other plan?
> 
> I'd prefer the common code to reside in one file (or one .c file and one header
> file), and the driver-specific code to stay in separate per-driver files.
> 
[Ken] I wrote a proto type for acpi_soc.c. please help to confirm if it
can match your ideal. if yes, i will submit a new patch after do more
test and refine codes. I think it will impact lpss driver greatly, even
i have taken it into account. below codes is for acpi_soc.c.


From fc323fb7b3b4cbb79bda05ce3b1d6d8dfe5e883b Mon Sep 17 00:00:00 2001
From: Ken Xue <Ken.Xue@amd.com>
Date: Wed, 26 Nov 2014 17:15:30 +0800
Subject: [PATCH] This is proto type for acpi_soc.

Signed-off-by: Ken Xue <Ken.Xue@amd.com>
---
 arch/x86/Kconfig        |  11 +++
 drivers/acpi/Makefile   |   2 +-
 drivers/acpi/acpi_apd.c | 123 ++++++++++++++++++++++++++++
 drivers/acpi/acpi_soc.c | 213
++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/acpi_soc.h |  91 +++++++++++++++++++++
 drivers/acpi/internal.h |   6 ++
 drivers/acpi/scan.c     |   1 +
 7 files changed, 446 insertions(+), 1 deletion(-)
 create mode 100755 drivers/acpi/acpi_apd.c
 create mode 100755 drivers/acpi/acpi_soc.c
 create mode 100755 drivers/acpi/acpi_soc.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ded8a67..6402c79f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -495,6 +495,17 @@ config X86_INTEL_LPSS
 	  things like clock tree (common clock framework) and pincontrol
 	  which are needed by the LPSS peripheral drivers.
 
+config X86_AMD_PLATFORM_DEVICE
+	bool "AMD ACPI2Platform devices support"
+	depends on ACPI
+	select COMMON_CLK
+	select PINCTRL
+	---help---
+	  Select to interpret AMD specific ACPI device to platform device
+	  such as I2C, UART found on AMD CARRIZO and later chipset. Selecting
+	  this option enables things like clock tree (common clock framework)
+	  and pinctrl.
+
 config IOSF_MBI
 	tristate "Intel SoC IOSF Sideband support for SoC platforms"
 	depends on PCI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index c3b2fcb..b07003a 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -40,7 +40,7 @@ acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) +=
processor_pdc.o
 acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o
-acpi-y				+= acpi_lpss.o
+acpi-y				+= acpi_soc.o acpi_lpss.o acpi_apd.o
 acpi-y				+= acpi_platform.o
 acpi-y				+= acpi_pnp.o
 acpi-y				+= int340x_thermal.o
diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
new file mode 100755
index 0000000..c7e916b
--- /dev/null
+++ b/drivers/acpi/acpi_apd.c
@@ -0,0 +1,123 @@
+/*
+ * AMD ACPI support for  ACPI2platform device.
+ *
+ * Copyright (c) 2014, AMD Corporation.
+ * Authors: Ken Xue <Ken.Xue@amd.com>
+ *	Wu, Jeff <Jeff.Wu@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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include "internal.h"
+#include "acpi_soc.h"
+
+struct acpi_soc asoc;
+
+#ifdef X86_AMD_PLATFORM_DEVICE
+static int acpi_apd_setup(struct acpi_soc_dev_private_data *pdata)
+{
+	struct acpi_soc_dev_desc *dev_desc = pdata->dev_desc;
+	struct clk *clk = ERR_PTR(-ENODEV);
+
+	if (dev_desc->clk)
+		return 0;
+	
+	if (dev_desc->fixed_clk_rate) {
+		clk = clk_register_fixed_rate(&pdata->adev->dev, 
+					dev_name(&pdata->adev->dev),
+					NULL, CLK_IS_ROOT, dev_desc->rate);
+		dev_desc->clk = clk;
+		clk_register_clkdev(clk, NULL, dev_name(&pdata->adev->dev));
+	}
+
+	return 0;
+}
+
+static struct acpi_soc_dev_desc cz_i2c_desc = {
+	.setup = acpi_apd_setup;
+	.fixed_clk_rate = 133000000,
+};
+
+static struct acpi_soc_dev_desc cz_uart_desc = {
+	.setup = acpi_apd_setup;
+	.fixed_clk_rate = 48000000,
+};
+
+#else
+
+#define APD_ADDR(desc) (0UL)
+
+#endif /* CONFIG_X86_INTEL_LPSS */
+
+static struct acpi_device_id acpi_apd_device_ids[] = {
+	/* Generic apd devices */
+	{ "AMD0010", APD_ADDR(cz_i2c_desc) },
+	{ "AMD0020", APD_ADDR(cz_uart_desc) },
+	{ }
+};
+
+#ifdef X86_AMD_PLATFORM_DEVICE
+
+static ssize_t apd_device_desc_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int ret;
+	struct acpi_device *adev;
+	struct acpi_soc_dev_private_data *pdata;
+	struct acpi_soc_dev_desc *dev_desc;
+
+	ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev);
+	if (WARN_ON(ret))
+		return ret;
+
+	pdata = acpi_driver_data(adev);
+	if (WARN_ON(!pdata || !pdata->dev_desc))
+		return -ENODEV;
+	
+	dev_desc = pdata->dev_desc;
+	if (dev_desc->fixed_clk_rate)
+		return sprintf(buf, "Required fix rate clk %s: %ld\n",
+			dev_desc->clk->name,
+			dev_desc->fixed_clk_rate);
+	else
+		return sprintf(buf, "No need clk\n");
+}
+
+static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL);
+
+static struct attribute *apd_attrs[] = {
+	&dev_attr_device_desc.attr,
+	NULL,
+};
+
+static struct attribute_group apd_attr_group = {
+	.attrs = apd_attrs,
+	.name = "apd",
+};
+
+void __init acpi_apd_init(void)
+{
+	asoc.ids = acpi_apd_device_ids;
+	asoc.attr_group = &apd_attr_group;
+	register_acpi_soc(&asoc, true);
+}
+
+#else
+
+void __init acpi_apd_init(void)
+{
+	asoc.ids = acpi_apd_device_ids;
+	register_acpi_soc(&asoc, false);
+}
+#endif
diff --git a/drivers/acpi/acpi_soc.c b/drivers/acpi/acpi_soc.c
new file mode 100755
index 0000000..99df1ab
--- /dev/null
+++ b/drivers/acpi/acpi_soc.c
@@ -0,0 +1,213 @@
+/*
+ * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD.
+ *
+ * Copyright (C) 2015, Intel Corporation & AMD Corporation
+ * Authors: Ken Xue <Ken.Xue@amd.com>
+ *		Mika Westerberg <mika.westerberg@linux.intel.com>
+ *		Rafael J. Wysocki <rafael.j.wysocki@intel.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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/delay.h>
+#include <linux/pm_domain.h>
+
+#include "internal.h"
+#include "acpi_soc.h"
+
+
+ACPI_MODULE_NAME("acpi_soc");
+
+/*A list for all acpi soc device*/
+static LIST_HEAD(asoc_list);
+
+static int is_memory(struct acpi_resource *res, void *not_used)
+{
+	struct resource r;
+	return !acpi_dev_resource_memory(res, &r);
+}
+
+static int acpi_soc_create_device(struct acpi_device *adev,
+				   const struct acpi_device_id *id)
+{
+	struct acpi_soc_dev_desc *dev_desc;
+	struct acpi_soc_dev_private_data *pdata;
+	struct resource_list_entry *rentry;
+	struct list_head resource_list;
+	struct platform_device *pdev;
+	int ret;
+
+	dev_desc = (struct acpi_soc_dev_desc *)id->driver_data;
+	if (!dev_desc) {
+		pdev = acpi_create_platform_device(adev);
+		return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1;
+	}
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&resource_list);
+	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
+	if (ret < 0)
+		goto err_out;
+
+	list_for_each_entry(rentry, &resource_list, node)
+		if (resource_type(&rentry->res) == IORESOURCE_MEM) {
+			if (dev_desc->mem_size_override)
+				pdata->mmio_size = dev_desc->mem_size_override;
+			else
+				pdata->mmio_size = resource_size(&rentry->res);
+			pdata->mmio_base = ioremap(rentry->res.start,
+						   pdata->mmio_size);
+			break;
+		}
+
+	acpi_dev_free_resource_list(&resource_list);
+
+	pdata->dev_desc = dev_desc;
+
+	/*setup device by a hook routine*/
+	if (dev_desc->setup)
+		dev_desc->setup(pdata);
+
+	/*
+	 * This works around a known issue in ACPI tables where acpi soc
devices
+	 * have _PS0 and _PS3 without _PSC (and no power resources), so
+	 * acpi_bus_init_power() will assume that the BIOS has put them into
D0.
+	 */
+	ret = acpi_device_fix_up_power(adev);
+	if (ret) {
+		/* Skip the device, but continue the namespace scan. */
+		ret = 0;
+		goto err_out;
+	}
+
+	adev->driver_data = pdata;
+	pdev = acpi_create_platform_device(adev);
+	if (!IS_ERR_OR_NULL(pdev)) {
+		return 1;
+	}
+
+	ret = PTR_ERR(pdev);
+	adev->driver_data = NULL;
+
+ err_out:
+	kfree(pdata);
+	return ret;
+}
+
+static int acpi_soc_platform_notify(struct notifier_block *nb,
+				     unsigned long action, void *data)
+{
+	struct platform_device *pdev = to_platform_device(data);
+	struct acpi_soc_dev_private_data *pdata;
+	struct acpi_device *adev;
+	struct acpi_soc *asoc_entry;
+	const struct acpi_device_id *id = NULL;
+
+	list_for_each_entry(asoc_entry, &asoc_list, list) {
+		id = acpi_match_device(asoc_entry->ids, &pdev->dev);
+		if (!id)
+			break;
+	}
+	
+	if (!id || !id->driver_data)
+		return 0;
+
+	if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
+		return 0;
+
+	pdata = acpi_driver_data(adev);
+	if (!pdata || !pdata->mmio_base)
+		return 0;
+
+	switch (action) {
+	case BUS_NOTIFY_BOUND_DRIVER:
+		if ((pdata->dev_desc->flags & ACPI_SOC_PM)){
+			if (asoc_entry->pm_domain)
+				pdev->dev.pm_domain = asoc_entry->pm_domain;
+			else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON)
+					dev_pm_domain_attach(&pdev->dev, true);
+			else
+					dev_pm_domain_attach(&pdev->dev, false);
+		}	
+		break;
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		if ((pdata->dev_desc->flags & ACPI_SOC_PM)){
+			if (asoc_entry->pm_domain)
+				pdev->dev.pm_domain = asoc_entry->pm_domain;
+			else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON)
+					dev_pm_domain_detach(&pdev->dev, true);
+			else
+					dev_pm_domain_detach(&pdev->dev, false);
+		}	
+		break;
+	case BUS_NOTIFY_ADD_DEVICE:
+		if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) &&
asoc_entry->pm_domain)
+			return sysfs_create_group(&pdev->dev.kobj,
+						  asoc_entry->attr_group);
+	case BUS_NOTIFY_DEL_DEVICE:
+		if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) &&
asoc_entry->pm_domain)
+			sysfs_remove_group(&pdev->dev.kobj, asoc_entry->attr_group);
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static struct notifier_block acpi_soc_nb = {
+	.notifier_call = acpi_soc_platform_notify,
+};
+
+static void acpi_soc_bind(struct device *dev)
+{
+	struct acpi_soc_dev_private_data *pdata;
+
+	pdata = acpi_driver_data(ACPI_COMPANION(dev));
+
+	if (!pdata || !pdata->dev_desc || !pdata->dev_desc->bind)
+		return;
+
+	pdata->dev_desc->bind(pdata);
+	return;
+}
+
+static void acpi_soc_unbind(struct device *dev)
+{
+	struct acpi_soc_dev_private_data *pdata;
+
+	pdata = acpi_driver_data(ACPI_COMPANION(dev));
+
+	if (!pdata || !pdata->dev_desc || !pdata->dev_desc->unbind)
+		return;
+
+	pdata->dev_desc->unbind(pdata);
+	return;
+}
+
+void register_acpi_soc(struct acpi_soc *asoc, bool
disable_scan_handler)
+{
+	struct acpi_scan_handler *acpi_soc_handler;
+	
+	INIT_LIST_HEAD(&asoc->list);
+	list_add(&asoc->list, &asoc_list);
+
+	acpi_soc_handler = kzalloc(sizeof(struct acpi_scan_handler),
GFP_KERNEL);
+	acpi_soc_handler->ids = asoc->ids;
+	if (!disable_scan_handler) {
+		acpi_soc_handler->attach = acpi_soc_create_device;
+		acpi_soc_handler->bind = acpi_soc_bind;
+		acpi_soc_handler->unbind = acpi_soc_unbind;
+	}	
+	acpi_scan_add_handler(acpi_soc_handler);
+	bus_register_notifier(&platform_bus_type, &acpi_soc_nb);
+}
+EXPORT_SYMBOL_GPL(register_acpi_soc);
diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h
new file mode 100755
index 0000000..96db30e
--- /dev/null
+++ b/drivers/acpi/acpi_soc.h
@@ -0,0 +1,91 @@
+/*
+ * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD.
+ *
+ * Copyright (C) 2015, Intel Corporation & AMD Corporation
+ * Authors: Ken Xue <Ken.Xue@amd.com>
+ *		Mika Westerberg <mika.westerberg@linux.intel.com>
+ *		Rafael J. Wysocki <rafael.j.wysocki@intel.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 _ACPI_SOC_H
+#define _ACPI_SOC_H
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/pm.h>
+
+/* Flags */
+#define ACPI_SOC_SYSFS	BIT(0)
+#define ACPI_SOC_PM		BIT(1)
+#define ACPI_SOC_PM_ON	BIT(2)
+
+struct acpi_soc_dev_private_data;
+
+/**
+ * struct acpi_soc - acpi soc 
+ * @list: list head
+ * @ids: all acpi device ids for acpi soc 
+ * @pm_domain: power domain for all acpi device;can be NULL
+ * @attr_group: attribute group for sysfs support of acpi soc;can be
NULL
+ */
+struct acpi_soc {
+	struct list_head list;
+	struct acpi_device_id	*ids;
+	struct dev_pm_domain	*pm_domain;
+	struct attribute_group 	*attr_group;
+};
+
+/**
+ * struct acpi_soc_dev_desc - a descriptor for acpi device 
+ * @flags: some device feature flags
+ * @clk: clock device
+ * @fixed_clk_rate: fixed rate input clock source for acpi device;
+ *				0 means no fixed rate input clock source
+ * @mem_size_override: a workaround for override device memsize;
+ *				0 means no needs for this WA
+ * @setup: a hook routine to set device resource during create platform
device
+ * @bind: a hook of acpi_scan_handler.bind
+ * @unbind: a hook of acpi_scan_handler.unbind
+ *
+ *device description defined as acpi_device_id.driver_data
+ */
+struct acpi_soc_dev_desc {
+	unsigned int flags;
+	struct clk *clk;
+	unsigned int fixed_clk_rate;
+	size_t mem_size_override;
+	int (*setup)(struct acpi_soc_dev_private_data *pdata);
+	void (*bind)(struct acpi_soc_dev_private_data *pdata);
+	void (*unbind)(struct acpi_soc_dev_private_data *pdata);
+};
+
+/**
+ * struct acpi_soc_dev_private_data - acpi device private data
+ * @mmio_base: virtual memory base addr of the device
+ * @mmio_size: device memory size
+ * @dev_desc: device description
+ * @adev: apci device
+ *
+ */
+struct acpi_soc_dev_private_data {
+	void __iomem *mmio_base;
+	resource_size_t mmio_size;
+	
+	const struct acpi_soc_dev_desc *dev_desc;
+	struct acpi_device *adev;
+};
+
+/**
+ * register_acpi_soc - register a new acpi soc
+ * @asoc: acpi soc
+ * @disable_scan_handler: true means remove deafult scan handle
+ *					false means use deafult scan handle
+ * 
+ * register a new acpi soc into asoc_list and install deafult scan
handle.
+ */
+void register_acpi_soc(struct acpi_soc *asoc, bool
disable_scan_handler);
+
+#endif
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 447f6d6..c8a0e8e 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void)
{ return; }
 #endif
 void acpi_lpss_init(void);
 
+#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
+void acpi_apd_init(void);
+#else
+static inline void acpi_apd_init(void) {}
+#endif
+
 acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
 bool acpi_queue_hotplug_work(struct work_struct *work);
 void acpi_device_hotplug(struct acpi_device *adev, u32 src);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0476e90..24fef2b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void)
 	acpi_pci_link_init();
 	acpi_processor_init();
 	acpi_lpss_init();
+	acpi_apd_init();
 	acpi_cmos_rtc_init();
 	acpi_container_init();
 	acpi_memory_hotplug_init();
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
@ 2014-11-26 10:31         ` Ken Xue
  0 siblings, 0 replies; 15+ messages in thread
From: Ken Xue @ 2014-11-26 10:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: lenb, linux-acpi, linux-kernel, Wu, Jeff, Mika Westerberg,
	Andriy Shevchenko

On 一, 2014-11-24 at 02:47 +0100, Rafael J. Wysocki wrote:
> On Monday, November 24, 2014 01:02:30 AM Xue, Ken wrote:
> > 
> > On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote:
> > > This new feature is to interpret AMD specific ACPI device to platform 
> > > device such as I2C, UART found on AMD CZ and later chipsets. It is 
> > > based on example INTEL LPSS. Now, it can support AMD I2C & UART.
> > > 
> > > Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> > > Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>
> > 
> > Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead.  What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)?
> > 
> > Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen.
> > 
> > [ken] sounds fair enough.  Let me take action to merge drivers to acpi_soc.c ? or you have other plan?
> 
> I'd prefer the common code to reside in one file (or one .c file and one header
> file), and the driver-specific code to stay in separate per-driver files.
> 
[Ken] I wrote a proto type for acpi_soc.c. please help to confirm if it
can match your ideal. if yes, i will submit a new patch after do more
test and refine codes. I think it will impact lpss driver greatly, even
i have taken it into account. below codes is for acpi_soc.c.


>From fc323fb7b3b4cbb79bda05ce3b1d6d8dfe5e883b Mon Sep 17 00:00:00 2001
From: Ken Xue <Ken.Xue@amd.com>
Date: Wed, 26 Nov 2014 17:15:30 +0800
Subject: [PATCH] This is proto type for acpi_soc.

Signed-off-by: Ken Xue <Ken.Xue@amd.com>
---
 arch/x86/Kconfig        |  11 +++
 drivers/acpi/Makefile   |   2 +-
 drivers/acpi/acpi_apd.c | 123 ++++++++++++++++++++++++++++
 drivers/acpi/acpi_soc.c | 213
++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/acpi_soc.h |  91 +++++++++++++++++++++
 drivers/acpi/internal.h |   6 ++
 drivers/acpi/scan.c     |   1 +
 7 files changed, 446 insertions(+), 1 deletion(-)
 create mode 100755 drivers/acpi/acpi_apd.c
 create mode 100755 drivers/acpi/acpi_soc.c
 create mode 100755 drivers/acpi/acpi_soc.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ded8a67..6402c79f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -495,6 +495,17 @@ config X86_INTEL_LPSS
 	  things like clock tree (common clock framework) and pincontrol
 	  which are needed by the LPSS peripheral drivers.
 
+config X86_AMD_PLATFORM_DEVICE
+	bool "AMD ACPI2Platform devices support"
+	depends on ACPI
+	select COMMON_CLK
+	select PINCTRL
+	---help---
+	  Select to interpret AMD specific ACPI device to platform device
+	  such as I2C, UART found on AMD CARRIZO and later chipset. Selecting
+	  this option enables things like clock tree (common clock framework)
+	  and pinctrl.
+
 config IOSF_MBI
 	tristate "Intel SoC IOSF Sideband support for SoC platforms"
 	depends on PCI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index c3b2fcb..b07003a 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -40,7 +40,7 @@ acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) +=
processor_pdc.o
 acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o
-acpi-y				+= acpi_lpss.o
+acpi-y				+= acpi_soc.o acpi_lpss.o acpi_apd.o
 acpi-y				+= acpi_platform.o
 acpi-y				+= acpi_pnp.o
 acpi-y				+= int340x_thermal.o
diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
new file mode 100755
index 0000000..c7e916b
--- /dev/null
+++ b/drivers/acpi/acpi_apd.c
@@ -0,0 +1,123 @@
+/*
+ * AMD ACPI support for  ACPI2platform device.
+ *
+ * Copyright (c) 2014, AMD Corporation.
+ * Authors: Ken Xue <Ken.Xue@amd.com>
+ *	Wu, Jeff <Jeff.Wu@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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include "internal.h"
+#include "acpi_soc.h"
+
+struct acpi_soc asoc;
+
+#ifdef X86_AMD_PLATFORM_DEVICE
+static int acpi_apd_setup(struct acpi_soc_dev_private_data *pdata)
+{
+	struct acpi_soc_dev_desc *dev_desc = pdata->dev_desc;
+	struct clk *clk = ERR_PTR(-ENODEV);
+
+	if (dev_desc->clk)
+		return 0;
+	
+	if (dev_desc->fixed_clk_rate) {
+		clk = clk_register_fixed_rate(&pdata->adev->dev, 
+					dev_name(&pdata->adev->dev),
+					NULL, CLK_IS_ROOT, dev_desc->rate);
+		dev_desc->clk = clk;
+		clk_register_clkdev(clk, NULL, dev_name(&pdata->adev->dev));
+	}
+
+	return 0;
+}
+
+static struct acpi_soc_dev_desc cz_i2c_desc = {
+	.setup = acpi_apd_setup;
+	.fixed_clk_rate = 133000000,
+};
+
+static struct acpi_soc_dev_desc cz_uart_desc = {
+	.setup = acpi_apd_setup;
+	.fixed_clk_rate = 48000000,
+};
+
+#else
+
+#define APD_ADDR(desc) (0UL)
+
+#endif /* CONFIG_X86_INTEL_LPSS */
+
+static struct acpi_device_id acpi_apd_device_ids[] = {
+	/* Generic apd devices */
+	{ "AMD0010", APD_ADDR(cz_i2c_desc) },
+	{ "AMD0020", APD_ADDR(cz_uart_desc) },
+	{ }
+};
+
+#ifdef X86_AMD_PLATFORM_DEVICE
+
+static ssize_t apd_device_desc_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int ret;
+	struct acpi_device *adev;
+	struct acpi_soc_dev_private_data *pdata;
+	struct acpi_soc_dev_desc *dev_desc;
+
+	ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev);
+	if (WARN_ON(ret))
+		return ret;
+
+	pdata = acpi_driver_data(adev);
+	if (WARN_ON(!pdata || !pdata->dev_desc))
+		return -ENODEV;
+	
+	dev_desc = pdata->dev_desc;
+	if (dev_desc->fixed_clk_rate)
+		return sprintf(buf, "Required fix rate clk %s: %ld\n",
+			dev_desc->clk->name,
+			dev_desc->fixed_clk_rate);
+	else
+		return sprintf(buf, "No need clk\n");
+}
+
+static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL);
+
+static struct attribute *apd_attrs[] = {
+	&dev_attr_device_desc.attr,
+	NULL,
+};
+
+static struct attribute_group apd_attr_group = {
+	.attrs = apd_attrs,
+	.name = "apd",
+};
+
+void __init acpi_apd_init(void)
+{
+	asoc.ids = acpi_apd_device_ids;
+	asoc.attr_group = &apd_attr_group;
+	register_acpi_soc(&asoc, true);
+}
+
+#else
+
+void __init acpi_apd_init(void)
+{
+	asoc.ids = acpi_apd_device_ids;
+	register_acpi_soc(&asoc, false);
+}
+#endif
diff --git a/drivers/acpi/acpi_soc.c b/drivers/acpi/acpi_soc.c
new file mode 100755
index 0000000..99df1ab
--- /dev/null
+++ b/drivers/acpi/acpi_soc.c
@@ -0,0 +1,213 @@
+/*
+ * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD.
+ *
+ * Copyright (C) 2015, Intel Corporation & AMD Corporation
+ * Authors: Ken Xue <Ken.Xue@amd.com>
+ *		Mika Westerberg <mika.westerberg@linux.intel.com>
+ *		Rafael J. Wysocki <rafael.j.wysocki@intel.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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/delay.h>
+#include <linux/pm_domain.h>
+
+#include "internal.h"
+#include "acpi_soc.h"
+
+
+ACPI_MODULE_NAME("acpi_soc");
+
+/*A list for all acpi soc device*/
+static LIST_HEAD(asoc_list);
+
+static int is_memory(struct acpi_resource *res, void *not_used)
+{
+	struct resource r;
+	return !acpi_dev_resource_memory(res, &r);
+}
+
+static int acpi_soc_create_device(struct acpi_device *adev,
+				   const struct acpi_device_id *id)
+{
+	struct acpi_soc_dev_desc *dev_desc;
+	struct acpi_soc_dev_private_data *pdata;
+	struct resource_list_entry *rentry;
+	struct list_head resource_list;
+	struct platform_device *pdev;
+	int ret;
+
+	dev_desc = (struct acpi_soc_dev_desc *)id->driver_data;
+	if (!dev_desc) {
+		pdev = acpi_create_platform_device(adev);
+		return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1;
+	}
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&resource_list);
+	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
+	if (ret < 0)
+		goto err_out;
+
+	list_for_each_entry(rentry, &resource_list, node)
+		if (resource_type(&rentry->res) == IORESOURCE_MEM) {
+			if (dev_desc->mem_size_override)
+				pdata->mmio_size = dev_desc->mem_size_override;
+			else
+				pdata->mmio_size = resource_size(&rentry->res);
+			pdata->mmio_base = ioremap(rentry->res.start,
+						   pdata->mmio_size);
+			break;
+		}
+
+	acpi_dev_free_resource_list(&resource_list);
+
+	pdata->dev_desc = dev_desc;
+
+	/*setup device by a hook routine*/
+	if (dev_desc->setup)
+		dev_desc->setup(pdata);
+
+	/*
+	 * This works around a known issue in ACPI tables where acpi soc
devices
+	 * have _PS0 and _PS3 without _PSC (and no power resources), so
+	 * acpi_bus_init_power() will assume that the BIOS has put them into
D0.
+	 */
+	ret = acpi_device_fix_up_power(adev);
+	if (ret) {
+		/* Skip the device, but continue the namespace scan. */
+		ret = 0;
+		goto err_out;
+	}
+
+	adev->driver_data = pdata;
+	pdev = acpi_create_platform_device(adev);
+	if (!IS_ERR_OR_NULL(pdev)) {
+		return 1;
+	}
+
+	ret = PTR_ERR(pdev);
+	adev->driver_data = NULL;
+
+ err_out:
+	kfree(pdata);
+	return ret;
+}
+
+static int acpi_soc_platform_notify(struct notifier_block *nb,
+				     unsigned long action, void *data)
+{
+	struct platform_device *pdev = to_platform_device(data);
+	struct acpi_soc_dev_private_data *pdata;
+	struct acpi_device *adev;
+	struct acpi_soc *asoc_entry;
+	const struct acpi_device_id *id = NULL;
+
+	list_for_each_entry(asoc_entry, &asoc_list, list) {
+		id = acpi_match_device(asoc_entry->ids, &pdev->dev);
+		if (!id)
+			break;
+	}
+	
+	if (!id || !id->driver_data)
+		return 0;
+
+	if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
+		return 0;
+
+	pdata = acpi_driver_data(adev);
+	if (!pdata || !pdata->mmio_base)
+		return 0;
+
+	switch (action) {
+	case BUS_NOTIFY_BOUND_DRIVER:
+		if ((pdata->dev_desc->flags & ACPI_SOC_PM)){
+			if (asoc_entry->pm_domain)
+				pdev->dev.pm_domain = asoc_entry->pm_domain;
+			else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON)
+					dev_pm_domain_attach(&pdev->dev, true);
+			else
+					dev_pm_domain_attach(&pdev->dev, false);
+		}	
+		break;
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		if ((pdata->dev_desc->flags & ACPI_SOC_PM)){
+			if (asoc_entry->pm_domain)
+				pdev->dev.pm_domain = asoc_entry->pm_domain;
+			else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON)
+					dev_pm_domain_detach(&pdev->dev, true);
+			else
+					dev_pm_domain_detach(&pdev->dev, false);
+		}	
+		break;
+	case BUS_NOTIFY_ADD_DEVICE:
+		if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) &&
asoc_entry->pm_domain)
+			return sysfs_create_group(&pdev->dev.kobj,
+						  asoc_entry->attr_group);
+	case BUS_NOTIFY_DEL_DEVICE:
+		if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) &&
asoc_entry->pm_domain)
+			sysfs_remove_group(&pdev->dev.kobj, asoc_entry->attr_group);
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static struct notifier_block acpi_soc_nb = {
+	.notifier_call = acpi_soc_platform_notify,
+};
+
+static void acpi_soc_bind(struct device *dev)
+{
+	struct acpi_soc_dev_private_data *pdata;
+
+	pdata = acpi_driver_data(ACPI_COMPANION(dev));
+
+	if (!pdata || !pdata->dev_desc || !pdata->dev_desc->bind)
+		return;
+
+	pdata->dev_desc->bind(pdata);
+	return;
+}
+
+static void acpi_soc_unbind(struct device *dev)
+{
+	struct acpi_soc_dev_private_data *pdata;
+
+	pdata = acpi_driver_data(ACPI_COMPANION(dev));
+
+	if (!pdata || !pdata->dev_desc || !pdata->dev_desc->unbind)
+		return;
+
+	pdata->dev_desc->unbind(pdata);
+	return;
+}
+
+void register_acpi_soc(struct acpi_soc *asoc, bool
disable_scan_handler)
+{
+	struct acpi_scan_handler *acpi_soc_handler;
+	
+	INIT_LIST_HEAD(&asoc->list);
+	list_add(&asoc->list, &asoc_list);
+
+	acpi_soc_handler = kzalloc(sizeof(struct acpi_scan_handler),
GFP_KERNEL);
+	acpi_soc_handler->ids = asoc->ids;
+	if (!disable_scan_handler) {
+		acpi_soc_handler->attach = acpi_soc_create_device;
+		acpi_soc_handler->bind = acpi_soc_bind;
+		acpi_soc_handler->unbind = acpi_soc_unbind;
+	}	
+	acpi_scan_add_handler(acpi_soc_handler);
+	bus_register_notifier(&platform_bus_type, &acpi_soc_nb);
+}
+EXPORT_SYMBOL_GPL(register_acpi_soc);
diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h
new file mode 100755
index 0000000..96db30e
--- /dev/null
+++ b/drivers/acpi/acpi_soc.h
@@ -0,0 +1,91 @@
+/*
+ * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD.
+ *
+ * Copyright (C) 2015, Intel Corporation & AMD Corporation
+ * Authors: Ken Xue <Ken.Xue@amd.com>
+ *		Mika Westerberg <mika.westerberg@linux.intel.com>
+ *		Rafael J. Wysocki <rafael.j.wysocki@intel.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 _ACPI_SOC_H
+#define _ACPI_SOC_H
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/pm.h>
+
+/* Flags */
+#define ACPI_SOC_SYSFS	BIT(0)
+#define ACPI_SOC_PM		BIT(1)
+#define ACPI_SOC_PM_ON	BIT(2)
+
+struct acpi_soc_dev_private_data;
+
+/**
+ * struct acpi_soc - acpi soc 
+ * @list: list head
+ * @ids: all acpi device ids for acpi soc 
+ * @pm_domain: power domain for all acpi device;can be NULL
+ * @attr_group: attribute group for sysfs support of acpi soc;can be
NULL
+ */
+struct acpi_soc {
+	struct list_head list;
+	struct acpi_device_id	*ids;
+	struct dev_pm_domain	*pm_domain;
+	struct attribute_group 	*attr_group;
+};
+
+/**
+ * struct acpi_soc_dev_desc - a descriptor for acpi device 
+ * @flags: some device feature flags
+ * @clk: clock device
+ * @fixed_clk_rate: fixed rate input clock source for acpi device;
+ *				0 means no fixed rate input clock source
+ * @mem_size_override: a workaround for override device memsize;
+ *				0 means no needs for this WA
+ * @setup: a hook routine to set device resource during create platform
device
+ * @bind: a hook of acpi_scan_handler.bind
+ * @unbind: a hook of acpi_scan_handler.unbind
+ *
+ *device description defined as acpi_device_id.driver_data
+ */
+struct acpi_soc_dev_desc {
+	unsigned int flags;
+	struct clk *clk;
+	unsigned int fixed_clk_rate;
+	size_t mem_size_override;
+	int (*setup)(struct acpi_soc_dev_private_data *pdata);
+	void (*bind)(struct acpi_soc_dev_private_data *pdata);
+	void (*unbind)(struct acpi_soc_dev_private_data *pdata);
+};
+
+/**
+ * struct acpi_soc_dev_private_data - acpi device private data
+ * @mmio_base: virtual memory base addr of the device
+ * @mmio_size: device memory size
+ * @dev_desc: device description
+ * @adev: apci device
+ *
+ */
+struct acpi_soc_dev_private_data {
+	void __iomem *mmio_base;
+	resource_size_t mmio_size;
+	
+	const struct acpi_soc_dev_desc *dev_desc;
+	struct acpi_device *adev;
+};
+
+/**
+ * register_acpi_soc - register a new acpi soc
+ * @asoc: acpi soc
+ * @disable_scan_handler: true means remove deafult scan handle
+ *					false means use deafult scan handle
+ * 
+ * register a new acpi soc into asoc_list and install deafult scan
handle.
+ */
+void register_acpi_soc(struct acpi_soc *asoc, bool
disable_scan_handler);
+
+#endif
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 447f6d6..c8a0e8e 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void)
{ return; }
 #endif
 void acpi_lpss_init(void);
 
+#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
+void acpi_apd_init(void);
+#else
+static inline void acpi_apd_init(void) {}
+#endif
+
 acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
 bool acpi_queue_hotplug_work(struct work_struct *work);
 void acpi_device_hotplug(struct acpi_device *adev, u32 src);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0476e90..24fef2b 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void)
 	acpi_pci_link_init();
 	acpi_processor_init();
 	acpi_lpss_init();
+	acpi_apd_init();
 	acpi_cmos_rtc_init();
 	acpi_container_init();
 	acpi_memory_hotplug_init();
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-26 10:31         ` Ken Xue
@ 2014-11-27 11:46           ` Mika Westerberg
  -1 siblings, 0 replies; 15+ messages in thread
From: Mika Westerberg @ 2014-11-27 11:46 UTC (permalink / raw)
  To: Ken Xue
  Cc: Rafael J. Wysocki, lenb, linux-acpi, linux-kernel, Wu, Jeff,
	Andriy Shevchenko

On Wed, Nov 26, 2014 at 06:31:38PM +0800, Ken Xue wrote:
> On 一, 2014-11-24 at 02:47 +0100, Rafael J. Wysocki wrote:
> > On Monday, November 24, 2014 01:02:30 AM Xue, Ken wrote:
> > > 
> > > On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote:
> > > > This new feature is to interpret AMD specific ACPI device to platform 
> > > > device such as I2C, UART found on AMD CZ and later chipsets. It is 
> > > > based on example INTEL LPSS. Now, it can support AMD I2C & UART.
> > > > 
> > > > Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> > > > Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>
> > > 
> > > Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead.  What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)?
> > > 
> > > Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen.
> > > 
> > > [ken] sounds fair enough.  Let me take action to merge drivers to acpi_soc.c ? or you have other plan?
> > 
> > I'd prefer the common code to reside in one file (or one .c file and one header
> > file), and the driver-specific code to stay in separate per-driver files.
> > 
> [Ken] I wrote a proto type for acpi_soc.c. please help to confirm if it
> can match your ideal. if yes, i will submit a new patch after do more
> test and refine codes. I think it will impact lpss driver greatly, even
> i have taken it into account. below codes is for acpi_soc.c.

In general looks good. I have few comments though.

> 
> >From fc323fb7b3b4cbb79bda05ce3b1d6d8dfe5e883b Mon Sep 17 00:00:00 2001
> From: Ken Xue <Ken.Xue@amd.com>
> Date: Wed, 26 Nov 2014 17:15:30 +0800
> Subject: [PATCH] This is proto type for acpi_soc.
> 
> Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> ---
>  arch/x86/Kconfig        |  11 +++
>  drivers/acpi/Makefile   |   2 +-
>  drivers/acpi/acpi_apd.c | 123 ++++++++++++++++++++++++++++
>  drivers/acpi/acpi_soc.c | 213
> ++++++++++++++++++++++++++++++++++++++++++++++++

This is line-wrapped, please make sure when you submit the actual patch
that it is formatted properly. Also you need proper changelog etc.

>  drivers/acpi/acpi_soc.h |  91 +++++++++++++++++++++
>  drivers/acpi/internal.h |   6 ++
>  drivers/acpi/scan.c     |   1 +
>  7 files changed, 446 insertions(+), 1 deletion(-)
>  create mode 100755 drivers/acpi/acpi_apd.c
>  create mode 100755 drivers/acpi/acpi_soc.c
>  create mode 100755 drivers/acpi/acpi_soc.h
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index ded8a67..6402c79f 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -495,6 +495,17 @@ config X86_INTEL_LPSS
>  	  things like clock tree (common clock framework) and pincontrol
>  	  which are needed by the LPSS peripheral drivers.
>  
> +config X86_AMD_PLATFORM_DEVICE
> +	bool "AMD ACPI2Platform devices support"
> +	depends on ACPI
> +	select COMMON_CLK
> +	select PINCTRL
> +	---help---
> +	  Select to interpret AMD specific ACPI device to platform device
> +	  such as I2C, UART found on AMD CARRIZO and later chipset. Selecting
> +	  this option enables things like clock tree (common clock framework)
> +	  and pinctrl.
> +
>  config IOSF_MBI
>  	tristate "Intel SoC IOSF Sideband support for SoC platforms"
>  	depends on PCI
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index c3b2fcb..b07003a 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -40,7 +40,7 @@ acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) +=
> processor_pdc.o
>  acpi-y				+= ec.o
>  acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
>  acpi-y				+= pci_root.o pci_link.o pci_irq.o
> -acpi-y				+= acpi_lpss.o
> +acpi-y				+= acpi_soc.o acpi_lpss.o acpi_apd.o
>  acpi-y				+= acpi_platform.o
>  acpi-y				+= acpi_pnp.o
>  acpi-y				+= int340x_thermal.o
> diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
> new file mode 100755
> index 0000000..c7e916b
> --- /dev/null
> +++ b/drivers/acpi/acpi_apd.c
> @@ -0,0 +1,123 @@
> +/*
> + * AMD ACPI support for  ACPI2platform device.
> + *
> + * Copyright (c) 2014, AMD Corporation.
> + * Authors: Ken Xue <Ken.Xue@amd.com>
> + *	Wu, Jeff <Jeff.Wu@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.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "internal.h"
> +#include "acpi_soc.h"
> +
> +struct acpi_soc asoc;

static?

Also Is "asoc" good name? That might get confused with Alsa SoC (ASoC).

> +
> +#ifdef X86_AMD_PLATFORM_DEVICE
> +static int acpi_apd_setup(struct acpi_soc_dev_private_data *pdata)
> +{
> +	struct acpi_soc_dev_desc *dev_desc = pdata->dev_desc;
> +	struct clk *clk = ERR_PTR(-ENODEV);
> +
> +	if (dev_desc->clk)
> +		return 0;
> +	
> +	if (dev_desc->fixed_clk_rate) {
> +		clk = clk_register_fixed_rate(&pdata->adev->dev, 
> +					dev_name(&pdata->adev->dev),
> +					NULL, CLK_IS_ROOT, dev_desc->rate);
> +		dev_desc->clk = clk;
> +		clk_register_clkdev(clk, NULL, dev_name(&pdata->adev->dev));
> +	}
> +
> +	return 0;
> +}
> +
> +static struct acpi_soc_dev_desc cz_i2c_desc = {
> +	.setup = acpi_apd_setup;
> +	.fixed_clk_rate = 133000000,

Oh, good so now we can get rid the hack we did for
i2c-designware-platdrv.c with this commit:

a445900c906092f i2c: designware: Add support for AMD I2C controller

Since now you have means to pass clock to the driver.

Are you going to handle that driver as well?

> +};
> +
> +static struct acpi_soc_dev_desc cz_uart_desc = {
> +	.setup = acpi_apd_setup;
> +	.fixed_clk_rate = 48000000,
> +};
> +
> +#else
> +
> +#define APD_ADDR(desc) (0UL)
> +
> +#endif /* CONFIG_X86_INTEL_LPSS */
> +
> +static struct acpi_device_id acpi_apd_device_ids[] = {

const?

> +	/* Generic apd devices */
> +	{ "AMD0010", APD_ADDR(cz_i2c_desc) },
> +	{ "AMD0020", APD_ADDR(cz_uart_desc) },
> +	{ }
> +};
> +
> +#ifdef X86_AMD_PLATFORM_DEVICE
> +
> +static ssize_t apd_device_desc_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	int ret;
> +	struct acpi_device *adev;
> +	struct acpi_soc_dev_private_data *pdata;
> +	struct acpi_soc_dev_desc *dev_desc;
> +
> +	ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev);
> +	if (WARN_ON(ret))
> +		return ret;
> +
> +	pdata = acpi_driver_data(adev);
> +	if (WARN_ON(!pdata || !pdata->dev_desc))
> +		return -ENODEV;
> +	
> +	dev_desc = pdata->dev_desc;
> +	if (dev_desc->fixed_clk_rate)
> +		return sprintf(buf, "Required fix rate clk %s: %ld\n",
> +			dev_desc->clk->name,
> +			dev_desc->fixed_clk_rate);
> +	else
> +		return sprintf(buf, "No need clk\n");
> +}
> +
> +static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL);
> +
> +static struct attribute *apd_attrs[] = {
> +	&dev_attr_device_desc.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group apd_attr_group = {
> +	.attrs = apd_attrs,
> +	.name = "apd",
> +};

This requires updating sysfs ABI but then again, do you really need the
above? And does it belong to sysfs in the first place?

> +
> +void __init acpi_apd_init(void)
> +{
> +	asoc.ids = acpi_apd_device_ids;
> +	asoc.attr_group = &apd_attr_group;
> +	register_acpi_soc(&asoc, true);
> +}
> +
> +#else
> +
> +void __init acpi_apd_init(void)
> +{
> +	asoc.ids = acpi_apd_device_ids;
> +	register_acpi_soc(&asoc, false);
> +}
> +#endif
> diff --git a/drivers/acpi/acpi_soc.c b/drivers/acpi/acpi_soc.c
> new file mode 100755
> index 0000000..99df1ab
> --- /dev/null
> +++ b/drivers/acpi/acpi_soc.c
> @@ -0,0 +1,213 @@
> +/*
> + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD.
> + *
> + * Copyright (C) 2015, Intel Corporation & AMD Corporation
> + * Authors: Ken Xue <Ken.Xue@amd.com>
> + *		Mika Westerberg <mika.westerberg@linux.intel.com>
> + *		Rafael J. Wysocki <rafael.j.wysocki@intel.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.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/delay.h>
> +#include <linux/pm_domain.h>
> +
> +#include "internal.h"
> +#include "acpi_soc.h"
> +
> +

Delete the extra blank line

> +ACPI_MODULE_NAME("acpi_soc");
> +
> +/*A list for all acpi soc device*/
> +static LIST_HEAD(asoc_list);
> +
> +static int is_memory(struct acpi_resource *res, void *not_used)
> +{
> +	struct resource r;
> +	return !acpi_dev_resource_memory(res, &r);
> +}
> +
> +static int acpi_soc_create_device(struct acpi_device *adev,
> +				   const struct acpi_device_id *id)
> +{
> +	struct acpi_soc_dev_desc *dev_desc;
> +	struct acpi_soc_dev_private_data *pdata;
> +	struct resource_list_entry *rentry;
> +	struct list_head resource_list;
> +	struct platform_device *pdev;
> +	int ret;
> +
> +	dev_desc = (struct acpi_soc_dev_desc *)id->driver_data;
> +	if (!dev_desc) {
> +		pdev = acpi_create_platform_device(adev);
> +		return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1;
> +	}
> +	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&resource_list);
> +	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
> +	if (ret < 0)
> +		goto err_out;
> +
> +	list_for_each_entry(rentry, &resource_list, node)
> +		if (resource_type(&rentry->res) == IORESOURCE_MEM) {
> +			if (dev_desc->mem_size_override)
> +				pdata->mmio_size = dev_desc->mem_size_override;
> +			else
> +				pdata->mmio_size = resource_size(&rentry->res);
> +			pdata->mmio_base = ioremap(rentry->res.start,
> +						   pdata->mmio_size);
> +			break;
> +		}
> +
> +	acpi_dev_free_resource_list(&resource_list);
> +
> +	pdata->dev_desc = dev_desc;
> +
> +	/*setup device by a hook routine*/

The comment should look like this

	/* Setup device by hook routine */

but I think it does not provide any new information so you can just drop
it.

> +	if (dev_desc->setup)
> +		dev_desc->setup(pdata);
> +
> +	/*
> +	 * This works around a known issue in ACPI tables where acpi soc
> devices
> +	 * have _PS0 and _PS3 without _PSC (and no power resources), so
> +	 * acpi_bus_init_power() will assume that the BIOS has put them into
> D0.
> +	 */
> +	ret = acpi_device_fix_up_power(adev);
> +	if (ret) {
> +		/* Skip the device, but continue the namespace scan. */
> +		ret = 0;
> +		goto err_out;
> +	}
> +
> +	adev->driver_data = pdata;
> +	pdev = acpi_create_platform_device(adev);
> +	if (!IS_ERR_OR_NULL(pdev)) {
> +		return 1;
> +	}
> +
> +	ret = PTR_ERR(pdev);
> +	adev->driver_data = NULL;
> +
> + err_out:
> +	kfree(pdata);
> +	return ret;
> +}
> +
> +static int acpi_soc_platform_notify(struct notifier_block *nb,
> +				     unsigned long action, void *data)
> +{
> +	struct platform_device *pdev = to_platform_device(data);
> +	struct acpi_soc_dev_private_data *pdata;
> +	struct acpi_device *adev;
> +	struct acpi_soc *asoc_entry;
> +	const struct acpi_device_id *id = NULL;
> +
> +	list_for_each_entry(asoc_entry, &asoc_list, list) {
> +		id = acpi_match_device(asoc_entry->ids, &pdev->dev);
> +		if (!id)
> +			break;
> +	}
> +	
> +	if (!id || !id->driver_data)
> +		return 0;
> +
> +	if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
> +		return 0;
> +
> +	pdata = acpi_driver_data(adev);
> +	if (!pdata || !pdata->mmio_base)
> +		return 0;
> +
> +	switch (action) {
> +	case BUS_NOTIFY_BOUND_DRIVER:
> +		if ((pdata->dev_desc->flags & ACPI_SOC_PM)){
> +			if (asoc_entry->pm_domain)
> +				pdev->dev.pm_domain = asoc_entry->pm_domain;
> +			else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON)
> +					dev_pm_domain_attach(&pdev->dev, true);
> +			else
> +					dev_pm_domain_attach(&pdev->dev, false);
> +		}	
> +		break;
> +	case BUS_NOTIFY_UNBOUND_DRIVER:
> +		if ((pdata->dev_desc->flags & ACPI_SOC_PM)){
> +			if (asoc_entry->pm_domain)
> +				pdev->dev.pm_domain = asoc_entry->pm_domain;
> +			else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON)
> +					dev_pm_domain_detach(&pdev->dev, true);
> +			else
> +					dev_pm_domain_detach(&pdev->dev, false);
> +		}	
> +		break;
> +	case BUS_NOTIFY_ADD_DEVICE:
> +		if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) &&
> asoc_entry->pm_domain)
> +			return sysfs_create_group(&pdev->dev.kobj,
> +						  asoc_entry->attr_group);
> +	case BUS_NOTIFY_DEL_DEVICE:
> +		if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) &&
> asoc_entry->pm_domain)
> +			sysfs_remove_group(&pdev->dev.kobj, asoc_entry->attr_group);
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block acpi_soc_nb = {
> +	.notifier_call = acpi_soc_platform_notify,
> +};
> +
> +static void acpi_soc_bind(struct device *dev)
> +{
> +	struct acpi_soc_dev_private_data *pdata;
> +
> +	pdata = acpi_driver_data(ACPI_COMPANION(dev));
> +
> +	if (!pdata || !pdata->dev_desc || !pdata->dev_desc->bind)
> +		return;
> +
> +	pdata->dev_desc->bind(pdata);
> +	return;
> +}
> +
> +static void acpi_soc_unbind(struct device *dev)
> +{
> +	struct acpi_soc_dev_private_data *pdata;
> +
> +	pdata = acpi_driver_data(ACPI_COMPANION(dev));
> +
> +	if (!pdata || !pdata->dev_desc || !pdata->dev_desc->unbind)
> +		return;
> +
> +	pdata->dev_desc->unbind(pdata);
> +	return;
> +}
> +
> +void register_acpi_soc(struct acpi_soc *asoc, bool
> disable_scan_handler)
> +{
> +	struct acpi_scan_handler *acpi_soc_handler;
> +	
> +	INIT_LIST_HEAD(&asoc->list);
> +	list_add(&asoc->list, &asoc_list);
> +
> +	acpi_soc_handler = kzalloc(sizeof(struct acpi_scan_handler),
> GFP_KERNEL);
> +	acpi_soc_handler->ids = asoc->ids;
> +	if (!disable_scan_handler) {
> +		acpi_soc_handler->attach = acpi_soc_create_device;
> +		acpi_soc_handler->bind = acpi_soc_bind;
> +		acpi_soc_handler->unbind = acpi_soc_unbind;
> +	}	
> +	acpi_scan_add_handler(acpi_soc_handler);
> +	bus_register_notifier(&platform_bus_type, &acpi_soc_nb);
> +}
> +EXPORT_SYMBOL_GPL(register_acpi_soc);

I don't think we want to export these to modules.

> diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h
> new file mode 100755
> index 0000000..96db30e
> --- /dev/null
> +++ b/drivers/acpi/acpi_soc.h

And all this belongs to drivers/acpi/internal.h.

> @@ -0,0 +1,91 @@
> +/*
> + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD.
> + *
> + * Copyright (C) 2015, Intel Corporation & AMD Corporation
> + * Authors: Ken Xue <Ken.Xue@amd.com>
> + *		Mika Westerberg <mika.westerberg@linux.intel.com>
> + *		Rafael J. Wysocki <rafael.j.wysocki@intel.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 _ACPI_SOC_H
> +#define _ACPI_SOC_H
> +
> +#include <linux/acpi.h>
> +#include <linux/clk.h>
> +#include <linux/pm.h>
> +
> +/* Flags */
> +#define ACPI_SOC_SYSFS	BIT(0)
> +#define ACPI_SOC_PM		BIT(1)
> +#define ACPI_SOC_PM_ON	BIT(2)
> +
> +struct acpi_soc_dev_private_data;
> +
> +/**
> + * struct acpi_soc - acpi soc 
> + * @list: list head
> + * @ids: all acpi device ids for acpi soc 
> + * @pm_domain: power domain for all acpi device;can be NULL
> + * @attr_group: attribute group for sysfs support of acpi soc;can be
> NULL
> + */
> +struct acpi_soc {
> +	struct list_head list;
> +	struct acpi_device_id	*ids;
> +	struct dev_pm_domain	*pm_domain;
> +	struct attribute_group 	*attr_group;
> +};
> +
> +/**
> + * struct acpi_soc_dev_desc - a descriptor for acpi device 
> + * @flags: some device feature flags
> + * @clk: clock device
> + * @fixed_clk_rate: fixed rate input clock source for acpi device;
> + *				0 means no fixed rate input clock source
> + * @mem_size_override: a workaround for override device memsize;
> + *				0 means no needs for this WA
> + * @setup: a hook routine to set device resource during create platform
> device
> + * @bind: a hook of acpi_scan_handler.bind
> + * @unbind: a hook of acpi_scan_handler.unbind
> + *
> + *device description defined as acpi_device_id.driver_data
> + */
> +struct acpi_soc_dev_desc {
> +	unsigned int flags;
> +	struct clk *clk;
> +	unsigned int fixed_clk_rate;
> +	size_t mem_size_override;
> +	int (*setup)(struct acpi_soc_dev_private_data *pdata);
> +	void (*bind)(struct acpi_soc_dev_private_data *pdata);
> +	void (*unbind)(struct acpi_soc_dev_private_data *pdata);
> +};
> +
> +/**
> + * struct acpi_soc_dev_private_data - acpi device private data
> + * @mmio_base: virtual memory base addr of the device
> + * @mmio_size: device memory size
> + * @dev_desc: device description
> + * @adev: apci device
> + *

Delete the above line.

> + */
> +struct acpi_soc_dev_private_data {
> +	void __iomem *mmio_base;
> +	resource_size_t mmio_size;
> +	
> +	const struct acpi_soc_dev_desc *dev_desc;
> +	struct acpi_device *adev;
> +};
> +
> +/**
> + * register_acpi_soc - register a new acpi soc
> + * @asoc: acpi soc
> + * @disable_scan_handler: true means remove deafult scan handle
> + *					false means use deafult scan handle
> + * 
> + * register a new acpi soc into asoc_list and install deafult scan
> handle.
> + */
> +void register_acpi_soc(struct acpi_soc *asoc, bool
> disable_scan_handler);
> +
> +#endif
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 447f6d6..c8a0e8e 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void)
> { return; }
>  #endif
>  void acpi_lpss_init(void);
>  
> +#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
> +void acpi_apd_init(void);
> +#else
> +static inline void acpi_apd_init(void) {}
> +#endif
> +
>  acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
>  bool acpi_queue_hotplug_work(struct work_struct *work);
>  void acpi_device_hotplug(struct acpi_device *adev, u32 src);
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 0476e90..24fef2b 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void)
>  	acpi_pci_link_init();
>  	acpi_processor_init();
>  	acpi_lpss_init();
> +	acpi_apd_init();
>  	acpi_cmos_rtc_init();
>  	acpi_container_init();
>  	acpi_memory_hotplug_init();
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
@ 2014-11-27 11:46           ` Mika Westerberg
  0 siblings, 0 replies; 15+ messages in thread
From: Mika Westerberg @ 2014-11-27 11:46 UTC (permalink / raw)
  To: Ken Xue
  Cc: Rafael J. Wysocki, lenb, linux-acpi, linux-kernel, Wu, Jeff,
	Andriy Shevchenko

On Wed, Nov 26, 2014 at 06:31:38PM +0800, Ken Xue wrote:
> On 一, 2014-11-24 at 02:47 +0100, Rafael J. Wysocki wrote:
> > On Monday, November 24, 2014 01:02:30 AM Xue, Ken wrote:
> > > 
> > > On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote:
> > > > This new feature is to interpret AMD specific ACPI device to platform 
> > > > device such as I2C, UART found on AMD CZ and later chipsets. It is 
> > > > based on example INTEL LPSS. Now, it can support AMD I2C & UART.
> > > > 
> > > > Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> > > > Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>
> > > 
> > > Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead.  What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)?
> > > 
> > > Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen.
> > > 
> > > [ken] sounds fair enough.  Let me take action to merge drivers to acpi_soc.c ? or you have other plan?
> > 
> > I'd prefer the common code to reside in one file (or one .c file and one header
> > file), and the driver-specific code to stay in separate per-driver files.
> > 
> [Ken] I wrote a proto type for acpi_soc.c. please help to confirm if it
> can match your ideal. if yes, i will submit a new patch after do more
> test and refine codes. I think it will impact lpss driver greatly, even
> i have taken it into account. below codes is for acpi_soc.c.

In general looks good. I have few comments though.

> 
> >From fc323fb7b3b4cbb79bda05ce3b1d6d8dfe5e883b Mon Sep 17 00:00:00 2001
> From: Ken Xue <Ken.Xue@amd.com>
> Date: Wed, 26 Nov 2014 17:15:30 +0800
> Subject: [PATCH] This is proto type for acpi_soc.
> 
> Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> ---
>  arch/x86/Kconfig        |  11 +++
>  drivers/acpi/Makefile   |   2 +-
>  drivers/acpi/acpi_apd.c | 123 ++++++++++++++++++++++++++++
>  drivers/acpi/acpi_soc.c | 213
> ++++++++++++++++++++++++++++++++++++++++++++++++

This is line-wrapped, please make sure when you submit the actual patch
that it is formatted properly. Also you need proper changelog etc.

>  drivers/acpi/acpi_soc.h |  91 +++++++++++++++++++++
>  drivers/acpi/internal.h |   6 ++
>  drivers/acpi/scan.c     |   1 +
>  7 files changed, 446 insertions(+), 1 deletion(-)
>  create mode 100755 drivers/acpi/acpi_apd.c
>  create mode 100755 drivers/acpi/acpi_soc.c
>  create mode 100755 drivers/acpi/acpi_soc.h
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index ded8a67..6402c79f 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -495,6 +495,17 @@ config X86_INTEL_LPSS
>  	  things like clock tree (common clock framework) and pincontrol
>  	  which are needed by the LPSS peripheral drivers.
>  
> +config X86_AMD_PLATFORM_DEVICE
> +	bool "AMD ACPI2Platform devices support"
> +	depends on ACPI
> +	select COMMON_CLK
> +	select PINCTRL
> +	---help---
> +	  Select to interpret AMD specific ACPI device to platform device
> +	  such as I2C, UART found on AMD CARRIZO and later chipset. Selecting
> +	  this option enables things like clock tree (common clock framework)
> +	  and pinctrl.
> +
>  config IOSF_MBI
>  	tristate "Intel SoC IOSF Sideband support for SoC platforms"
>  	depends on PCI
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index c3b2fcb..b07003a 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -40,7 +40,7 @@ acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) +=
> processor_pdc.o
>  acpi-y				+= ec.o
>  acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
>  acpi-y				+= pci_root.o pci_link.o pci_irq.o
> -acpi-y				+= acpi_lpss.o
> +acpi-y				+= acpi_soc.o acpi_lpss.o acpi_apd.o
>  acpi-y				+= acpi_platform.o
>  acpi-y				+= acpi_pnp.o
>  acpi-y				+= int340x_thermal.o
> diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
> new file mode 100755
> index 0000000..c7e916b
> --- /dev/null
> +++ b/drivers/acpi/acpi_apd.c
> @@ -0,0 +1,123 @@
> +/*
> + * AMD ACPI support for  ACPI2platform device.
> + *
> + * Copyright (c) 2014, AMD Corporation.
> + * Authors: Ken Xue <Ken.Xue@amd.com>
> + *	Wu, Jeff <Jeff.Wu@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.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "internal.h"
> +#include "acpi_soc.h"
> +
> +struct acpi_soc asoc;

static?

Also Is "asoc" good name? That might get confused with Alsa SoC (ASoC).

> +
> +#ifdef X86_AMD_PLATFORM_DEVICE
> +static int acpi_apd_setup(struct acpi_soc_dev_private_data *pdata)
> +{
> +	struct acpi_soc_dev_desc *dev_desc = pdata->dev_desc;
> +	struct clk *clk = ERR_PTR(-ENODEV);
> +
> +	if (dev_desc->clk)
> +		return 0;
> +	
> +	if (dev_desc->fixed_clk_rate) {
> +		clk = clk_register_fixed_rate(&pdata->adev->dev, 
> +					dev_name(&pdata->adev->dev),
> +					NULL, CLK_IS_ROOT, dev_desc->rate);
> +		dev_desc->clk = clk;
> +		clk_register_clkdev(clk, NULL, dev_name(&pdata->adev->dev));
> +	}
> +
> +	return 0;
> +}
> +
> +static struct acpi_soc_dev_desc cz_i2c_desc = {
> +	.setup = acpi_apd_setup;
> +	.fixed_clk_rate = 133000000,

Oh, good so now we can get rid the hack we did for
i2c-designware-platdrv.c with this commit:

a445900c906092f i2c: designware: Add support for AMD I2C controller

Since now you have means to pass clock to the driver.

Are you going to handle that driver as well?

> +};
> +
> +static struct acpi_soc_dev_desc cz_uart_desc = {
> +	.setup = acpi_apd_setup;
> +	.fixed_clk_rate = 48000000,
> +};
> +
> +#else
> +
> +#define APD_ADDR(desc) (0UL)
> +
> +#endif /* CONFIG_X86_INTEL_LPSS */
> +
> +static struct acpi_device_id acpi_apd_device_ids[] = {

const?

> +	/* Generic apd devices */
> +	{ "AMD0010", APD_ADDR(cz_i2c_desc) },
> +	{ "AMD0020", APD_ADDR(cz_uart_desc) },
> +	{ }
> +};
> +
> +#ifdef X86_AMD_PLATFORM_DEVICE
> +
> +static ssize_t apd_device_desc_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	int ret;
> +	struct acpi_device *adev;
> +	struct acpi_soc_dev_private_data *pdata;
> +	struct acpi_soc_dev_desc *dev_desc;
> +
> +	ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev);
> +	if (WARN_ON(ret))
> +		return ret;
> +
> +	pdata = acpi_driver_data(adev);
> +	if (WARN_ON(!pdata || !pdata->dev_desc))
> +		return -ENODEV;
> +	
> +	dev_desc = pdata->dev_desc;
> +	if (dev_desc->fixed_clk_rate)
> +		return sprintf(buf, "Required fix rate clk %s: %ld\n",
> +			dev_desc->clk->name,
> +			dev_desc->fixed_clk_rate);
> +	else
> +		return sprintf(buf, "No need clk\n");
> +}
> +
> +static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL);
> +
> +static struct attribute *apd_attrs[] = {
> +	&dev_attr_device_desc.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group apd_attr_group = {
> +	.attrs = apd_attrs,
> +	.name = "apd",
> +};

This requires updating sysfs ABI but then again, do you really need the
above? And does it belong to sysfs in the first place?

> +
> +void __init acpi_apd_init(void)
> +{
> +	asoc.ids = acpi_apd_device_ids;
> +	asoc.attr_group = &apd_attr_group;
> +	register_acpi_soc(&asoc, true);
> +}
> +
> +#else
> +
> +void __init acpi_apd_init(void)
> +{
> +	asoc.ids = acpi_apd_device_ids;
> +	register_acpi_soc(&asoc, false);
> +}
> +#endif
> diff --git a/drivers/acpi/acpi_soc.c b/drivers/acpi/acpi_soc.c
> new file mode 100755
> index 0000000..99df1ab
> --- /dev/null
> +++ b/drivers/acpi/acpi_soc.c
> @@ -0,0 +1,213 @@
> +/*
> + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD.
> + *
> + * Copyright (C) 2015, Intel Corporation & AMD Corporation
> + * Authors: Ken Xue <Ken.Xue@amd.com>
> + *		Mika Westerberg <mika.westerberg@linux.intel.com>
> + *		Rafael J. Wysocki <rafael.j.wysocki@intel.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.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/delay.h>
> +#include <linux/pm_domain.h>
> +
> +#include "internal.h"
> +#include "acpi_soc.h"
> +
> +

Delete the extra blank line

> +ACPI_MODULE_NAME("acpi_soc");
> +
> +/*A list for all acpi soc device*/
> +static LIST_HEAD(asoc_list);
> +
> +static int is_memory(struct acpi_resource *res, void *not_used)
> +{
> +	struct resource r;
> +	return !acpi_dev_resource_memory(res, &r);
> +}
> +
> +static int acpi_soc_create_device(struct acpi_device *adev,
> +				   const struct acpi_device_id *id)
> +{
> +	struct acpi_soc_dev_desc *dev_desc;
> +	struct acpi_soc_dev_private_data *pdata;
> +	struct resource_list_entry *rentry;
> +	struct list_head resource_list;
> +	struct platform_device *pdev;
> +	int ret;
> +
> +	dev_desc = (struct acpi_soc_dev_desc *)id->driver_data;
> +	if (!dev_desc) {
> +		pdev = acpi_create_platform_device(adev);
> +		return IS_ERR_OR_NULL(pdev) ? PTR_ERR(pdev) : 1;
> +	}
> +	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&resource_list);
> +	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
> +	if (ret < 0)
> +		goto err_out;
> +
> +	list_for_each_entry(rentry, &resource_list, node)
> +		if (resource_type(&rentry->res) == IORESOURCE_MEM) {
> +			if (dev_desc->mem_size_override)
> +				pdata->mmio_size = dev_desc->mem_size_override;
> +			else
> +				pdata->mmio_size = resource_size(&rentry->res);
> +			pdata->mmio_base = ioremap(rentry->res.start,
> +						   pdata->mmio_size);
> +			break;
> +		}
> +
> +	acpi_dev_free_resource_list(&resource_list);
> +
> +	pdata->dev_desc = dev_desc;
> +
> +	/*setup device by a hook routine*/

The comment should look like this

	/* Setup device by hook routine */

but I think it does not provide any new information so you can just drop
it.

> +	if (dev_desc->setup)
> +		dev_desc->setup(pdata);
> +
> +	/*
> +	 * This works around a known issue in ACPI tables where acpi soc
> devices
> +	 * have _PS0 and _PS3 without _PSC (and no power resources), so
> +	 * acpi_bus_init_power() will assume that the BIOS has put them into
> D0.
> +	 */
> +	ret = acpi_device_fix_up_power(adev);
> +	if (ret) {
> +		/* Skip the device, but continue the namespace scan. */
> +		ret = 0;
> +		goto err_out;
> +	}
> +
> +	adev->driver_data = pdata;
> +	pdev = acpi_create_platform_device(adev);
> +	if (!IS_ERR_OR_NULL(pdev)) {
> +		return 1;
> +	}
> +
> +	ret = PTR_ERR(pdev);
> +	adev->driver_data = NULL;
> +
> + err_out:
> +	kfree(pdata);
> +	return ret;
> +}
> +
> +static int acpi_soc_platform_notify(struct notifier_block *nb,
> +				     unsigned long action, void *data)
> +{
> +	struct platform_device *pdev = to_platform_device(data);
> +	struct acpi_soc_dev_private_data *pdata;
> +	struct acpi_device *adev;
> +	struct acpi_soc *asoc_entry;
> +	const struct acpi_device_id *id = NULL;
> +
> +	list_for_each_entry(asoc_entry, &asoc_list, list) {
> +		id = acpi_match_device(asoc_entry->ids, &pdev->dev);
> +		if (!id)
> +			break;
> +	}
> +	
> +	if (!id || !id->driver_data)
> +		return 0;
> +
> +	if (acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
> +		return 0;
> +
> +	pdata = acpi_driver_data(adev);
> +	if (!pdata || !pdata->mmio_base)
> +		return 0;
> +
> +	switch (action) {
> +	case BUS_NOTIFY_BOUND_DRIVER:
> +		if ((pdata->dev_desc->flags & ACPI_SOC_PM)){
> +			if (asoc_entry->pm_domain)
> +				pdev->dev.pm_domain = asoc_entry->pm_domain;
> +			else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON)
> +					dev_pm_domain_attach(&pdev->dev, true);
> +			else
> +					dev_pm_domain_attach(&pdev->dev, false);
> +		}	
> +		break;
> +	case BUS_NOTIFY_UNBOUND_DRIVER:
> +		if ((pdata->dev_desc->flags & ACPI_SOC_PM)){
> +			if (asoc_entry->pm_domain)
> +				pdev->dev.pm_domain = asoc_entry->pm_domain;
> +			else if (pdata->dev_desc->flags & ACPI_SOC_PM_ON)
> +					dev_pm_domain_detach(&pdev->dev, true);
> +			else
> +					dev_pm_domain_detach(&pdev->dev, false);
> +		}	
> +		break;
> +	case BUS_NOTIFY_ADD_DEVICE:
> +		if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) &&
> asoc_entry->pm_domain)
> +			return sysfs_create_group(&pdev->dev.kobj,
> +						  asoc_entry->attr_group);
> +	case BUS_NOTIFY_DEL_DEVICE:
> +		if ((pdata->dev_desc->flags & ACPI_SOC_SYSFS) &&
> asoc_entry->pm_domain)
> +			sysfs_remove_group(&pdev->dev.kobj, asoc_entry->attr_group);
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block acpi_soc_nb = {
> +	.notifier_call = acpi_soc_platform_notify,
> +};
> +
> +static void acpi_soc_bind(struct device *dev)
> +{
> +	struct acpi_soc_dev_private_data *pdata;
> +
> +	pdata = acpi_driver_data(ACPI_COMPANION(dev));
> +
> +	if (!pdata || !pdata->dev_desc || !pdata->dev_desc->bind)
> +		return;
> +
> +	pdata->dev_desc->bind(pdata);
> +	return;
> +}
> +
> +static void acpi_soc_unbind(struct device *dev)
> +{
> +	struct acpi_soc_dev_private_data *pdata;
> +
> +	pdata = acpi_driver_data(ACPI_COMPANION(dev));
> +
> +	if (!pdata || !pdata->dev_desc || !pdata->dev_desc->unbind)
> +		return;
> +
> +	pdata->dev_desc->unbind(pdata);
> +	return;
> +}
> +
> +void register_acpi_soc(struct acpi_soc *asoc, bool
> disable_scan_handler)
> +{
> +	struct acpi_scan_handler *acpi_soc_handler;
> +	
> +	INIT_LIST_HEAD(&asoc->list);
> +	list_add(&asoc->list, &asoc_list);
> +
> +	acpi_soc_handler = kzalloc(sizeof(struct acpi_scan_handler),
> GFP_KERNEL);
> +	acpi_soc_handler->ids = asoc->ids;
> +	if (!disable_scan_handler) {
> +		acpi_soc_handler->attach = acpi_soc_create_device;
> +		acpi_soc_handler->bind = acpi_soc_bind;
> +		acpi_soc_handler->unbind = acpi_soc_unbind;
> +	}	
> +	acpi_scan_add_handler(acpi_soc_handler);
> +	bus_register_notifier(&platform_bus_type, &acpi_soc_nb);
> +}
> +EXPORT_SYMBOL_GPL(register_acpi_soc);

I don't think we want to export these to modules.

> diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h
> new file mode 100755
> index 0000000..96db30e
> --- /dev/null
> +++ b/drivers/acpi/acpi_soc.h

And all this belongs to drivers/acpi/internal.h.

> @@ -0,0 +1,91 @@
> +/*
> + * ACPI SOC support for Intel Lynxpoint LPSS and AMD APD.
> + *
> + * Copyright (C) 2015, Intel Corporation & AMD Corporation
> + * Authors: Ken Xue <Ken.Xue@amd.com>
> + *		Mika Westerberg <mika.westerberg@linux.intel.com>
> + *		Rafael J. Wysocki <rafael.j.wysocki@intel.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 _ACPI_SOC_H
> +#define _ACPI_SOC_H
> +
> +#include <linux/acpi.h>
> +#include <linux/clk.h>
> +#include <linux/pm.h>
> +
> +/* Flags */
> +#define ACPI_SOC_SYSFS	BIT(0)
> +#define ACPI_SOC_PM		BIT(1)
> +#define ACPI_SOC_PM_ON	BIT(2)
> +
> +struct acpi_soc_dev_private_data;
> +
> +/**
> + * struct acpi_soc - acpi soc 
> + * @list: list head
> + * @ids: all acpi device ids for acpi soc 
> + * @pm_domain: power domain for all acpi device;can be NULL
> + * @attr_group: attribute group for sysfs support of acpi soc;can be
> NULL
> + */
> +struct acpi_soc {
> +	struct list_head list;
> +	struct acpi_device_id	*ids;
> +	struct dev_pm_domain	*pm_domain;
> +	struct attribute_group 	*attr_group;
> +};
> +
> +/**
> + * struct acpi_soc_dev_desc - a descriptor for acpi device 
> + * @flags: some device feature flags
> + * @clk: clock device
> + * @fixed_clk_rate: fixed rate input clock source for acpi device;
> + *				0 means no fixed rate input clock source
> + * @mem_size_override: a workaround for override device memsize;
> + *				0 means no needs for this WA
> + * @setup: a hook routine to set device resource during create platform
> device
> + * @bind: a hook of acpi_scan_handler.bind
> + * @unbind: a hook of acpi_scan_handler.unbind
> + *
> + *device description defined as acpi_device_id.driver_data
> + */
> +struct acpi_soc_dev_desc {
> +	unsigned int flags;
> +	struct clk *clk;
> +	unsigned int fixed_clk_rate;
> +	size_t mem_size_override;
> +	int (*setup)(struct acpi_soc_dev_private_data *pdata);
> +	void (*bind)(struct acpi_soc_dev_private_data *pdata);
> +	void (*unbind)(struct acpi_soc_dev_private_data *pdata);
> +};
> +
> +/**
> + * struct acpi_soc_dev_private_data - acpi device private data
> + * @mmio_base: virtual memory base addr of the device
> + * @mmio_size: device memory size
> + * @dev_desc: device description
> + * @adev: apci device
> + *

Delete the above line.

> + */
> +struct acpi_soc_dev_private_data {
> +	void __iomem *mmio_base;
> +	resource_size_t mmio_size;
> +	
> +	const struct acpi_soc_dev_desc *dev_desc;
> +	struct acpi_device *adev;
> +};
> +
> +/**
> + * register_acpi_soc - register a new acpi soc
> + * @asoc: acpi soc
> + * @disable_scan_handler: true means remove deafult scan handle
> + *					false means use deafult scan handle
> + * 
> + * register a new acpi soc into asoc_list and install deafult scan
> handle.
> + */
> +void register_acpi_soc(struct acpi_soc *asoc, bool
> disable_scan_handler);
> +
> +#endif
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 447f6d6..c8a0e8e 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -68,6 +68,12 @@ static inline void acpi_debugfs_init(void)
> { return; }
>  #endif
>  void acpi_lpss_init(void);
>  
> +#ifdef CONFIG_X86_AMD_PLATFORM_DEVICE
> +void acpi_apd_init(void);
> +#else
> +static inline void acpi_apd_init(void) {}
> +#endif
> +
>  acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
>  bool acpi_queue_hotplug_work(struct work_struct *work);
>  void acpi_device_hotplug(struct acpi_device *adev, u32 src);
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 0476e90..24fef2b 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2349,6 +2349,7 @@ int __init acpi_scan_init(void)
>  	acpi_pci_link_init();
>  	acpi_processor_init();
>  	acpi_lpss_init();
> +	acpi_apd_init();
>  	acpi_cmos_rtc_init();
>  	acpi_container_init();
>  	acpi_memory_hotplug_init();
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-27 17:06           ` Rafael J. Wysocki
@ 2014-11-27 16:48             ` Mika Westerberg
  0 siblings, 0 replies; 15+ messages in thread
From: Mika Westerberg @ 2014-11-27 16:48 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Ken Xue, lenb, linux-acpi, linux-kernel, Wu, Jeff, Andriy Shevchenko

On Thu, Nov 27, 2014 at 06:06:35PM +0100, Rafael J. Wysocki wrote:
> On Thursday, November 27, 2014 01:46:37 PM Mika Westerberg wrote:
> > On Wed, Nov 26, 2014 at 06:31:38PM +0800, Ken Xue wrote:
> 
> [cut]
> 
> > > diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h
> > > new file mode 100755
> > > index 0000000..96db30e
> > > --- /dev/null
> > > +++ b/drivers/acpi/acpi_soc.h
> > 
> > And all this belongs to drivers/acpi/internal.h.
> 
> I requested the new file.
> 
> I don't really want things specific to SoC drivers to be mixed up with ACPI core
> stuff.  It may even be good to create a separate subdir for the SoC stuff, but
> that can be done later.

I see, thanks for the explanation.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-27 11:46           ` Mika Westerberg
  (?)
@ 2014-11-27 17:06           ` Rafael J. Wysocki
  2014-11-27 16:48             ` Mika Westerberg
  -1 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2014-11-27 17:06 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Ken Xue, lenb, linux-acpi, linux-kernel, Wu, Jeff, Andriy Shevchenko

On Thursday, November 27, 2014 01:46:37 PM Mika Westerberg wrote:
> On Wed, Nov 26, 2014 at 06:31:38PM +0800, Ken Xue wrote:

[cut]

> > diff --git a/drivers/acpi/acpi_soc.h b/drivers/acpi/acpi_soc.h
> > new file mode 100755
> > index 0000000..96db30e
> > --- /dev/null
> > +++ b/drivers/acpi/acpi_soc.h
> 
> And all this belongs to drivers/acpi/internal.h.

I requested the new file.

I don't really want things specific to SoC drivers to be mixed up with ACPI core
stuff.  It may even be good to create a separate subdir for the SoC stuff, but
that can be done later.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system.
  2014-11-27 11:46           ` Mika Westerberg
  (?)
  (?)
@ 2014-11-28  1:33           ` Ken Xue
  -1 siblings, 0 replies; 15+ messages in thread
From: Ken Xue @ 2014-11-28  1:33 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Rafael J. Wysocki, lenb, linux-acpi, linux-kernel, Wu, Jeff,
	Andriy Shevchenko

On Thu, 2014-11-27 at 13:46 +0200, Mika Westerberg wrote:
> On Wed, Nov 26, 2014 at 06:31:38PM +0800, Ken Xue wrote:
> > On Monday, 2014-11-24 at 02:47 +0100, Rafael J. Wysocki wrote:
> > > On Monday, November 24, 2014 01:02:30 AM Xue, Ken wrote:
> > > > 
> > > > On Tuesday, November 18, 2014 01:58:11 PM Ken Xue wrote:
> > > > > This new feature is to interpret AMD specific ACPI device to platform 
> > > > > device such as I2C, UART found on AMD CZ and later chipsets. It is 
> > > > > based on example INTEL LPSS. Now, it can support AMD I2C & UART.
> > > > > 
> > > > > Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> > > > > Signed-off-by: Jeff Wu <Jeff.Wu@amd.com>
> > > > 
> > > > Generally speaking, this seems to duplicate much code from acpi_lpss which should be re-used instead.  What about moving the code that will be common between acpi_lpss and the new driver into a new file (say acpi_soc.c)?
> > > > 
> > > > Also, you need to avoid automatic creation of platform devices when !X86_AMD_PLATFORM_DEVICE in analogy with what acpi_lpss does, or bad things will happen.
> > > > 
> > > > [ken] sounds fair enough.  Let me take action to merge drivers to acpi_soc.c ? or you have other plan?
> > > 
> > > I'd prefer the common code to reside in one file (or one .c file and one header
> > > file), and the driver-specific code to stay in separate per-driver files.
> > > 
> > [Ken] I wrote a proto type for acpi_soc.c. please help to confirm if it
> > can match your ideal. if yes, i will submit a new patch after do more
> > test and refine codes. I think it will impact lpss driver greatly, even
> > i have taken it into account. below codes is for acpi_soc.c.
> 
> In general looks good. I have few comments though.
[Ken] thanks for your comments.

> > 
> > >From fc323fb7b3b4cbb79bda05ce3b1d6d8dfe5e883b Mon Sep 17 00:00:00 2001
> > From: Ken Xue <Ken.Xue@amd.com>
> > Date: Wed, 26 Nov 2014 17:15:30 +0800
> > Subject: [PATCH] This is proto type for acpi_soc.
> > 
> > Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> > ---
> >  arch/x86/Kconfig        |  11 +++
> >  drivers/acpi/Makefile   |   2 +-
> >  drivers/acpi/acpi_apd.c | 123 ++++++++++++++++++++++++++++
> >  drivers/acpi/acpi_soc.c | 213
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> 
> This is line-wrapped, please make sure when you submit the actual patch
> that it is formatted properly. Also you need proper changelog etc.
[Ken] sure. 

> > +#include "internal.h"
> > +#include "acpi_soc.h"
> > +
> > +struct acpi_soc asoc;
> 
> static?
> 
> Also Is "asoc" good name? That might get confused with Alsa SoC (ASoC).
[Ken] I will use "static", and change name to be a_soc.


> > +
> > +static struct acpi_soc_dev_desc cz_i2c_desc = {
> > +	.setup = acpi_apd_setup;
> > +	.fixed_clk_rate = 133000000,
> 
> Oh, good so now we can get rid the hack we did for
> i2c-designware-platdrv.c with this commit:
> 
> a445900c906092f i2c: designware: Add support for AMD I2C controller
> 
> Since now you have means to pass clock to the driver.
> 
> Are you going to handle that driver as well?
[Ken]sure, thanks. this was one of reasons to create AMD APD.

> > +};
> > +
> > +static struct acpi_soc_dev_desc cz_uart_desc = {
> > +	.setup = acpi_apd_setup;
> > +	.fixed_clk_rate = 48000000,
> > +};
> > +
> > +#else
> > +
> > +#define APD_ADDR(desc) (0UL)
> > +
> > +#endif /* CONFIG_X86_INTEL_LPSS */
> > +
> > +static struct acpi_device_id acpi_apd_device_ids[] = {
> 
> const?
[ken]No. "acpi_soc_dev_desc" may be modified later.

> > +	/* Generic apd devices */
> > +	{ "AMD0010", APD_ADDR(cz_i2c_desc) },
> > +	{ "AMD0020", APD_ADDR(cz_uart_desc) },
> > +	{ }
> > +};
> > +
> > +#ifdef X86_AMD_PLATFORM_DEVICE
> > +
> > +static ssize_t apd_device_desc_show(struct device *dev,
> > +				struct device_attribute *attr, char *buf)
> > +{
> > +	int ret;
> > +	struct acpi_device *adev;
> > +	struct acpi_soc_dev_private_data *pdata;
> > +	struct acpi_soc_dev_desc *dev_desc;
> > +
> > +	ret = acpi_bus_get_device(ACPI_HANDLE(dev), &adev);
> > +	if (WARN_ON(ret))
> > +		return ret;
> > +
> > +	pdata = acpi_driver_data(adev);
> > +	if (WARN_ON(!pdata || !pdata->dev_desc))
> > +		return -ENODEV;
> > +	
> > +	dev_desc = pdata->dev_desc;
> > +	if (dev_desc->fixed_clk_rate)
> > +		return sprintf(buf, "Required fix rate clk %s: %ld\n",
> > +			dev_desc->clk->name,
> > +			dev_desc->fixed_clk_rate);
> > +	else
> > +		return sprintf(buf, "No need clk\n");
> > +}
> > +
> > +static DEVICE_ATTR(device_desc, S_IRUSR, apd_device_desc_show, NULL);
> > +
> > +static struct attribute *apd_attrs[] = {
> > +	&dev_attr_device_desc.attr,
> > +	NULL,
> > +};
> > +
> > +static struct attribute_group apd_attr_group = {
> > +	.attrs = apd_attrs,
> > +	.name = "apd",
> > +};
> 
> This requires updating sysfs ABI but then again, do you really need the
> above? And does it belong to sysfs in the first place?
[Ken] just want to output some debug information with sysfs. I think i
can add sysfs interface after APD has rich features in the future.

> > 
> > +#include "internal.h"
> > +#include "acpi_soc.h"
> > +
> > +
> 
> Delete the extra blank line
[ken] got it.

> >
> > +	pdata->dev_desc = dev_desc;
> > +
> > +	/*setup device by a hook routine*/
> 
> The comment should look like this
> 
> 	/* Setup device by hook routine */
> 
> but I think it does not provide any new information so you can just drop
> it.
[Ken] i will remove it.

> > +
> > +void register_acpi_soc(struct acpi_soc *asoc, bool
> > disable_scan_handler)
> > +{
> > +	struct acpi_scan_handler *acpi_soc_handler;
> > +	
> > +	INIT_LIST_HEAD(&asoc->list);
> > +	list_add(&asoc->list, &asoc_list);
> > +
> > +	acpi_soc_handler = kzalloc(sizeof(struct acpi_scan_handler),
> > GFP_KERNEL);
> > +	acpi_soc_handler->ids = asoc->ids;
> > +	if (!disable_scan_handler) {
> > +		acpi_soc_handler->attach = acpi_soc_create_device;
> > +		acpi_soc_handler->bind = acpi_soc_bind;
> > +		acpi_soc_handler->unbind = acpi_soc_unbind;
> > +	}	
> > +	acpi_scan_add_handler(acpi_soc_handler);
> > +	bus_register_notifier(&platform_bus_type, &acpi_soc_nb);
> > +}
> > +EXPORT_SYMBOL_GPL(register_acpi_soc);
> 
> I don't think we want to export these to modules.
[ken]i will remove it.


> > +
> > +/**
> > + * struct acpi_soc_dev_private_data - acpi device private data
> > + * @mmio_base: virtual memory base addr of the device
> > + * @mmio_size: device memory size
> > + * @dev_desc: device description
> > + * @adev: apci device
> > + *
> 
> Delete the above line.
[Ken]ok.





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-11-28  1:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-18  5:58 [PATCH] acpi:apd:add AMD ACPI2Platform device support for x86 system Ken Xue
2014-11-18  5:58 ` Ken Xue
2014-11-21  5:15 ` Xue, Ken
2014-11-21 14:58   ` Rafael J. Wysocki
2014-11-24  1:15 ` Rafael J. Wysocki
2014-11-24  1:02   ` Xue, Ken
2014-11-24  1:02     ` Xue, Ken
2014-11-24  1:47     ` Rafael J. Wysocki
2014-11-26 10:31       ` Ken Xue
2014-11-26 10:31         ` Ken Xue
2014-11-27 11:46         ` Mika Westerberg
2014-11-27 11:46           ` Mika Westerberg
2014-11-27 17:06           ` Rafael J. Wysocki
2014-11-27 16:48             ` Mika Westerberg
2014-11-28  1:33           ` Ken Xue

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.