linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATH v7 0/2] Xilinx ZynqMP IPI Mailbox Controller Driver
@ 2018-12-20 17:32 Wendy Liang
  2018-12-20 17:32 ` [PATH v7 1/2] mailbox: ZynqMP IPI mailbox controller Wendy Liang
  2018-12-20 17:32 ` [PATH v7 2/2] dt-bindings: mailbox: Add Xilinx IPI Mailbox Wendy Liang
  0 siblings, 2 replies; 5+ messages in thread
From: Wendy Liang @ 2018-12-20 17:32 UTC (permalink / raw)
  To: jassisinghbrar, michal.simek, robh+dt, mark.rutland
  Cc: linux-kernel, linux-arm-kernel, devicetree, Wendy Liang

Introduce mailbox controller driver for ZynqMP IPI(Inter-processor
interrupt) IP core.

As the device tree bindings have been updated. Do not have "Reviewed-by"
nor "Acked-by" in the dt-bindings commit.

v7:
 - Fix sparse warning, and documentation check.
 - Add emply block to the end of of_device_id.

v6:
 - dts-binding, remove compatible property from IPI subnode

v5:
 - fix check patch warning on write a paragraph to describe the kconfig
   symbol.

v4:
 - make IPI mailboxes as subnodes to the IPI agent device node to properly
   describe the hardware.

v3:
 - add NULL entry to of_device_id of IPI controller

v2:
 - change SPDX-License-Identifier license text style in .c file
 - replace xlnx-ipi-ids with xlnx,ipi-ids


Wendy Liang (2):
  mailbox: ZynqMP IPI mailbox controller
  dt-bindings: mailbox: Add Xilinx IPI Mailbox

 .../bindings/mailbox/xlnx,zynqmp-ipi-mailbox.txt   | 127 ++++
 drivers/mailbox/Kconfig                            |  11 +
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/zynqmp-ipi-mailbox.c               | 764 +++++++++++++++++++++
 4 files changed, 904 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.txt
 create mode 100644 drivers/mailbox/zynqmp-ipi-mailbox.c

-- 
2.7.4


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

* [PATH v7 1/2] mailbox: ZynqMP IPI mailbox controller
  2018-12-20 17:32 [PATH v7 0/2] Xilinx ZynqMP IPI Mailbox Controller Driver Wendy Liang
