[PATCH v2 0/2] i2c: xgene-slimpro: Add ACPI support

[PATCH v2 0/2] i2c: xgene-slimpro: Add ACPI support

[Hoan Tran]

This patch set adds ACPI support by using PCC mailbox communication interface.

v2:
 * Alphabeltical order
 * Use reinit_completion
 * Remove mutex
 * Use ENOENT instead of ENODEV

Hoan Tran (2):
  i2c: xgene-slimpro: Use a single function to send command message
  i2c: xgene-slimpro: Add ACPI support by using PCC mailbox

 drivers/i2c/busses/i2c-xgene-slimpro.c | 241 +++++++++++++++++++++++++--------
 1 file changed, 187 insertions(+), 54 deletions(-)

-- 
1.9.1

[PATCH v2 1/2] i2c: xgene-slimpro: Use a single function to send command message

[Hoan Tran]

This patch refactors the code to use a single message function to
send command message.

Signed-off-by: Hoan Tran <hotran@apm.com>
---
 drivers/i2c/busses/i2c-xgene-slimpro.c | 67 +++++++++++++---------------------
 1 file changed, 26 insertions(+), 41 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c b/drivers/i2c/busses/i2c-xgene-slimpro.c
index dbe7e44..96545aa 100644
--- a/drivers/i2c/busses/i2c-xgene-slimpro.c
+++ b/drivers/i2c/busses/i2c-xgene-slimpro.c
@@ -144,49 +144,52 @@ static int start_i2c_msg_xfer(struct slimpro_i2c_dev *ctx)
 	return 0;
 }
 
-static int slimpro_i2c_rd(struct slimpro_i2c_dev *ctx, u32 chip,
-			  u32 addr, u32 addrlen, u32 protocol,
-			  u32 readlen, u32 *data)
+static int slimpro_i2c_send_msg(struct slimpro_i2c_dev *ctx,
+				u32 *msg,
+				u32 *data)
 {
-	u32 msg[3];
 	int rc;
 
-	msg[0] = SLIMPRO_IIC_ENCODE_MSG(SLIMPRO_IIC_BUS, chip,
-					SLIMPRO_IIC_READ, protocol, addrlen, readlen);
-	msg[1] = SLIMPRO_IIC_ENCODE_ADDR(addr);
-	msg[2] = 0;
 	ctx->resp_msg = data;
-	rc = mbox_send_message(ctx->mbox_chan, &msg);
+
+	rc = mbox_send_message(ctx->mbox_chan, msg);
 	if (rc < 0)
 		goto err;
 
 	rc = start_i2c_msg_xfer(ctx);
+
 err:
 	ctx->resp_msg = NULL;
+
 	return rc;
 }
 
