[PATCH v9] PCC: Platform Communication Channel

[PATCH v9] PCC: Platform Communication Channel

[Ashwin Chaugule]

Changes since V8:
- Removed unncessary header files.
- Added kerneldoc comments.
- Added intro about PCC in pcc.c

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 (1):
  Mailbox: Add support for Platform Communication Channel

 drivers/mailbox/Kconfig   |  12 ++
 drivers/mailbox/Makefile  |   2 +
 drivers/mailbox/mailbox.c |   4 +-
 drivers/mailbox/mailbox.h |  16 ++
 drivers/mailbox/pcc.c     | 369 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 400 insertions(+), 3 deletions(-)
 create mode 100644 drivers/mailbox/mailbox.h
 create mode 100644 drivers/mailbox/pcc.c

-- 
1.9.1

[PATCH v9] 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.

Reviewed-by: Mark Brown <broonie@kernel.org>
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     | 369 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 400 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 9fd9c67..c04fed9 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -33,4 +33,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 94ed7ce..dd412c2 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -5,3 +5,5 @@ obj-$(CONFIG_MAILBOX)		+= mailbox.o
 obj-$(CONFIG_PL320_MBOX)	+= pl320-ipc.o
 
 obj-$(CONFIG_OMAP2PLUS_MBOX)	+= omap-mailbox.o
+
+obj-$(CONFIG_PCC)		+= pcc.o
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index afcb430..17e9e4a 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..0006fc3
--- /dev/null
+++ b/drivers/mailbox/pcc.c
@@ -0,0 +1,369 @@
+/*
+ *	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.
+ *
+ *  PCC (Platform Communication Channel) is defined in the ACPI 5.0+
+ *  specification. It is a mailbox like mechanism to allow clients
+ *  such as CPPC (Collaborative Processor Performance Control), RAS
+ *  (Reliability, Availability and Serviceability) and MPST (Memory
+ *  Node Power State Table) to talk to the platform (e.g. BMC) through
+ *  shared memory regions as defined in the PCC table entries. The PCC
+ *  specification supports a Doorbell mechanism for the PCC clients
+ *  to notify the platform about new data. This Doorbell information
+ *  is also specified in each PCC table entry. See pcc_send_data()
+ *  and pcc_tx_done() for basic mode of operation.
+ *
+ *  For more details about PCC, please see the ACPI specification from
+ *  http://www.uefi.org/ACPIv5.1 Section 14.
+ *
+ *  This file implements PCC as a Mailbox controller and allows for PCC
+ *  clients to be implemented as its Mailbox Client Channels.
+ */
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/init.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 = {};
+
+/**
+ * pcc_mbox_request_channel - PCC clients call this function to
+ *		request a pointer to their PCC subspace, from which they
+ *		can get the details of communicating with the remote.
+ * @cl: Pointer to Mailbox client, so we know where to bind the
+ *		Channel.
+ * @index: The PCC Subspace index as parsed in the PCC client
+ *		ACPI package. This is used to lookup the array of PCC
+ *		subspaces as parsed by the PCC Mailbox controller.
+ *
+ * Return: Pointer to the Mailbox Channel if successful or
+ *		ERR_PTR.
+ */
+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);
+
+/**
+ * pcc_mbox_free_channel - Clients call this to free their Channel.
+ *
+ * @chan: Pointer to the mailbox channel as returned by
+ *		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);
+
+/**
+ * pcc_tx_done - Callback from Mailbox controller code to
+ *		check if PCC message transmission completed.
+ * @chan: Pointer to Mailbox channel on which previous
+ *		transmission occurred.
+ *
+ * Return: TRUE if succeeded.
+ */
+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;
+}
+
+/**
+ * get_subspace_id - Given a Mailbox channel, find out the
+ *		PCC subspace id.
+ * @chan: Pointer to Mailbox Channel from which we want
+ *		the index.
+ * Return: Errno if not found, else positive index number.
+ */
+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;
+}
+
+/**
+ * pcc_send_data - Called from Mailbox Controller code to finally
+ *	transmit data over channel.
+ * @chan: Pointer to Mailbox channel over which to send data.
+ * @data: Actual data to be written over channel.
+ *
+ * Return: Err if something failed else 0 for success.
+ */
+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,
+};
+
+/**
+ * parse_pcc_subspace - Parse the PCC table and extract PCC subspace
+ *		entries. There should be one entry per PCC client.
+ * @header: Pointer to the ACPI subtable header under the PCCT.
+ * @end: End of subtable entry.
+ *
+ * Return: 0 for Success, else errno.
+ *
+ * This gets called for each entry in the PCC table.
+ */
+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;
+}
+
+/**
+ * acpi_pcc_probe - Parse the ACPI tree for the PCCT.
+ *
+ * Return: 0 for Success, else errno.
+ */
+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;
+}
+
+/**
+ * pcc_mbox_probe - Called when we find a match for the
+ *	PCCT platform device. This is purely used to represent
+ *	the PCCT as a virtual device for registering with the
+ *	generic Mailbox framework.
+ *
+ * @pdev: Pointer to platform device returned when a match
+ *	is found.
+ *
+ *	Return: 0 for Success, else errno.
+ */
+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