@ 2018-12-20 17:32 ` Wendy Liang
  2018-12-22  1:59   ` Jassi Brar
  2018-12-20 17:32 ` [PATH v7 2/2] dt-bindings: mailbox: Add Xilinx IPI Mailbox Wendy Liang
  1 sibling, 1 reply; 5+ messages in thread
From: Wendy Liang @ 2018-12-20 17:32 UTC (permalink / raw)
  To: jassisinghbrar, michal.simek, robh+dt, mark.rutland
  Cc: linux-kernel, linux-arm-kernel, devicetree, Wendy Liang

This patch is to introduce ZynqMP IPI mailbox controller driver
to use the ZynqMP IPI block as mailboxes.

Signed-off-by: Wendy Liang <wendy.liang@xilinx.com>
---
 drivers/mailbox/Kconfig              |  11 +
 drivers/mailbox/Makefile             |   2 +
 drivers/mailbox/zynqmp-ipi-mailbox.c | 764 +++++++++++++++++++++++++++++++++++
 3 files changed, 777 insertions(+)
 create mode 100644 drivers/mailbox/zynqmp-ipi-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 3eeb12e9..d86e7a4 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -205,4 +205,15 @@ config MTK_CMDQ_MBOX
 	  mailbox driver. The CMDQ is used to help read/write registers with
 	  critical time limitation, such as updating display configuration
 	  during the vblank.
+
+config ZYNQMP_IPI_MBOX
+	bool "Xilinx ZynqMP IPI Mailbox"
+	depends on ARCH_ZYNQMP && OF
+	help
+	  Say yes here to add support for Xilinx IPI mailbox driver.
+	  This mailbox driver is used to send notification or short message
+	  between processors with Xilinx ZynqMP IPI. It will place the
+	  message to the IPI buffer and will access the IPI control
+	  registers to kick the other processor or enquire status.
+
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index c818b5d..8be3bcb 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -44,3 +44,5 @@ obj-$(CONFIG_TEGRA_HSP_MBOX)	+= tegra-hsp.o
 obj-$(CONFIG_STM32_IPCC) 	+= stm32-ipcc.o
 
 obj-$(CONFIG_MTK_CMDQ_MBOX)	+= mtk-cmdq-mailbox.o
+
+obj-$(CONFIG_ZYNQMP_IPI_MBOX)	+= zynqmp-ipi-mailbox.o
diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
new file mode 100644
index 0000000..bbddfd5
--- /dev/null
+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
@@ -0,0 +1,764 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx Inter Processor Interrupt(IPI) Mailbox Driver
+ *
+ * Copyright (C) 2018 Xilinx, Inc.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox/zynqmp-ipi-message.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+
+/* IPI agent ID any */
+#define IPI_ID_ANY 0xFFUL
+
+/* indicate if ZynqMP IPI mailbox driver uses SMC calls or HVC calls */
+#define USE_SMC 0
+#define USE_HVC 1
+
+/* Default IPI SMC function IDs */
+#define SMC_IPI_MAILBOX_OPEN		0x82001000U
+#define SMC_IPI_MAILBOX_RELEASE		0x82001001U
+#define SMC_IPI_MAILBOX_STATUS_ENQUIRY	0x82001002U
+#define SMC_IPI_MAILBOX_NOTIFY		0x82001003U
+#define SMC_IPI_MAILBOX_ACK		0x82001004U
+#define SMC_IPI_MAILBOX_ENABLE_IRQ	0x82001005U
+#define SMC_IPI_MAILBOX_DISABLE_IRQ	0x82001006U
+
+/* IPI SMC Macros */
+#define IPI_SMC_OPEN_IRQ_MASK		0x00000001UL /* IRQ enable bit in IPI
+						      * open SMC call
+						      */
+#define IPI_SMC_NOTIFY_BLOCK_MASK	0x00000001UL /* Flag to indicate if
+						      * IPI notification needs
+						      * to be blocking.
+						      */
+#define IPI_SMC_ENQUIRY_DIRQ_MASK	0x00000001UL /* Flag to indicate if
+						      * notification interrupt
+						      * to be disabled.
+						      */
+#define IPI_SMC_ACK_EIRQ_MASK		0x00000001UL /* Flag to indicate if
+						      * notification interrupt
+						      * to be enabled.
+						      */
+
+/* IPI mailbox status */
+#define IPI_MB_STATUS_IDLE		0
+#define IPI_MB_STATUS_SEND_PENDING	1
+#define IPI_MB_STATUS_RECV_PENDING	2
+
+#define IPI_MB_CHNL_TX	0 /* IPI mailbox TX channel */
+#define IPI_MB_CHNL_RX	1 /* IPI mailbox RX channel */
+
+/**
+ * struct zynqmp_ipi_mchan - Description of a Xilinx ZynqMP IPI mailbox channel
+ * @is_opened: indicate if the IPI channel is opened
+ * @req_buf: local to remote request buffer start address
+ * @resp_buf: local to remote response buffer start address
+ * @req_buf_size: request buffer size
+ * @resp_buf_size: response buffer size
+ * @rx_buf: receive buffer to pass received message to client
+ * @chan_type: channel type
+ */
+struct zynqmp_ipi_mchan {
+	int is_opened;
+	void __iomem *req_buf;
+	void __iomem *resp_buf;
+	void *rx_buf;
+	size_t req_buf_size;
+	size_t resp_buf_size;
+	unsigned int chan_type;
+};
+
+/**
+ * struct zynqmp_ipi_mbox - Description of a ZynqMP IPI mailbox
+ *                          platform data.
+ * @pdata:		  pointer to the IPI private data
+ * @dev:                  device pointer corresponding to the Xilinx ZynqMP
+ *                        IPI mailbox
+ * @remote_id:            remote IPI agent ID
+ * @mbox:                 mailbox Controller
+ * @mchans:               array for channels, tx channel and rx channel.
+ * @irq:                  IPI agent interrupt ID
+ */
+struct zynqmp_ipi_mbox {
+	struct zynqmp_ipi_pdata *pdata;
+	struct device dev;
+	u32 remote_id;
+	struct mbox_controller mbox;
+	struct zynqmp_ipi_mchan mchans[2];
+};
+
+/**
+ * struct zynqmp_ipi_pdata - Description of z ZynqMP IPI agent platform data.
+ *
+ * @dev:                  device pointer corresponding to the Xilinx ZynqMP
+ *                        IPI agent
+ * @irq:                  IPI agent interrupt ID
+ * @method:               IPI SMC or HVC is going to be used
+ * @local_id:             local IPI agent ID
+ * @num_mboxes:           number of mailboxes of this IPI agent
+ * @ipi_mboxes:           IPI mailboxes of this IPI agent
+ */
+struct zynqmp_ipi_pdata {
+	struct device *dev;
+	int irq;
+	unsigned int method;
+	u32 local_id;
+	int num_mboxes;
+	struct zynqmp_ipi_mbox *ipi_mboxes;
+};
+
+static struct device_driver zynqmp_ipi_mbox_driver = {
+	.owner = THIS_MODULE,
+	.name = "zynqmp-ipi-mbox",
+};
+
+static void zynqmp_ipi_fw_call(struct zynqmp_ipi_mbox *ipi_mbox,
+			       unsigned long a0, unsigned long a3,
+			       unsigned long a4, unsigned long a5,
+			       unsigned long a6, unsigned long a7,
+			       struct arm_smccc_res *res)
+{
+	struct zynqmp_ipi_pdata *pdata = ipi_mbox->pdata;
+	unsigned long a1, a2;
+
+	a1 = pdata->local_id;
+	a2 = ipi_mbox->remote_id;
+	if (pdata->method == USE_SMC)
+		arm_smccc_smc(a0, a1, a2, a3, a4, a5, a6, a7, res);
+	else
+		arm_smccc_hvc(a0, a1, a2, a3, a4, a5, a6, a7, res);
+}
+
+/**
+ * zynqmp_ipi_interrupt - Interrupt handler for IPI notification
+ *
+ * @irq:  Interrupt number
+ * @data: ZynqMP IPI mailbox platform data.
+ *
+ * Return: -EINVAL if there is no instance
+ * IRQ_NONE if the interrupt is not ours.
+ * IRQ_HANDLED if the rx interrupt was successfully handled.
+ */
+static irqreturn_t zynqmp_ipi_interrupt(int irq, void *data)
+{
+	struct zynqmp_ipi_pdata *pdata = data;
+	struct mbox_chan *chan;
+	struct zynqmp_ipi_mbox *ipi_mbox;
+	struct zynqmp_ipi_mchan *mchan;
+	struct zynqmp_ipi_message *msg;
+	u64 arg0, arg3;
+	struct arm_smccc_res res;
+	int ret, i;
+
+	(void)irq;
+	arg0 = SMC_IPI_MAILBOX_STATUS_ENQUIRY;
+	arg3 = IPI_SMC_ENQUIRY_DIRQ_MASK;
+	for (i = 0; i < pdata->num_mboxes; i++) {
+		ipi_mbox = &pdata->ipi_mboxes[i];
+		mchan = &ipi_mbox->mchans[IPI_MB_CHNL_RX];
+		chan = &ipi_mbox->mbox.chans[IPI_MB_CHNL_RX];
+		zynqmp_ipi_fw_call(ipi_mbox, arg0, arg3, 0, 0, 0, 0, &res);
+		ret = (int)(res.a0 & 0xFFFFFFFF);
+		if (ret > 0 && ret & IPI_MB_STATUS_RECV_PENDING) {
+			if (mchan->is_opened) {
+				msg = mchan->rx_buf;
+				msg->len = mchan->req_buf_size;
+				memcpy_fromio(msg->data, mchan->req_buf,
+					      msg->len);
+				mbox_chan_received_data(chan, (void *)msg);
+				return IRQ_HANDLED;
+			}
+		}
+	}
+	return IRQ_NONE;
+}
+
+/**
+ * zynqmp_ipi_peek_data - Peek to see if there are any rx messages.
+ *
+ * @chan: Channel Pointer
+ *
+ * Return: 'true' if there is pending rx data, 'false' if there is none.
+ */
+static bool zynqmp_ipi_peek_data(struct mbox_chan *chan)
+{
+	struct device *dev = chan->mbox->dev;
+	struct zynqmp_ipi_mbox *ipi_mbox = dev_get_drvdata(dev);
+	struct zynqmp_ipi_mchan *mchan = chan->con_priv;
+	int ret;
+	u64 arg0;
+	struct arm_smccc_res res;
+
+	if (WARN_ON(!ipi_mbox)) {
+		dev_err(dev, "no platform drv data??\n");
+		return false;
+	}
+
+	arg0 = SMC_IPI_MAILBOX_STATUS_ENQUIRY;
+	zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
+	ret = (int)(res.a0 & 0xFFFFFFFF);
+
+	if (mchan->chan_type == IPI_MB_CHNL_TX) {
+		/* TX channel, check if the message has been acked
+		 * by the remote, if yes, response is available.
+		 */
+		if (ret < 0 || ret & IPI_MB_STATUS_SEND_PENDING)
+			return false;
+		else
+			return true;
+	} else if (ret > 0 && ret & IPI_MB_STATUS_RECV_PENDING) {
+		/* RX channel, check if there is message arrived. */
+		return true;
+	}
+	return false;
+}
+
+/**
+ * zynqmp_ipi_last_tx_done - See if the last tx message is sent
+ *
+ * @chan: Channel pointer
+ *
+ * Return: 'true' is no pending tx data, 'false' if there are any.
+ */
+static bool zynqmp_ipi_last_tx_done(struct mbox_chan *chan)
+{
+	struct device *dev = chan->mbox->dev;
+	struct zynqmp_ipi_mbox *ipi_mbox = dev_get_drvdata(dev);
+	struct zynqmp_ipi_mchan *mchan = chan->con_priv;
+	int ret;
+	u64 arg0;
+	struct arm_smccc_res res;
+	struct zynqmp_ipi_message *msg;
+
+	if (WARN_ON(!ipi_mbox)) {
+		dev_err(dev, "no platform drv data??\n");
+		return false;
+	}
+
+	if (mchan->chan_type == IPI_MB_CHNL_TX) {
+		/* We only need to check if the message been taken
+		 * by the remote in the TX channel
+		 */
+		arg0 = SMC_IPI_MAILBOX_STATUS_ENQUIRY;
+		zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
+		/* Check the SMC call status, a0 of the result */
+		ret = (int)(res.a0 & 0xFFFFFFFF);
+		if (ret < 0 || ret & IPI_MB_STATUS_SEND_PENDING)
+			return false;
+
+		msg = mchan->rx_buf;
+		msg->len = mchan->resp_buf_size;
+		memcpy_fromio(msg->data, mchan->resp_buf, msg->len);
+		mbox_chan_received_data(chan, (void *)msg);
+		return true;
+	}
+	/* Always true for the response message in RX channel */
+	return true;
+}
+
+/**
+ * zynqmp_ipi_send_data - Send data
+ *
+ * @chan: Channel Pointer
+ * @data: Message Pointer
+ *
+ * Return: 0 if all goes good, else appropriate error messages.
+ */
+static int zynqmp_ipi_send_data(struct mbox_chan *chan, void *data)
+{
+	struct device *dev = chan->mbox->dev;
+	struct zynqmp_ipi_mbox *ipi_mbox = dev_get_drvdata(dev);
+	struct zynqmp_ipi_mchan *mchan = chan->con_priv;
+	struct zynqmp_ipi_message *msg = data;
+	u64 arg0;
+	struct arm_smccc_res res;
+	u32 timeout;
+	int ret;
+
+	if (WARN_ON(!ipi_mbox)) {
+		dev_err(dev, "no platform drv data??\n");
+		return -EINVAL;
+	}
+
+	if (mchan->chan_type == IPI_MB_CHNL_TX) {
+		/* Send request message */
+		if (msg && msg->len > mchan->req_buf_size) {
+			dev_err(dev, "channel %d message length %u > max %lu\n",
+				mchan->chan_type, (unsigned int)msg->len,
+				mchan->req_buf_size);
+			return -EINVAL;
+		}
+		/* Enquire if the mailbox is free to send message */
+		arg0 = SMC_IPI_MAILBOX_STATUS_ENQUIRY;
+		timeout = 10;
+		if (msg && msg->len) {
+			timeout = 10;
+			do {
+				zynqmp_ipi_fw_call(ipi_mbox, arg0,
+						   0, 0, 0, 0, 0, &res);
+				ret = res.a0 & 0xFFFFFFFF;
+				if (ret >= 0 &&
+				    !(ret & IPI_MB_STATUS_SEND_PENDING))
+					break;
+				usleep_range(1, 2);
+				timeout--;
+			} while (timeout);
+			if (!timeout) {
+				dev_warn(dev, "chan %d sending msg timeout.\n",
+					 ipi_mbox->remote_id);
+				return -ETIME;
+			}
+			memcpy_toio(mchan->req_buf, msg->data, msg->len);
+		}
+		/* Kick IPI mailbox to send message */
+		arg0 = SMC_IPI_MAILBOX_NOTIFY;
+		zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
+	} else {
+		/* Send response message */
+		if (msg && msg->len > mchan->resp_buf_size) {
+			dev_err(dev, "channel %d message length %u > max %lu\n",
+				mchan->chan_type, (unsigned int)msg->len,
+				mchan->resp_buf_size);
+			return -EINVAL;
+		}
+		if (msg && msg->len)
+			memcpy_toio(mchan->resp_buf, msg->data, msg->len);
+		arg0 = SMC_IPI_MAILBOX_NOTIFY;
+		arg0 = SMC_IPI_MAILBOX_ACK;
+		zynqmp_ipi_fw_call(ipi_mbox, arg0, IPI_SMC_ACK_EIRQ_MASK,
+				   0, 0, 0, 0, &res);
+	}
+	return 0;
+}
+
+/**
+ * zynqmp_ipi_startup - Startup the IPI channel
+ *
+ * @chan: Channel pointer
+ *
+ * Return: 0 if all goes good, else return corresponding error message
+ */
+static int zynqmp_ipi_startup(struct mbox_chan *chan)
+{
+	struct device *dev = chan->mbox->dev;
+	struct zynqmp_ipi_mbox *ipi_mbox = dev_get_drvdata(dev);
+	struct zynqmp_ipi_mchan *mchan = chan->con_priv;
+	u64 arg0;
+	struct arm_smccc_res res;
+	int ret = 0;
+	unsigned int nchan_type;
+
+	if (mchan->is_opened)
+		return 0;
+
+	/* If no channel has been opened, open the IPI mailbox */
+	nchan_type = (mchan->chan_type + 1) % 2;
+	if (!ipi_mbox->mchans[nchan_type].is_opened) {
+		arg0 = SMC_IPI_MAILBOX_OPEN;
+		zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
+		/* Check the SMC call status, a0 of the result */
+		ret = (int)(res.a0 & 0xFFFFFFFF);
+		if (ret < 0) {
+			dev_err(dev, "SMC to open the IPI channel failed.\n");
+			return ret;
+		}
+		ret = 0;
+	}
+
+	/* If it is RX channel, enable the IPI notification interrupt */
+	if (mchan->chan_type == IPI_MB_CHNL_RX) {
+		arg0 = SMC_IPI_MAILBOX_ENABLE_IRQ;
+		zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
+	}
+	mchan->is_opened = 1;
+
+	return ret;
+}
+
+/**
+ * zynqmp_ipi_shutdown - Shutdown the IPI channel
+ *
+ * @chan: Channel pointer
+ */
+static void zynqmp_ipi_shutdown(struct mbox_chan *chan)
+{
+	struct device *dev = chan->mbox->dev;
+	struct zynqmp_ipi_mbox *ipi_mbox = dev_get_drvdata(dev);
+	struct zynqmp_ipi_mchan *mchan = chan->con_priv;
+	u64 arg0;
+	struct arm_smccc_res res;
+	unsigned int chan_type;
+
+	if (!mchan->is_opened)
+		return;
+
+	/* If it is RX channel, disable notification interrupt */
+	chan_type = mchan->chan_type;
+	if (chan_type == IPI_MB_CHNL_RX) {
+		arg0 = SMC_IPI_MAILBOX_DISABLE_IRQ;
+		zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
+	}
+	/* Release IPI mailbox if no other channel is opened */
+	chan_type = (chan_type + 1) % 2;
+	if (!ipi_mbox->mchans[chan_type].is_opened) {
+		arg0 = SMC_IPI_MAILBOX_RELEASE;
+		zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
+	}
+
+	mchan->is_opened = 0;
+}
+
+/* ZynqMP IPI mailbox operations */
+static const struct mbox_chan_ops zynqmp_ipi_chan_ops = {
+	.startup = zynqmp_ipi_startup,
+	.shutdown = zynqmp_ipi_shutdown,
+	.peek_data = zynqmp_ipi_peek_data,
+	.last_tx_done = zynqmp_ipi_last_tx_done,
+	.send_data = zynqmp_ipi_send_data,
+};
+
+/**
+ * zynqmp_ipi_of_xlate - Translate of phandle to IPI mailbox channel
+ *
+ * @mbox: mailbox controller pointer
+ * @p:    phandle pointer
+ *
+ * Return: Mailbox channel, else return error pointer.
+ */
+static struct mbox_chan *zynqmp_ipi_of_xlate(struct mbox_controller *mbox,
+					     const struct of_phandle_args *p)
+{
+	struct mbox_chan *chan;
+	struct device *dev = mbox->dev;
+	unsigned int chan_type;
+
+	/* Only supports TX and RX channels */
+	chan_type = p->args[0];
+	if (chan_type != IPI_MB_CHNL_TX && chan_type != IPI_MB_CHNL_RX) {
+		dev_err(dev, "req chnl failure: invalid chnl type %u.\n",
+			chan_type);
+		return ERR_PTR(-EINVAL);
+	}
+	chan = &mbox->chans[chan_type];
+	return chan;
+}
+
+static const struct of_device_id zynqmp_ipi_of_match[] = {
+	{ .compatible = "xlnx,zynqmp-ipi-mailbox" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, zynqmp_ipi_of_match);
+
+/**
+ * zynqmp_ipi_mbox_get_buf_res - Get buffer resource from the IPI dev node
+ *
+ * @node: IPI mbox device child node
+ * @name: name of the IPI buffer
+ * @res: pointer to where the resource information will be stored.
+ *
+ * Return: 0 for success, negative value for failure
+ */
+static int zynqmp_ipi_mbox_get_buf_res(struct device_node *node,
+				       const char *name,
+				       struct resource *res)
+{
+	int ret, index;
+
+	index = of_property_match_string(node, "reg-names", name);
+	if (index >= 0) {
+		ret = of_address_to_resource(node, index, res);
+		if (ret < 0)
+			return -EINVAL;
+		return 0;
+	}
+	return -ENODEV;
+}
+
+/**
+ * zynqmp_ipi_mbox_dev_release() - release the existence of a ipi mbox dev
+ *
+ * @dev: the ipi mailbox device
+ *
+ * This is to avoid the no device release() function kernel warning.
+ *
+ */
+static void zynqmp_ipi_mbox_dev_release(struct device *dev)
+{
+	(void)dev;
+}
+
+/**
+ * zynqmp_ipi_mbox_probe - probe IPI mailbox resource from device node
+ *
+ * @ipi_mbox: pointer to IPI mailbox private data structure
+ * @node: IPI mailbox device node
+ *
+ * Return: 0 for success, negative value for failure
+ */
+static int zynqmp_ipi_mbox_probe(struct zynqmp_ipi_mbox *ipi_mbox,
+				 struct device_node *node)
+{
+	struct zynqmp_ipi_mchan *mchan;
+	struct mbox_chan *chans;
+	struct mbox_controller *mbox;
+	struct resource res;
+	struct device *dev, *mdev;
+	const char *name;
+	int ret;
+
+	dev = ipi_mbox->pdata->dev;
+	/* Initialize dev for IPI mailbox */
+	ipi_mbox->dev.parent = dev;
+	ipi_mbox->dev.release = NULL;
+	ipi_mbox->dev.of_node = node;
+	dev_set_name(&ipi_mbox->dev, "%s", of_node_full_name(node));
+	dev_set_drvdata(&ipi_mbox->dev, ipi_mbox);
+	ipi_mbox->dev.release = zynqmp_ipi_mbox_dev_release;
+	ipi_mbox->dev.driver = &zynqmp_ipi_mbox_driver;
+	ret = device_register(&ipi_mbox->dev);
+	if (ret) {
+		dev_err(dev, "Failed to register ipi mbox dev.\n");
+		return ret;
+	}
+	mdev = &ipi_mbox->dev;
+
+	mchan = &ipi_mbox->mchans[IPI_MB_CHNL_TX];
+	name = "local_request_region";
+	ret = zynqmp_ipi_mbox_get_buf_res(node, name, &res);
+	if (!ret) {
+		mchan->req_buf_size = resource_size(&res);
+		mchan->req_buf = devm_ioremap(mdev, res.start,
+					      mchan->req_buf_size);
+		if (IS_ERR(mchan->req_buf)) {
+			dev_err(mdev, "Unable to map IPI buffer I/O memory\n");
+			ret = PTR_ERR(mchan->req_buf);
+			return ret;
+		}
+	} else if (ret != -ENODEV) {
+		dev_err(mdev, "Unmatched resource %s, %d.\n", name, ret);
+		return ret;
+	}
+
+	name = "remote_response_region";
+	ret = zynqmp_ipi_mbox_get_buf_res(node, name, &res);
+	if (!ret) {
+		mchan->resp_buf_size = resource_size(&res);
+		mchan->resp_buf = devm_ioremap(mdev, res.start,
+					       mchan->resp_buf_size);
+		if (IS_ERR(mchan->resp_buf)) {
+			dev_err(mdev, "Unable to map IPI buffer I/O memory\n");
+			ret = PTR_ERR(mchan->resp_buf);
+			return ret;
+		}
+	} else if (ret != -ENODEV) {
+		dev_err(mdev, "Unmatched resource %s.\n", name);
+		return ret;
+	}
+	mchan->rx_buf = devm_kzalloc(mdev,
+				     mchan->resp_buf_size +
+				     sizeof(struct zynqmp_ipi_message),
+				     GFP_KERNEL);
+	if (!mchan->rx_buf)
+		return -ENOMEM;
+
+	mchan = &ipi_mbox->mchans[IPI_MB_CHNL_RX];
+	name = "remote_request_region";
+	ret = zynqmp_ipi_mbox_get_buf_res(node, name, &res);
+	if (!ret) {
+		mchan->req_buf_size = resource_size(&res);
+		mchan->req_buf = devm_ioremap(mdev, res.start,
+					      mchan->req_buf_size);
+		if (IS_ERR(mchan->req_buf)) {
+			dev_err(mdev, "Unable to map IPI buffer I/O memory\n");
+			ret = PTR_ERR(mchan->req_buf);
+			return ret;
+		}
+	} else if (ret != -ENODEV) {
+		dev_err(mdev, "Unmatched resource %s.\n", name);
+		return ret;
+	}
+
+	name = "local_response_region";
+	ret = zynqmp_ipi_mbox_get_buf_res(node, name, &res);
+	if (!ret) {
+		mchan->resp_buf_size = resource_size(&res);
+		mchan->resp_buf = devm_ioremap(mdev, res.start,
+					       mchan->resp_buf_size);
+		if (IS_ERR(mchan->resp_buf)) {
+			dev_err(mdev, "Unable to map IPI buffer I/O memory\n");
+			ret = PTR_ERR(mchan->resp_buf);
+			return ret;
+		}
+	} else if (ret != -ENODEV) {
+		dev_err(mdev, "Unmatched resource %s.\n", name);
+		return ret;
+	}
+	mchan->rx_buf = devm_kzalloc(mdev,
+				     mchan->resp_buf_size +
+				     sizeof(struct zynqmp_ipi_message),
+				     GFP_KERNEL);
+	if (!mchan->rx_buf)
+		return -ENOMEM;
+
+	/* Get the IPI remote agent ID */
+	ret = of_property_read_u32(node, "xlnx,ipi-id", &ipi_mbox->remote_id);
+	if (ret < 0) {
+		dev_err(dev, "No IPI remote ID is specified.\n");
+		return ret;
+	}
+
+	mbox = &ipi_mbox->mbox;
+	mbox->dev = mdev;
+	mbox->ops = &zynqmp_ipi_chan_ops;
+	mbox->num_chans = 2;
+	mbox->txdone_irq = false;
+	mbox->txdone_poll = true;
+	mbox->txpoll_period = 5;
+	mbox->of_xlate = zynqmp_ipi_of_xlate;
+	chans = devm_kzalloc(mdev, 2 * sizeof(*chans), GFP_KERNEL);
+	if (!chans)
+		return -ENOMEM;
+	mbox->chans = chans;
+	chans[IPI_MB_CHNL_TX].con_priv = &ipi_mbox->mchans[IPI_MB_CHNL_TX];
+	chans[IPI_MB_CHNL_RX].con_priv = &ipi_mbox->mchans[IPI_MB_CHNL_RX];
+	ipi_mbox->mchans[IPI_MB_CHNL_TX].chan_type = IPI_MB_CHNL_TX;
+	ipi_mbox->mchans[IPI_MB_CHNL_RX].chan_type = IPI_MB_CHNL_RX;
+	ret = mbox_controller_register(mbox);
+	if (ret)
+		dev_err(mdev,
+			"Failed to register mbox_controller(%d)\n", ret);
+	else
+		dev_info(mdev, "Probed ZynqMP IPI Mailbox driver.\n");
+	return ret;
+}
+
+/**
+ * zynqmp_ipi_free_mboxes - Free IPI mailboxes devices
+ *
+ * @pdata: IPI private data
+ */
+static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata)
+{
+	struct zynqmp_ipi_mbox *ipi_mbox;
+	int i;
+
+	i = pdata->num_mboxes;
+	for (; i >= 0; i--) {
+		ipi_mbox = &pdata->ipi_mboxes[i];
+		if (ipi_mbox->dev.parent) {
+			mbox_controller_unregister(&ipi_mbox->mbox);
+			device_unregister(&ipi_mbox->dev);
+		}
+	}
+}
+
+static int zynqmp_ipi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *nc, *np = pdev->dev.of_node;
+	struct zynqmp_ipi_pdata *pdata;
+	struct zynqmp_ipi_mbox *mbox;
+	int i, ret = -EINVAL;
+
+	i = 0;
+	for_each_available_child_of_node(np, nc)
+		i++;
+	pdata = devm_kzalloc(dev, sizeof(*pdata) + (i * sizeof(*mbox)),
+			     GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+	pdata->dev = dev;
+
+	/* Get the IPI local agents ID */
+	ret = of_property_read_u32(np, "xlnx,ipi-id", &pdata->local_id);
+	if (ret < 0) {
+		dev_err(dev, "No IPI local ID is specified.\n");
+		return ret;
+	}
+
+	pdata->num_mboxes = i;
+	pdata->ipi_mboxes = (struct zynqmp_ipi_mbox *)
+			    ((char *)pdata + sizeof(*pdata));
+
+	mbox = pdata->ipi_mboxes;
+	for_each_available_child_of_node(np, nc) {
+		mbox->pdata = pdata;
+		ret = zynqmp_ipi_mbox_probe(mbox, nc);
+		if (ret) {
+			dev_err(dev, "failed to probe subdev.\n");
+			ret = -EINVAL;
+			goto free_mbox_dev;
+		}
+		mbox++;
+	}
+
+	/* IPI IRQ */
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0) {
+		dev_err(dev, "unable to find IPI IRQ.\n");
+		goto free_mbox_dev;
+	}
+	pdata->irq = ret;
+	ret = devm_request_irq(dev, pdata->irq, zynqmp_ipi_interrupt,
+			       IRQF_SHARED, dev_name(dev), pdata);
+	if (ret) {
+		dev_err(dev, "IRQ %d is not requested successfully.\n",
+			pdata->irq);
+		goto free_mbox_dev;
+	}
+
+	platform_set_drvdata(pdev, pdata);
+	return ret;
+
+free_mbox_dev:
+	zynqmp_ipi_free_mboxes(pdata);
+	return ret;
+}
+
+static int zynqmp_ipi_remove(struct platform_device *pdev)
+{
+	struct zynqmp_ipi_pdata *pdata;
+
+	pdata = platform_get_drvdata(pdev);
+	zynqmp_ipi_free_mboxes(pdata);
+
+	return 0;
+}
+
+static struct platform_driver zynqmp_ipi_driver = {
+	.probe = zynqmp_ipi_probe,
+	.remove = zynqmp_ipi_remove,
+	.driver = {
+		   .name = "zynqmp-ipi",
+		   .of_match_table = of_match_ptr(zynqmp_ipi_of_match),
+	},
+};
+
+static int __init zynqmp_ipi_init(void)
+{
+	return platform_driver_register(&zynqmp_ipi_driver);
+}
+subsys_initcall(zynqmp_ipi_init);
+
+static void __exit zynqmp_ipi_exit(void)
+{
+	platform_driver_unregister(&zynqmp_ipi_driver);
+}
+module_exit(zynqmp_ipi_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Xilinx ZynqMP IPI Mailbox driver");
+MODULE_AUTHOR("Xilinx Inc.");
-- 
2.7.4


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

* [PATH v7 2/2] dt-bindings: mailbox: Add Xilinx IPI Mailbox
  2018-12-20 17:32 [PATH v7 0/2] Xilinx ZynqMP IPI Mailbox Controller Driver Wendy Liang
  2018-12-20 17:32 ` [PATH v7 1/2] mailbox: ZynqMP IPI mailbox controller Wendy Liang
