[PATCH v8 0/2] PCC: Platform Communication Channel

[PATCH v8 0/2] PCC: Platform Communication Channel

[Ashwin Chaugule]

Changes since V7:
- Added timeout to tx method in case the remote dies.
- Restructured usage of acpi_status. Had inverted logic previously.

Changes since V6:
- Cosmetic changes based on Lv's suggestions

Changes since V5:
- Optimize loop that matches channel request.
- Use platform_create_bundle.
- Replace ioread/writes.
- Remove redundant code and headers.
- Restructure common mailbox macros.
- Reformat PCC cmd parsing.

Changes since V4:
- Folded PCC Mailbox helpers into pcc.c

Changes since V3:
- Added PCC helper functions to work around "struct device" limitations.
- PCC driver changes to work with PCC specific Mailbox helpers.

Changes since V2:
- Rebased on top of git://git.linaro.org/landing-teams/working/fujitsu/integration.git
        branch mailbox-for-3.17
- Added PCC API to mailbox framework as per Arnd's suggestion to allow usage without ACPI.

Changes since V1:
- Integration with Mailbox framework - https://lkml.org/lkml/2014/5/15/49

This patchset adds support for the PCC (Platform Communication Channel)
interface as described in the current ACPI 5.0 spec. See Section 14 of the
ACPI spec - http://acpi.info/DOWNLOADS/ACPI_5_Errata%20A.pdf for more details
on how PCC works.

In brief PCC is a generic means for PCC clients, to talk to the firmware. The
PCC register space is typically memory mapped IO and uses a doorbell mechanism
to communicate synchronously from the OS to the firmware. The PCC driver is
completely agnostic to the protocol implemented by the PCC clients. It only
implements the enumeration of PCC channels and the low level transport mechanism
and leaves the rest to the PCC clients.

The PCC is meant to be useable in the future by clients such as CPPC
(Collaborative Processor Performance Control), RAS (Reliability,
Availability and Serviceability) and MPST (Memory Power State Tables) and possibly others.

Ashwin Chaugule (2):
  Mailbox: Add support for Platform Communication Channel
  PCC test

 drivers/mailbox/Kconfig    |  12 ++
 drivers/mailbox/Makefile   |   2 +
 drivers/mailbox/mailbox.c  |   4 +-
 drivers/mailbox/mailbox.h  |  16 +++
 drivers/mailbox/pcc-test.c | 204 +++++++++++++++++++++++++++++++
 drivers/mailbox/pcc.c      | 292 +++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 527 insertions(+), 3 deletions(-)
 create mode 100644 drivers/mailbox/mailbox.h
 create mode 100644 drivers/mailbox/pcc-test.c
 create mode 100644 drivers/mailbox/pcc.c

-- 
1.9.1

[PATCH v8 1/2] Mailbox: Add support for Platform Communication Channel

[Ashwin Chaugule]

ACPI 5.0+ spec defines a generic mode of communication
between the OS and a platform such as the BMC. This medium
(PCC) is typically used by CPPC (ACPI CPU Performance management),
RAS (ACPI reliability protocol) and MPST (ACPI Memory power
states).

This patch adds PCC support as a Mailbox Controller.

Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
---
 drivers/mailbox/Kconfig   |  12 ++
 drivers/mailbox/Makefile  |   2 +
 drivers/mailbox/mailbox.c |   4 +-
 drivers/mailbox/mailbox.h |  16 +++
 drivers/mailbox/pcc.c     | 292 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 323 insertions(+), 3 deletions(-)
 create mode 100644 drivers/mailbox/mailbox.h
 create mode 100644 drivers/mailbox/pcc.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index c8b5c13..e08cc83 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -50,4 +50,16 @@ config OMAP_MBOX_KFIFO_SIZE
 	  Specify the default size of mailbox's kfifo buffers (bytes).
 	  This can also be changed at runtime (via the mbox_kfifo_size
 	  module parameter).
+
+config PCC
+	bool "Platform Communication Channel Driver"
+	depends on ACPI
+	help
+		ACPI 5.0+ spec defines a generic mode of communication
+		between the OS and a platform such as the BMC. This medium
+		(PCC) is typically used by CPPC (ACPI CPU Performance management),
+		RAS (ACPI reliability protocol) and MPST (ACPI Memory power
+		states). Select this driver if your platform implements the
+		PCC clients mentioned above.
+
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 2fa343a..0b09d6f 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -9,3 +9,5 @@ obj-$(CONFIG_OMAP1_MBOX)	+= mailbox_omap1.o
 mailbox_omap1-objs		:= mailbox-omap1.o
 obj-$(CONFIG_OMAP2PLUS_MBOX)	+= mailbox_omap2.o
 mailbox_omap2-objs		:= mailbox-omap2.o
+
+obj-$(CONFIG_PCC)			+= pcc.o
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 63ecc17..9d9366a 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -21,9 +21,7 @@
 #include <linux/mailbox_client.h>
 #include <linux/mailbox_controller.h>
 
-#define TXDONE_BY_IRQ	BIT(0) /* controller has remote RTR irq */
-#define TXDONE_BY_POLL	BIT(1) /* controller can read status of last TX */
-#define TXDONE_BY_ACK	BIT(2) /* S/W ACK recevied by Client ticks the TX */
+#include "mailbox.h"
 
 static LIST_HEAD(mbox_cons);
 static DEFINE_MUTEX(con_mutex);
