Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[One Thousand Gnomes]

On Tue, 21 Apr 2015 07:04:29 +0800
"qipeng.zha" <qipeng.zha@intel.com> wrote:

> From: "qipeng.zha" <qipeng.zha@intel.com>
> 
> This driver provides support for PMC control on Broxton platforms,
> PMC is ARC processor which defined some IPC commands for communication
> with other entities running in IA

This appears to mostly be a clone of 

drivers/platform/x86/intel_scu_ipc.c

so is there any reason why it has to be another rewrite of the ancient
driver that was already cleaned up and upstreamed once.

Alan

[PATCH] platform:x86: add Intel Broxton PMC IPC driver

[qipeng.zha]

From: "qipeng.zha" <qipeng.zha@intel.com>

This driver provides support for PMC control on Broxton platforms,
PMC is ARC processor which defined some IPC commands for communication
with other entities running in IA

Signed-off-by: qipeng.zha <qipeng.zha@intel.com>
---
 arch/x86/include/asm/intel_pmc_ipc.h |  79 ++++
 drivers/platform/x86/Kconfig         |   7 +
 drivers/platform/x86/Makefile        |   1 +
 drivers/platform/x86/intel_pmc_ipc.c | 740 +++++++++++++++++++++++++++++++++++
 4 files changed, 827 insertions(+)
 create mode 100644 arch/x86/include/asm/intel_pmc_ipc.h
 create mode 100644 drivers/platform/x86/intel_pmc_ipc.c

diff --git a/arch/x86/include/asm/intel_pmc_ipc.h b/arch/x86/include/asm/intel_pmc_ipc.h
new file mode 100644
index 0000000..690b55f
--- /dev/null
+++ b/arch/x86/include/asm/intel_pmc_ipc.h
@@ -0,0 +1,79 @@
+#ifndef _ASM_X86_INTEL_PMC_IPC_H_
+#define  _ASM_X86_INTEL_PMC_IPC_H_
+
+/*commands*/
+#define PMC_IPC_PMIC_ACCESS		0xFF
+#define		PMC_IPC_PMIC_ACCESS_READ	0x0
+#define		PMC_IPC_PMIC_ACCESS_WRITE	0x1
+#define PMC_IPC_USB_PWR_CTRL		0xF0
+#define PMC_IPC_PMIC_BLACKLIST_SEL	0xEF
+#define PMC_IPC_PHY_CONFIG		0xEE
+#define PMC_IPC_NORTHPEAK_CTRL		0xED
+#define PMC_IPC_PM_DEBUG		0xEC
+#define PMC_IPC_PMC_TELEMTRY		0xEB
+#define PMC_IPC_PMC_FW_MSG_CTRL		0xEA
+
+/*IPC return code*/
+#define IPC_ERR_NONE			0
+#define IPC_ERR_CMD_NOT_SUPPORTED	1
+#define IPC_ERR_CMD_NOT_SERVICED	2
+#define IPC_ERR_UNABLE_TO_SERVICE	3
+#define IPC_ERR_CMD_INVALID		4
+#define IPC_ERR_CMD_FAILED		5
+#define IPC_ERR_EMSECURITY		6
+#define IPC_ERR_UNSIGNEDKERNEL		7
+
+#ifdef CONFIG_INTEL_PMC_IPC
+/*
+* intel_pmc_ipc_simple_command
+* @cmd: command
+* @sub: sub type
+*/
+int intel_pmc_ipc_simple_command(int cmd, int sub);
+/*
+* intel_pmc_ipc_raw_cmd
+* @cmd: command
+* @sub: sub type
+* @in: input data
+* @inlen: input length in bytes
+* @out: output data
+* @outlen: output length in dwords
+* @sptr: data writing to SPTR register
+* @dptr: data writing to DPTR register
+*/
+int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
+		u32 *out, u32 outlen, u32 dptr, u32 sptr);
+/*
+* intel_pmc_ipc_raw_cmd
+* @cmd: command
+* @sub: sub type
+* @in: input data
+* @inlen: input length in bytes
+* @out: output data
+* @outlen: output length in dwords
+*/
+int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
+		u32 *out, u32 outlen);
+
+#else
+
+static inline int intel_pmc_ipc_simple_command(int cmd, int sub)
+{
+	return -EINVAL;
+}
+
+static inline int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
+		u32 *out, u32 outlen, u32 dptr, u32 sptr)
+{
+	return -EINVAL;
+}
+
+static inline int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
+		u32 *out, u32 outlen)
+{
+	return -EINVAL;
+}
+
+#endif /*CONFIG_INTEL_PMC_IPC*/
+
+#endif
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 9752761..4eb15eb 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -884,4 +884,11 @@ config PVPANIC
 	  a paravirtualized device provided by QEMU; it lets a virtual machine
 	  (guest) communicate panic events to the host.
 
+config INTEL_PMC_IPC
+	tristate "Intel PMC IPC Driver"
+	---help---
+	This driver provides support for PMC control on some Intel platforms,
+	PMC is ARC processor which defined some IPC commands for communication
+	with other entities which running in IA.
+
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index f82232b..1051372 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)	+= intel-smartconnect.o
 
 obj-$(CONFIG_PVPANIC)           += pvpanic.o
 obj-$(CONFIG_ALIENWARE_WMI)	+= alienware-wmi.o