@ 2018-12-20 17:32 ` Wendy Liang
  1 sibling, 0 replies; 5+ messages in thread
From: Wendy Liang @ 2018-12-20 17:32 UTC (permalink / raw)
  To: jassisinghbrar, michal.simek, robh+dt, mark.rutland
  Cc: linux-kernel, linux-arm-kernel, devicetree, Wendy Liang

Xilinx ZynqMP IPI(Inter Processor Interrupt) is a hardware block
in ZynqMP SoC used for the communication between various processor
systems.

Signed-off-by: Wendy Liang <wendy.liang@xilinx.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/mailbox/xlnx,zynqmp-ipi-mailbox.txt   | 127 +++++++++++++++++++++
 1 file changed, 127 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.txt

diff --git a/Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.txt b/Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.txt
new file mode 100644
index 0000000..4438432
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/xlnx,zynqmp-ipi-mailbox.txt
@@ -0,0 +1,127 @@
+Xilinx IPI Mailbox Controller
+========================================
+
+The Xilinx IPI(Inter Processor Interrupt) mailbox controller is to manage
+messaging between two Xilinx Zynq UltraScale+ MPSoC IPI agents. Each IPI
+agent owns registers used for notification and buffers for message.
+
+               +-------------------------------------+
+               | Xilinx ZynqMP IPI Controller        |
+               +-------------------------------------+
+    +--------------------------------------------------+
+ATF                    |                     |
+                       |                     |
+                       |                     |
+    +--------------------------+             |
+                       |                     |
+                       |                     |
+    +--------------------------------------------------+
+            +------------------------------------------+
+            |  +----------------+   +----------------+ |
+Hardware    |  |  IPI Agent     |   |  IPI Buffers   | |
+            |  |  Registers     |   |                | |
+            |  |                |   |                | |
+            |  +----------------+   +----------------+ |
+            |                                          |
+            | Xilinx IPI Agent Block                   |
+            +------------------------------------------+
+
+
+Controller Device Node:
+===========================
+Required properties:
+--------------------
+IPI agent node:
+- compatible:		Shall be: "xlnx,zynqmp-ipi-mailbox"
+- interrupt-parent:	Phandle for the interrupt controller
+- interrupts:		Interrupt information corresponding to the
+			interrupt-names property.
+- xlnx,ipi-id:		local Xilinx IPI agent ID
+- #address-cells:	number of address cells of internal IPI mailbox nodes
+- #size-cells:		number of size cells of internal IPI mailbox nodes
+
+Internal IPI mailbox node:
+- reg:			IPI buffers address ranges
+- reg-names:		Names of the reg resources. It should have:
+			* local_request_region
+			  - IPI request msg buffer written by local and read
+			    by remote
+			* local_response_region
+			  - IPI response msg buffer written by local and read
+			    by remote
+			* remote_request_region
+			  - IPI request msg buffer written by remote and read
+			    by local
+			* remote_response_region
+			  - IPI response msg buffer written by remote and read
+			    by local
+- #mbox-cells:		Shall be 1. It contains:
+			* tx(0) or rx(1) channel
+- xlnx,ipi-id:		remote Xilinx IPI agent ID of which the mailbox is
+			connected to.
+
+Optional properties:
+--------------------
+- method:              The method of accessing the IPI agent registers.
+                       Permitted values are: "smc" and "hvc". Default is
+                       "smc".
+
+Client Device Node:
+===========================
+Required properties:
+--------------------
+- mboxes:		Standard property to specify a mailbox
+			(See ./mailbox.txt)
+- mbox-names:		List of identifier  strings for each mailbox
+			channel.
+
+Example:
+===========================
+	zynqmp_ipi {
+		compatible = "xlnx,zynqmp-ipi-mailbox";
+		interrupt-parent = <&gic>;
+		interrupts = <0 29 4>;
+		xlnx,ipi-id = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		/* APU<->RPU0 IPI mailbox controller */
+		ipi_mailbox_rpu0: mailbox@ff90400 {
+			reg = <0xff990400 0x20>,
+			      <0xff990420 0x20>,
+			      <0xff990080 0x20>,
+			      <0xff9900a0 0x20>;
+			reg-names = "local_request_region",
+				    "local_response_region",
+				    "remote_request_region",
+				    "remote_response_region";
+			#mbox-cells = <1>;
+			xlnx,ipi-id = <1>;
+		};
+		/* APU<->RPU1 IPI mailbox controller */
+		ipi_mailbox_rpu1: mailbox@ff990440 {
+			reg = <0xff990440 0x20>,
+			      <0xff990460 0x20>,
+			      <0xff990280 0x20>,
+			      <0xff9902a0 0x20>;
+			reg-names = "local_request_region",
+				    "local_response_region",
+				    "remote_request_region",
+				    "remote_response_region";
+			#mbox-cells = <1>;
+			xlnx,ipi-id = <2>;
+		};
+	};
+	rpu0 {
+		...
+		mboxes = <&ipi_mailbox_rpu0 0>,
+			 <&ipi_mailbox_rpu0 1>;
+		mbox-names = "tx", "rx";
+	};
+	rpu1 {
+		...
+		mboxes = <&ipi_mailbox_rpu1 0>,
+			 <&ipi_mailbox_rpu1 1>;
+		mbox-names = "tx", "rx";
+	};
-- 
2.7.4


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

* Re: [PATH v7 1/2] mailbox: ZynqMP IPI mailbox controller
  2018-12-20 17:32 ` [PATH v7 1/2] mailbox: ZynqMP IPI mailbox controller Wendy Liang