diff --git a/drivers/mailbox/mailbox.h b/drivers/mailbox/mailbox.h
new file mode 100644
index 0000000..5a15a25
--- /dev/null
+++ b/drivers/mailbox/mailbox.h
@@ -0,0 +1,16 @@
+/*
+ * 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 __MAILBOX_H
+#define __MAILBOX_H
+
+#define TXDONE_BY_IRQ	BIT(0) /* controller has remote RTR irq */
+#define TXDONE_BY_POLL	BIT(1) /* controller can read status of last TX */
+#define TXDONE_BY_ACK	BIT(2) /* S/W ACK recevied by Client ticks the TX */
+
+#endif /* __MAILBOX_H */
+
+
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
new file mode 100644
index 0000000..a16991e
--- /dev/null
+++ b/drivers/mailbox/pcc.c
@@ -0,0 +1,292 @@
+/*
+ *	Copyright (C) 2014 Linaro Ltd.
+ *	Author:	Ashwin Chaugule <ashwin.chaugule@linaro.org>
+ *
+ *  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; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/cpufreq.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/ioctl.h>
+#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <linux/vmalloc.h>
+#include <linux/platform_device.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox_client.h>
+
+#include "mailbox.h"
+
+#define MAX_PCC_SUBSPACES	256
+#define PCCS_SS_SIG_MAGIC	0x50434300
+#define PCC_CMD_COMPLETE	0x1
+
+static struct mbox_chan pcc_mbox_chan[MAX_PCC_SUBSPACES];
+static struct mbox_controller pcc_mbox_ctrl = {};
+
+struct mbox_chan *pcc_mbox_request_channel(struct mbox_client *cl,
+		int index)
+{
+	struct device *dev = pcc_mbox_ctrl.dev;
+	struct mbox_chan *chan;
+	unsigned long flags;
+
+	/*
+	 * Each PCC Subspace is a Mailbox Channel.
+	 * The PCC Clients get their PCC Subspace ID
+	 * from their own tables and pass it here.
+	 * This returns a pointer to the PCC subspace
+	 * for the Client to operate on.
+	 */
+	chan = &pcc_mbox_chan[index];
+
+	if (!chan || chan->cl) {
+		dev_err(dev, "%s: PCC mailbox not free\n", __func__);
+		return ERR_PTR(-EBUSY);
+	}
+
+	spin_lock_irqsave(&chan->lock, flags);
+	chan->msg_free = 0;
+	chan->msg_count = 0;
+	chan->active_req = NULL;
+	chan->cl = cl;
+	init_completion(&chan->tx_complete);
+
+	if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
+		chan->txdone_method |= TXDONE_BY_ACK;
+
+	spin_unlock_irqrestore(&chan->lock, flags);
+
+	return chan;
+}
+EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
+
+void pcc_mbox_free_channel(struct mbox_chan *chan)
+{
+	unsigned long flags;
+
+	if (!chan || !chan->cl)
+		return;
+
+	spin_lock_irqsave(&chan->lock, flags);
+	chan->cl = NULL;
+	chan->active_req = NULL;
+	if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
+		chan->txdone_method = TXDONE_BY_POLL;
+
+	spin_unlock_irqrestore(&chan->lock, flags);
+}
+EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
+
+static bool pcc_tx_done(struct mbox_chan *chan)
+{
+	struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
+	struct acpi_pcct_shared_memory *generic_comm_base =
+		(struct acpi_pcct_shared_memory *) pcct_ss->base_address;
+	u16 cmd_delay = pcct_ss->min_turnaround_time;
+	unsigned int retries = 0;
+
+	/* Try a few times while waiting for platform to consume */
+	while (!(readw_relaxed(&generic_comm_base->status)
+		    & PCC_CMD_COMPLETE)) {
+
+		if (retries++ < 5)
+			udelay(cmd_delay);
+		else {
+			/*
+			 * If the remote is dead, this will cause the Mbox
+			 * controller to timeout after mbox client.tx_tout
+			 * msecs.
+			 */
+			pr_err("PCC platform did not respond.\n");
+			return false;
+		}
+	}
+	return true;
+}
+
+static int get_subspace_id(struct mbox_chan *chan)
+{
+	int id = chan - pcc_mbox_chan;
+
+	if (id < 0 || id > pcc_mbox_ctrl.num_chans)
+		return -ENOENT;
+
+	return id;
+}
+
+/* Channel lock is already held by mbox controller code. */
+static int pcc_send_data(struct mbox_chan *chan, void *data)
+{
+	struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
+	struct acpi_pcct_shared_memory *generic_comm_base =
+		(struct acpi_pcct_shared_memory *) pcct_ss->base_address;
+	struct acpi_generic_address doorbell;
+	u64 doorbell_preserve;
+	u64 doorbell_val;
+	u64 doorbell_write;
+	u16 cmd = *(u16 *) data;
+	u16 ss_idx = -1;
+	int ret = 0;
+
+	ss_idx = get_subspace_id(chan);
+
+	if (ss_idx < 0) {
+		pr_err("Invalid Subspace ID from PCC client\n");
+		ret = -EINVAL;
+		goto out_err;
+	}
+
+	doorbell = pcct_ss->doorbell_register;
+	doorbell_preserve = pcct_ss->preserve_mask;
+	doorbell_write = pcct_ss->write_mask;
+
+	/* Write to the shared comm region. */
+	writew(cmd, &generic_comm_base->command);
+
+	/* Write Subspace MAGIC value so platform can identify destination. */
+	writel((PCCS_SS_SIG_MAGIC | ss_idx), &generic_comm_base->signature);
+
+	/* Flip CMD COMPLETE bit */
+	writew(0, &generic_comm_base->status);
+
+	/* Sync notification from OSPM to Platform. */
+	acpi_read(&doorbell_val, &doorbell);
+	acpi_write((doorbell_val & doorbell_preserve) | doorbell_write,
+			&doorbell);
+
+out_err:
+	return ret;
+}
+
+static struct mbox_chan_ops pcc_chan_ops = {
+	.send_data = pcc_send_data,
+	.last_tx_done = pcc_tx_done,
+};
+
+static int parse_pcc_subspace(struct acpi_subtable_header *header,
+		const unsigned long end)
+{
+	struct acpi_pcct_subspace *pcct_ss;
+
+	if (pcc_mbox_ctrl.num_chans <= MAX_PCC_SUBSPACES) {
+		pcct_ss = (struct acpi_pcct_subspace *) header;
+
+		if (pcct_ss->header.type != ACPI_PCCT_TYPE_GENERIC_SUBSPACE) {
+			pr_err("Incorrect PCC Subspace type detected\n");
+			return -EINVAL;
+		}
+
+		/* New mbox channel entry for each PCC subspace detected. */
+		pcc_mbox_chan[pcc_mbox_ctrl.num_chans].con_priv = pcct_ss;
+		pcc_mbox_ctrl.num_chans++;
+
+	} else {
+		pr_err("No more space for PCC subspaces.\n");
+		return -ENOSPC;
+	}
+
+	return 0;
+}
+
+static int __init acpi_pcc_probe(void)
+{
+	acpi_status status = AE_OK;
+	acpi_size pcct_tbl_header_size;
+	int count;
+	struct acpi_table_pcct *pcct_tbl;
+
+	/* Search for PCCT */
+	status = acpi_get_table_with_size(ACPI_SIG_PCCT, 0,
+			(struct acpi_table_header **)&pcct_tbl,
+			&pcct_tbl_header_size);
+
+	if (ACPI_FAILURE(status) || !pcct_tbl) {
+		pr_warn("PCCT header not found.\n");
+		return -ENODEV;
+	}
+
+	count = acpi_table_parse_entries(ACPI_SIG_PCCT,
+			sizeof(struct acpi_table_pcct),
+			ACPI_PCCT_TYPE_GENERIC_SUBSPACE,
+			parse_pcc_subspace, MAX_PCC_SUBSPACES);
+
+	if (count <= 0) {
+		pr_err("Error parsing PCC subspaces from PCCT\n");
+		return -EINVAL;
+	}
+
+	pr_info("Detected %d PCC Subspaces\n", pcc_mbox_ctrl.num_chans);
+
+	return 0;
+}
+
+static int pcc_mbox_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+
+	pcc_mbox_ctrl.chans = pcc_mbox_chan;
+	pcc_mbox_ctrl.ops = &pcc_chan_ops;
+	pcc_mbox_ctrl.txdone_poll = true;
+	pcc_mbox_ctrl.txpoll_period = 10;
+	pcc_mbox_ctrl.dev = &pdev->dev;
+
+	pr_info("Registering PCC driver as Mailbox controller\n");
+	ret = mbox_controller_register(&pcc_mbox_ctrl);
+
+	if (ret) {
+		pr_err("Err registering PCC as Mailbox controller: %d\n", ret);
+		ret = -ENODEV;
+	}
+
+	return ret;
+}
+
+struct platform_driver pcc_mbox_driver = {
+	.probe = pcc_mbox_probe,
+	.driver = {
+		.name = "PCCT",
+		.owner = THIS_MODULE,
+	},
+};
+
+static int __init pcc_init(void)
+{
+	int ret;
+	struct platform_device *pcc_pdev;
+
+	if (acpi_disabled)
+		return -ENODEV;
+
+	/* Check if PCC support is available. */
+	ret = acpi_pcc_probe();
+
+	if (ret) {
+		pr_err("ACPI PCC probe failed.\n");
+		return -ENODEV;
+	}
+
+	pcc_pdev = platform_create_bundle(&pcc_mbox_driver,
+			pcc_mbox_probe, NULL, 0, NULL, 0);
+
+	if (!pcc_pdev) {
+		pr_err("Err creating PCC platform bundle\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+device_initcall(pcc_init);
-- 
1.9.1

[PATCH v8 2/2] PCC test

[Ashwin Chaugule]

Not for upstreaming. This is only to show how the PCC driver was
tested.

echo 2 > /proc/pcc_test [ to get the PCC base address from the PCCT ]

echo 1 > /proc/pcc_test [ to trigger a PCC write ]

echo 0 > /proc/pcc_test [ to trigger a PCC read ]

dmesg shows the output.

Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
---
 drivers/mailbox/Makefile   |   2 +-
 drivers/mailbox/pcc-test.c | 204 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 205 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mailbox/pcc-test.c

diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 0b09d6f..028b8b1 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -10,4 +10,4 @@ mailbox_omap1-objs		:= mailbox-omap1.o
 obj-$(CONFIG_OMAP2PLUS_MBOX)	+= mailbox_omap2.o
 mailbox_omap2-objs		:= mailbox-omap2.o
 
-obj-$(CONFIG_PCC)			+= pcc.o
+obj-$(CONFIG_PCC)			+= pcc.o pcc-test.o
diff --git a/drivers/mailbox/pcc-test.c b/drivers/mailbox/pcc-test.c
new file mode 100644
index 0000000..5faa255
--- /dev/null
+++ b/drivers/mailbox/pcc-test.c
@@ -0,0 +1,204 @@
+/*
+ *	Copyright (C) 2014 Linaro Ltd.
+ *	Author:	Ashwin Chaugule <ashwin.chaugule@linaro.org>
+ *
+ *  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; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+#include <linux/acpi.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+#include <linux/init.h>
+#include <linux/cpufreq.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include <linux/mailbox_client.h>
+#include <linux/mailbox_controller.h>
+#include <linux/platform_device.h>
+#include <asm/uaccess.h>
+
+#include <acpi/actbl.h>
+
+static void __iomem *comm_base_addr; 	/* For use after ioremap */
+
+extern struct mbox_chan *pcc_mbox_request_channel(struct mbox_client *, unsigned int);
+extern int mbox_send_message(struct mbox_chan *chan, void *mssg);
+
+/* XXX: The PCC Subspace id is hard coded here only for test purposes.
+ * In reality it should be parsed from the PCC package as described
+ * in the PCC client table. e.g. CPC for CPPC 
+ */
+#define PCC_SUBSPACE_IDX 0
+#define CMD_COMPLETE    1
+
+struct mbox_chan *pcc_test_chan;
+enum ppc_cmds {
+	CMD_READ,
+	CMD_WRITE,
+	RESERVED,
+};
+
+static u64 reg1, reg2;
+
+static void run_pcc_test_read(void)
+{
+	u64 reg1_addr = (u64)comm_base_addr + 0x100;
+	u64 reg2_addr = (u64)comm_base_addr + 0x110;
+	u16 cmd = CMD_READ;
+	int ret;
+
+	/* READ part of the test */
+	pr_info("Sending PCC read req from Channel base addr: %llx\n", (u64)comm_base_addr);
+	ret = mbox_send_message(pcc_test_chan, &cmd);
+	if (ret >= 0) {
+		pr_info("Read updated values from Platform.\n");
+		reg1 = readq((void*)reg1_addr);
+		reg2 = readq((void*)reg2_addr);
+		pr_info("updated value of reg1:%llx\n", reg1);
+		pr_info("updated value of reg2:%llx\n", reg2);
+	} else
+		pr_err("Failed to read PCC parameters: ret=%d\n", ret);
+}
+
+static void run_pcc_test_write(void)
+{
+	u64 reg1_addr = (u64)comm_base_addr + 0x100;
+	u64 reg2_addr = (u64)comm_base_addr + 0x110;
+	u16 cmd = CMD_WRITE;
+	int ret;
+
+	/* WRITE part of the test */
+	reg1++;
+	reg2++;
+
+	writeq(reg1, (void *)reg1_addr);
+	writeq(reg2, (void *)reg2_addr);
+
+	pr_info("Sending PCC write req from Channel base addr: %llx\n", (u64)comm_base_addr);
+	ret = mbox_send_message(pcc_test_chan, &cmd);
+
+	if (ret >= 0)
+		pr_info("OSPM successfully sent PCC write cmd to platform\n");
+	else
+		pr_err("Failed to write PCC parameters. ret= %d\n", ret);
+}
+
+static void pcc_chan_tx_done(struct mbox_client *cl, void *mssg, int ret)
+{
+	if (!ret)
+		pr_info("PCC channel TX successfully completed. CMD sent = %x\n", *(u16*)mssg);
+	else
+		pr_warn("PCC channel TX did not complete: CMD sent = %x\n", *(u16*)mssg);
+}
+
+struct mbox_client pcc_mbox_cl = {
+	.tx_done	=	pcc_chan_tx_done,
+	.tx_block	=	true,
+	.tx_tout	=	10,
+};
+
+void get_pcc_comm(void)
+{
+	u64 base_addr;
+	u32 len;
+	struct acpi_pcct_subspace *pcc_ss;
+
+	pcc_test_chan = pcc_mbox_request_channel(&pcc_mbox_cl, PCC_SUBSPACE_IDX);
+
+	if (!pcc_test_chan) {
+		pr_err("PCC Channel not found!\n");
+		return;
+	}
+
+	pcc_ss = pcc_test_chan->con_priv;
+
+	base_addr = pcc_ss->base_address;
+	len = pcc_ss->length;
+
+	pr_info("ioremapping: %llx, len: %x\n", base_addr, len);
+
+//	comm_base_addr = ioremap_nocache(base_addr, len);
+	/* HACK to test PCC commands */
+	comm_base_addr = kzalloc(PAGE_SIZE, GFP_KERNEL);
+
+	if (!comm_base_addr) {
+		pr_err("Could not ioremap channel\n");
+		return;
+	}
+
+	pcc_ss->base_address = (u64)comm_base_addr;
+
+	pr_info("Comm_base_addr: %llx\n", (u64)comm_base_addr);
+}
+
+static ssize_t pcc_test_write(struct file *file, const char __user *buf,
+		size_t count, loff_t *offs)
+{
+	char ctl[2];
+
+	if (count != 2 || *offs)
+		return -EINVAL;
+
+	if (copy_from_user(ctl, buf, count))
+		return -EFAULT;
+
+	switch (ctl[0]) {
+		case '0':
+			/* PCC read */
+			run_pcc_test_read();
+			break;
+		case '1':
+			/* PCC write */
+			run_pcc_test_write();
+			break;
+		case '2':
+			/* Get PCC channel */
+			get_pcc_comm();
+			break;
+		default:
+			pr_err("Unknown val\n");
+			break;
+	}
+
+	return count;
+}
+
+static int pcc_test_open(struct inode *inode, struct file *filp)
+{
+	return 0;
+}
+
+static int pcc_test_release(struct inode *inode, struct file *filp)
+{
+	return 0;
+}
+
+static const struct file_operations pcc_test_fops = {
+	.open		= pcc_test_open,
+//	.read		= seq_read,
+	.write		= pcc_test_write,
+	.release	= pcc_test_release,
+};
+
+static int __init pcc_test(void)
+{
+	struct proc_dir_entry *pe;
+
+	pe = proc_create("pcc_test", 0644, NULL, &pcc_test_fops);
+
+	if (!pe)
+		return -ENOMEM;
+
+	return 0;
+}
+
+late_initcall(pcc_test);
-- 
1.9.1

Re: [PATCH v8 1/2] Mailbox: Add support for Platform Communication Channel

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 371 bytes --]

On Wed, Oct 01, 2014 at 04:46:12PM -0400, Ashwin Chaugule wrote:
> ACPI 5.0+ spec defines a generic mode of communication
> between the OS and a platform such as the BMC. This medium
> (PCC) is typically used by CPPC (ACPI CPU Performance management),
> RAS (ACPI reliability protocol) and MPST (ACPI Memory power
> states).

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v8 1/2] Mailbox: Add support for Platform Communication Channel

[Ashwin Chaugule]

On 2 October 2014 13:25, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 01, 2014 at 04:46:12PM -0400, Ashwin Chaugule wrote:
>> ACPI 5.0+ spec defines a generic mode of communication
>> between the OS and a platform such as the BMC. This medium
>> (PCC) is typically used by CPPC (ACPI CPU Performance management),
>> RAS (ACPI reliability protocol) and MPST (ACPI Memory power
>> states).
>
> Reviewed-by: Mark Brown <broonie@kernel.org>

Thanks for the quick turn Mark!

Arnd, Lv, since you both helped a lot as well, let me know if I can
add your Reviewed-by tags to this patch set.

Thanks,
Ashwin

Re: [PATCH v8 1/2] Mailbox: Add support for Platform Communication Channel

[Rafael J. Wysocki]

On Wednesday, October 01, 2014 04:46:12 PM Ashwin Chaugule wrote:
> ACPI 5.0+ spec defines a generic mode of communication
> between the OS and a platform such as the BMC. This medium
> (PCC) is typically used by CPPC (ACPI CPU Performance management),
> RAS (ACPI reliability protocol) and MPST (ACPI Memory power
> states).
> 
> This patch adds PCC support as a Mailbox Controller.
> 
> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
> ---
>  drivers/mailbox/Kconfig   |  12 ++
>  drivers/mailbox/Makefile  |   2 +
>  drivers/mailbox/mailbox.c |   4 +-
>  drivers/mailbox/mailbox.h |  16 +++
>  drivers/mailbox/pcc.c     | 292 ++++++++++++++++++++++++++++++++++++++++++++++

I'd prefer this new file to be called acpi_pcc.c ->

>  5 files changed, 323 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/mailbox/mailbox.h
>  create mode 100644 drivers/mailbox/pcc.c
> 
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c8b5c13..e08cc83 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -50,4 +50,16 @@ config OMAP_MBOX_KFIFO_SIZE
>  	  Specify the default size of mailbox's kfifo buffers (bytes).
>  	  This can also be changed at runtime (via the mbox_kfifo_size
>  	  module parameter).
> +
> +config PCC

-> and this new symbol to be called ACPI_PCC.

> +	bool "Platform Communication Channel Driver"
> +	depends on ACPI
> +	help
> +		ACPI 5.0+ spec defines a generic mode of communication
> +		between the OS and a platform such as the BMC. This medium
> +		(PCC) is typically used by CPPC (ACPI CPU Performance management),
> +		RAS (ACPI reliability protocol) and MPST (ACPI Memory power
> +		states). Select this driver if your platform implements the
> +		PCC clients mentioned above.
> +
>  endif
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index 2fa343a..0b09d6f 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -9,3 +9,5 @@ obj-$(CONFIG_OMAP1_MBOX)	+= mailbox_omap1.o
>  mailbox_omap1-objs		:= mailbox-omap1.o
>  obj-$(CONFIG_OMAP2PLUS_MBOX)	+= mailbox_omap2.o
>  mailbox_omap2-objs		:= mailbox-omap2.o
> +
> +obj-$(CONFIG_PCC)			+= pcc.o
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index 63ecc17..9d9366a 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -21,9 +21,7 @@
>  #include <linux/mailbox_client.h>
>  #include <linux/mailbox_controller.h>
>  
> -#define TXDONE_BY_IRQ	BIT(0) /* controller has remote RTR irq */
> -#define TXDONE_BY_POLL	BIT(1) /* controller can read status of last TX */
> -#define TXDONE_BY_ACK	BIT(2) /* S/W ACK recevied by Client ticks the TX */
> +#include "mailbox.h"
>  
>  static LIST_HEAD(mbox_cons);
>  static DEFINE_MUTEX(con_mutex);
> diff --git a/drivers/mailbox/mailbox.h b/drivers/mailbox/mailbox.h
> new file mode 100644
> index 0000000..5a15a25
> --- /dev/null
> +++ b/drivers/mailbox/mailbox.h
> @@ -0,0 +1,16 @@
> +/*
> + * 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 __MAILBOX_H
> +#define __MAILBOX_H
> +
> +#define TXDONE_BY_IRQ	BIT(0) /* controller has remote RTR irq */
> +#define TXDONE_BY_POLL	BIT(1) /* controller can read status of last TX */
> +#define TXDONE_BY_ACK	BIT(2) /* S/W ACK recevied by Client ticks the TX */
> +
> +#endif /* __MAILBOX_H */
> +
> +
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> new file mode 100644
> index 0000000..a16991e
> --- /dev/null
> +++ b/drivers/mailbox/pcc.c
> @@ -0,0 +1,292 @@
> +/*
> + *	Copyright (C) 2014 Linaro Ltd.
> + *	Author:	Ashwin Chaugule <ashwin.chaugule@linaro.org>
> + *
> + *  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; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + */

Any chance to say something about what this file provides, what the PCC
is and so on?  Any pointers to the relevant part of the spec in case
people want to see themselves?

> +
> +#include <linux/acpi.h>
> +#include <linux/cpufreq.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/ioctl.h>
> +#include <linux/module.h>
> +#include <linux/uaccess.h>
> +#include <linux/vmalloc.h>
> +#include <linux/platform_device.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/mailbox_client.h>

Do you really need all these headers?

> +
> +#include "mailbox.h"
> +
> +#define MAX_PCC_SUBSPACES	256
> +#define PCCS_SS_SIG_MAGIC	0x50434300
> +#define PCC_CMD_COMPLETE	0x1
> +
> +static struct mbox_chan pcc_mbox_chan[MAX_PCC_SUBSPACES];
> +static struct mbox_controller pcc_mbox_ctrl = {};
> +
> +struct mbox_chan *pcc_mbox_request_channel(struct mbox_client *cl,
> +		int index)
> +{
> +	struct device *dev = pcc_mbox_ctrl.dev;
> +	struct mbox_chan *chan;
> +	unsigned long flags;
> +
> +	/*
> +	 * Each PCC Subspace is a Mailbox Channel.
> +	 * The PCC Clients get their PCC Subspace ID
> +	 * from their own tables and pass it here.
> +	 * This returns a pointer to the PCC subspace
> +	 * for the Client to operate on.
> +	 */
> +	chan = &pcc_mbox_chan[index];
> +
> +	if (!chan || chan->cl) {
> +		dev_err(dev, "%s: PCC mailbox not free\n", __func__);
> +		return ERR_PTR(-EBUSY);
> +	}
> +
> +	spin_lock_irqsave(&chan->lock, flags);
> +	chan->msg_free = 0;
> +	chan->msg_count = 0;
> +	chan->active_req = NULL;
> +	chan->cl = cl;
> +	init_completion(&chan->tx_complete);
> +
> +	if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
> +		chan->txdone_method |= TXDONE_BY_ACK;
> +
> +	spin_unlock_irqrestore(&chan->lock, flags);
> +
> +	return chan;
> +}
> +EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
> +
> +void pcc_mbox_free_channel(struct mbox_chan *chan)
> +{
> +	unsigned long flags;
> +
> +	if (!chan || !chan->cl)
> +		return;
> +
> +	spin_lock_irqsave(&chan->lock, flags);
> +	chan->cl = NULL;
> +	chan->active_req = NULL;
> +	if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
> +		chan->txdone_method = TXDONE_BY_POLL;
> +
> +	spin_unlock_irqrestore(&chan->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
> +
> +static bool pcc_tx_done(struct mbox_chan *chan)
> +{
> +	struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
> +	struct acpi_pcct_shared_memory *generic_comm_base =
> +		(struct acpi_pcct_shared_memory *) pcct_ss->base_address;
> +	u16 cmd_delay = pcct_ss->min_turnaround_time;
> +	unsigned int retries = 0;
> +
> +	/* Try a few times while waiting for platform to consume */
> +	while (!(readw_relaxed(&generic_comm_base->status)
> +		    & PCC_CMD_COMPLETE)) {
> +
> +		if (retries++ < 5)
> +			udelay(cmd_delay);
> +		else {
> +			/*
> +			 * If the remote is dead, this will cause the Mbox
> +			 * controller to timeout after mbox client.tx_tout
> +			 * msecs.
> +			 */
> +			pr_err("PCC platform did not respond.\n");
> +			return false;
> +		}
> +	}
> +	return true;
> +}
> +
> +static int get_subspace_id(struct mbox_chan *chan)
> +{
> +	int id = chan - pcc_mbox_chan;
> +
> +	if (id < 0 || id > pcc_mbox_ctrl.num_chans)
> +		return -ENOENT;
> +
> +	return id;
> +}
> +
> +/* Channel lock is already held by mbox controller code. */
> +static int pcc_send_data(struct mbox_chan *chan, void *data)
> +{
> +	struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
> +	struct acpi_pcct_shared_memory *generic_comm_base =
> +		(struct acpi_pcct_shared_memory *) pcct_ss->base_address;
> +	struct acpi_generic_address doorbell;
> +	u64 doorbell_preserve;
> +	u64 doorbell_val;
> +	u64 doorbell_write;
> +	u16 cmd = *(u16 *) data;
> +	u16 ss_idx = -1;
> +	int ret = 0;
> +
> +	ss_idx = get_subspace_id(chan);
> +
> +	if (ss_idx < 0) {
> +		pr_err("Invalid Subspace ID from PCC client\n");
> +		ret = -EINVAL;
> +		goto out_err;
> +	}
> +
> +	doorbell = pcct_ss->doorbell_register;
> +	doorbell_preserve = pcct_ss->preserve_mask;
> +	doorbell_write = pcct_ss->write_mask;
> +
> +	/* Write to the shared comm region. */
> +	writew(cmd, &generic_comm_base->command);
> +
> +	/* Write Subspace MAGIC value so platform can identify destination. */
> +	writel((PCCS_SS_SIG_MAGIC | ss_idx), &generic_comm_base->signature);
> +
> +	/* Flip CMD COMPLETE bit */
> +	writew(0, &generic_comm_base->status);
> +
> +	/* Sync notification from OSPM to Platform. */
> +	acpi_read(&doorbell_val, &doorbell);
> +	acpi_write((doorbell_val & doorbell_preserve) | doorbell_write,
> +			&doorbell);
> +
> +out_err:
> +	return ret;
> +}
> +
> +static struct mbox_chan_ops pcc_chan_ops = {
> +	.send_data = pcc_send_data,
> +	.last_tx_done = pcc_tx_done,
> +};
> +
> +static int parse_pcc_subspace(struct acpi_subtable_header *header,
> +		const unsigned long end)
> +{
> +	struct acpi_pcct_subspace *pcct_ss;
> +
> +	if (pcc_mbox_ctrl.num_chans <= MAX_PCC_SUBSPACES) {
> +		pcct_ss = (struct acpi_pcct_subspace *) header;
> +
> +		if (pcct_ss->header.type != ACPI_PCCT_TYPE_GENERIC_SUBSPACE) {
> +			pr_err("Incorrect PCC Subspace type detected\n");
> +			return -EINVAL;
> +		}
> +
> +		/* New mbox channel entry for each PCC subspace detected. */
> +		pcc_mbox_chan[pcc_mbox_ctrl.num_chans].con_priv = pcct_ss;
> +		pcc_mbox_ctrl.num_chans++;
> +
> +	} else {
> +		pr_err("No more space for PCC subspaces.\n");
> +		return -ENOSPC;
> +	}
> +
> +	return 0;
> +}
> +
> +static int __init acpi_pcc_probe(void)
> +{
> +	acpi_status status = AE_OK;
> +	acpi_size pcct_tbl_header_size;
> +	int count;
> +	struct acpi_table_pcct *pcct_tbl;
> +
> +	/* Search for PCCT */
> +	status = acpi_get_table_with_size(ACPI_SIG_PCCT, 0,
> +			(struct acpi_table_header **)&pcct_tbl,
> +			&pcct_tbl_header_size);
> +
> +	if (ACPI_FAILURE(status) || !pcct_tbl) {
> +		pr_warn("PCCT header not found.\n");
> +		return -ENODEV;
> +	}
> +
> +	count = acpi_table_parse_entries(ACPI_SIG_PCCT,
> +			sizeof(struct acpi_table_pcct),
> +			ACPI_PCCT_TYPE_GENERIC_SUBSPACE,
> +			parse_pcc_subspace, MAX_PCC_SUBSPACES);
> +
> +	if (count <= 0) {
> +		pr_err("Error parsing PCC subspaces from PCCT\n");
> +		return -EINVAL;
> +	}
> +
> +	pr_info("Detected %d PCC Subspaces\n", pcc_mbox_ctrl.num_chans);
> +
> +	return 0;
> +}
> +
> +static int pcc_mbox_probe(struct platform_device *pdev)
> +{
> +	int ret = 0;
> +
> +	pcc_mbox_ctrl.chans = pcc_mbox_chan;
> +	pcc_mbox_ctrl.ops = &pcc_chan_ops;
> +	pcc_mbox_ctrl.txdone_poll = true;
> +	pcc_mbox_ctrl.txpoll_period = 10;
> +	pcc_mbox_ctrl.dev = &pdev->dev;
> +
> +	pr_info("Registering PCC driver as Mailbox controller\n");
> +	ret = mbox_controller_register(&pcc_mbox_ctrl);
> +
> +	if (ret) {
> +		pr_err("Err registering PCC as Mailbox controller: %d\n", ret);
> +		ret = -ENODEV;
> +	}
> +
> +	return ret;
> +}
> +
> +struct platform_driver pcc_mbox_driver = {
> +	.probe = pcc_mbox_probe,
> +	.driver = {
> +		.name = "PCCT",
> +		.owner = THIS_MODULE,
> +	},
> +};
> +
> +static int __init pcc_init(void)
> +{
> +	int ret;
> +	struct platform_device *pcc_pdev;
> +
> +	if (acpi_disabled)
> +		return -ENODEV;
> +
> +	/* Check if PCC support is available. */
> +	ret = acpi_pcc_probe();
> +
> +	if (ret) {
> +		pr_err("ACPI PCC probe failed.\n");
> +		return -ENODEV;
> +	}
> +
> +	pcc_pdev = platform_create_bundle(&pcc_mbox_driver,
> +			pcc_mbox_probe, NULL, 0, NULL, 0);
> +
> +	if (!pcc_pdev) {
> +		pr_err("Err creating PCC platform bundle\n");
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +device_initcall(pcc_init);
> 

Care to add some kerneldoc comments to the functions?  Especially the ones
that are then used as callbacks.

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

Re: [PATCH v8 1/2] Mailbox: Add support for Platform Communication Channel

[Ashwin Chaugule]

Hi Rafael,

On 6 October 2014 19:00, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Wednesday, October 01, 2014 04:46:12 PM Ashwin Chaugule wrote:
>> ACPI 5.0+ spec defines a generic mode of communication
>> between the OS and a platform such as the BMC. This medium
>> (PCC) is typically used by CPPC (ACPI CPU Performance management),
>> RAS (ACPI reliability protocol) and MPST (ACPI Memory power
>> states).
>>
>> This patch adds PCC support as a Mailbox Controller.
>>
>> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@linaro.org>
>> ---
>>  drivers/mailbox/Kconfig   |  12 ++
>>  drivers/mailbox/Makefile  |   2 +
>>  drivers/mailbox/mailbox.c |   4 +-
>>  drivers/mailbox/mailbox.h |  16 +++
>>  drivers/mailbox/pcc.c     | 292 ++++++++++++++++++++++++++++++++++++++++++++++
>
> I'd prefer this new file to be called acpi_pcc.c ->

[..]

>
>>  5 files changed, 323 insertions(+), 3 deletions(-)
>>  create mode 100644 drivers/mailbox/mailbox.h
>>  create mode 100644 drivers/mailbox/pcc.c
>>
>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
>> index c8b5c13..e08cc83 100644
>> --- a/drivers/mailbox/Kconfig
>> +++ b/drivers/mailbox/Kconfig
>> @@ -50,4 +50,16 @@ config OMAP_MBOX_KFIFO_SIZE
>>         Specify the default size of mailbox's kfifo buffers (bytes).
>>         This can also be changed at runtime (via the mbox_kfifo_size
>>         module parameter).
>> +
>> +config PCC
>
> -> and this new symbol to be called ACPI_PCC.

I agree that PCC is an ACPI defined concept, but based on very early
feedback from Arnd, we decided to make this generic enough for DT
support if anybody wants to add it. That would require a new
dt_pcc_probe() and a slightly modified pcc_send_data() to write the
doorbell reg. Since the changes should be minimal, they could live in
one file.

>
>> +     bool "Platform Communication Channel Driver"
>> +     depends on ACPI
>> +     help
>> +             ACPI 5.0+ spec defines a generic mode of communication
>> +             between the OS and a platform such as the BMC. This medium
>> +             (PCC) is typically used by CPPC (ACPI CPU Performance management),
>> +             RAS (ACPI reliability protocol) and MPST (ACPI Memory power
>> +             states). Select this driver if your platform implements the
>> +             PCC clients mentioned above.
>> +
>>  endif
>> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
>> index 2fa343a..0b09d6f 100644
>> --- a/drivers/mailbox/Makefile
>> +++ b/drivers/mailbox/Makefile
>> @@ -9,3 +9,5 @@ obj-$(CONFIG_OMAP1_MBOX)      += mailbox_omap1.o
>>  mailbox_omap1-objs           := mailbox-omap1.o
>>  obj-$(CONFIG_OMAP2PLUS_MBOX) += mailbox_omap2.o
>>  mailbox_omap2-objs           := mailbox-omap2.o
>> +
>> +obj-$(CONFIG_PCC)                    += pcc.o
>> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
>> index 63ecc17..9d9366a 100644
>> --- a/drivers/mailbox/mailbox.c
>> +++ b/drivers/mailbox/mailbox.c
>> @@ -21,9 +21,7 @@
>>  #include <linux/mailbox_client.h>
>>  #include <linux/mailbox_controller.h>
>>
>> -#define TXDONE_BY_IRQ        BIT(0) /* controller has remote RTR irq */
>> -#define TXDONE_BY_POLL       BIT(1) /* controller can read status of last TX */
>> -#define TXDONE_BY_ACK        BIT(2) /* S/W ACK recevied by Client ticks the TX */
>> +#include "mailbox.h"
>>
>>  static LIST_HEAD(mbox_cons);
>>  static DEFINE_MUTEX(con_mutex);
>> diff --git a/drivers/mailbox/mailbox.h b/drivers/mailbox/mailbox.h
>> new file mode 100644
>> index 0000000..5a15a25
>> --- /dev/null
>> +++ b/drivers/mailbox/mailbox.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * 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 __MAILBOX_H
>> +#define __MAILBOX_H
>> +
>> +#define TXDONE_BY_IRQ        BIT(0) /* controller has remote RTR irq */
>> +#define TXDONE_BY_POLL       BIT(1) /* controller can read status of last TX */
>> +#define TXDONE_BY_ACK        BIT(2) /* S/W ACK recevied by Client ticks the TX */
>> +
>> +#endif /* __MAILBOX_H */
>> +
>> +
>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
>> new file mode 100644
>> index 0000000..a16991e
>> --- /dev/null
>> +++ b/drivers/mailbox/pcc.c
>> @@ -0,0 +1,292 @@
>> +/*
>> + *   Copyright (C) 2014 Linaro Ltd.
>> + *   Author: Ashwin Chaugule <ashwin.chaugule@linaro.org>
>> + *
>> + *  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; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + */
>
> Any chance to say something about what this file provides, what the PCC
> is and so on?  Any pointers to the relevant part of the spec in case
> people want to see themselves?

Will do.

>
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/cpufreq.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/init.h>
>> +#include <linux/ioctl.h>
>> +#include <linux/module.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/vmalloc.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/mailbox_controller.h>
>> +#include <linux/mailbox_client.h>
>
> Do you really need all these headers?

D'oh! left over from early prototypes. Will fix.

>
>> +
>> +#include "mailbox.h"
>> +
>> +#define MAX_PCC_SUBSPACES    256
>> +#define PCCS_SS_SIG_MAGIC    0x50434300
>> +#define PCC_CMD_COMPLETE     0x1
>> +
>> +static struct mbox_chan pcc_mbox_chan[MAX_PCC_SUBSPACES];
>> +static struct mbox_controller pcc_mbox_ctrl = {};
>> +
>> +struct mbox_chan *pcc_mbox_request_channel(struct mbox_client *cl,
>> +             int index)
>> +{
>> +     struct device *dev = pcc_mbox_ctrl.dev;
>> +     struct mbox_chan *chan;
>> +     unsigned long flags;
>> +
>> +     /*
>> +      * Each PCC Subspace is a Mailbox Channel.
>> +      * The PCC Clients get their PCC Subspace ID
>> +      * from their own tables and pass it here.
>> +      * This returns a pointer to the PCC subspace
>> +      * for the Client to operate on.
>> +      */
>> +     chan = &pcc_mbox_chan[index];
>> +
>> +     if (!chan || chan->cl) {
>> +             dev_err(dev, "%s: PCC mailbox not free\n", __func__);
>> +             return ERR_PTR(-EBUSY);
>> +     }
>> +
>> +     spin_lock_irqsave(&chan->lock, flags);
>> +     chan->msg_free = 0;
>> +     chan->msg_count = 0;
>> +     chan->active_req = NULL;
>> +     chan->cl = cl;
>> +     init_completion(&chan->tx_complete);
>> +
>> +     if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone)
>> +             chan->txdone_method |= TXDONE_BY_ACK;
>> +
>> +     spin_unlock_irqrestore(&chan->lock, flags);
>> +
>> +     return chan;
>> +}
>> +EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
>> +
>> +void pcc_mbox_free_channel(struct mbox_chan *chan)
>> +{
>> +     unsigned long flags;
>> +
>> +     if (!chan || !chan->cl)
>> +             return;
>> +
>> +     spin_lock_irqsave(&chan->lock, flags);
>> +     chan->cl = NULL;
>> +     chan->active_req = NULL;
>> +     if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
>> +             chan->txdone_method = TXDONE_BY_POLL;
>> +
>> +     spin_unlock_irqrestore(&chan->lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
>> +
>> +static bool pcc_tx_done(struct mbox_chan *chan)
>> +{
>> +     struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
>> +     struct acpi_pcct_shared_memory *generic_comm_base =
>> +             (struct acpi_pcct_shared_memory *) pcct_ss->base_address;
>> +     u16 cmd_delay = pcct_ss->min_turnaround_time;
>> +     unsigned int retries = 0;
>> +
>> +     /* Try a few times while waiting for platform to consume */
>> +     while (!(readw_relaxed(&generic_comm_base->status)
>> +                 & PCC_CMD_COMPLETE)) {
>> +
>> +             if (retries++ < 5)
>> +                     udelay(cmd_delay);
>> +             else {
>> +                     /*
>> +                      * If the remote is dead, this will cause the Mbox
>> +                      * controller to timeout after mbox client.tx_tout
>> +                      * msecs.
>> +                      */
>> +                     pr_err("PCC platform did not respond.\n");
>> +                     return false;
>> +             }
>> +     }
>> +     return true;
>> +}
>> +
>> +static int get_subspace_id(struct mbox_chan *chan)
>> +{
>> +     int id = chan - pcc_mbox_chan;
>> +
>> +     if (id < 0 || id > pcc_mbox_ctrl.num_chans)
>> +             return -ENOENT;
>> +
>> +     return id;
>> +}
>> +
>> +/* Channel lock is already held by mbox controller code. */
>> +static int pcc_send_data(struct mbox_chan *chan, void *data)
>> +{
>> +     struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
>> +     struct acpi_pcct_shared_memory *generic_comm_base =
>> +             (struct acpi_pcct_shared_memory *) pcct_ss->base_address;
>> +     struct acpi_generic_address doorbell;
>> +     u64 doorbell_preserve;
>> +     u64 doorbell_val;
>> +     u64 doorbell_write;
>> +     u16 cmd = *(u16 *) data;
>> +     u16 ss_idx = -1;
>> +     int ret = 0;
>> +
>> +     ss_idx = get_subspace_id(chan);
>> +
>> +     if (ss_idx < 0) {
>> +             pr_err("Invalid Subspace ID from PCC client\n");
>> +             ret = -EINVAL;
>> +             goto out_err;
>> +     }
>> +
>> +     doorbell = pcct_ss->doorbell_register;
>> +     doorbell_preserve = pcct_ss->preserve_mask;
>> +     doorbell_write = pcct_ss->write_mask;
>> +
>> +     /* Write to the shared comm region. */
>> +     writew(cmd, &generic_comm_base->command);
>> +
>> +     /* Write Subspace MAGIC value so platform can identify destination. */
>> +     writel((PCCS_SS_SIG_MAGIC | ss_idx), &generic_comm_base->signature);
>> +
>> +     /* Flip CMD COMPLETE bit */
>> +     writew(0, &generic_comm_base->status);
>> +
>> +     /* Sync notification from OSPM to Platform. */
>> +     acpi_read(&doorbell_val, &doorbell);
>> +     acpi_write((doorbell_val & doorbell_preserve) | doorbell_write,
>> +                     &doorbell);
>> +
>> +out_err:
>> +     return ret;
>> +}
>> +
>> +static struct mbox_chan_ops pcc_chan_ops = {
>> +     .send_data = pcc_send_data,
>> +     .last_tx_done = pcc_tx_done,
>> +};
>> +
>> +static int parse_pcc_subspace(struct acpi_subtable_header *header,
>> +             const unsigned long end)
>> +{
>> +     struct acpi_pcct_subspace *pcct_ss;
>> +
>> +     if (pcc_mbox_ctrl.num_chans <= MAX_PCC_SUBSPACES) {
>> +             pcct_ss = (struct acpi_pcct_subspace *) header;
>> +
>> +             if (pcct_ss->header.type != ACPI_PCCT_TYPE_GENERIC_SUBSPACE) {
>> +                     pr_err("Incorrect PCC Subspace type detected\n");
>> +                     return -EINVAL;
>> +             }
>> +
>> +             /* New mbox channel entry for each PCC subspace detected. */
>> +             pcc_mbox_chan[pcc_mbox_ctrl.num_chans].con_priv = pcct_ss;
>> +             pcc_mbox_ctrl.num_chans++;
>> +
>> +     } else {
>> +             pr_err("No more space for PCC subspaces.\n");
>> +             return -ENOSPC;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int __init acpi_pcc_probe(void)
>> +{
>> +     acpi_status status = AE_OK;
>> +     acpi_size pcct_tbl_header_size;
>> +     int count;
>> +     struct acpi_table_pcct *pcct_tbl;
>> +
>> +     /* Search for PCCT */
>> +     status = acpi_get_table_with_size(ACPI_SIG_PCCT, 0,
>> +                     (struct acpi_table_header **)&pcct_tbl,
>> +                     &pcct_tbl_header_size);
>> +
>> +     if (ACPI_FAILURE(status) || !pcct_tbl) {
>> +             pr_warn("PCCT header not found.\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     count = acpi_table_parse_entries(ACPI_SIG_PCCT,
>> +                     sizeof(struct acpi_table_pcct),
>> +                     ACPI_PCCT_TYPE_GENERIC_SUBSPACE,
>> +                     parse_pcc_subspace, MAX_PCC_SUBSPACES);
>> +
>> +     if (count <= 0) {
>> +             pr_err("Error parsing PCC subspaces from PCCT\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     pr_info("Detected %d PCC Subspaces\n", pcc_mbox_ctrl.num_chans);
>> +
>> +     return 0;
>> +}
>> +
>> +static int pcc_mbox_probe(struct platform_device *pdev)
>> +{
>> +     int ret = 0;
>> +
>> +     pcc_mbox_ctrl.chans = pcc_mbox_chan;
>> +     pcc_mbox_ctrl.ops = &pcc_chan_ops;
>> +     pcc_mbox_ctrl.txdone_poll = true;
>> +     pcc_mbox_ctrl.txpoll_period = 10;
>> +     pcc_mbox_ctrl.dev = &pdev->dev;
>> +
>> +     pr_info("Registering PCC driver as Mailbox controller\n");
>> +     ret = mbox_controller_register(&pcc_mbox_ctrl);
>> +
>> +     if (ret) {
>> +             pr_err("Err registering PCC as Mailbox controller: %d\n", ret);
>> +             ret = -ENODEV;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +struct platform_driver pcc_mbox_driver = {
>> +     .probe = pcc_mbox_probe,
>> +     .driver = {
>> +             .name = "PCCT",
>> +             .owner = THIS_MODULE,
>> +     },
>> +};
>> +
>> +static int __init pcc_init(void)
>> +{
>> +     int ret;
>> +     struct platform_device *pcc_pdev;
>> +
>> +     if (acpi_disabled)
>> +             return -ENODEV;
>> +
>> +     /* Check if PCC support is available. */
>> +     ret = acpi_pcc_probe();
>> +
>> +     if (ret) {
>> +             pr_err("ACPI PCC probe failed.\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     pcc_pdev = platform_create_bundle(&pcc_mbox_driver,
>> +                     pcc_mbox_probe, NULL, 0, NULL, 0);
>> +
>> +     if (!pcc_pdev) {
>> +             pr_err("Err creating PCC platform bundle\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +device_initcall(pcc_init);
>>
>
> Care to add some kerneldoc comments to the functions?  Especially the ones
> that are then used as callbacks.

Sure.


Thanks,
Ashwin