+obj-$(CONFIG_INTEL_PMC_IPC)	+= intel_pmc_ipc.o
diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
new file mode 100644
index 0000000..8610508
--- /dev/null
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -0,0 +1,740 @@
+/*
+ * intel_pmc_ipc.c: Driver for the Intel PMC IPC mechanism
+ *
+ * (C) Copyright 2014 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ *
+ * PMC running in ARC processor communicates with other entity running in IA
+ * core through IPC mechanism which in turn messaging between IA core ad PMC.
+ */
+
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/pm.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/pm_qos.h>
+#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/sched.h>
+#include <linux/atomic.h>
+#include <linux/notifier.h>
+#include <linux/suspend.h>
+#include <linux/acpi.h>
+#include <asm/intel_pmc_ipc.h>
+#include <linux/mfd/lpc_ich.h>
+
+/*
+* IPC registers
+*/
+#define IPC_CMD			0x0
+#define		IPC_CMD_MSI		0x100
+#define		IPC_CMD_SIZE		16
+#define		IPC_CMD_SUBCMD		12
+#define IPC_STATUS		0x04
+#define		IPC_STATUS_IRQ		0x4
+#define		IPC_STATUS_ERR		0x2
+#define		IPC_STATUS_BUSY		0x1
+#define IPC_SPTR		0x08
+#define IPC_DPTR		0x0C
+#define IPC_WRITE_BUFFER	0x80
+#define IPC_READ_BUFFER		0x90
+
+#define IPC_LOOP_CNT		3000000
+#define IPC_MAX_SEC		3
+
+/*How host to know command status from PMC*/
+#define IPC_TRIGGER_MODE_IRQ		true
+
+/*exported resources from IFWI*/
+#define PLAT_RESOURCE_IPC_INDEX		0
+#define PLAT_RESOURCE_IPC_SIZE		0x1000
+#define PLAT_RESOURCE_GCR_SIZE		0x1000
+#define PLAT_RESOURCE_PUNIT_DATA_INDEX	1
+#define PLAT_RESOURCE_PUNIT_INTER_INDEX	2
+#define PLAT_RESOURCE_ACPI_IO_INDEX	0
+
+/*BIOS don't create ACPI device for each PMC function,
+* but export multi resoures on one ACPI device(IPC) for
+* multi functions, so this driver is responsible to create
+* platform device and to export resource for those functions
+*/
+#define TCO_DEVICE_NAME			"iTCO_wdt"
+#define SMI_EN_OFFSET			0x30
+#define SMI_EN_SIZE			4
+#define TCO_BASE_OFFSET			0x60
+#define TCO_REGS_SIZE			16
+#define PUNIT_DEVICE_NAME		"intel_punit_ipc"
+
+static const int iTCO_version = 3;
+
+static struct intel_pmc_ipc_dev {
+	struct device *dev;
+	void __iomem *ipc_base;
+	bool irq_mode;
+	int irq;
+	int cmd;
+	struct completion cmd_complete;
+
+	/*Below bars in PMC share same ACPI device with IPC*/
+	void *acpi_io_base;
+	int acpi_io_size;
+	struct platform_device *tco_dev;
+
+	/*gcr*/
+	void __iomem *gcr_base;
+	int gcr_size;
+
+	/*punit*/
+	void *punit_base;
+	int punit_size;
+	void *punit_base2;
+	int punit_size2;
+	struct platform_device *punit_dev;
+} ipcdev;
+
+static char *ipc_err_sources[] = {
+	[IPC_ERR_NONE] =
+		"no error",
+	[IPC_ERR_CMD_NOT_SUPPORTED] =
+		"command not supported",
+	[IPC_ERR_CMD_NOT_SERVICED] =
+		"command not serviced",
+	[IPC_ERR_UNABLE_TO_SERVICE] =
+		"unable to service",
+	[IPC_ERR_CMD_INVALID] =
+		"command invalid",
+	[IPC_ERR_CMD_FAILED] =
+		"command failed",
+	[IPC_ERR_EMSECURITY] =
+		"Invalid Battery",
+	[IPC_ERR_UNSIGNEDKERNEL] =
+		"Unsigned kernel",
+};
+
+/*prevent multiple call to PMC*/
+static DEFINE_MUTEX(ipclock);
+
+/*
+* Command Register
+* |rsvd(8) | size(8) | command id(4) | rsvd(3) | msi(1) | command(8)|
+*/
+static inline void ipc_send_command(u32 cmd)
+{
+	ipcdev.cmd = cmd;
+	if (ipcdev.irq_mode) {
+		reinit_completion(&ipcdev.cmd_complete);
+		cmd |= IPC_CMD_MSI;
+	}
+	writel(cmd, ipcdev.ipc_base + IPC_CMD);
+}
+
+/*
+* Status Register
+* |rsvd(8)|error code(8)|initiator id(8)|cmd id(4)|irq(2)|error(1)|busy(1)|
+*/
+static inline u32 ipc_read_status(void)
+{
+	return readl(ipcdev.ipc_base + IPC_STATUS);
+}
+
+/*
+* IPC Write Buffer (Write Only):
+* 16-byte buffer for sending data associated with IPC command
+* Size of the data is specified in the IPC_CMD[size]
+*/
+static inline void ipc_data_writel(u32 data, u32 offset)
+{
+	writel(data, ipcdev.ipc_base + IPC_WRITE_BUFFER + offset);
+}
+
+static inline u8 ipc_data_readb(u32 offset)
+{
+	return readb(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
+}
+
+static inline u32 ipc_data_readl(u32 offset)
+{
+	return readl(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
+}
+
+int intel_pmc_ipc_check_status(void)
+{
+	int i;
+	int ret = 0;
+	int status;
+	int loop_count = IPC_LOOP_CNT;
+
+	if (ipcdev.irq_mode) {
+		if (0 == wait_for_completion_timeout(
+				&ipcdev.cmd_complete, IPC_MAX_SEC * HZ))
+			ret = -ETIMEDOUT;
+	} else {
+		while ((ipc_read_status() & IPC_STATUS_BUSY) && --loop_count)
+			udelay(1);
+		if (loop_count == 0)
+			ret = -ETIMEDOUT;
+	}
+
+	status = ipc_read_status();
+	if (ret == -ETIMEDOUT) {
+		dev_err(ipcdev.dev,
+			"IPC timed out, TS=0x%x, CMD=0x%x\n",
+			status, ipcdev.cmd);
+		return ret;
+	}
+
+	if (status & IPC_STATUS_ERR) {
+		ret = -EIO;
+		i = (status >> IPC_CMD_SIZE) & 0xFF;
+		if (i < ARRAY_SIZE(ipc_err_sources))
+			dev_err(ipcdev.dev,
+				"IPC failed: %s, STS=0x%x, CMD=0x%x\n",
+				ipc_err_sources[i], status, ipcdev.cmd);
+		else
+			dev_err(ipcdev.dev,
+				"IPC failed: unknown, STS=0x%x, CMD=0x%x\n",
+				 status, ipcdev.cmd);
+		if ((i == IPC_ERR_UNSIGNEDKERNEL) || (i == IPC_ERR_EMSECURITY))
+			ret = -EACCES;
+	}
+
+	return ret;
+}
+
+int intel_pmc_ipc_simple_command(int cmd, int sub)
+{
+	int ret;
+
+	if (ipcdev.dev == NULL)
+		return -ENODEV;
+
+	mutex_lock(&ipclock);
+	ipc_send_command(sub << IPC_CMD_SUBCMD | cmd);
+	ret = intel_pmc_ipc_check_status();
+	mutex_unlock(&ipclock);
+
+	return ret;
+}
+EXPORT_SYMBOL(intel_pmc_ipc_simple_command);
+
+int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen, u32 *out,
+		u32 outlen, u32 dptr, u32 sptr)
+{
+	int i, ret;
+	u32 wbuf[4] = { 0 };
+
+	if (ipcdev.dev == NULL)
+		return -ENODEV;
+	if (inlen > 16)
+		return -EINVAL;
+
+	memcpy(wbuf, in, inlen);
+	mutex_lock(&ipclock);
+	writel(dptr, ipcdev.ipc_base + IPC_DPTR);
+	writel(sptr, ipcdev.ipc_base + IPC_SPTR);
+	/*The input data register is 32bit register and inlen is in Byte*/
+	for (i = 0; i < ((inlen + 3) / 4); i++)
+		ipc_data_writel(wbuf[i], 4 * i);
+	ipc_send_command((inlen << IPC_CMD_SIZE) |
+			(sub << IPC_CMD_SUBCMD) | cmd);
+	ret = intel_pmc_ipc_check_status();
+	if (!ret) {
+		/*out is read from 32bit register and outlen is in 32bit*/
+		for (i = 0; i < outlen; i++)
+			*out++ = ipc_data_readl(4 * i);
+	}
+	mutex_unlock(&ipclock);
+	return ret;
+}
+EXPORT_SYMBOL(intel_pmc_ipc_raw_cmd);
+
+int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
+		u32 *out, u32 outlen)
+{
+	return intel_pmc_ipc_raw_cmd(cmd, sub, in, inlen, out, outlen, 0, 0);
+}
+EXPORT_SYMBOL_GPL(intel_pmc_ipc_command);
+
+static irqreturn_t ioc(int irq, void *dev_id)
+{
+	int status;
+
+	if (ipcdev.irq_mode) {
+		status = ipc_read_status();
+		writel(status | IPC_STATUS_IRQ, ipcdev.ipc_base + IPC_STATUS);
+	}
+	complete(&ipcdev.cmd_complete);
+
+	return IRQ_HANDLED;
+}
+
+static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	int ret;
+	int len;
+	resource_size_t pci_resource;
+
+	ipcdev.dev = &pci_dev_get(pdev)->dev;
+	ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ;
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	ret = pci_request_regions(pdev, "intel_pmc_ipc");
+	if (ret)
+		return ret;
+
+	pci_resource = pci_resource_start(pdev, 0);
+	len = pci_resource_len(pdev, 0);
+	if (!pci_resource || !len) {
+		dev_err(&pdev->dev, "Fail to get resource\n");
+		return -ENOMEM;
+	}
+
+	init_completion(&ipcdev.cmd_complete);
+
+	if (request_irq(pdev->irq, ioc, 0, "intel_pmc_ipc", &ipcdev)) {
+		dev_err(&pdev->dev, "Fail to request irq\n");
+		return -EBUSY;
+	}
+
+	ipcdev.ipc_base = ioremap_nocache(pci_resource, len);
+	if (!ipcdev.ipc_base) {
+		dev_err(&pdev->dev, "Fail to ioremap ipc base\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	return 0;
+err:
+	free_irq(pdev->irq, &ipcdev);
+	return ret;
+}
+
+static void ipc_pci_remove(struct pci_dev *pdev)
+{
+	free_irq(pdev->irq, &ipcdev);
+	pci_release_regions(pdev);
+	pci_dev_put(pdev);
+	iounmap(ipcdev.ipc_base);
+	ipcdev.dev = NULL;
+}
+
+static const struct pci_device_id ipc_pci_ids[] = {
+	{PCI_VDEVICE(INTEL, 0x0a94), 0},
+	{PCI_VDEVICE(INTEL, 0x1a94), 0},
+	{ 0,}
+};
+MODULE_DEVICE_TABLE(pci, ipc_pci_ids);
+
+static struct pci_driver ipc_pci_driver = {
+	.name = "intel_pmc_ipc",
+	.id_table = ipc_pci_ids,
+	.probe = ipc_pci_probe,
+	.remove = ipc_pci_remove,
+};
+
+static ssize_t intel_pmc_ipc_simple_cmd_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	int ret;
+	int cmd;
+	int subcmd;
+
+	ret = sscanf(buf, "%d %d", &cmd, &subcmd);
+	if (ret != 2) {
+		dev_err(dev, "Error args\n");
+		return -EINVAL;
+	}
+
+	ret = intel_pmc_ipc_simple_command(cmd, subcmd);
+	if (ret) {
+		dev_err(dev, "command %d error with %d\n", cmd, ret);
+		return ret;
+	}
+	return  count;
+}
+
+static ssize_t intel_pmc_ipc_northpeak_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	unsigned long val;
+	int subcmd;
+	int ret;
+
+	if (kstrtoul(buf, 0, &val))
+		return -EINVAL;
+
+	if (val)
+		subcmd = 1;
+	else
+		subcmd = 0;
+	ret = intel_pmc_ipc_simple_command(PMC_IPC_NORTHPEAK_CTRL, subcmd);
+	if (ret) {
+		dev_err(dev, "command north %d error with %d\n", subcmd, ret);
+		return ret;
+	}
+	return count;
+}
+
+static DEVICE_ATTR(simplecmd, S_IWUSR | S_IRUSR,
+		NULL,
+		intel_pmc_ipc_simple_cmd_store);
+static DEVICE_ATTR(northpeak, S_IWUSR | S_IRUSR,
+		NULL,
+		intel_pmc_ipc_northpeak_store);
+
+static struct attribute *intel_ipc_attrs[] = {
+	&dev_attr_northpeak.attr,
+	&dev_attr_simplecmd.attr,
+	NULL
+};
+
+static const struct attribute_group intel_ipc_group = {
+	.attrs = intel_ipc_attrs,
+};
+
+#define PUNIT_RESOURCE_INTER		1
+static struct resource punit_res[] = {
+	/*Punit*/
+	{
+		.flags = IORESOURCE_MEM,
+	},
+	{
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+#define TCO_RESOURCE_ACPI_IO		0
+#define TCO_RESOURCE_SMI_EN_IO		1
+#define TCO_RESOURCE_GCR_MEM		2
+static struct resource tco_res[] = {
+	/*ACPI - TCO*/
+	{
+		.flags = IORESOURCE_IO,
+	},
+	/*ACPI - SMI*/
+	{
+		.flags = IORESOURCE_IO,
+	},
+	/*GCS*/
+	{
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+static struct lpc_ich_info tco_info = {
+	.name = "Broxton SoC",
+	.iTCO_version = 3,
+};
+
+static int ipc_create_punit_device(void)
+{
+	int ret;
+	struct resource *res;
+	struct platform_device *pdev;
+
+	pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1);
+	if (!pdev) {
+		dev_err(ipcdev.dev, "Fail to alloc punit platform device\n");
+		return -ENOMEM;
+	}
+
+	pdev->dev.parent = ipcdev.dev;
+
+	res = punit_res;
+	res->start = (resource_size_t)ipcdev.punit_base;
+	res->end = res->start + ipcdev.punit_size - 1;
+
+	res = punit_res + PUNIT_RESOURCE_INTER;
+	res->start = (resource_size_t)ipcdev.punit_base2;
+	res->end = res->start + ipcdev.punit_size2 - 1;
+
+	ret = platform_device_add_resources(pdev,
+				punit_res, ARRAY_SIZE(punit_res));
+	if (ret) {
+		dev_err(ipcdev.dev, "Fail to add platform punit resources\n");
+		goto err;
+	}
+
+	ret = platform_device_add(pdev);
+	if (ret) {
+		dev_err(ipcdev.dev, "Fail to add punit platform devcie\n");
+		goto err;
+	}
+	ipcdev.punit_dev = pdev;
+
+	return 0;
+err:
+	platform_device_put(pdev);
+	return ret;
+}
+
+static int ipc_create_tco_device(void)
+{
+	int ret;
+	struct resource *res;
+	struct platform_device *pdev;
+
+	pdev = platform_device_alloc(TCO_DEVICE_NAME, -1);
+	if (!pdev) {
+		dev_err(ipcdev.dev, "Fail to alloc tco platform device\n");
+		return -ENOMEM;
+	}
+
+	pdev->dev.parent = ipcdev.dev;
+
+	res = tco_res + TCO_RESOURCE_ACPI_IO;
+	res->start = (resource_size_t)ipcdev.acpi_io_base + TCO_BASE_OFFSET;
+	res->end = res->start + TCO_REGS_SIZE - 1;
+
+	res = tco_res + TCO_RESOURCE_SMI_EN_IO;
+	res->start = (resource_size_t)ipcdev.acpi_io_base + SMI_EN_OFFSET;
+	res->end = res->start + SMI_EN_SIZE - 1;
+
+	res = tco_res + TCO_RESOURCE_GCR_MEM;
+	res->start = (resource_size_t)ipcdev.gcr_base;
+	res->end = res->start + ipcdev.gcr_size - 1;
+
+	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
+	if (ret) {
+		dev_err(ipcdev.dev, "Fail to add tco platform resources\n");
+		goto err;
+	}
+
+	ret = platform_device_add_data(pdev, &tco_info,
+					sizeof(struct lpc_ich_info));
+	if (ret) {
+		dev_err(ipcdev.dev, "Fail to add tco platform data\n");
+		goto err;
+	}
+
+	ret = platform_device_add(pdev);
+	if (ret) {
+		dev_err(ipcdev.dev, "Fail to add tco platform devcie\n");
+		goto err;
+	}
+	ipcdev.tco_dev = pdev;
+
+	return 0;
+err:
+	platform_device_put(pdev);
+	return ret;
+}
+
+static int ipc_create_pmc_devices(void)
+{
+	int ret;
+
+	ret = ipc_create_tco_device();
+	if (ret) {
+		dev_err(ipcdev.dev, "Fail to add tco platform device\n");
+		return ret;
+	}
+	ret = ipc_create_punit_device();
+	if (ret) {
+		dev_err(ipcdev.dev, "Fail to add punit platform device\n");
+		platform_device_unregister(ipcdev.tco_dev);
+	}
+	return ret;
+}
+
+static int ipc_plat_get_res(struct platform_device *pdev)
+{
+	struct resource *res;
+	void __iomem *addr;
+	int size;
+
+	res = platform_get_resource(pdev, IORESOURCE_IO,
+				PLAT_RESOURCE_ACPI_IO_INDEX);
+	if (!res) {
+		dev_err(&pdev->dev, "Fail to get io resource\n");
+		return -ENXIO;
+	}
+	size = resource_size(res);
+	ipcdev.acpi_io_base = (void *)res->start;
+	ipcdev.acpi_io_size = size;
+	dev_info(&pdev->dev, "io res: %llx %x\n",
+			res->start, (int)resource_size(res));
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM,
+				PLAT_RESOURCE_PUNIT_DATA_INDEX);
+	if (!res) {
+		dev_err(&pdev->dev, "Fail to get punit resource\n");
+		return -ENXIO;
+	}
+	size = resource_size(res);
+	ipcdev.punit_base = (void *)res->start;
+	ipcdev.punit_size = size;
+	dev_info(&pdev->dev, "punit data res: %llx %x\n",
+			res->start, (int)resource_size(res));
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM,
+				PLAT_RESOURCE_PUNIT_INTER_INDEX);
+	if (!res) {
+		dev_err(&pdev->dev, "Fail to get punit inter resource\n");
+		return -ENXIO;
+	}
+	size = resource_size(res);
+	ipcdev.punit_base2 = (void *)res->start;
+	ipcdev.punit_size2 = size;
+	dev_info(&pdev->dev, "punit interface res: %llx %x\n",
+			res->start, (int)resource_size(res));
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM,
+				PLAT_RESOURCE_IPC_INDEX);
+	if (!res) {
+		dev_err(&pdev->dev, "Fail to get ipc resource\n");
+		return -ENXIO;
+	}
+	size = PLAT_RESOURCE_IPC_SIZE;
+	if (!request_mem_region(res->start, size, pdev->name)) {
+		dev_err(&pdev->dev, "Fail to request ipc resouce\n");
+		return -EBUSY;
+	}
+	addr = ioremap_nocache(res->start, size);
+	if (!addr) {
+		dev_err(&pdev->dev, "I/O memory remapping failed\n");
+		release_mem_region(res->start, size);
+		return -ENOMEM;
+	}
+	ipcdev.ipc_base = addr;
+
+	ipcdev.gcr_base = (void *)(res->start + size);
+	ipcdev.gcr_size = PLAT_RESOURCE_GCR_SIZE;
+	dev_info(&pdev->dev, "ipc res: %llx %x\n",
+			res->start, (int)resource_size(res));
+
+	return 0;
+}
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id ipc_acpi_ids[] = {
+	{ "INT34D2", 0},
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, ipc_acpi_ids);
+#endif
+
+static int ipc_plat_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct resource *res;
+
+	ipcdev.dev = &pdev->dev;
+	ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ;
+	init_completion(&ipcdev.cmd_complete);
+
+	ipcdev.irq = platform_get_irq(pdev, 0);
+	if (ipcdev.irq < 0) {
+		dev_err(&pdev->dev, "Fail to get irq\n");
+		return -EINVAL;
+	}
+
+	ret = ipc_plat_get_res(pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Fail to request resource\n");
+		return ret;
+	}
+
+	ret = ipc_create_pmc_devices();
+	if (ret) {
+		dev_err(&pdev->dev, "Fail to create pmc devices\n");
+		goto err_device;
+	}
+
+	if (request_irq(ipcdev.irq, ioc, 0, "intel_pmc_ipc", &ipcdev)) {
+		dev_err(&pdev->dev, "Fail to request irq\n");
+		ret = -EBUSY;
+		goto err_irq;
+	}
+
+	ret = sysfs_create_group(&pdev->dev.kobj, &intel_ipc_group);
+	if (ret) {
+		dev_err(&pdev->dev, "Fail to create sysfs group %d\n",
+			ret);
+		goto err_sys;
+	}
+
+	return 0;
+err_sys:
+	free_irq(ipcdev.irq, &ipcdev);
+err_irq:
+	platform_device_unregister(ipcdev.tco_dev);
+	platform_device_unregister(ipcdev.punit_dev);
+err_device:
+	iounmap(ipcdev.ipc_base);
+	res = platform_get_resource(pdev, IORESOURCE_MEM,
+				PLAT_RESOURCE_IPC_INDEX);
+	if (res)
+		release_mem_region(res->start, PLAT_RESOURCE_IPC_SIZE);
+	return ret;
+}
+
+static int ipc_plat_remove(struct platform_device *pdev)
+{
+	struct resource *res;
+
+	sysfs_remove_group(&pdev->dev.kobj, &intel_ipc_group);
+	free_irq(ipcdev.irq, &ipcdev);
+	platform_device_unregister(ipcdev.tco_dev);
+	platform_device_unregister(ipcdev.punit_dev);
+	iounmap(ipcdev.ipc_base);
+	res = platform_get_resource(pdev, IORESOURCE_MEM,
+				PLAT_RESOURCE_IPC_INDEX);
+	if (res)
+		release_mem_region(res->start, PLAT_RESOURCE_IPC_SIZE);
+	ipcdev.dev = NULL;
+	return 0;
+}
+
+static struct platform_driver ipc_plat_driver = {
+	.remove = ipc_plat_remove,
+	.probe = ipc_plat_probe,
+	.driver = {
+		.name = "pmc-ipc-plat",
+		.owner = THIS_MODULE,
+		.acpi_match_table = ACPI_PTR(ipc_acpi_ids),
+	},
+};
+
+static int __init intel_pmc_ipc_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&ipc_plat_driver);
+	if (ret) {
+		pr_err("Fail to register PMC ipc platform driver\n");
+		return ret;
+	}
+	ret = pci_register_driver(&ipc_pci_driver);
+	if (ret) {
+		pr_err("Fail to register PMC ipc pci driver\n");
+		platform_driver_unregister(&ipc_plat_driver);
+		return ret;
+	}
+	return ret;
+}
+
+static void __exit intel_pmc_ipc_exit(void)
+{
+	pci_unregister_driver(&ipc_pci_driver);
+	platform_driver_unregister(&ipc_plat_driver);
+}
+
+MODULE_AUTHOR("qipeng.zha@intel.com");
+MODULE_DESCRIPTION("Intel PMC IPC driver");
+MODULE_LICENSE("GPL V2");
+
+fs_initcall(intel_pmc_ipc_init);
+module_exit(intel_pmc_ipc_exit);
-- 
1.8.3.2