@ 2018-12-22  1:59   ` Jassi Brar
  2019-01-11  0:58     ` Jiaying Liang
  0 siblings, 1 reply; 5+ messages in thread
From: Jassi Brar @ 2018-12-22  1:59 UTC (permalink / raw)
  To: Wendy Liang
  Cc: Michal Simek, Rob Herring, Mark Rutland,
	Linux Kernel Mailing List, linux-arm-kernel, Devicetree List

On Thu, Dec 20, 2018 at 11:32 AM Wendy Liang <wendy.liang@xilinx.com> wrote:


> diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
> new file mode 100644
> index 0000000..bbddfd5
> --- /dev/null
> +++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
> @@ -0,0 +1,764 @@

......
> +
> +/* IPI SMC Macros */
> +#define IPI_SMC_OPEN_IRQ_MASK          0x00000001UL /* IRQ enable bit in IPI
> +                                                     * open SMC call
> +                                                     */
> +#define IPI_SMC_NOTIFY_BLOCK_MASK      0x00000001UL /* Flag to indicate if
> +                                                     * IPI notification needs
> +                                                     * to be blocking.
> +                                                     */
> +#define IPI_SMC_ENQUIRY_DIRQ_MASK      0x00000001UL /* Flag to indicate if
> +                                                     * notification interrupt
> +                                                     * to be disabled.
> +                                                     */
> +#define IPI_SMC_ACK_EIRQ_MASK          0x00000001UL /* Flag to indicate if
> +                                                     * notification interrupt
> +                                                     * to be enabled.
> +                                                     */
> +
The first two are unused.