+static int slimpro_i2c_rd(struct slimpro_i2c_dev *ctx, u32 chip,
+			  u32 addr, u32 addrlen, u32 protocol,
+			  u32 readlen, u32 *data)
+{
+	u32 msg[3];
+
+	msg[0] = SLIMPRO_IIC_ENCODE_MSG(SLIMPRO_IIC_BUS, chip,
+					SLIMPRO_IIC_READ, protocol, addrlen, readlen);
+	msg[1] = SLIMPRO_IIC_ENCODE_ADDR(addr);
+	msg[2] = 0;
+
+	return slimpro_i2c_send_msg(ctx, msg, data);
+}
+
 static int slimpro_i2c_wr(struct slimpro_i2c_dev *ctx, u32 chip,
 			  u32 addr, u32 addrlen, u32 protocol, u32 writelen,
 			  u32 data)
 {
 	u32 msg[3];
-	int rc;
 
 	msg[0] = SLIMPRO_IIC_ENCODE_MSG(SLIMPRO_IIC_BUS, chip,
 					SLIMPRO_IIC_WRITE, protocol, addrlen, writelen);
 	msg[1] = SLIMPRO_IIC_ENCODE_ADDR(addr);
 	msg[2] = data;
-	ctx->resp_msg = msg;
-
-	rc = mbox_send_message(ctx->mbox_chan, &msg);
-	if (rc < 0)
-		goto err;
 
-	rc = start_i2c_msg_xfer(ctx);
-err:
-	ctx->resp_msg = NULL;
-	return rc;
+	return slimpro_i2c_send_msg(ctx, msg, msg);
 }
 
 static int slimpro_i2c_blkrd(struct slimpro_i2c_dev *ctx, u32 chip, u32 addr,
@@ -201,8 +204,7 @@ static int slimpro_i2c_blkrd(struct slimpro_i2c_dev *ctx, u32 chip, u32 addr,
 	if (dma_mapping_error(ctx->dev, paddr)) {
 		dev_err(&ctx->adapter.dev, "Error in mapping dma buffer %p\n",
 			ctx->dma_buffer);
-		rc = -ENOMEM;
-		goto err;
+		return -ENOMEM;
 	}
 
 	msg[0] = SLIMPRO_IIC_ENCODE_MSG(SLIMPRO_IIC_BUS, chip, SLIMPRO_IIC_READ,
@@ -212,21 +214,13 @@ static int slimpro_i2c_blkrd(struct slimpro_i2c_dev *ctx, u32 chip, u32 addr,
 		 SLIMPRO_IIC_ENCODE_UPPER_BUFADDR(paddr) |
 		 SLIMPRO_IIC_ENCODE_ADDR(addr);
 	msg[2] = (u32)paddr;
-	ctx->resp_msg = msg;
 
-	rc = mbox_send_message(ctx->mbox_chan, &msg);
-	if (rc < 0)
-		goto err_unmap;
-
-	rc = start_i2c_msg_xfer(ctx);
+	rc = slimpro_i2c_send_msg(ctx, msg, msg);
 
 	/* Copy to destination */
 	memcpy(data, ctx->dma_buffer, readlen);
 
-err_unmap:
 	dma_unmap_single(ctx->dev, paddr, readlen, DMA_FROM_DEVICE);
-err:
-	ctx->resp_msg = NULL;
 	return rc;
 }
 
@@ -244,8 +238,7 @@ static int slimpro_i2c_blkwr(struct slimpro_i2c_dev *ctx, u32 chip,
 	if (dma_mapping_error(ctx->dev, paddr)) {
 		dev_err(&ctx->adapter.dev, "Error in mapping dma buffer %p\n",
 			ctx->dma_buffer);
-		rc = -ENOMEM;
-		goto err;
+		return -ENOMEM;
 	}
 
 	msg[0] = SLIMPRO_IIC_ENCODE_MSG(SLIMPRO_IIC_BUS, chip, SLIMPRO_IIC_WRITE,
@@ -254,21 +247,13 @@ static int slimpro_i2c_blkwr(struct slimpro_i2c_dev *ctx, u32 chip,
 		 SLIMPRO_IIC_ENCODE_UPPER_BUFADDR(paddr) |
 		 SLIMPRO_IIC_ENCODE_ADDR(addr);
 	msg[2] = (u32)paddr;
-	ctx->resp_msg = msg;
 
 	if (ctx->mbox_client.tx_block)
 		reinit_completion(&ctx->rd_complete);
 
-	rc = mbox_send_message(ctx->mbox_chan, &msg);
-	if (rc < 0)
-		goto err_unmap;
-
-	rc = start_i2c_msg_xfer(ctx);
+	rc = slimpro_i2c_send_msg(ctx, msg, msg);
 
-err_unmap:
 	dma_unmap_single(ctx->dev, paddr, writelen, DMA_TO_DEVICE);
-err:
-	ctx->resp_msg = NULL;
 	return rc;
 }
 
-- 
1.9.1

[PATCH v2 2/2] i2c: xgene-slimpro: Add ACPI support by using PCC mailbox

[Hoan Tran]

This patch adds ACPI support by using PCC mailbox communication
interface.

Signed-off-by: Hoan Tran <hotran@apm.com>
---
 drivers/i2c/busses/i2c-xgene-slimpro.c | 174 ++++++++++++++++++++++++++++++---
 1 file changed, 161 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c b/drivers/i2c/busses/i2c-xgene-slimpro.c
index 96545aa..d81bf50 100644
--- a/drivers/i2c/busses/i2c-xgene-slimpro.c
+++ b/drivers/i2c/busses/i2c-xgene-slimpro.c
@@ -22,6 +22,7 @@
  * using the APM X-Gene SLIMpro mailbox driver.
  *
  */
+#include <acpi/pcc.h>
 #include <linux/acpi.h>
 #include <linux/dma-mapping.h>
 #include <linux/i2c.h>
@@ -89,6 +90,8 @@
 	((addrlen << SLIMPRO_IIC_ADDRLEN_SHIFT) & SLIMPRO_IIC_ADDRLEN_MASK) | \
 	((datalen << SLIMPRO_IIC_DATALEN_SHIFT) & SLIMPRO_IIC_DATALEN_MASK))
 
+#define SLIMPRO_MSG_TYPE(v)             (((v) & 0xF0000000) >> 28)
+
 /*
  * Encode for upper address for block data
  */
@@ -99,19 +102,47 @@
 								& 0x3FF00000))
 #define SLIMPRO_IIC_ENCODE_ADDR(a)			((a) & 0x000FFFFF)
 
+#define SLIMPRO_IIC_MSG_DWORD_COUNT			3
+
+/* PCC related defines */
+#define PCC_SIGNATURE			0x50424300
+#define PCC_STS_CMD_COMPLETE		BIT(0)
+#define PCC_STS_SCI_DOORBELL		BIT(1)
+#define PCC_STS_ERR			BIT(2)
+#define PCC_STS_PLAT_NOTIFY		BIT(3)
+#define PCC_CMD_GENERATE_DB_INT		BIT(15)
+
 struct slimpro_i2c_dev {
 	struct i2c_adapter adapter;
 	struct device *dev;
 	struct mbox_chan *mbox_chan;
 	struct mbox_client mbox_client;
+	int mbox_idx;
 	struct completion rd_complete;
 	u8 dma_buffer[I2C_SMBUS_BLOCK_MAX + 1]; /* dma_buffer[0] is used for length */
 	u32 *resp_msg;
+	phys_addr_t comm_base_addr;
+	void *pcc_comm_addr;
 };
 
 #define to_slimpro_i2c_dev(cl)	\
 		container_of(cl, struct slimpro_i2c_dev, mbox_client)
 
+/*
+ * This function tests and clears a bitmask then returns its old value
+ */
+static u16 xgene_word_tst_and_clr(u16 *addr, u16 mask)
+{
+	u16 ret, val;
+
+	val = le16_to_cpu(READ_ONCE(*addr));
+	ret = val & mask;
+	val &= ~mask;
+	WRITE_ONCE(*addr, cpu_to_le16(val));
+
+	return ret;
+}
+
 static void slimpro_i2c_rx_cb(struct mbox_client *cl, void *mssg)
 {
 	struct slimpro_i2c_dev *ctx = to_slimpro_i2c_dev(cl);
@@ -129,9 +160,53 @@ static void slimpro_i2c_rx_cb(struct mbox_client *cl, void *mssg)
 		complete(&ctx->rd_complete);
 }
 
+static void slimpro_i2c_pcc_rx_cb(struct mbox_client *cl, void *msg)
+{
+	struct slimpro_i2c_dev *ctx = to_slimpro_i2c_dev(cl);
+	struct acpi_pcct_shared_memory *generic_comm_base = ctx->pcc_comm_addr;
+
+	/* Check if platform sends interrupt */
+	if (!xgene_word_tst_and_clr(&generic_comm_base->status,
+				    PCC_STS_SCI_DOORBELL))
+		return;
+
+	if (xgene_word_tst_and_clr(&generic_comm_base->status,
+				   PCC_STS_CMD_COMPLETE)) {
+		msg = generic_comm_base + 1;
+
+		/* Response message msg[1] contains the return value. */
+		if (ctx->resp_msg)
+			*ctx->resp_msg = ((u32 *)msg)[1];
+
+		complete(&ctx->rd_complete);
+	}
+}
+
+static void slimpro_i2c_pcc_tx_prepare(struct slimpro_i2c_dev *ctx, u32 *msg)
+{
+	struct acpi_pcct_shared_memory *generic_comm_base = ctx->pcc_comm_addr;
+	u32 *ptr = (void *)(generic_comm_base + 1);
+	u16 status;
+	int i;
+
+	WRITE_ONCE(generic_comm_base->signature,
+		   cpu_to_le32(PCC_SIGNATURE | ctx->mbox_idx));
+
+	WRITE_ONCE(generic_comm_base->command,
+		   cpu_to_le16(SLIMPRO_MSG_TYPE(msg[0]) | PCC_CMD_GENERATE_DB_INT));
+
+	status = le16_to_cpu(READ_ONCE(generic_comm_base->status));
+	status &= ~PCC_STS_CMD_COMPLETE;
+	WRITE_ONCE(generic_comm_base->status, cpu_to_le16(status));
+
+	/* Copy the message to the PCC comm space */
+	for (i = 0; i < SLIMPRO_IIC_MSG_DWORD_COUNT; i++)
+		WRITE_ONCE(ptr[i], cpu_to_le32(msg[i]));
+}
+
 static int start_i2c_msg_xfer(struct slimpro_i2c_dev *ctx)
 {
-	if (ctx->mbox_client.tx_block) {
+	if (ctx->mbox_client.tx_block || !acpi_disabled) {
 		if (!wait_for_completion_timeout(&ctx->rd_complete,
 						 msecs_to_jiffies(MAILBOX_OP_TIMEOUT)))
 			return -ETIMEDOUT;
@@ -152,6 +227,11 @@ static int slimpro_i2c_send_msg(struct slimpro_i2c_dev *ctx,
 
 	ctx->resp_msg = data;
 
+	if (!acpi_disabled) {
+		reinit_completion(&ctx->rd_complete);
+		slimpro_i2c_pcc_tx_prepare(ctx, msg);
+	}
+
 	rc = mbox_send_message(ctx->mbox_chan, msg);
 	if (rc < 0)
 		goto err;
@@ -159,6 +239,9 @@ static int slimpro_i2c_send_msg(struct slimpro_i2c_dev *ctx,
 	rc = start_i2c_msg_xfer(ctx);
 
 err:
+	if (!acpi_disabled) {
+		mbox_chan_txdone(ctx->mbox_chan, 0);
+	}
 	ctx->resp_msg = NULL;
 
 	return rc;
@@ -379,17 +462,73 @@ static int xgene_slimpro_i2c_probe(struct platform_device *pdev)
 
 	/* Request mailbox channel */
 	cl->dev = &pdev->dev;
-	cl->rx_callback = slimpro_i2c_rx_cb;
-	cl->tx_block = true;
 	init_completion(&ctx->rd_complete);
 	cl->tx_tout = MAILBOX_OP_TIMEOUT;
 	cl->knows_txdone = false;
-	ctx->mbox_chan = mbox_request_channel(cl, MAILBOX_I2C_INDEX);
-	if (IS_ERR(ctx->mbox_chan)) {
-		dev_err(&pdev->dev, "i2c mailbox channel request failed\n");
-		return PTR_ERR(ctx->mbox_chan);
-	}
+	if (acpi_disabled) {
+		cl->tx_block = true;
+		cl->rx_callback = slimpro_i2c_rx_cb;
+		ctx->mbox_chan = mbox_request_channel(cl, MAILBOX_I2C_INDEX);
+		if (IS_ERR(ctx->mbox_chan)) {
+			dev_err(&pdev->dev, "i2c mailbox channel request failed\n");
+			return PTR_ERR(ctx->mbox_chan);
+		}
+	} else {
+		struct acpi_pcct_hw_reduced *cppc_ss;
+
+		if (device_property_read_u32(&pdev->dev, "pcc-channel",
+					     &ctx->mbox_idx))
+			ctx->mbox_idx = MAILBOX_I2C_INDEX;
+
+		cl->tx_block = false;
+		cl->rx_callback = slimpro_i2c_pcc_rx_cb;
+		ctx->mbox_chan = pcc_mbox_request_channel(cl, ctx->mbox_idx);
+		if (IS_ERR(ctx->mbox_chan)) {
+			dev_err(&pdev->dev, "PCC mailbox channel request failed\n");
+			return PTR_ERR(ctx->mbox_chan);
+		}
+
+		/*
+		 * The PCC mailbox controller driver should
+		 * have parsed the PCCT (global table of all
+		 * PCC channels) and stored pointers to the
+		 * subspace communication region in con_priv.
+		 */
+		cppc_ss = ctx->mbox_chan->con_priv;
+		if (!cppc_ss) {
+			dev_err(&pdev->dev, "PPC subspace not found\n");
+			rc = -ENOENT;
+			goto mbox_err;
+		}
+
+		if (!ctx->mbox_chan->mbox->txdone_irq) {
+			dev_err(&pdev->dev, "PCC IRQ not supported\n");
+			rc = -ENOENT;
+			goto mbox_err;
+		}
 
+		/*
+		 * This is the shared communication region
+		 * for the OS and Platform to communicate over.
+		 */
+		ctx->comm_base_addr = cppc_ss->base_address;
+		if (ctx->comm_base_addr) {
+			ctx->pcc_comm_addr = memremap(ctx->comm_base_addr,
+						      cppc_ss->length,
+						      MEMREMAP_WB);
+		} else {
+			dev_err(&pdev->dev, "Failed to get PCC comm region\n");
+			rc = -ENOENT;
+			goto mbox_err;
+		}
+
+		if (!ctx->pcc_comm_addr) {
+			dev_err(&pdev->dev,
+				"Failed to ioremap PCC comm region\n");
+			rc = -ENOMEM;
+			goto mbox_err;
+		}
+	}
 	rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (rc)
 		dev_warn(&pdev->dev, "Unable to set dma mask\n");
@@ -403,13 +542,19 @@ static int xgene_slimpro_i2c_probe(struct platform_device *pdev)
 	adapter->dev.of_node = pdev->dev.of_node;
 	i2c_set_adapdata(adapter, ctx);
 	rc = i2c_add_adapter(adapter);
-	if (rc) {
-		mbox_free_channel(ctx->mbox_chan);
-		return rc;
-	}
+	if (rc)
+		goto mbox_err;
 
 	dev_info(&pdev->dev, "Mailbox I2C Adapter registered\n");
 	return 0;
+
+mbox_err:
+	if (acpi_disabled)
+		mbox_free_channel(ctx->mbox_chan);
+	else
+		pcc_mbox_free_channel(ctx->mbox_chan);
+
+	return rc;
 }
 
 static int xgene_slimpro_i2c_remove(struct platform_device *pdev)
@@ -418,7 +563,10 @@ static int xgene_slimpro_i2c_remove(struct platform_device *pdev)
 
 	i2c_del_adapter(&ctx->adapter);
 
-	mbox_free_channel(ctx->mbox_chan);
+	if (acpi_disabled)
+		mbox_free_channel(ctx->mbox_chan);
+	else
+		pcc_mbox_free_channel(ctx->mbox_chan);
 
 	return 0;
 }
-- 
1.9.1

Re: [PATCH v2 0/2] i2c: xgene-slimpro: Add ACPI support

[Hoan Tran]

Hi All,

Do you have any comments on this patch set?

Thanks
Hoan

On Mon, Apr 24, 2017 at 11:00 AM, Hoan Tran <hotran@apm.com> wrote:
> This patch set adds ACPI support by using PCC mailbox communication interface.
>
> v2:
>  * Alphabeltical order
>  * Use reinit_completion
>  * Remove mutex
>  * Use ENOENT instead of ENODEV
>
> Hoan Tran (2):
>   i2c: xgene-slimpro: Use a single function to send command message
>   i2c: xgene-slimpro: Add ACPI support by using PCC mailbox
>
>  drivers/i2c/busses/i2c-xgene-slimpro.c | 241 +++++++++++++++++++++++++--------
>  1 file changed, 187 insertions(+), 54 deletions(-)
>
> --
> 1.9.1
>

Re: [PATCH v2 1/2] i2c: xgene-slimpro: Use a single function to send command message

[Wolfram Sang]

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

On Mon, Apr 24, 2017 at 11:00:25AM -0700, Hoan Tran wrote:
> This patch refactors the code to use a single message function to
> send command message.
> 
> Signed-off-by: Hoan Tran <hotran@apm.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 2/2] i2c: xgene-slimpro: Add ACPI support by using PCC mailbox

[Wolfram Sang]

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

On Mon, Apr 24, 2017 at 11:00:26AM -0700, Hoan Tran wrote:
> This patch adds ACPI support by using PCC mailbox communication
> interface.
> 
> Signed-off-by: Hoan Tran <hotran@apm.com>

Please make use checkpatch:

WARNING: braces {} are not necessary for single statement blocks
#149: FILE: drivers/i2c/busses/i2c-xgene-slimpro.c:242:
+	if (!acpi_disabled) {
+		mbox_chan_txdone(ctx->mbox_chan, 0);
+	}

Fixed that and applied to for-next, thanks!

BTW is there anyone inside apm.com who is willing to take over maintainership
for this driver? This person needs to review patches thoroughly, yet it usually
means patches will go in faster.

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 2/2] i2c: xgene-slimpro: Add ACPI support by using PCC mailbox

[Loc Ho]

Hi Wolfram,

>> This patch adds ACPI support by using PCC mailbox communication
>> interface.
>>
>> Signed-off-by: Hoan Tran <hotran@apm.com>
>
> Please make use checkpatch:
>
> WARNING: braces {} are not necessary for single statement blocks
> #149: FILE: drivers/i2c/busses/i2c-xgene-slimpro.c:242:
> +       if (!acpi_disabled) {
> +               mbox_chan_txdone(ctx->mbox_chan, 0);
> +       }
>
> Fixed that and applied to for-next, thanks!
>
> BTW is there anyone inside apm.com who is willing to take over maintainership
> for this driver? This person needs to review patches thoroughly, yet it usually
> means patches will go in faster.

Thanks for fixing the warning message. Given that Duc has left MACOM
(previously APM), we are ramping up another person.

Thanks,
Loc

Re: [PATCH v2 2/2] i2c: xgene-slimpro: Add ACPI support by using PCC mailbox

[Hoan Tran]

On Fri, Jun 2, 2017 at 1:31 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Mon, Apr 24, 2017 at 11:00:26AM -0700, Hoan Tran wrote:
>> This patch adds ACPI support by using PCC mailbox communication
>> interface.
>>
>> Signed-off-by: Hoan Tran <hotran@apm.com>
>
> Please make use checkpatch:
>
> WARNING: braces {} are not necessary for single statement blocks
> #149: FILE: drivers/i2c/busses/i2c-xgene-slimpro.c:242:
> +       if (!acpi_disabled) {
> +               mbox_chan_txdone(ctx->mbox_chan, 0);
> +       }
>
> Fixed that and applied to for-next, thanks!

Thanks, Wolfram!

Regards,
Hoan

>
> BTW is there anyone inside apm.com who is willing to take over maintainership
> for this driver? This person needs to review patches thoroughly, yet it usually
> means patches will go in faster.
>
> Regards,
>
>    Wolfram
>