RE: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[Zha, Qipeng]

Hi Alan

Fei, owner of intel_scu_ipc.c, suggested separate these two, since they are different hw,
Not prefer to manage them in one driver.




Best wishes
Qipeng

-----Original Message-----
From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-x86-owner@vger.kernel.org] On Behalf Of One Thousand Gnomes
Sent: Tuesday, April 21, 2015 1:26 AM
To: Zha, Qipeng
Cc: platform-driver-x86@vger.kernel.org; dvhart@infradead.org; Yang, Fei; Zhong, Huiquan; Chen, Jason CJ; Zheng, Qi
Subject: Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

On Tue, 21 Apr 2015 07:04:29 +0800
"qipeng.zha" <qipeng.zha@intel.com> wrote:

> From: "qipeng.zha" <qipeng.zha@intel.com>
> 
> This driver provides support for PMC control on Broxton platforms, PMC 
> is ARC processor which defined some IPC commands for communication 
> with other entities running in IA

This appears to mostly be a clone of 

drivers/platform/x86/intel_scu_ipc.c

so is there any reason why it has to be another rewrite of the ancient driver that was already cleaned up and upstreamed once.

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

Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[One Thousand Gnomes]

On Tue, 21 Apr 2015 00:59:35 +0000
"Zha, Qipeng" <qipeng.zha@intel.com> wrote:

> Hi Alan
> 
> Fei, owner of intel_scu_ipc.c, suggested separate these two, since they are different hw,
> Not prefer to manage them in one driver.

They don't look that different to me except that it lacks some of the
cleanups as well as the naming to make it more Linxulike
(intel_scu_ipc_iowrite16 etc) that were done to the scu_ipc code.

What we really want to avoid is code elsewhere that ends up doing

	if (broxton)
		intel_pmc_ipc_foo(x)
	else
		intel_scu_ipc_foo(x);

I guess to judge that really means seeing who uses these routines. At the
moment the user you've posted takes the existing not very pretty API and
wraps it up again in another one with no obvious reason for all the
layering, but it does at least then provide the upper layer as a sort of
abstraction.

For the code itself

- there are quite a few things that could be const
- you keep the register bases in both the platform resources of the
  subdevices and directly, yet don't seem to use them directly ?

Alan

RE: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[Zha, Qipeng]

Hi Alan

a) Register access is one of ipc commands, and we do provide more generic APIs based on regmap subsystem in other patch,
    with that, there will be not necessary to check platform(Broxton) for register access.
b) we separate scu and pmc ipc in two drivers, is because they are many difference in hw itself,  it composite several sub devices
in hardware layer, and supported ipc commands are different,    I expected user driver will not check platform(Broxston...) to do
ipc with scu or pmc when no common commands.

  

Best wishes
Qipeng

-----Original Message-----
From: One Thousand Gnomes [mailto:gnomes@lxorguk.ukuu.org.uk] 
Sent: Tuesday, April 21, 2015 7:59 PM
To: Zha, Qipeng
Cc: platform-driver-x86@vger.kernel.org; dvhart@infradead.org; Yang, Fei; Zhong, Huiquan; Chen, Jason CJ; Zheng, Qi
Subject: Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

On Tue, 21 Apr 2015 00:59:35 +0000
"Zha, Qipeng" <qipeng.zha@intel.com> wrote:

> Hi Alan
> 
> Fei, owner of intel_scu_ipc.c, suggested separate these two, since 
> they are different hw, Not prefer to manage them in one driver.

They don't look that different to me except that it lacks some of the cleanups as well as the naming to make it more Linxulike
(intel_scu_ipc_iowrite16 etc) that were done to the scu_ipc code.

What we really want to avoid is code elsewhere that ends up doing

	if (broxton)
		intel_pmc_ipc_foo(x)
	else
		intel_scu_ipc_foo(x);

I guess to judge that really means seeing who uses these routines. At the moment the user you've posted takes the existing not very pretty API and wraps it up again in another one with no obvious reason for all the layering, but it does at least then provide the upper layer as a sort of abstraction.

For the code itself

- there are quite a few things that could be const
- you keep the register bases in both the platform resources of the
  subdevices and directly, yet don't seem to use them directly ?

Alan

RE: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[Yang, Fei]

>> Hi Alan
>> 
>> Fei, owner of intel_scu_ipc.c, suggested separate these two, since 
>> they are different hw, Not prefer to manage them in one driver.
>
> They don't look that different to me except that it lacks some of the cleanups as well as the naming to make it more Linxulike
> (intel_scu_ipc_iowrite16 etc) that were done to the scu_ipc code.

I proposed a different name because the so called SCU is phasing out, and I don't think anyone is
actively maintaining the scu_ipc driver. On the other hand the PMC design is expected to have a much
longer life time, so I think it's better to have fresh start, not confused with the old SCU IPC.

-Fei

RE: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[Zha, Qipeng]

Hi Alan

Thanks.
The register  access ipc command is only used to r/w registers on PMIC which is controlled by PMC, Based on current pmic core driver design and Fei's suggestion, we implement regmap interface to Access such registers based on PMC ipc command.
Then pmic device driver(or other driver need to access pmic registers) can use generic regmap_read/_write Directly.




Best wishes
Qipeng

-----Original Message-----
From: One Thousand Gnomes [mailto:gnomes@lxorguk.ukuu.org.uk]
Sent: Wednesday, April 22, 2015 6:37 PM
To: Zha, Qipeng
Cc: "platform-driver-x86@vger.kernel.org\" <platform-driver-x86@vger.kernel.org>, " dvhart@infradead.org " <dvhart@infradead.org>, " Yang; Fei " <fei.yang@intel.com>, " Zhong@lxorguk.ukuu.org.uk; Huiquan " <huiquan.zhong@intel.com>, " Chen@lxorguk.ukuu.org.uk; Jason.CJ " <jason.cj.chen@intel.com>, " Zheng@lxorguk.ukuu.org.uk; Qi " <qi.zheng@intel.com>"@lxorguk.ukuu.org.uk
Subject: Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

On Wed, 22 Apr 2015 10:32:44 +0000
"Zha, Qipeng" <qipeng.zha@intel.com> wrote:

> Hi Alan
> 
> a) Register access is one of ipc commands, and we do provide more generic APIs based on regmap subsystem in other patch,
>     with that, there will be not necessary to check platform(Broxton) for register access.

Ok that is good... but can I ask why you have two layers of IPC commands and then regmap - not just a single file that solely provides the regmap interface ?

> b) we separate scu and pmc ipc in two drivers, is because they are many difference in hw itself,  it composite several sub devices
> in hardware layer, and supported ipc commands are different,    I expected user driver will not check platform(Broxston...) to do
> ipc with scu or pmc when no common commands.

Ok

Alan

RE: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[Yang, Fei]

Qipeng,
I think what Alan suggested is to implement regmap without PMC IPC driver, in other words, combine the regmap and pmc_ipc into one driver. That seems to make a lot of sense to me, we really don't need another set of driver APIs to be exposed when there is already regmap in place.

-Fei

-----Original Message-----
From: Zha, Qipeng 
Sent: Wednesday, April 22, 2015 6:07 PM
To: platform-driver-x86@vger.kernel.org; dvhart@infradead.org; One Thousand Gnomes
Cc: Zheng, Qi; Zhong, Huiquan; Chen, Jason CJ; Yang, Fei
Subject: RE: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

Hi Alan

Thanks.
The register  access ipc command is only used to r/w registers on PMIC which is controlled by PMC, Based on current pmic core driver design and Fei's suggestion, we implement regmap interface to Access such registers based on PMC ipc command.
Then pmic device driver(or other driver need to access pmic registers) can use generic regmap_read/_write Directly.




Best wishes
Qipeng

-----Original Message-----
From: One Thousand Gnomes [mailto:gnomes@lxorguk.ukuu.org.uk]
Sent: Wednesday, April 22, 2015 6:37 PM
To: Zha, Qipeng
Cc: "platform-driver-x86@vger.kernel.org\" <platform-driver-x86@vger.kernel.org>, " dvhart@infradead.org " <dvhart@infradead.org>, " Yang; Fei " <fei.yang@intel.com>, " Zhong@lxorguk.ukuu.org.uk; Huiquan " <huiquan.zhong@intel.com>, " Chen@lxorguk.ukuu.org.uk; Jason.CJ " <jason.cj.chen@intel.com>, " Zheng@lxorguk.ukuu.org.uk; Qi " <qi.zheng@intel.com>"@lxorguk.ukuu.org.uk
Subject: Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

On Wed, 22 Apr 2015 10:32:44 +0000
"Zha, Qipeng" <qipeng.zha@intel.com> wrote:

> Hi Alan
> 
> a) Register access is one of ipc commands, and we do provide more generic APIs based on regmap subsystem in other patch,
>     with that, there will be not necessary to check platform(Broxton) for register access.

Ok that is good... but can I ask why you have two layers of IPC commands and then regmap - not just a single file that solely provides the regmap interface ?

> b) we separate scu and pmc ipc in two drivers, is because they are many difference in hw itself,  it composite several sub devices
> in hardware layer, and supported ipc commands are different,    I expected user driver will not check platform(Broxston...) to do
> ipc with scu or pmc when no common commands.

Ok

Alan

RE: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[Zha, Qipeng]

Fei

Registers access is one of ipc functions and other ipc commands can't be wrapped as register access,
So I don't think we can only keep regmap apis.




Best wishes
Qipeng

-----Original Message-----
From: Yang, Fei 
Sent: Thursday, April 23, 2015 12:30 PM
To: Zha, Qipeng; platform-driver-x86@vger.kernel.org; dvhart@infradead.org; One Thousand Gnomes
Cc: Zheng, Qi; Zhong, Huiquan; Chen, Jason CJ
Subject: RE: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

Qipeng,
I think what Alan suggested is to implement regmap without PMC IPC driver, in other words, combine the regmap and pmc_ipc into one driver. That seems to make a lot of sense to me, we really don't need another set of driver APIs to be exposed when there is already regmap in place.

-Fei

-----Original Message-----
From: Zha, Qipeng 
Sent: Wednesday, April 22, 2015 6:07 PM
To: platform-driver-x86@vger.kernel.org; dvhart@infradead.org; One Thousand Gnomes
Cc: Zheng, Qi; Zhong, Huiquan; Chen, Jason CJ; Yang, Fei
Subject: RE: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

Hi Alan

Thanks.
The register  access ipc command is only used to r/w registers on PMIC which is controlled by PMC, Based on current pmic core driver design and Fei's suggestion, we implement regmap interface to Access such registers based on PMC ipc command.
Then pmic device driver(or other driver need to access pmic registers) can use generic regmap_read/_write Directly.




Best wishes
Qipeng

-----Original Message-----
From: One Thousand Gnomes [mailto:gnomes@lxorguk.ukuu.org.uk]
Sent: Wednesday, April 22, 2015 6:37 PM
To: Zha, Qipeng
Cc: "platform-driver-x86@vger.kernel.org\" <platform-driver-x86@vger.kernel.org>, " dvhart@infradead.org " <dvhart@infradead.org>, " Yang; Fei " <fei.yang@intel.com>, " Zhong@lxorguk.ukuu.org.uk; Huiquan " <huiquan.zhong@intel.com>, " Chen@lxorguk.ukuu.org.uk; Jason.CJ " <jason.cj.chen@intel.com>, " Zheng@lxorguk.ukuu.org.uk; Qi " <qi.zheng@intel.com>"@lxorguk.ukuu.org.uk
Subject: Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

On Wed, 22 Apr 2015 10:32:44 +0000
"Zha, Qipeng" <qipeng.zha@intel.com> wrote:

> Hi Alan
> 
> a) Register access is one of ipc commands, and we do provide more generic APIs based on regmap subsystem in other patch,
>     with that, there will be not necessary to check platform(Broxton) for register access.

Ok that is good... but can I ask why you have two layers of IPC commands and then regmap - not just a single file that solely provides the regmap interface ?

> b) we separate scu and pmc ipc in two drivers, is because they are many difference in hw itself,  it composite several sub devices
> in hardware layer, and supported ipc commands are different,    I expected user driver will not check platform(Broxston...) to do
> ipc with scu or pmc when no common commands.

Ok

Alan

Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[Darren Hart]

On Tue, Apr 21, 2015 at 07:04:29AM +0800, qipeng.zha wrote:
> From: "qipeng.zha" <qipeng.zha@intel.com>
> 
> This driver provides support for PMC control on Broxton platforms,
> PMC is ARC processor which defined some IPC commands for communication
> with other entities running in IA
> 

+Jacob Pan

Jacob, any thoughts on this or Alan's specific concerns about combining it with
the previous gen driver?


> Signed-off-by: qipeng.zha <qipeng.zha@intel.com>
> ---
>  arch/x86/include/asm/intel_pmc_ipc.h |  79 ++++
>  drivers/platform/x86/Kconfig         |   7 +
>  drivers/platform/x86/Makefile        |   1 +
>  drivers/platform/x86/intel_pmc_ipc.c | 740 +++++++++++++++++++++++++++++++++++
>  4 files changed, 827 insertions(+)
>  create mode 100644 arch/x86/include/asm/intel_pmc_ipc.h
>  create mode 100644 drivers/platform/x86/intel_pmc_ipc.c
> 
> diff --git a/arch/x86/include/asm/intel_pmc_ipc.h b/arch/x86/include/asm/intel_pmc_ipc.h
> new file mode 100644
> index 0000000..690b55f
> --- /dev/null
> +++ b/arch/x86/include/asm/intel_pmc_ipc.h
> @@ -0,0 +1,79 @@
> +#ifndef _ASM_X86_INTEL_PMC_IPC_H_
> +#define  _ASM_X86_INTEL_PMC_IPC_H_
> +
> +/*commands*/
> +#define PMC_IPC_PMIC_ACCESS		0xFF
> +#define		PMC_IPC_PMIC_ACCESS_READ	0x0
> +#define		PMC_IPC_PMIC_ACCESS_WRITE	0x1
> +#define PMC_IPC_USB_PWR_CTRL		0xF0
> +#define PMC_IPC_PMIC_BLACKLIST_SEL	0xEF
> +#define PMC_IPC_PHY_CONFIG		0xEE
> +#define PMC_IPC_NORTHPEAK_CTRL		0xED
> +#define PMC_IPC_PM_DEBUG		0xEC
> +#define PMC_IPC_PMC_TELEMTRY		0xEB
> +#define PMC_IPC_PMC_FW_MSG_CTRL		0xEA
> +
> +/*IPC return code*/
> +#define IPC_ERR_NONE			0
> +#define IPC_ERR_CMD_NOT_SUPPORTED	1
> +#define IPC_ERR_CMD_NOT_SERVICED	2
> +#define IPC_ERR_UNABLE_TO_SERVICE	3
> +#define IPC_ERR_CMD_INVALID		4
> +#define IPC_ERR_CMD_FAILED		5
> +#define IPC_ERR_EMSECURITY		6
> +#define IPC_ERR_UNSIGNEDKERNEL		7
> +
> +#ifdef CONFIG_INTEL_PMC_IPC
> +/*
> +* intel_pmc_ipc_simple_command
> +* @cmd: command
> +* @sub: sub type
> +*/
> +int intel_pmc_ipc_simple_command(int cmd, int sub);
> +/*
> +* intel_pmc_ipc_raw_cmd
> +* @cmd: command
> +* @sub: sub type
> +* @in: input data
> +* @inlen: input length in bytes
> +* @out: output data
> +* @outlen: output length in dwords
> +* @sptr: data writing to SPTR register
> +* @dptr: data writing to DPTR register
> +*/
> +int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
> +		u32 *out, u32 outlen, u32 dptr, u32 sptr);
> +/*
> +* intel_pmc_ipc_raw_cmd
> +* @cmd: command
> +* @sub: sub type
> +* @in: input data
> +* @inlen: input length in bytes
> +* @out: output data
> +* @outlen: output length in dwords
> +*/
> +int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
> +		u32 *out, u32 outlen);
> +
> +#else
> +
> +static inline int intel_pmc_ipc_simple_command(int cmd, int sub)
> +{
> +	return -EINVAL;
> +}
> +
> +static inline int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
> +		u32 *out, u32 outlen, u32 dptr, u32 sptr)
> +{
> +	return -EINVAL;
> +}
> +
> +static inline int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
> +		u32 *out, u32 outlen)
> +{
> +	return -EINVAL;
> +}
> +
> +#endif /*CONFIG_INTEL_PMC_IPC*/
> +
> +#endif
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9752761..4eb15eb 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -884,4 +884,11 @@ config PVPANIC
>  	  a paravirtualized device provided by QEMU; it lets a virtual machine
>  	  (guest) communicate panic events to the host.
>  
> +config INTEL_PMC_IPC
> +	tristate "Intel PMC IPC Driver"
> +	---help---
> +	This driver provides support for PMC control on some Intel platforms,
> +	PMC is ARC processor which defined some IPC commands for communication
> +	with other entities which running in IA.
> +
>  endif # X86_PLATFORM_DEVICES
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index f82232b..1051372 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -58,3 +58,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)	+= intel-smartconnect.o
>  
>  obj-$(CONFIG_PVPANIC)           += pvpanic.o
>  obj-$(CONFIG_ALIENWARE_WMI)	+= alienware-wmi.o
> +obj-$(CONFIG_INTEL_PMC_IPC)	+= intel_pmc_ipc.o
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> new file mode 100644
> index 0000000..8610508
> --- /dev/null
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -0,0 +1,740 @@
> +/*
> + * intel_pmc_ipc.c: Driver for the Intel PMC IPC mechanism
> + *
> + * (C) Copyright 2014 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + *
> + * PMC running in ARC processor communicates with other entity running in IA
> + * core through IPC mechanism which in turn messaging between IA core ad PMC.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/pm.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/pm_qos.h>
> +#include <linux/kernel.h>
> +#include <linux/bitops.h>
> +#include <linux/sched.h>
> +#include <linux/atomic.h>
> +#include <linux/notifier.h>
> +#include <linux/suspend.h>
> +#include <linux/acpi.h>
> +#include <asm/intel_pmc_ipc.h>
> +#include <linux/mfd/lpc_ich.h>
> +
> +/*
> +* IPC registers
> +*/
> +#define IPC_CMD			0x0
> +#define		IPC_CMD_MSI		0x100
> +#define		IPC_CMD_SIZE		16
> +#define		IPC_CMD_SUBCMD		12
> +#define IPC_STATUS		0x04
> +#define		IPC_STATUS_IRQ		0x4
> +#define		IPC_STATUS_ERR		0x2
> +#define		IPC_STATUS_BUSY		0x1
> +#define IPC_SPTR		0x08
> +#define IPC_DPTR		0x0C
> +#define IPC_WRITE_BUFFER	0x80
> +#define IPC_READ_BUFFER		0x90
> +
> +#define IPC_LOOP_CNT		3000000
> +#define IPC_MAX_SEC		3
> +
> +/*How host to know command status from PMC*/
> +#define IPC_TRIGGER_MODE_IRQ		true
> +
> +/*exported resources from IFWI*/
> +#define PLAT_RESOURCE_IPC_INDEX		0
> +#define PLAT_RESOURCE_IPC_SIZE		0x1000
> +#define PLAT_RESOURCE_GCR_SIZE		0x1000
> +#define PLAT_RESOURCE_PUNIT_DATA_INDEX	1
> +#define PLAT_RESOURCE_PUNIT_INTER_INDEX	2
> +#define PLAT_RESOURCE_ACPI_IO_INDEX	0
> +
> +/*BIOS don't create ACPI device for each PMC function,
> +* but export multi resoures on one ACPI device(IPC) for
> +* multi functions, so this driver is responsible to create
> +* platform device and to export resource for those functions
> +*/
> +#define TCO_DEVICE_NAME			"iTCO_wdt"
> +#define SMI_EN_OFFSET			0x30
> +#define SMI_EN_SIZE			4
> +#define TCO_BASE_OFFSET			0x60
> +#define TCO_REGS_SIZE			16
> +#define PUNIT_DEVICE_NAME		"intel_punit_ipc"
> +
> +static const int iTCO_version = 3;
> +
> +static struct intel_pmc_ipc_dev {
> +	struct device *dev;
> +	void __iomem *ipc_base;
> +	bool irq_mode;
> +	int irq;
> +	int cmd;
> +	struct completion cmd_complete;
> +
> +	/*Below bars in PMC share same ACPI device with IPC*/
> +	void *acpi_io_base;
> +	int acpi_io_size;
> +	struct platform_device *tco_dev;
> +
> +	/*gcr*/
> +	void __iomem *gcr_base;
> +	int gcr_size;
> +
> +	/*punit*/
> +	void *punit_base;
> +	int punit_size;
> +	void *punit_base2;
> +	int punit_size2;
> +	struct platform_device *punit_dev;
> +} ipcdev;
> +
> +static char *ipc_err_sources[] = {
> +	[IPC_ERR_NONE] =
> +		"no error",
> +	[IPC_ERR_CMD_NOT_SUPPORTED] =
> +		"command not supported",
> +	[IPC_ERR_CMD_NOT_SERVICED] =
> +		"command not serviced",
> +	[IPC_ERR_UNABLE_TO_SERVICE] =
> +		"unable to service",
> +	[IPC_ERR_CMD_INVALID] =
> +		"command invalid",
> +	[IPC_ERR_CMD_FAILED] =
> +		"command failed",
> +	[IPC_ERR_EMSECURITY] =
> +		"Invalid Battery",
> +	[IPC_ERR_UNSIGNEDKERNEL] =
> +		"Unsigned kernel",
> +};
> +
> +/*prevent multiple call to PMC*/
> +static DEFINE_MUTEX(ipclock);
> +
> +/*
> +* Command Register
> +* |rsvd(8) | size(8) | command id(4) | rsvd(3) | msi(1) | command(8)|
> +*/
> +static inline void ipc_send_command(u32 cmd)
> +{
> +	ipcdev.cmd = cmd;
> +	if (ipcdev.irq_mode) {
> +		reinit_completion(&ipcdev.cmd_complete);
> +		cmd |= IPC_CMD_MSI;
> +	}
> +	writel(cmd, ipcdev.ipc_base + IPC_CMD);
> +}
> +
> +/*
> +* Status Register
> +* |rsvd(8)|error code(8)|initiator id(8)|cmd id(4)|irq(2)|error(1)|busy(1)|
> +*/
> +static inline u32 ipc_read_status(void)
> +{
> +	return readl(ipcdev.ipc_base + IPC_STATUS);
> +}
> +
> +/*
> +* IPC Write Buffer (Write Only):
> +* 16-byte buffer for sending data associated with IPC command
> +* Size of the data is specified in the IPC_CMD[size]
> +*/
> +static inline void ipc_data_writel(u32 data, u32 offset)
> +{
> +	writel(data, ipcdev.ipc_base + IPC_WRITE_BUFFER + offset);
> +}
> +
> +static inline u8 ipc_data_readb(u32 offset)
> +{
> +	return readb(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
> +}
> +
> +static inline u32 ipc_data_readl(u32 offset)
> +{
> +	return readl(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
> +}
> +
> +int intel_pmc_ipc_check_status(void)
> +{
> +	int i;
> +	int ret = 0;
> +	int status;
> +	int loop_count = IPC_LOOP_CNT;
> +
> +	if (ipcdev.irq_mode) {
> +		if (0 == wait_for_completion_timeout(
> +				&ipcdev.cmd_complete, IPC_MAX_SEC * HZ))
> +			ret = -ETIMEDOUT;
> +	} else {
> +		while ((ipc_read_status() & IPC_STATUS_BUSY) && --loop_count)
> +			udelay(1);
> +		if (loop_count == 0)
> +			ret = -ETIMEDOUT;
> +	}
> +
> +	status = ipc_read_status();
> +	if (ret == -ETIMEDOUT) {
> +		dev_err(ipcdev.dev,
> +			"IPC timed out, TS=0x%x, CMD=0x%x\n",
> +			status, ipcdev.cmd);
> +		return ret;
> +	}
> +
> +	if (status & IPC_STATUS_ERR) {
> +		ret = -EIO;
> +		i = (status >> IPC_CMD_SIZE) & 0xFF;
> +		if (i < ARRAY_SIZE(ipc_err_sources))
> +			dev_err(ipcdev.dev,
> +				"IPC failed: %s, STS=0x%x, CMD=0x%x\n",
> +				ipc_err_sources[i], status, ipcdev.cmd);
> +		else
> +			dev_err(ipcdev.dev,
> +				"IPC failed: unknown, STS=0x%x, CMD=0x%x\n",
> +				 status, ipcdev.cmd);
> +		if ((i == IPC_ERR_UNSIGNEDKERNEL) || (i == IPC_ERR_EMSECURITY))
> +			ret = -EACCES;
> +	}
> +
> +	return ret;
> +}
> +
> +int intel_pmc_ipc_simple_command(int cmd, int sub)
> +{
> +	int ret;
> +
> +	if (ipcdev.dev == NULL)
> +		return -ENODEV;
> +
> +	mutex_lock(&ipclock);
> +	ipc_send_command(sub << IPC_CMD_SUBCMD | cmd);
> +	ret = intel_pmc_ipc_check_status();
> +	mutex_unlock(&ipclock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(intel_pmc_ipc_simple_command);
> +
> +int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen, u32 *out,
> +		u32 outlen, u32 dptr, u32 sptr)
> +{
> +	int i, ret;
> +	u32 wbuf[4] = { 0 };
> +
> +	if (ipcdev.dev == NULL)
> +		return -ENODEV;
> +	if (inlen > 16)
> +		return -EINVAL;
> +
> +	memcpy(wbuf, in, inlen);
> +	mutex_lock(&ipclock);
> +	writel(dptr, ipcdev.ipc_base + IPC_DPTR);
> +	writel(sptr, ipcdev.ipc_base + IPC_SPTR);
> +	/*The input data register is 32bit register and inlen is in Byte*/
> +	for (i = 0; i < ((inlen + 3) / 4); i++)
> +		ipc_data_writel(wbuf[i], 4 * i);
> +	ipc_send_command((inlen << IPC_CMD_SIZE) |
> +			(sub << IPC_CMD_SUBCMD) | cmd);
> +	ret = intel_pmc_ipc_check_status();
> +	if (!ret) {
> +		/*out is read from 32bit register and outlen is in 32bit*/
> +		for (i = 0; i < outlen; i++)
> +			*out++ = ipc_data_readl(4 * i);
> +	}
> +	mutex_unlock(&ipclock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(intel_pmc_ipc_raw_cmd);
> +
> +int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
> +		u32 *out, u32 outlen)
> +{
> +	return intel_pmc_ipc_raw_cmd(cmd, sub, in, inlen, out, outlen, 0, 0);
> +}
> +EXPORT_SYMBOL_GPL(intel_pmc_ipc_command);
> +
> +static irqreturn_t ioc(int irq, void *dev_id)
> +{
> +	int status;
> +
> +	if (ipcdev.irq_mode) {
> +		status = ipc_read_status();
> +		writel(status | IPC_STATUS_IRQ, ipcdev.ipc_base + IPC_STATUS);
> +	}
> +	complete(&ipcdev.cmd_complete);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int ipc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	int ret;
> +	int len;
> +	resource_size_t pci_resource;
> +
> +	ipcdev.dev = &pci_dev_get(pdev)->dev;
> +	ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = pci_request_regions(pdev, "intel_pmc_ipc");
> +	if (ret)
> +		return ret;
> +
> +	pci_resource = pci_resource_start(pdev, 0);
> +	len = pci_resource_len(pdev, 0);
> +	if (!pci_resource || !len) {
> +		dev_err(&pdev->dev, "Fail to get resource\n");
> +		return -ENOMEM;
> +	}
> +
> +	init_completion(&ipcdev.cmd_complete);
> +
> +	if (request_irq(pdev->irq, ioc, 0, "intel_pmc_ipc", &ipcdev)) {
> +		dev_err(&pdev->dev, "Fail to request irq\n");
> +		return -EBUSY;
> +	}
> +
> +	ipcdev.ipc_base = ioremap_nocache(pci_resource, len);
> +	if (!ipcdev.ipc_base) {
> +		dev_err(&pdev->dev, "Fail to ioremap ipc base\n");
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	return 0;
> +err:
> +	free_irq(pdev->irq, &ipcdev);
> +	return ret;
> +}
> +
> +static void ipc_pci_remove(struct pci_dev *pdev)
> +{
> +	free_irq(pdev->irq, &ipcdev);
> +	pci_release_regions(pdev);
> +	pci_dev_put(pdev);
> +	iounmap(ipcdev.ipc_base);
> +	ipcdev.dev = NULL;
> +}
> +
> +static const struct pci_device_id ipc_pci_ids[] = {
> +	{PCI_VDEVICE(INTEL, 0x0a94), 0},
> +	{PCI_VDEVICE(INTEL, 0x1a94), 0},
> +	{ 0,}
> +};
> +MODULE_DEVICE_TABLE(pci, ipc_pci_ids);
> +
> +static struct pci_driver ipc_pci_driver = {
> +	.name = "intel_pmc_ipc",
> +	.id_table = ipc_pci_ids,
> +	.probe = ipc_pci_probe,
> +	.remove = ipc_pci_remove,
> +};
> +
> +static ssize_t intel_pmc_ipc_simple_cmd_store(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	int ret;
> +	int cmd;
> +	int subcmd;
> +
> +	ret = sscanf(buf, "%d %d", &cmd, &subcmd);
> +	if (ret != 2) {
> +		dev_err(dev, "Error args\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = intel_pmc_ipc_simple_command(cmd, subcmd);
> +	if (ret) {
> +		dev_err(dev, "command %d error with %d\n", cmd, ret);
> +		return ret;
> +	}
> +	return  count;
> +}
> +
> +static ssize_t intel_pmc_ipc_northpeak_store(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	unsigned long val;
> +	int subcmd;
> +	int ret;
> +
> +	if (kstrtoul(buf, 0, &val))
> +		return -EINVAL;
> +
> +	if (val)
> +		subcmd = 1;
> +	else
> +		subcmd = 0;
> +	ret = intel_pmc_ipc_simple_command(PMC_IPC_NORTHPEAK_CTRL, subcmd);
> +	if (ret) {
> +		dev_err(dev, "command north %d error with %d\n", subcmd, ret);
> +		return ret;
> +	}
> +	return count;
> +}
> +
> +static DEVICE_ATTR(simplecmd, S_IWUSR | S_IRUSR,
> +		NULL,
> +		intel_pmc_ipc_simple_cmd_store);
> +static DEVICE_ATTR(northpeak, S_IWUSR | S_IRUSR,
> +		NULL,
> +		intel_pmc_ipc_northpeak_store);
> +
> +static struct attribute *intel_ipc_attrs[] = {
> +	&dev_attr_northpeak.attr,
> +	&dev_attr_simplecmd.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group intel_ipc_group = {
> +	.attrs = intel_ipc_attrs,
> +};
> +
> +#define PUNIT_RESOURCE_INTER		1
> +static struct resource punit_res[] = {
> +	/*Punit*/
> +	{
> +		.flags = IORESOURCE_MEM,
> +	},
> +	{
> +		.flags = IORESOURCE_MEM,
> +	},
> +};
> +
> +#define TCO_RESOURCE_ACPI_IO		0
> +#define TCO_RESOURCE_SMI_EN_IO		1
> +#define TCO_RESOURCE_GCR_MEM		2
> +static struct resource tco_res[] = {
> +	/*ACPI - TCO*/
> +	{
> +		.flags = IORESOURCE_IO,
> +	},
> +	/*ACPI - SMI*/
> +	{
> +		.flags = IORESOURCE_IO,
> +	},
> +	/*GCS*/
> +	{
> +		.flags = IORESOURCE_MEM,
> +	},
> +};
> +
> +static struct lpc_ich_info tco_info = {
> +	.name = "Broxton SoC",
> +	.iTCO_version = 3,
> +};
> +
> +static int ipc_create_punit_device(void)
> +{
> +	int ret;
> +	struct resource *res;
> +	struct platform_device *pdev;
> +
> +	pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1);
> +	if (!pdev) {
> +		dev_err(ipcdev.dev, "Fail to alloc punit platform device\n");
> +		return -ENOMEM;
> +	}
> +
> +	pdev->dev.parent = ipcdev.dev;
> +
> +	res = punit_res;
> +	res->start = (resource_size_t)ipcdev.punit_base;
> +	res->end = res->start + ipcdev.punit_size - 1;
> +
> +	res = punit_res + PUNIT_RESOURCE_INTER;
> +	res->start = (resource_size_t)ipcdev.punit_base2;
> +	res->end = res->start + ipcdev.punit_size2 - 1;
> +
> +	ret = platform_device_add_resources(pdev,
> +				punit_res, ARRAY_SIZE(punit_res));
> +	if (ret) {
> +		dev_err(ipcdev.dev, "Fail to add platform punit resources\n");
> +		goto err;
> +	}
> +
> +	ret = platform_device_add(pdev);
> +	if (ret) {
> +		dev_err(ipcdev.dev, "Fail to add punit platform devcie\n");
> +		goto err;
> +	}
> +	ipcdev.punit_dev = pdev;
> +
> +	return 0;
> +err:
> +	platform_device_put(pdev);
> +	return ret;
> +}
> +
> +static int ipc_create_tco_device(void)
> +{
> +	int ret;
> +	struct resource *res;
> +	struct platform_device *pdev;
> +
> +	pdev = platform_device_alloc(TCO_DEVICE_NAME, -1);
> +	if (!pdev) {
> +		dev_err(ipcdev.dev, "Fail to alloc tco platform device\n");
> +		return -ENOMEM;
> +	}
> +
> +	pdev->dev.parent = ipcdev.dev;
> +
> +	res = tco_res + TCO_RESOURCE_ACPI_IO;
> +	res->start = (resource_size_t)ipcdev.acpi_io_base + TCO_BASE_OFFSET;
> +	res->end = res->start + TCO_REGS_SIZE - 1;
> +
> +	res = tco_res + TCO_RESOURCE_SMI_EN_IO;
> +	res->start = (resource_size_t)ipcdev.acpi_io_base + SMI_EN_OFFSET;
> +	res->end = res->start + SMI_EN_SIZE - 1;
> +
> +	res = tco_res + TCO_RESOURCE_GCR_MEM;
> +	res->start = (resource_size_t)ipcdev.gcr_base;
> +	res->end = res->start + ipcdev.gcr_size - 1;
> +
> +	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
> +	if (ret) {
> +		dev_err(ipcdev.dev, "Fail to add tco platform resources\n");
> +		goto err;
> +	}
> +
> +	ret = platform_device_add_data(pdev, &tco_info,
> +					sizeof(struct lpc_ich_info));
> +	if (ret) {
> +		dev_err(ipcdev.dev, "Fail to add tco platform data\n");
> +		goto err;
> +	}
> +
> +	ret = platform_device_add(pdev);
> +	if (ret) {
> +		dev_err(ipcdev.dev, "Fail to add tco platform devcie\n");
> +		goto err;
> +	}
> +	ipcdev.tco_dev = pdev;
> +
> +	return 0;
> +err:
> +	platform_device_put(pdev);
> +	return ret;
> +}
> +
> +static int ipc_create_pmc_devices(void)
> +{
> +	int ret;
> +
> +	ret = ipc_create_tco_device();
> +	if (ret) {
> +		dev_err(ipcdev.dev, "Fail to add tco platform device\n");
> +		return ret;
> +	}
> +	ret = ipc_create_punit_device();
> +	if (ret) {
> +		dev_err(ipcdev.dev, "Fail to add punit platform device\n");
> +		platform_device_unregister(ipcdev.tco_dev);
> +	}
> +	return ret;
> +}
> +
> +static int ipc_plat_get_res(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	void __iomem *addr;
> +	int size;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_IO,
> +				PLAT_RESOURCE_ACPI_IO_INDEX);
> +	if (!res) {
> +		dev_err(&pdev->dev, "Fail to get io resource\n");
> +		return -ENXIO;
> +	}
> +	size = resource_size(res);
> +	ipcdev.acpi_io_base = (void *)res->start;
> +	ipcdev.acpi_io_size = size;
> +	dev_info(&pdev->dev, "io res: %llx %x\n",
> +			res->start, (int)resource_size(res));
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> +				PLAT_RESOURCE_PUNIT_DATA_INDEX);
> +	if (!res) {
> +		dev_err(&pdev->dev, "Fail to get punit resource\n");
> +		return -ENXIO;
> +	}
> +	size = resource_size(res);
> +	ipcdev.punit_base = (void *)res->start;
> +	ipcdev.punit_size = size;
> +	dev_info(&pdev->dev, "punit data res: %llx %x\n",
> +			res->start, (int)resource_size(res));
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> +				PLAT_RESOURCE_PUNIT_INTER_INDEX);
> +	if (!res) {
> +		dev_err(&pdev->dev, "Fail to get punit inter resource\n");
> +		return -ENXIO;
> +	}
> +	size = resource_size(res);
> +	ipcdev.punit_base2 = (void *)res->start;
> +	ipcdev.punit_size2 = size;
> +	dev_info(&pdev->dev, "punit interface res: %llx %x\n",
> +			res->start, (int)resource_size(res));
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> +				PLAT_RESOURCE_IPC_INDEX);
> +	if (!res) {
> +		dev_err(&pdev->dev, "Fail to get ipc resource\n");
> +		return -ENXIO;
> +	}
> +	size = PLAT_RESOURCE_IPC_SIZE;
> +	if (!request_mem_region(res->start, size, pdev->name)) {
> +		dev_err(&pdev->dev, "Fail to request ipc resouce\n");
> +		return -EBUSY;
> +	}
> +	addr = ioremap_nocache(res->start, size);
> +	if (!addr) {
> +		dev_err(&pdev->dev, "I/O memory remapping failed\n");
> +		release_mem_region(res->start, size);
> +		return -ENOMEM;
> +	}
> +	ipcdev.ipc_base = addr;
> +
> +	ipcdev.gcr_base = (void *)(res->start + size);
> +	ipcdev.gcr_size = PLAT_RESOURCE_GCR_SIZE;
> +	dev_info(&pdev->dev, "ipc res: %llx %x\n",
> +			res->start, (int)resource_size(res));
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id ipc_acpi_ids[] = {
> +	{ "INT34D2", 0},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, ipc_acpi_ids);
> +#endif
> +
> +static int ipc_plat_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct resource *res;
> +
> +	ipcdev.dev = &pdev->dev;
> +	ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ;
> +	init_completion(&ipcdev.cmd_complete);
> +
> +	ipcdev.irq = platform_get_irq(pdev, 0);
> +	if (ipcdev.irq < 0) {
> +		dev_err(&pdev->dev, "Fail to get irq\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = ipc_plat_get_res(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Fail to request resource\n");
> +		return ret;
> +	}
> +
> +	ret = ipc_create_pmc_devices();
> +	if (ret) {
> +		dev_err(&pdev->dev, "Fail to create pmc devices\n");
> +		goto err_device;
> +	}
> +
> +	if (request_irq(ipcdev.irq, ioc, 0, "intel_pmc_ipc", &ipcdev)) {
> +		dev_err(&pdev->dev, "Fail to request irq\n");
> +		ret = -EBUSY;
> +		goto err_irq;
> +	}
> +
> +	ret = sysfs_create_group(&pdev->dev.kobj, &intel_ipc_group);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Fail to create sysfs group %d\n",
> +			ret);
> +		goto err_sys;
> +	}
> +
> +	return 0;
> +err_sys:
> +	free_irq(ipcdev.irq, &ipcdev);
> +err_irq:
> +	platform_device_unregister(ipcdev.tco_dev);
> +	platform_device_unregister(ipcdev.punit_dev);
> +err_device:
> +	iounmap(ipcdev.ipc_base);
> +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> +				PLAT_RESOURCE_IPC_INDEX);
> +	if (res)
> +		release_mem_region(res->start, PLAT_RESOURCE_IPC_SIZE);
> +	return ret;
> +}
> +
> +static int ipc_plat_remove(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +
> +	sysfs_remove_group(&pdev->dev.kobj, &intel_ipc_group);
> +	free_irq(ipcdev.irq, &ipcdev);
> +	platform_device_unregister(ipcdev.tco_dev);
> +	platform_device_unregister(ipcdev.punit_dev);
> +	iounmap(ipcdev.ipc_base);
> +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> +				PLAT_RESOURCE_IPC_INDEX);
> +	if (res)
> +		release_mem_region(res->start, PLAT_RESOURCE_IPC_SIZE);
> +	ipcdev.dev = NULL;
> +	return 0;
> +}
> +
> +static struct platform_driver ipc_plat_driver = {
> +	.remove = ipc_plat_remove,
> +	.probe = ipc_plat_probe,
> +	.driver = {
> +		.name = "pmc-ipc-plat",
> +		.owner = THIS_MODULE,
> +		.acpi_match_table = ACPI_PTR(ipc_acpi_ids),
> +	},
> +};
> +
> +static int __init intel_pmc_ipc_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&ipc_plat_driver);
> +	if (ret) {
> +		pr_err("Fail to register PMC ipc platform driver\n");
> +		return ret;
> +	}
> +	ret = pci_register_driver(&ipc_pci_driver);
> +	if (ret) {
> +		pr_err("Fail to register PMC ipc pci driver\n");
> +		platform_driver_unregister(&ipc_plat_driver);
> +		return ret;
> +	}
> +	return ret;
> +}
> +
> +static void __exit intel_pmc_ipc_exit(void)
> +{
> +	pci_unregister_driver(&ipc_pci_driver);
> +	platform_driver_unregister(&ipc_plat_driver);
> +}
> +
> +MODULE_AUTHOR("qipeng.zha@intel.com");
> +MODULE_DESCRIPTION("Intel PMC IPC driver");
> +MODULE_LICENSE("GPL V2");
> +
> +fs_initcall(intel_pmc_ipc_init);
> +module_exit(intel_pmc_ipc_exit);
> -- 
> 1.8.3.2
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[Jacob Pan]