Re: [PATCH v9] Mailbox: Add support for Platform Communication Channel

[Ashwin Chaugule]

Hello,

On 10 October 2014 16:13, Ashwin Chaugule <ashwin.chaugule@linaro.org> 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.
>
> Reviewed-by: Mark Brown <broonie@kernel.org>
> 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     | 369 ++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 400 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 9fd9c67..c04fed9 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -33,4 +33,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

Looks like Linus has agreed to pull in the Mailbox framework in 3.18
[1], so I was hoping we could move this patch forward.

When I started working on the PCC driver, I based it off of ACPI v5.0.
Since then the spec moved to v5.1 and added a trivial change to the
PCC section. This change adds a new subtable to support systems that
dont have an SCI for the doorbell irq. The new structure declaration
is already in mainline [2]. Given that there were no existing users of
PCC so far and that the future users are more likely to use the new
subtable, I'd like to include it here and make it the default from the
start. SCI support can be added later if it is needed. The change to
do this is also trivial and the diff is below.

Mark, are you okay with squashing this diff into the original patch
[v9] and keeping your reviewed-by tag? I can then send a git pull
request.

Arnd, Rafael, if there are no further comments on this patch, kindly
advise which tree to rebase on for the git pull.

--------------------8<-----------------------

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 0006fc3..592c6fa 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -130,7 +130,7 @@ 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_hw_reduced *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;
@@ -182,7 +182,7 @@ static int get_subspace_id(struct mbox_chan *chan)
  */
 static int pcc_send_data(struct mbox_chan *chan, void *data)
 {
-    struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
+    struct acpi_pcct_hw_reduced *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;
@@ -191,14 +191,12 @@ static int pcc_send_data(struct mbox_chan *chan,
void *data)
     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;
+        return -EINVAL;
     }

     doorbell = pcct_ss->doorbell_register;
@@ -219,8 +217,7 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
     acpi_write((doorbell_val & doorbell_preserve) | doorbell_write,
             &doorbell);