> +struct zynqmp_ipi_pdata {
> +       struct device *dev;
> +       int irq;
> +       unsigned int method;
>
'method' doesn't track the HVC option in the driver. Please have a look.

......
> +
> +static void zynqmp_ipi_fw_call(struct zynqmp_ipi_mbox *ipi_mbox,
> +                              unsigned long a0, unsigned long a3,
> +                              unsigned long a4, unsigned long a5,
> +                              unsigned long a6, unsigned long a7,
> +                              struct arm_smccc_res *res)
>
[a4,a7] are always 0, so maybe just drop them?


> +static bool zynqmp_ipi_last_tx_done(struct mbox_chan *chan)
> +{
> +       struct device *dev = chan->mbox->dev;
> +       struct zynqmp_ipi_mbox *ipi_mbox = dev_get_drvdata(dev);
> +       struct zynqmp_ipi_mchan *mchan = chan->con_priv;
> +       int ret;
> +       u64 arg0;
> +       struct arm_smccc_res res;
> +       struct zynqmp_ipi_message *msg;
> +
> +       if (WARN_ON(!ipi_mbox)) {
> +               dev_err(dev, "no platform drv data??\n");
> +               return false;
> +       }
> +
> +       if (mchan->chan_type == IPI_MB_CHNL_TX) {
> +               /* We only need to check if the message been taken
> +                * by the remote in the TX channel
> +                */
> +               arg0 = SMC_IPI_MAILBOX_STATUS_ENQUIRY;
> +               zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
> +               /* Check the SMC call status, a0 of the result */
> +               ret = (int)(res.a0 & 0xFFFFFFFF);
> +               if (ret < 0 || ret & IPI_MB_STATUS_SEND_PENDING)
> +                       return false;
> +
OK, but ...

> +               msg = mchan->rx_buf;
> +               msg->len = mchan->resp_buf_size;
> +               memcpy_fromio(msg->data, mchan->resp_buf, msg->len);
> +               mbox_chan_received_data(chan, (void *)msg);
>
.... wouldn't this be done from zynqmp_ipi_interrupt()?



> +static int zynqmp_ipi_send_data(struct mbox_chan *chan, void *data)
> +{
  .........
> +               /* Enquire if the mailbox is free to send message */
> +               arg0 = SMC_IPI_MAILBOX_STATUS_ENQUIRY;
> +               timeout = 10;
> +               if (msg && msg->len) {
> +                       timeout = 10;
> +                       do {
> +                               zynqmp_ipi_fw_call(ipi_mbox, arg0,
> +                                                  0, 0, 0, 0, 0, &res);
> +                               ret = res.a0 & 0xFFFFFFFF;
> +                               if (ret >= 0 &&
> +                                   !(ret & IPI_MB_STATUS_SEND_PENDING))
> +                                       break;
> +                               usleep_range(1, 2);
> +                               timeout--;
> +                       } while (timeout);
> +                       if (!timeout) {
> +                               dev_warn(dev, "chan %d sending msg timeout.\n",
> +                                        ipi_mbox->remote_id);
> +                               return -ETIME;
> +                       }
> +                       memcpy_toio(mchan->req_buf, msg->data, msg->len);
> +               }
>
This check should be done in last_tx_done, and not here.
send_data is never called unless last_tx_done returns true.

> +               /* Kick IPI mailbox to send message */
> +               arg0 = SMC_IPI_MAILBOX_NOTIFY;
> +               zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
> +       } else {
> +               /* Send response message */
> +               if (msg && msg->len > mchan->resp_buf_size) {
> +                       dev_err(dev, "channel %d message length %u > max %lu\n",
> +                               mchan->chan_type, (unsigned int)msg->len,
> +                               mchan->resp_buf_size);
> +                       return -EINVAL;
> +               }
> +               if (msg && msg->len)
> +                       memcpy_toio(mchan->resp_buf, msg->data, msg->len);
>

> +               arg0 = SMC_IPI_MAILBOX_NOTIFY;
> +               arg0 = SMC_IPI_MAILBOX_ACK;
>
This is obviously wrong.

....
> +       mbox->chans = chans;
> +       chans[IPI_MB_CHNL_TX].con_priv = &ipi_mbox->mchans[IPI_MB_CHNL_TX];
> +       chans[IPI_MB_CHNL_RX].con_priv = &ipi_mbox->mchans[IPI_MB_CHNL_RX];
> +       ipi_mbox->mchans[IPI_MB_CHNL_TX].chan_type = IPI_MB_CHNL_TX;
> +       ipi_mbox->mchans[IPI_MB_CHNL_RX].chan_type = IPI_MB_CHNL_RX;
> +       ret = mbox_controller_register(mbox);
>
while at it, you may want to start using
devm_mbox_controller_register() recently added by Thierry.

> +       if (ret)
> +               dev_err(mdev,
> +                       "Failed to register mbox_controller(%d)\n", ret);
> +       else
> +               dev_info(mdev, "Probed ZynqMP IPI Mailbox driver.\n");
>
You may want to also print something instance specific here.


> +static int zynqmp_ipi_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *nc, *np = pdev->dev.of_node;
> +       struct zynqmp_ipi_pdata *pdata;
> +       struct zynqmp_ipi_mbox *mbox;
> +       int i, ret = -EINVAL;
> +
> +       i = 0;
> +       for_each_available_child_of_node(np, nc)
> +               i++;
>
of_get_child_count() ?


Thnx

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

* RE: [PATH v7 1/2] mailbox: ZynqMP IPI mailbox controller
  2018-12-22  1:59   ` Jassi Brar
@ 2019-01-11  0:58     ` Jiaying Liang
  0 siblings, 0 replies; 5+ messages in thread
From: Jiaying Liang @ 2019-01-11  0:58 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Michal Simek, Rob Herring, Mark Rutland,
	Linux Kernel Mailing List, linux-arm-kernel, Devicetree List



> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@gmail.com]
> Sent: Friday, December 21, 2018 6:00 PM
> To: Jiaying Liang <jliang@xilinx.com>
> Cc: Michal Simek <michals@xilinx.com>; Rob Herring <robh+dt@kernel.org>;
> Mark Rutland <mark.rutland@arm.com>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; linux-arm-kernel@lists.infradead.org; Devicetree
> List <devicetree@vger.kernel.org>
> Subject: Re: [PATH v7 1/2] mailbox: ZynqMP IPI mailbox controller
> 
> On Thu, Dec 20, 2018 at 11:32 AM Wendy Liang <wendy.liang@xilinx.com>
> wrote:
> 
> 
> > diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c
> > b/drivers/mailbox/zynqmp-ipi-mailbox.c
> > new file mode 100644
> > index 0000000..bbddfd5
> > --- /dev/null
> > +++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
> > @@ -0,0 +1,764 @@
> 
> ......
> > +
> > +/* IPI SMC Macros */
> > +#define IPI_SMC_OPEN_IRQ_MASK          0x00000001UL /* IRQ enable bit
> in IPI
> > +                                                     * open SMC call
> > +                                                     */
> > +#define IPI_SMC_NOTIFY_BLOCK_MASK      0x00000001UL /* Flag to
> indicate if
> > +                                                     * IPI notification needs
> > +                                                     * to be blocking.
> > +                                                     */
> > +#define IPI_SMC_ENQUIRY_DIRQ_MASK      0x00000001UL /* Flag to
> indicate if
> > +                                                     * notification interrupt
> > +                                                     * to be disabled.
> > +                                                     */
> > +#define IPI_SMC_ACK_EIRQ_MASK          0x00000001UL /* Flag to indicate
> if
> > +                                                     * notification interrupt
> > +                                                     * to be enabled.
> > +                                                     */
> > +
> The first two are unused.
[Wendy] Will remove the unused macros

> 
> 
> > +struct zynqmp_ipi_pdata {
> > +       struct device *dev;
> > +       int irq;
> > +       unsigned int method;
> >
> 'method' doesn't track the HVC option in the driver. Please have a look.
[Wendy] I will add one more checking in the function implementation
to check HVC and error if it is neither SMC nor HVC.
> 
> ......
> > +
> > +static void zynqmp_ipi_fw_call(struct zynqmp_ipi_mbox *ipi_mbox,
> > +                              unsigned long a0, unsigned long a3,
> > +                              unsigned long a4, unsigned long a5,
> > +                              unsigned long a6, unsigned long a7,
> > +                              struct arm_smccc_res *res)
> >
> [a4,a7] are always 0, so maybe just drop them?
[Wendy] Will drop them from the API, and set them to 0.
> 
> 
> > +static bool zynqmp_ipi_last_tx_done(struct mbox_chan *chan) {
> > +       struct device *dev = chan->mbox->dev;
> > +       struct zynqmp_ipi_mbox *ipi_mbox = dev_get_drvdata(dev);
> > +       struct zynqmp_ipi_mchan *mchan = chan->con_priv;
> > +       int ret;
> > +       u64 arg0;
> > +       struct arm_smccc_res res;
> > +       struct zynqmp_ipi_message *msg;
> > +
> > +       if (WARN_ON(!ipi_mbox)) {
> > +               dev_err(dev, "no platform drv data??\n");
> > +               return false;
> > +       }
> > +
> > +       if (mchan->chan_type == IPI_MB_CHNL_TX) {
> > +               /* We only need to check if the message been taken
> > +                * by the remote in the TX channel
> > +                */
> > +               arg0 = SMC_IPI_MAILBOX_STATUS_ENQUIRY;
> > +               zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
> > +               /* Check the SMC call status, a0 of the result */
> > +               ret = (int)(res.a0 & 0xFFFFFFFF);
> > +               if (ret < 0 || ret & IPI_MB_STATUS_SEND_PENDING)
> > +                       return false;
> > +
> OK, but ...
> 
> > +               msg = mchan->rx_buf;
> > +               msg->len = mchan->resp_buf_size;
> > +               memcpy_fromio(msg->data, mchan->resp_buf, msg->len);
> > +               mbox_chan_received_data(chan, (void *)msg);
> >
> .... wouldn't this be done from zynqmp_ipi_interrupt()?
[Wendy] The IPI hardware supports both the synchronous mode and
Asynchronous mode.
The rx interrupt used for remote to notify host, or for responding asynchronous
Request. In synchronous mode, the IPI hardware allow remote to write response
To the tx response buffer, and the host side can poll the observation register
And get the response if the remote has acked.
The last_tx_done is implemented for this purpose.
> 
> 
> 
> > +static int zynqmp_ipi_send_data(struct mbox_chan *chan, void *data) {
>   .........
> > +               /* Enquire if the mailbox is free to send message */
> > +               arg0 = SMC_IPI_MAILBOX_STATUS_ENQUIRY;
> > +               timeout = 10;
> > +               if (msg && msg->len) {
> > +                       timeout = 10;
> > +                       do {
> > +                               zynqmp_ipi_fw_call(ipi_mbox, arg0,
> > +                                                  0, 0, 0, 0, 0, &res);
> > +                               ret = res.a0 & 0xFFFFFFFF;
> > +                               if (ret >= 0 &&
> > +                                   !(ret & IPI_MB_STATUS_SEND_PENDING))
> > +                                       break;
> > +                               usleep_range(1, 2);
> > +                               timeout--;
> > +                       } while (timeout);
> > +                       if (!timeout) {
> > +                               dev_warn(dev, "chan %d sending msg timeout.\n",
> > +                                        ipi_mbox->remote_id);
> > +                               return -ETIME;
> > +                       }
> > +                       memcpy_toio(mchan->req_buf, msg->data, msg->len);
> > +               }
> >
> This check should be done in last_tx_done, and not here.
> send_data is never called unless last_tx_done returns true.
[Wendy] will remove it.
> 
> > +               /* Kick IPI mailbox to send message */
> > +               arg0 = SMC_IPI_MAILBOX_NOTIFY;
> > +               zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, 0, 0, 0, 0, &res);
> > +       } else {
> > +               /* Send response message */
> > +               if (msg && msg->len > mchan->resp_buf_size) {
> > +                       dev_err(dev, "channel %d message length %u > max %lu\n",
> > +                               mchan->chan_type, (unsigned int)msg->len,
> > +                               mchan->resp_buf_size);
> > +                       return -EINVAL;
> > +               }
> > +               if (msg && msg->len)
> > +                       memcpy_toio(mchan->resp_buf, msg->data,
> > + msg->len);
> >
> 
> > +               arg0 = SMC_IPI_MAILBOX_NOTIFY;
> > +               arg0 = SMC_IPI_MAILBOX_ACK;
> >
> This is obviously wrong.
[Wendy] will fix
> 
> ....
> > +       mbox->chans = chans;
> > +       chans[IPI_MB_CHNL_TX].con_priv = &ipi_mbox-
> >mchans[IPI_MB_CHNL_TX];
> > +       chans[IPI_MB_CHNL_RX].con_priv = &ipi_mbox-
> >mchans[IPI_MB_CHNL_RX];
> > +       ipi_mbox->mchans[IPI_MB_CHNL_TX].chan_type = IPI_MB_CHNL_TX;
> > +       ipi_mbox->mchans[IPI_MB_CHNL_RX].chan_type = IPI_MB_CHNL_RX;
> > +       ret = mbox_controller_register(mbox);
> >
> while at it, you may want to start using
> devm_mbox_controller_register() recently added by Thierry.
[Wendy] will take a look at this function call
> 
> > +       if (ret)
> > +               dev_err(mdev,
> > +                       "Failed to register mbox_controller(%d)\n", ret);
> > +       else
> > +               dev_info(mdev, "Probed ZynqMP IPI Mailbox driver.\n");
> >
> You may want to also print something instance specific here.
[Wendy] will add more information such as the number of channels.
> 
> 
> > +static int zynqmp_ipi_probe(struct platform_device *pdev) {
> > +       struct device *dev = &pdev->dev;
> > +       struct device_node *nc, *np = pdev->dev.of_node;
> > +       struct zynqmp_ipi_pdata *pdata;
> > +       struct zynqmp_ipi_mbox *mbox;
> > +       int i, ret = -EINVAL;
> > +
> > +       i = 0;
> > +       for_each_available_child_of_node(np, nc)
> > +               i++;
> >
> of_get_child_count() ?
[Wendy] Will change.

Thanks,
Wendy
> 
> 
> Thnx

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

end of thread, other threads:[~2019-01-11  0:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-20 17:32 [PATH v7 0/2] Xilinx ZynqMP IPI Mailbox Controller Driver Wendy Liang
2018-12-20 17:32 ` [PATH v7 1/2] mailbox: ZynqMP IPI mailbox controller Wendy Liang
2018-12-22  1:59   ` Jassi Brar
2019-01-11  0:58     ` Jiaying Liang
2018-12-20 17:32 ` [PATH v7 2/2] dt-bindings: mailbox: Add Xilinx IPI Mailbox Wendy Liang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).