On Fri, 24 Apr 2015 15:28:01 -0700
Darren Hart <dvhart@infradead.org> wrote:

> On Tue, Apr 21, 2015 at 07:04:29AM +0800, qipeng.zha wrote:
> > From: "qipeng.zha" <qipeng.zha@intel.com>
> > 
> > This driver provides support for PMC control on Broxton platforms,
> > PMC is ARC processor which defined some IPC commands for
> > communication with other entities running in IA
> > 
> 
> +Jacob Pan
> 
> Jacob, any thoughts on this or Alan's specific concerns about
> combining it with the previous gen driver?
> 
while i think combining IPC is a good idea in general, the question is
where do we do that.

For this Broxton PMC IPC, I think the plan is to follow up with adding a
regmap bus driver to decouple end users such as PMIC function drivers
from the IPC or I2C access methods. Then, the same PMIC driver can work
on platform with i2c or IPC. The goal is also to eliminate direct
driver to driver API calls.

Since regmap is already an abstraction, I think it might be better to
combine IPC at regmap layer, i.e. per IPC regmap bus type.

For the older IPC drivers such as SCU IPC, there is no regmap layer so
converting all of them to regmap would be lots of work, API changes
plus platform data enumeration etc.

Thanks,

Jacob
> 
> > Signed-off-by: qipeng.zha <qipeng.zha@intel.com>
> > ---
> >  arch/x86/include/asm/intel_pmc_ipc.h |  79 ++++
> >  drivers/platform/x86/Kconfig         |   7 +
> >  drivers/platform/x86/Makefile        |   1 +
> >  drivers/platform/x86/intel_pmc_ipc.c | 740
> > +++++++++++++++++++++++++++++++++++ 4 files changed, 827
> > insertions(+) create mode 100644
> > arch/x86/include/asm/intel_pmc_ipc.h create mode 100644
> > drivers/platform/x86/intel_pmc_ipc.c
> > 
> > diff --git a/arch/x86/include/asm/intel_pmc_ipc.h
> > b/arch/x86/include/asm/intel_pmc_ipc.h new file mode 100644
> > index 0000000..690b55f
> > --- /dev/null
> > +++ b/arch/x86/include/asm/intel_pmc_ipc.h
> > @@ -0,0 +1,79 @@
> > +#ifndef _ASM_X86_INTEL_PMC_IPC_H_
> > +#define  _ASM_X86_INTEL_PMC_IPC_H_
> > +
> > +/*commands*/
> > +#define PMC_IPC_PMIC_ACCESS		0xFF
> > +#define		PMC_IPC_PMIC_ACCESS_READ	0x0
> > +#define		PMC_IPC_PMIC_ACCESS_WRITE	0x1
> > +#define PMC_IPC_USB_PWR_CTRL		0xF0
> > +#define PMC_IPC_PMIC_BLACKLIST_SEL	0xEF
> > +#define PMC_IPC_PHY_CONFIG		0xEE
> > +#define PMC_IPC_NORTHPEAK_CTRL		0xED
> > +#define PMC_IPC_PM_DEBUG		0xEC
> > +#define PMC_IPC_PMC_TELEMTRY		0xEB
> > +#define PMC_IPC_PMC_FW_MSG_CTRL		0xEA
> > +
> > +/*IPC return code*/
> > +#define IPC_ERR_NONE			0
> > +#define IPC_ERR_CMD_NOT_SUPPORTED	1
> > +#define IPC_ERR_CMD_NOT_SERVICED	2
> > +#define IPC_ERR_UNABLE_TO_SERVICE	3
> > +#define IPC_ERR_CMD_INVALID		4
> > +#define IPC_ERR_CMD_FAILED		5
> > +#define IPC_ERR_EMSECURITY		6
> > +#define IPC_ERR_UNSIGNEDKERNEL		7
> > +
> > +#ifdef CONFIG_INTEL_PMC_IPC
> > +/*
> > +* intel_pmc_ipc_simple_command
> > +* @cmd: command
> > +* @sub: sub type
> > +*/
> > +int intel_pmc_ipc_simple_command(int cmd, int sub);
> > +/*
> > +* intel_pmc_ipc_raw_cmd
> > +* @cmd: command
> > +* @sub: sub type
> > +* @in: input data
> > +* @inlen: input length in bytes
> > +* @out: output data
> > +* @outlen: output length in dwords
> > +* @sptr: data writing to SPTR register
> > +* @dptr: data writing to DPTR register
> > +*/
> > +int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
> > +		u32 *out, u32 outlen, u32 dptr, u32 sptr);
> > +/*
> > +* intel_pmc_ipc_raw_cmd
> > +* @cmd: command
> > +* @sub: sub type
> > +* @in: input data
> > +* @inlen: input length in bytes
> > +* @out: output data
> > +* @outlen: output length in dwords
> > +*/
> > +int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
> > +		u32 *out, u32 outlen);
> > +
> > +#else
> > +
> > +static inline int intel_pmc_ipc_simple_command(int cmd, int sub)
> > +{
> > +	return -EINVAL;
> > +}
> > +
> > +static inline int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in,
> > u32 inlen,
> > +		u32 *out, u32 outlen, u32 dptr, u32 sptr)
> > +{
> > +	return -EINVAL;
> > +}
> > +
> > +static inline int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in,
> > u32 inlen,
> > +		u32 *out, u32 outlen)
> > +{
> > +	return -EINVAL;
> > +}
> > +
> > +#endif /*CONFIG_INTEL_PMC_IPC*/
> > +
> > +#endif
> > diff --git a/drivers/platform/x86/Kconfig
> > b/drivers/platform/x86/Kconfig index 9752761..4eb15eb 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -884,4 +884,11 @@ config PVPANIC
> >  	  a paravirtualized device provided by QEMU; it lets a
> > virtual machine (guest) communicate panic events to the host.
> >  
> > +config INTEL_PMC_IPC
> > +	tristate "Intel PMC IPC Driver"
> > +	---help---
> > +	This driver provides support for PMC control on some Intel
> > platforms,
> > +	PMC is ARC processor which defined some IPC commands for
> > communication
> > +	with other entities which running in IA.
> > +
> >  endif # X86_PLATFORM_DEVICES
> > diff --git a/drivers/platform/x86/Makefile
> > b/drivers/platform/x86/Makefile index f82232b..1051372 100644
> > --- a/drivers/platform/x86/Makefile
> > +++ b/drivers/platform/x86/Makefile
> > @@ -58,3 +58,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)	+=
> > intel-smartconnect.o 
> >  obj-$(CONFIG_PVPANIC)           += pvpanic.o
> >  obj-$(CONFIG_ALIENWARE_WMI)	+= alienware-wmi.o
> > +obj-$(CONFIG_INTEL_PMC_IPC)	+= intel_pmc_ipc.o
> > diff --git a/drivers/platform/x86/intel_pmc_ipc.c
> > b/drivers/platform/x86/intel_pmc_ipc.c new file mode 100644
> > index 0000000..8610508
> > --- /dev/null
> > +++ b/drivers/platform/x86/intel_pmc_ipc.c
> > @@ -0,0 +1,740 @@
> > +/*
> > + * intel_pmc_ipc.c: Driver for the Intel PMC IPC mechanism
> > + *
> > + * (C) Copyright 2014 Intel Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; version 2
> > + * of the License.
> > + *
> > + * PMC running in ARC processor communicates with other entity
> > running in IA
> > + * core through IPC mechanism which in turn messaging between IA
> > core ad PMC.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/delay.h>
> > +#include <linux/errno.h>
> > +#include <linux/init.h>
> > +#include <linux/device.h>
> > +#include <linux/pm.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/pm_qos.h>
> > +#include <linux/kernel.h>
> > +#include <linux/bitops.h>
> > +#include <linux/sched.h>
> > +#include <linux/atomic.h>
> > +#include <linux/notifier.h>
> > +#include <linux/suspend.h>
> > +#include <linux/acpi.h>
> > +#include <asm/intel_pmc_ipc.h>
> > +#include <linux/mfd/lpc_ich.h>
> > +
> > +/*
> > +* IPC registers
> > +*/
> > +#define IPC_CMD			0x0
> > +#define		IPC_CMD_MSI		0x100
> > +#define		IPC_CMD_SIZE		16
> > +#define		IPC_CMD_SUBCMD		12
> > +#define IPC_STATUS		0x04
> > +#define		IPC_STATUS_IRQ		0x4
> > +#define		IPC_STATUS_ERR		0x2
> > +#define		IPC_STATUS_BUSY		0x1
> > +#define IPC_SPTR		0x08
> > +#define IPC_DPTR		0x0C
> > +#define IPC_WRITE_BUFFER	0x80
> > +#define IPC_READ_BUFFER		0x90
> > +
> > +#define IPC_LOOP_CNT		3000000
> > +#define IPC_MAX_SEC		3
> > +
> > +/*How host to know command status from PMC*/
> > +#define IPC_TRIGGER_MODE_IRQ		true
> > +
> > +/*exported resources from IFWI*/
> > +#define PLAT_RESOURCE_IPC_INDEX		0
> > +#define PLAT_RESOURCE_IPC_SIZE		0x1000
> > +#define PLAT_RESOURCE_GCR_SIZE		0x1000
> > +#define PLAT_RESOURCE_PUNIT_DATA_INDEX	1
> > +#define PLAT_RESOURCE_PUNIT_INTER_INDEX	2
> > +#define PLAT_RESOURCE_ACPI_IO_INDEX	0
> > +
> > +/*BIOS don't create ACPI device for each PMC function,
> > +* but export multi resoures on one ACPI device(IPC) for
> > +* multi functions, so this driver is responsible to create
> > +* platform device and to export resource for those functions
> > +*/
> > +#define TCO_DEVICE_NAME			"iTCO_wdt"
> > +#define SMI_EN_OFFSET			0x30
> > +#define SMI_EN_SIZE			4
> > +#define TCO_BASE_OFFSET			0x60
> > +#define TCO_REGS_SIZE			16
> > +#define PUNIT_DEVICE_NAME		"intel_punit_ipc"
> > +
> > +static const int iTCO_version = 3;
> > +
> > +static struct intel_pmc_ipc_dev {
> > +	struct device *dev;
> > +	void __iomem *ipc_base;
> > +	bool irq_mode;
> > +	int irq;
> > +	int cmd;
> > +	struct completion cmd_complete;
> > +
> > +	/*Below bars in PMC share same ACPI device with IPC*/
> > +	void *acpi_io_base;
> > +	int acpi_io_size;
> > +	struct platform_device *tco_dev;
> > +
> > +	/*gcr*/
> > +	void __iomem *gcr_base;
> > +	int gcr_size;
> > +
> > +	/*punit*/
> > +	void *punit_base;
> > +	int punit_size;
> > +	void *punit_base2;
> > +	int punit_size2;
> > +	struct platform_device *punit_dev;
> > +} ipcdev;
> > +
> > +static char *ipc_err_sources[] = {
> > +	[IPC_ERR_NONE] =
> > +		"no error",
> > +	[IPC_ERR_CMD_NOT_SUPPORTED] =
> > +		"command not supported",
> > +	[IPC_ERR_CMD_NOT_SERVICED] =
> > +		"command not serviced",
> > +	[IPC_ERR_UNABLE_TO_SERVICE] =
> > +		"unable to service",
> > +	[IPC_ERR_CMD_INVALID] =
> > +		"command invalid",
> > +	[IPC_ERR_CMD_FAILED] =
> > +		"command failed",
> > +	[IPC_ERR_EMSECURITY] =
> > +		"Invalid Battery",
> > +	[IPC_ERR_UNSIGNEDKERNEL] =
> > +		"Unsigned kernel",
> > +};
> > +
> > +/*prevent multiple call to PMC*/
> > +static DEFINE_MUTEX(ipclock);
> > +
> > +/*
> > +* Command Register
> > +* |rsvd(8) | size(8) | command id(4) | rsvd(3) | msi(1) |
> > command(8)| +*/
> > +static inline void ipc_send_command(u32 cmd)
> > +{
> > +	ipcdev.cmd = cmd;
> > +	if (ipcdev.irq_mode) {
> > +		reinit_completion(&ipcdev.cmd_complete);
> > +		cmd |= IPC_CMD_MSI;
> > +	}
> > +	writel(cmd, ipcdev.ipc_base + IPC_CMD);
> > +}
> > +
> > +/*
> > +* Status Register
> > +* |rsvd(8)|error code(8)|initiator id(8)|cmd
> > id(4)|irq(2)|error(1)|busy(1)| +*/
> > +static inline u32 ipc_read_status(void)
> > +{
> > +	return readl(ipcdev.ipc_base + IPC_STATUS);
> > +}
> > +
> > +/*
> > +* IPC Write Buffer (Write Only):
> > +* 16-byte buffer for sending data associated with IPC command
> > +* Size of the data is specified in the IPC_CMD[size]
> > +*/
> > +static inline void ipc_data_writel(u32 data, u32 offset)
> > +{
> > +	writel(data, ipcdev.ipc_base + IPC_WRITE_BUFFER + offset);
> > +}
> > +
> > +static inline u8 ipc_data_readb(u32 offset)
> > +{
> > +	return readb(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
> > +}
> > +
> > +static inline u32 ipc_data_readl(u32 offset)
> > +{
> > +	return readl(ipcdev.ipc_base + IPC_READ_BUFFER + offset);
> > +}
> > +
> > +int intel_pmc_ipc_check_status(void)
> > +{
> > +	int i;
> > +	int ret = 0;
> > +	int status;
> > +	int loop_count = IPC_LOOP_CNT;
> > +
> > +	if (ipcdev.irq_mode) {
> > +		if (0 == wait_for_completion_timeout(
> > +				&ipcdev.cmd_complete, IPC_MAX_SEC
> > * HZ))
> > +			ret = -ETIMEDOUT;
> > +	} else {
> > +		while ((ipc_read_status() & IPC_STATUS_BUSY) &&
> > --loop_count)
> > +			udelay(1);
> > +		if (loop_count == 0)
> > +			ret = -ETIMEDOUT;
> > +	}
> > +
> > +	status = ipc_read_status();
> > +	if (ret == -ETIMEDOUT) {
> > +		dev_err(ipcdev.dev,
> > +			"IPC timed out, TS=0x%x, CMD=0x%x\n",
> > +			status, ipcdev.cmd);
> > +		return ret;
> > +	}
> > +
> > +	if (status & IPC_STATUS_ERR) {
> > +		ret = -EIO;
> > +		i = (status >> IPC_CMD_SIZE) & 0xFF;
> > +		if (i < ARRAY_SIZE(ipc_err_sources))
> > +			dev_err(ipcdev.dev,
> > +				"IPC failed: %s, STS=0x%x,
> > CMD=0x%x\n",
> > +				ipc_err_sources[i], status,
> > ipcdev.cmd);
> > +		else
> > +			dev_err(ipcdev.dev,
> > +				"IPC failed: unknown, STS=0x%x,
> > CMD=0x%x\n",
> > +				 status, ipcdev.cmd);
> > +		if ((i == IPC_ERR_UNSIGNEDKERNEL) || (i ==
> > IPC_ERR_EMSECURITY))
> > +			ret = -EACCES;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +int intel_pmc_ipc_simple_command(int cmd, int sub)
> > +{
> > +	int ret;
> > +
> > +	if (ipcdev.dev == NULL)
> > +		return -ENODEV;
> > +
> > +	mutex_lock(&ipclock);
> > +	ipc_send_command(sub << IPC_CMD_SUBCMD | cmd);
> > +	ret = intel_pmc_ipc_check_status();
> > +	mutex_unlock(&ipclock);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(intel_pmc_ipc_simple_command);
> > +
> > +int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen, u32
> > *out,
> > +		u32 outlen, u32 dptr, u32 sptr)
> > +{
> > +	int i, ret;
> > +	u32 wbuf[4] = { 0 };
> > +
> > +	if (ipcdev.dev == NULL)
> > +		return -ENODEV;
> > +	if (inlen > 16)
> > +		return -EINVAL;
> > +
> > +	memcpy(wbuf, in, inlen);
> > +	mutex_lock(&ipclock);
> > +	writel(dptr, ipcdev.ipc_base + IPC_DPTR);
> > +	writel(sptr, ipcdev.ipc_base + IPC_SPTR);
> > +	/*The input data register is 32bit register and inlen is
> > in Byte*/
> > +	for (i = 0; i < ((inlen + 3) / 4); i++)
> > +		ipc_data_writel(wbuf[i], 4 * i);
> > +	ipc_send_command((inlen << IPC_CMD_SIZE) |
> > +			(sub << IPC_CMD_SUBCMD) | cmd);
> > +	ret = intel_pmc_ipc_check_status();
> > +	if (!ret) {
> > +		/*out is read from 32bit register and outlen is in
> > 32bit*/
> > +		for (i = 0; i < outlen; i++)
> > +			*out++ = ipc_data_readl(4 * i);
> > +	}
> > +	mutex_unlock(&ipclock);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(intel_pmc_ipc_raw_cmd);
> > +
> > +int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
> > +		u32 *out, u32 outlen)
> > +{
> > +	return intel_pmc_ipc_raw_cmd(cmd, sub, in, inlen, out,
> > outlen, 0, 0); +}
> > +EXPORT_SYMBOL_GPL(intel_pmc_ipc_command);
> > +
> > +static irqreturn_t ioc(int irq, void *dev_id)
> > +{
> > +	int status;
> > +
> > +	if (ipcdev.irq_mode) {
> > +		status = ipc_read_status();
> > +		writel(status | IPC_STATUS_IRQ, ipcdev.ipc_base +
> > IPC_STATUS);
> > +	}
> > +	complete(&ipcdev.cmd_complete);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int ipc_pci_probe(struct pci_dev *pdev, const struct
> > pci_device_id *id) +{
> > +	int ret;
> > +	int len;
> > +	resource_size_t pci_resource;
> > +
> > +	ipcdev.dev = &pci_dev_get(pdev)->dev;
> > +	ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ;
> > +
> > +	ret = pci_enable_device(pdev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = pci_request_regions(pdev, "intel_pmc_ipc");
> > +	if (ret)
> > +		return ret;
> > +
> > +	pci_resource = pci_resource_start(pdev, 0);
> > +	len = pci_resource_len(pdev, 0);
> > +	if (!pci_resource || !len) {
> > +		dev_err(&pdev->dev, "Fail to get resource\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	init_completion(&ipcdev.cmd_complete);
> > +
> > +	if (request_irq(pdev->irq, ioc, 0, "intel_pmc_ipc",
> > &ipcdev)) {
> > +		dev_err(&pdev->dev, "Fail to request irq\n");
> > +		return -EBUSY;
> > +	}
> > +
> > +	ipcdev.ipc_base = ioremap_nocache(pci_resource, len);
> > +	if (!ipcdev.ipc_base) {
> > +		dev_err(&pdev->dev, "Fail to ioremap ipc base\n");
> > +		ret = -ENOMEM;
> > +		goto err;
> > +	}
> > +
> > +	return 0;
> > +err:
> > +	free_irq(pdev->irq, &ipcdev);
> > +	return ret;
> > +}
> > +
> > +static void ipc_pci_remove(struct pci_dev *pdev)
> > +{
> > +	free_irq(pdev->irq, &ipcdev);
> > +	pci_release_regions(pdev);
> > +	pci_dev_put(pdev);
> > +	iounmap(ipcdev.ipc_base);
> > +	ipcdev.dev = NULL;
> > +}
> > +
> > +static const struct pci_device_id ipc_pci_ids[] = {
> > +	{PCI_VDEVICE(INTEL, 0x0a94), 0},
> > +	{PCI_VDEVICE(INTEL, 0x1a94), 0},
> > +	{ 0,}
> > +};
> > +MODULE_DEVICE_TABLE(pci, ipc_pci_ids);
> > +
> > +static struct pci_driver ipc_pci_driver = {
> > +	.name = "intel_pmc_ipc",
> > +	.id_table = ipc_pci_ids,
> > +	.probe = ipc_pci_probe,
> > +	.remove = ipc_pci_remove,
> > +};
> > +
> > +static ssize_t intel_pmc_ipc_simple_cmd_store(struct device *dev,
> > +	struct device_attribute *attr, const char *buf, size_t
> > count) +{
> > +	int ret;
> > +	int cmd;
> > +	int subcmd;
> > +
> > +	ret = sscanf(buf, "%d %d", &cmd, &subcmd);
> > +	if (ret != 2) {
> > +		dev_err(dev, "Error args\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = intel_pmc_ipc_simple_command(cmd, subcmd);
> > +	if (ret) {
> > +		dev_err(dev, "command %d error with %d\n", cmd,
> > ret);
> > +		return ret;
> > +	}
> > +	return  count;
> > +}
> > +
> > +static ssize_t intel_pmc_ipc_northpeak_store(struct device *dev,
> > +	struct device_attribute *attr, const char *buf, size_t
> > count) +{
> > +	unsigned long val;
> > +	int subcmd;
> > +	int ret;
> > +
> > +	if (kstrtoul(buf, 0, &val))
> > +		return -EINVAL;
> > +
> > +	if (val)
> > +		subcmd = 1;
> > +	else
> > +		subcmd = 0;
> > +	ret = intel_pmc_ipc_simple_command(PMC_IPC_NORTHPEAK_CTRL,
> > subcmd);
> > +	if (ret) {
> > +		dev_err(dev, "command north %d error with %d\n",
> > subcmd, ret);
> > +		return ret;
> > +	}
> > +	return count;
> > +}
> > +
> > +static DEVICE_ATTR(simplecmd, S_IWUSR | S_IRUSR,
> > +		NULL,
> > +		intel_pmc_ipc_simple_cmd_store);
> > +static DEVICE_ATTR(northpeak, S_IWUSR | S_IRUSR,
> > +		NULL,
> > +		intel_pmc_ipc_northpeak_store);
> > +
> > +static struct attribute *intel_ipc_attrs[] = {
> > +	&dev_attr_northpeak.attr,
> > +	&dev_attr_simplecmd.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group intel_ipc_group = {
> > +	.attrs = intel_ipc_attrs,
> > +};
> > +
> > +#define PUNIT_RESOURCE_INTER		1
> > +static struct resource punit_res[] = {
> > +	/*Punit*/
> > +	{
> > +		.flags = IORESOURCE_MEM,
> > +	},
> > +	{
> > +		.flags = IORESOURCE_MEM,
> > +	},
> > +};
> > +
> > +#define TCO_RESOURCE_ACPI_IO		0
> > +#define TCO_RESOURCE_SMI_EN_IO		1
> > +#define TCO_RESOURCE_GCR_MEM		2
> > +static struct resource tco_res[] = {
> > +	/*ACPI - TCO*/
> > +	{
> > +		.flags = IORESOURCE_IO,
> > +	},
> > +	/*ACPI - SMI*/
> > +	{
> > +		.flags = IORESOURCE_IO,
> > +	},
> > +	/*GCS*/
> > +	{
> > +		.flags = IORESOURCE_MEM,
> > +	},
> > +};
> > +
> > +static struct lpc_ich_info tco_info = {
> > +	.name = "Broxton SoC",
> > +	.iTCO_version = 3,
> > +};
> > +
> > +static int ipc_create_punit_device(void)
> > +{
> > +	int ret;
> > +	struct resource *res;
> > +	struct platform_device *pdev;
> > +
> > +	pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1);
> > +	if (!pdev) {
> > +		dev_err(ipcdev.dev, "Fail to alloc punit platform
> > device\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	pdev->dev.parent = ipcdev.dev;
> > +
> > +	res = punit_res;
> > +	res->start = (resource_size_t)ipcdev.punit_base;
> > +	res->end = res->start + ipcdev.punit_size - 1;
> > +
> > +	res = punit_res + PUNIT_RESOURCE_INTER;
> > +	res->start = (resource_size_t)ipcdev.punit_base2;
> > +	res->end = res->start + ipcdev.punit_size2 - 1;
> > +
> > +	ret = platform_device_add_resources(pdev,
> > +				punit_res, ARRAY_SIZE(punit_res));
> > +	if (ret) {
> > +		dev_err(ipcdev.dev, "Fail to add platform punit
> > resources\n");
> > +		goto err;
> > +	}
> > +
> > +	ret = platform_device_add(pdev);
> > +	if (ret) {
> > +		dev_err(ipcdev.dev, "Fail to add punit platform
> > devcie\n");
> > +		goto err;
> > +	}
> > +	ipcdev.punit_dev = pdev;
> > +
> > +	return 0;
> > +err:
> > +	platform_device_put(pdev);
> > +	return ret;
> > +}
> > +
> > +static int ipc_create_tco_device(void)
> > +{
> > +	int ret;
> > +	struct resource *res;
> > +	struct platform_device *pdev;
> > +
> > +	pdev = platform_device_alloc(TCO_DEVICE_NAME, -1);
> > +	if (!pdev) {
> > +		dev_err(ipcdev.dev, "Fail to alloc tco platform
> > device\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	pdev->dev.parent = ipcdev.dev;
> > +
> > +	res = tco_res + TCO_RESOURCE_ACPI_IO;
> > +	res->start = (resource_size_t)ipcdev.acpi_io_base +
> > TCO_BASE_OFFSET;
> > +	res->end = res->start + TCO_REGS_SIZE - 1;
> > +
> > +	res = tco_res + TCO_RESOURCE_SMI_EN_IO;
> > +	res->start = (resource_size_t)ipcdev.acpi_io_base +
> > SMI_EN_OFFSET;
> > +	res->end = res->start + SMI_EN_SIZE - 1;
> > +
> > +	res = tco_res + TCO_RESOURCE_GCR_MEM;
> > +	res->start = (resource_size_t)ipcdev.gcr_base;
> > +	res->end = res->start + ipcdev.gcr_size - 1;
> > +
> > +	ret = platform_device_add_resources(pdev, tco_res,
> > ARRAY_SIZE(tco_res));
> > +	if (ret) {
> > +		dev_err(ipcdev.dev, "Fail to add tco platform
> > resources\n");
> > +		goto err;
> > +	}
> > +
> > +	ret = platform_device_add_data(pdev, &tco_info,
> > +					sizeof(struct
> > lpc_ich_info));
> > +	if (ret) {
> > +		dev_err(ipcdev.dev, "Fail to add tco platform
> > data\n");
> > +		goto err;
> > +	}
> > +
> > +	ret = platform_device_add(pdev);
> > +	if (ret) {
> > +		dev_err(ipcdev.dev, "Fail to add tco platform
> > devcie\n");
> > +		goto err;
> > +	}
> > +	ipcdev.tco_dev = pdev;
> > +
> > +	return 0;
> > +err:
> > +	platform_device_put(pdev);
> > +	return ret;
> > +}
> > +
> > +static int ipc_create_pmc_devices(void)
> > +{
> > +	int ret;
> > +
> > +	ret = ipc_create_tco_device();
> > +	if (ret) {
> > +		dev_err(ipcdev.dev, "Fail to add tco platform
> > device\n");
> > +		return ret;
> > +	}
> > +	ret = ipc_create_punit_device();
> > +	if (ret) {
> > +		dev_err(ipcdev.dev, "Fail to add punit platform
> > device\n");
> > +		platform_device_unregister(ipcdev.tco_dev);
> > +	}
> > +	return ret;
> > +}
> > +
> > +static int ipc_plat_get_res(struct platform_device *pdev)
> > +{
> > +	struct resource *res;
> > +	void __iomem *addr;
> > +	int size;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_IO,
> > +				PLAT_RESOURCE_ACPI_IO_INDEX);
> > +	if (!res) {
> > +		dev_err(&pdev->dev, "Fail to get io resource\n");
> > +		return -ENXIO;
> > +	}
> > +	size = resource_size(res);
> > +	ipcdev.acpi_io_base = (void *)res->start;
> > +	ipcdev.acpi_io_size = size;
> > +	dev_info(&pdev->dev, "io res: %llx %x\n",
> > +			res->start, (int)resource_size(res));
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> > +				PLAT_RESOURCE_PUNIT_DATA_INDEX);
> > +	if (!res) {
> > +		dev_err(&pdev->dev, "Fail to get punit
> > resource\n");
> > +		return -ENXIO;
> > +	}
> > +	size = resource_size(res);
> > +	ipcdev.punit_base = (void *)res->start;
> > +	ipcdev.punit_size = size;
> > +	dev_info(&pdev->dev, "punit data res: %llx %x\n",
> > +			res->start, (int)resource_size(res));
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> > +				PLAT_RESOURCE_PUNIT_INTER_INDEX);
> > +	if (!res) {
> > +		dev_err(&pdev->dev, "Fail to get punit inter
> > resource\n");
> > +		return -ENXIO;
> > +	}
> > +	size = resource_size(res);
> > +	ipcdev.punit_base2 = (void *)res->start;
> > +	ipcdev.punit_size2 = size;
> > +	dev_info(&pdev->dev, "punit interface res: %llx %x\n",
> > +			res->start, (int)resource_size(res));
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> > +				PLAT_RESOURCE_IPC_INDEX);
> > +	if (!res) {
> > +		dev_err(&pdev->dev, "Fail to get ipc resource\n");
> > +		return -ENXIO;
> > +	}
> > +	size = PLAT_RESOURCE_IPC_SIZE;
> > +	if (!request_mem_region(res->start, size, pdev->name)) {
> > +		dev_err(&pdev->dev, "Fail to request ipc
> > resouce\n");
> > +		return -EBUSY;
> > +	}
> > +	addr = ioremap_nocache(res->start, size);
> > +	if (!addr) {
> > +		dev_err(&pdev->dev, "I/O memory remapping
> > failed\n");
> > +		release_mem_region(res->start, size);
> > +		return -ENOMEM;
> > +	}
> > +	ipcdev.ipc_base = addr;
> > +
> > +	ipcdev.gcr_base = (void *)(res->start + size);
> > +	ipcdev.gcr_size = PLAT_RESOURCE_GCR_SIZE;
> > +	dev_info(&pdev->dev, "ipc res: %llx %x\n",
> > +			res->start, (int)resource_size(res));
> > +
> > +	return 0;
> > +}
> > +
> > +#ifdef CONFIG_ACPI
> > +static const struct acpi_device_id ipc_acpi_ids[] = {
> > +	{ "INT34D2", 0},
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(acpi, ipc_acpi_ids);
> > +#endif
> > +
> > +static int ipc_plat_probe(struct platform_device *pdev)
> > +{
> > +	int ret;
> > +	struct resource *res;
> > +
> > +	ipcdev.dev = &pdev->dev;
> > +	ipcdev.irq_mode = IPC_TRIGGER_MODE_IRQ;
> > +	init_completion(&ipcdev.cmd_complete);
> > +
> > +	ipcdev.irq = platform_get_irq(pdev, 0);
> > +	if (ipcdev.irq < 0) {
> > +		dev_err(&pdev->dev, "Fail to get irq\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = ipc_plat_get_res(pdev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Fail to request resource\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = ipc_create_pmc_devices();
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Fail to create pmc
> > devices\n");
> > +		goto err_device;
> > +	}
> > +
> > +	if (request_irq(ipcdev.irq, ioc, 0, "intel_pmc_ipc",
> > &ipcdev)) {
> > +		dev_err(&pdev->dev, "Fail to request irq\n");
> > +		ret = -EBUSY;
> > +		goto err_irq;
> > +	}
> > +
> > +	ret = sysfs_create_group(&pdev->dev.kobj,
> > &intel_ipc_group);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Fail to create sysfs group
> > %d\n",
> > +			ret);
> > +		goto err_sys;
> > +	}
> > +
> > +	return 0;
> > +err_sys:
> > +	free_irq(ipcdev.irq, &ipcdev);
> > +err_irq:
> > +	platform_device_unregister(ipcdev.tco_dev);
> > +	platform_device_unregister(ipcdev.punit_dev);
> > +err_device:
> > +	iounmap(ipcdev.ipc_base);
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> > +				PLAT_RESOURCE_IPC_INDEX);
> > +	if (res)
> > +		release_mem_region(res->start,
> > PLAT_RESOURCE_IPC_SIZE);
> > +	return ret;
> > +}
> > +
> > +static int ipc_plat_remove(struct platform_device *pdev)
> > +{
> > +	struct resource *res;
> > +
> > +	sysfs_remove_group(&pdev->dev.kobj, &intel_ipc_group);
> > +	free_irq(ipcdev.irq, &ipcdev);
> > +	platform_device_unregister(ipcdev.tco_dev);
> > +	platform_device_unregister(ipcdev.punit_dev);
> > +	iounmap(ipcdev.ipc_base);
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM,
> > +				PLAT_RESOURCE_IPC_INDEX);
> > +	if (res)
> > +		release_mem_region(res->start,
> > PLAT_RESOURCE_IPC_SIZE);
> > +	ipcdev.dev = NULL;
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver ipc_plat_driver = {
> > +	.remove = ipc_plat_remove,
> > +	.probe = ipc_plat_probe,
> > +	.driver = {
> > +		.name = "pmc-ipc-plat",
> > +		.owner = THIS_MODULE,
> > +		.acpi_match_table = ACPI_PTR(ipc_acpi_ids),
> > +	},
> > +};
> > +
> > +static int __init intel_pmc_ipc_init(void)
> > +{
> > +	int ret;
> > +
> > +	ret = platform_driver_register(&ipc_plat_driver);
> > +	if (ret) {
> > +		pr_err("Fail to register PMC ipc platform
> > driver\n");
> > +		return ret;
> > +	}
> > +	ret = pci_register_driver(&ipc_pci_driver);
> > +	if (ret) {
> > +		pr_err("Fail to register PMC ipc pci driver\n");
> > +		platform_driver_unregister(&ipc_plat_driver);
> > +		return ret;
> > +	}
> > +	return ret;
> > +}
> > +
> > +static void __exit intel_pmc_ipc_exit(void)
> > +{
> > +	pci_unregister_driver(&ipc_pci_driver);
> > +	platform_driver_unregister(&ipc_plat_driver);
> > +}
> > +
> > +MODULE_AUTHOR("qipeng.zha@intel.com");
> > +MODULE_DESCRIPTION("Intel PMC IPC driver");
> > +MODULE_LICENSE("GPL V2");
> > +
> > +fs_initcall(intel_pmc_ipc_init);
> > +module_exit(intel_pmc_ipc_exit);
> > -- 
> > 1.8.3.2
> > 
> > 
> 
> -- 
> Darren Hart
> Intel Open Source Technology Center




-- 
Jacob Pan

Re: [PATCH] platform:x86: add Intel Broxton PMC IPC driver

[Darren Hart]

On Tue, Apr 21, 2015 at 07:04:29AM +0800, qipeng.zha wrote:
> From: "qipeng.zha" <qipeng.zha@intel.com>
> 
> This driver provides support for PMC control on Broxton platforms,
> PMC is ARC processor which defined some IPC commands for communication
> with other entities running in IA
> 

Hi Qipeng,

After the responses from Alan, Fei, and Jacob, I assume a second version of this
patch is in the works.

A few points I do want to call out before that one lands here.

1) There are a number of whitespace errors throughout this patch which make it
harder to review and focus on the technical implementation. Please abide by
coding style and when in doubt, compare with the SCU driver this so closely
resembles.

2) There are a number of undocumented functions which have well documented
parallels in the SCU driver, please pull those in.

3) Alan mentioned a number of cleanups for the SCU driver that are missing from
this one. I wasn't around for those, but I did notice a few rather severe
differences. See the commentary below regarding the ipclock mutex for an
example.

4) As a general rule, and when possible, please order your local variable
declarations in decreasing line length:

	int really_log_variable;
	int a_shorter_one;
	int ret;

Please consider these in your second version.

> Signed-off-by: qipeng.zha <qipeng.zha@intel.com>
> ---
>  arch/x86/include/asm/intel_pmc_ipc.h |  79 ++++
>  drivers/platform/x86/Kconfig         |   7 +
>  drivers/platform/x86/Makefile        |   1 +
>  drivers/platform/x86/intel_pmc_ipc.c | 740 +++++++++++++++++++++++++++++++++++
>  4 files changed, 827 insertions(+)
>  create mode 100644 arch/x86/include/asm/intel_pmc_ipc.h
>  create mode 100644 drivers/platform/x86/intel_pmc_ipc.c
> 

...

> +
> +int intel_pmc_ipc_simple_command(int cmd, int sub)
> +{
> +	int ret;
> +
> +	if (ipcdev.dev == NULL)
> +		return -ENODEV;
> +
> +	mutex_lock(&ipclock);

In the SCU driver, the check for ipcdev.dev == NULL is done with ipclock held. I
suspect this is because the assignment of ipcdev.ipc is done with the lock held.
While you can catch NULL and exit here without serious detriment in the case of
a race, if you continued after the check and it was set to NULL just before
acquiring the lock, you will end up with a kernel panic.

Other examples of this exist as well.

> +	ipc_send_command(sub << IPC_CMD_SUBCMD | cmd);
> +	ret = intel_pmc_ipc_check_status();
> +	mutex_unlock(&ipclock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(intel_pmc_ipc_simple_command);

...

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center