-out_err:
-    return ret;
+    return 0;
 }

 static struct mbox_chan_ops pcc_chan_ops = {
@@ -241,12 +238,12 @@ static struct mbox_chan_ops pcc_chan_ops = {
 static int parse_pcc_subspace(struct acpi_subtable_header *header,
         const unsigned long end)
 {
-    struct acpi_pcct_subspace *pcct_ss;
+    struct acpi_pcct_hw_reduced *pcct_ss;

     if (pcc_mbox_ctrl.num_chans <= MAX_PCC_SUBSPACES) {
-        pcct_ss = (struct acpi_pcct_subspace *) header;
+        pcct_ss = (struct acpi_pcct_hw_reduced *) header;

-        if (pcct_ss->header.type != ACPI_PCCT_TYPE_GENERIC_SUBSPACE) {
+        if (pcct_ss->header.type != ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE) {
             pr_err("Incorrect PCC Subspace type detected\n");
             return -EINVAL;
         }
@@ -287,7 +284,7 @@ static int __init acpi_pcc_probe(void)

     count = acpi_table_parse_entries(ACPI_SIG_PCCT,
             sizeof(struct acpi_table_pcct),
-            ACPI_PCCT_TYPE_GENERIC_SUBSPACE,
+            ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE,
             parse_pcc_subspace, MAX_PCC_SUBSPACES);

     if (count <= 0) {

---------------8------------------------


Thanks,
Ashwin

[1] - https://lkml.org/lkml/2014/10/21/660
[2] - https://lkml.org/lkml/2014/7/30/9

Re: [PATCH v9] Mailbox: Add support for Platform Communication Channel

[Ashwin Chaugule]

Hi,

On 23 October 2014 11:36, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
> Hello,
>
> On 10 October 2014 16:13, Ashwin Chaugule <ashwin.chaugule@linaro.org> 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.
>>
>> Reviewed-by: Mark Brown <broonie@kernel.org>
>> 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     | 369 ++++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 400 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 9fd9c67..c04fed9 100644
>> --- a/drivers/mailbox/Kconfig
>> +++ b/drivers/mailbox/Kconfig
>> @@ -33,4 +33,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
>
> Looks like Linus has agreed to pull in the Mailbox framework in 3.18
> [1], so I was hoping we could move this patch forward.
>
> When I started working on the PCC driver, I based it off of ACPI v5.0.
> Since then the spec moved to v5.1 and added a trivial change to the
> PCC section. This change adds a new subtable to support systems that
> dont have an SCI for the doorbell irq. The new structure declaration
> is already in mainline [2]. Given that there were no existing users of
> PCC so far and that the future users are more likely to use the new
> subtable, I'd like to include it here and make it the default from the
> start. SCI support can be added later if it is needed. The change to
> do this is also trivial and the diff is below.
>
> Mark, are you okay with squashing this diff into the original patch
> [v9] and keeping your reviewed-by tag? I can then send a git pull
> request.
>
> Arnd, Rafael, if there are no further comments on this patch, kindly
> advise which tree to rebase on for the git pull.

Guessing there are no objections to this patch and the diff? Can we
please merge this upstream?

Thanks,
Ashwin

>
> --------------------8<-----------------------
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 0006fc3..592c6fa 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -130,7 +130,7 @@ 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_hw_reduced *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;
> @@ -182,7 +182,7 @@ static int get_subspace_id(struct mbox_chan *chan)
>   */
>  static int pcc_send_data(struct mbox_chan *chan, void *data)
>  {
> -    struct acpi_pcct_subspace *pcct_ss = chan->con_priv;
> +    struct acpi_pcct_hw_reduced *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;
> @@ -191,14 +191,12 @@ static int pcc_send_data(struct mbox_chan *chan,
> void *data)
>      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;
> +        return -EINVAL;
>      }
>
>      doorbell = pcct_ss->doorbell_register;
> @@ -219,8 +217,7 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
>      acpi_write((doorbell_val & doorbell_preserve) | doorbell_write,
>              &doorbell);
>
> -out_err:
> -    return ret;
> +    return 0;
>  }
>
>  static struct mbox_chan_ops pcc_chan_ops = {
> @@ -241,12 +238,12 @@ static struct mbox_chan_ops pcc_chan_ops = {
>  static int parse_pcc_subspace(struct acpi_subtable_header *header,
>          const unsigned long end)
>  {
> -    struct acpi_pcct_subspace *pcct_ss;
> +    struct acpi_pcct_hw_reduced *pcct_ss;
>
>      if (pcc_mbox_ctrl.num_chans <= MAX_PCC_SUBSPACES) {
> -        pcct_ss = (struct acpi_pcct_subspace *) header;
> +        pcct_ss = (struct acpi_pcct_hw_reduced *) header;
>
> -        if (pcct_ss->header.type != ACPI_PCCT_TYPE_GENERIC_SUBSPACE) {
> +        if (pcct_ss->header.type != ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE) {
>              pr_err("Incorrect PCC Subspace type detected\n");
>              return -EINVAL;
>          }
> @@ -287,7 +284,7 @@ static int __init acpi_pcc_probe(void)
>
>      count = acpi_table_parse_entries(ACPI_SIG_PCCT,
>              sizeof(struct acpi_table_pcct),
> -            ACPI_PCCT_TYPE_GENERIC_SUBSPACE,
> +            ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE,
>              parse_pcc_subspace, MAX_PCC_SUBSPACES);
>
>      if (count <= 0) {
>
> ---------------8------------------------
>
>
> Thanks,
> Ashwin
>
> [1] - https://lkml.org/lkml/2014/10/21/660
> [2] - https://lkml.org/lkml/2014/7/30/9