linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] mailbox: imx-mailbox: add SCU protocol support
@ 2018-07-11 16:32 Dong Aisheng
  2018-07-11 16:32 ` [RFC PATCH v6 1/2] dt-bindings: arm: fsl: add mu binding doc Dong Aisheng
  2018-07-11 16:32 ` [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support Dong Aisheng
  0 siblings, 2 replies; 7+ messages in thread
From: Dong Aisheng @ 2018-07-11 16:32 UTC (permalink / raw)
  To: linux-arm-kernel

This is per Sascha's reques to implemented SCU protocol support
based on Oleksij's generic mailbox driver for communicate with M4.

That driver is also still under review.
https://www.spinics.net/lists/arm-kernel/msg659013.html

Due to the protocol is totally different, most of code can
not be reused for SCU. So we implement a separate mbox_chan_ops
for SCU protocol to avoid mixing the difference.

Dong Aisheng (2):
  dt-bindings: arm: fsl: add mu binding doc
  mailbox: imx-mailbox: add scu protocol support

 .../devicetree/bindings/mailbox/fsl,mu.txt         |  34 +++
 drivers/mailbox/Kconfig                            |   6 +-
 drivers/mailbox/imx-mailbox.c                      | 252 ++++++++++++++++++---
 3 files changed, 257 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/fsl,mu.txt

-- 
2.7.4

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

* [RFC PATCH v6 1/2] dt-bindings: arm: fsl: add mu binding doc
  2018-07-11 16:32 [RFC PATCH 0/2] mailbox: imx-mailbox: add SCU protocol support Dong Aisheng
@ 2018-07-11 16:32 ` Dong Aisheng
  2018-07-11 16:32 ` [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support Dong Aisheng
  1 sibling, 0 replies; 7+ messages in thread
From: Dong Aisheng @ 2018-07-11 16:32 UTC (permalink / raw)
  To: linux-arm-kernel

The Messaging Unit module enables two processors within
the SoC to communicate and coordinate by passing messages
(e.g. data, status and control) through the MU interface.

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree at vger.kernel.org
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 .../devicetree/bindings/mailbox/fsl,mu.txt         | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/fsl,mu.txt

diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
new file mode 100644
index 0000000..90e4905
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt
@@ -0,0 +1,34 @@
+NXP i.MX Messaging Unit (MU)
+--------------------------------------------------------------------
+
+The Messaging Unit module enables two processors within the SoC to
+communicate and coordinate by passing messages (e.g. data, status
+and control) through the MU interface. The MU also provides the ability
+for one processor to signal the other processor using interrupts.
+
+Because the MU manages the messaging between processors, the MU uses
+different clocks (from each side of the different peripheral buses).
+Therefore, the MU must synchronize the accesses from one side to the
+other. The MU accomplishes synchronization using two sets of matching
+registers (Processor A-facing, Processor B-facing).
+
+Messaging Unit Device Node:
+=============================
+
+Required properties:
+-------------------
+- compatible :	should be "fsl,<chip>-mu", the supported chips include
+		imx8qxp, imx8qm.
+- reg :		Should contain the registers location and length
+- interrupts :	Interrupt number. The interrupt specifier format depends
+		on the interrupt controller parent.
+- #mbox-cells:  Must be 0. Number of cells in a mailbox
+
+Examples:
+--------
+lsio_mu0: mailbox at 5d1b0000 {
+	compatible = "fsl,imx8qxp-mu";
+	reg = <0x0 0x5d1b0000 0x0 0x10000>;
+	interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
+	#mbox-cells = <0>;
+};
-- 
2.7.4

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

* [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support
  2018-07-11 16:32 [RFC PATCH 0/2] mailbox: imx-mailbox: add SCU protocol support Dong Aisheng
  2018-07-11 16:32 ` [RFC PATCH v6 1/2] dt-bindings: arm: fsl: add mu binding doc Dong Aisheng
@ 2018-07-11 16:32 ` Dong Aisheng
  2018-07-12  7:01   ` Sascha Hauer
  1 sibling, 1 reply; 7+ messages in thread
From: Dong Aisheng @ 2018-07-11 16:32 UTC (permalink / raw)
  To: linux-arm-kernel

Add SCU protocol support in the generic mailbox driver.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/mailbox/Kconfig       |   6 +-
 drivers/mailbox/imx-mailbox.c | 252 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 223 insertions(+), 35 deletions(-)

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 91dd507..40b8736 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -17,9 +17,11 @@ config ARM_MHU
 
 config IMX_MBOX
 	tristate "iMX Mailbox"
-	depends on SOC_IMX7D || COMPILE_TEST
+	depends on ARCH_MXC || COMPILE_TEST
 	help
-	  Mailbox implementation for iMX7D Messaging Unit (MU).
+	  An implementation of the i.MX MU Mailbox. It is used to send message
+	  between application processors and other processors/MCU/DSP. Select
+	  Y here if you want to use i.MX MU Mailbox controller.
 
 config PLATFORM_MHU
 	tristate "Platform MHU Mailbox"
diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 5f925dc..2931d57 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -1,11 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (c) 2018 Pengutronix, Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
+ * Copyright 2018 NXP, Dong Aisheng <aisheng.dong@nxp.com>
  */
 
 #include <linux/clk.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/mailbox_controller.h>
 #include <linux/module.h>
@@ -23,6 +25,12 @@
 
 /* Control Register */
 #define IMX_MU_xCR		0x24
+#define IMX_MU_xCR_GIEn_MASK	GENMASK(37, 28)
+#define IMX_MU_xCR_RIEn_MASK	GENMASK(27, 24)
+#define IMX_MU_xCR_TIEn_MASK	GENMASK(23, 20)
+#define IMX_MU_xCR_GIRn_MASK	GENMASK(19, 16)
+#define IMX_MU_xCR_Fn_MASK	GENMASK(2, 0)
+
 /* Transmit Interrupt Enable */
 #define IMX_MU_xCR_TIEn(x)	BIT(20 + (x))
 /* Receive Interrupt Enable */
@@ -30,6 +38,8 @@
 
 #define IMX_MU_MAX_CHANS	4u
 
+#define MU_DATA_TIME_OUT_US	(100 * USEC_PER_MSEC)
+
 struct imx_mu_priv;
 
 struct imx_mu_cfg {
@@ -38,7 +48,6 @@ struct imx_mu_cfg {
 };
 
 struct imx_mu_con_priv {
-	int			irq;
 	unsigned int		bidx;
 	unsigned int		idx;
 };
@@ -53,6 +62,11 @@ struct imx_mu_priv {
 
 	struct imx_mu_con_priv  con_priv[IMX_MU_MAX_CHANS];
 	struct clk		*clk;
+	int			irq;
+
+	bool			is_scu_pro;
+	/* for runtime scu msg store */
+	u32 *msg;
 };
 
 static struct imx_mu_priv *to_imx_mu_priv(struct mbox_controller *mbox)
@@ -82,6 +96,161 @@ static u32 imx_mu_rmw(struct imx_mu_priv *priv, u32 offs, u32 set, u32 clr)
 	return val;
 }
 
+static struct mbox_chan *imx_mu_scu_index_xlate(struct mbox_controller *mbox,
+						const struct of_phandle_args *sp)
+{
+	if (sp->args_count != 0) {
+		dev_err(mbox->dev,
+			"incorrect mu channel specified in devicetree\n");
+		return NULL;
+	}
+
+	return &mbox->chans[0];
+}
+
+/*
+ * Wait to receive message from the other core.
+ */
+static int imx_mu_scu_receive_msg(struct mbox_chan *chan, u32 index, u32 *msg)
+{
+	struct imx_mu_priv *priv = chan->con_priv;
+	u32 mask, sr;
+	int ret;
+
+	mask = IMX_MU_xSR_RFn(3 - index);
+
+	/* Wait RX register to be full. */
+	ret = readl_poll_timeout_atomic(priv->base + IMX_MU_xSR, sr, sr & mask,
+					0, MU_DATA_TIME_OUT_US);
+	if (ret) {
+		dev_err(chan->mbox->dev,
+			"Waiting MU receive register (%u) full timeout!\n",
+			index);
+		return ret;
+	}
+
+	*msg = imx_mu_read(priv, IMX_MU_xRRn(index));
+
+	return 0;
+}
+
+static bool imx_mu_scu_peek_data(struct mbox_chan *chan)
+{
+	struct imx_mu_priv *priv = chan->con_priv;
+	u8 *raw_data;
+	int i, size;
+	int ret;
+
+	ret = imx_mu_scu_receive_msg(chan, 0, priv->msg);
+	if (ret)
+		return false;
+
+	raw_data = (u8 *)priv->msg;
+	size = raw_data[1];
+
+	dev_dbg(chan->mbox->dev, "receive data: hdr 0x%x size %d\n",
+		*priv->msg, size);
+
+	for (i = 1; i < size; i++) {
+		ret = imx_mu_scu_receive_msg(chan, i % 4, priv->msg + i);
+		if (ret)
+			return false;
+	}
+
+	mbox_chan_received_data(chan, (void *)priv->msg);
+
+	return true;
+}
+
+
+/*
+ * Wait and send message to the other core.
+ */
+static int imx_mu_scu_send_msg(struct mbox_chan *chan, u32 index, u32 msg)
+{
+	struct imx_mu_priv *priv = chan->con_priv;
+	u32 mask, sr;
+	int ret;
+
+	mask = IMX_MU_xSR_TEn(3 - index);
+
+	/* Wait TX register to be empty. */
+	ret = readl_poll_timeout_atomic(priv->base + IMX_MU_xSR, sr, sr & mask,
+					0, MU_DATA_TIME_OUT_US);
+	if (ret) {
+		dev_err(chan->mbox->dev,
+			"Waiting MU transmit register (%u) empty timeout!\n",
+			index);
+		return ret;
+	}
+
+	imx_mu_write(priv, msg, IMX_MU_xTRn(index));
+
+	return 0;
+}
+
+static int imx_mu_scu_send_data(struct mbox_chan *chan, void *data)
+{
+	struct imx_mu_priv *priv = chan->con_priv;
+	u8 *raw_data = data;
+	int i, ret;
+	int size;
+
+	if (!data)
+		return -EINVAL;
+
+	priv->msg = data;
+
+	/* SCU protocol size position is at the second u8 */
+	size = raw_data[1];
+
+	dev_dbg(chan->mbox->dev, "send data (size %d)\n", size);
+
+	for (i = 0; i < size; i++) {
+		ret = imx_mu_scu_send_msg(chan, i % 4, *(priv->msg + i));
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int imx_mu_scu_startup(struct mbox_chan *chan)
+{
+	struct imx_mu_priv *priv = chan->con_priv;
+	u32 val;
+
+	/* Init MU */
+	val = imx_mu_read(priv, IMX_MU_xCR);
+	/* Clear GIEn, RIEn, TIEn, GIRn and ABFn. */
+	val &= ~(IMX_MU_xCR_GIEn_MASK | IMX_MU_xCR_RIEn_MASK |
+		 IMX_MU_xCR_TIEn_MASK | IMX_MU_xCR_GIRn_MASK |
+		 IMX_MU_xCR_Fn_MASK);
+	imx_mu_write(priv, val, IMX_MU_xCR);
+
+	return 0;
+}
+
+static const struct mbox_chan_ops imx_mu_scu_ops = {
+	.startup = imx_mu_scu_startup,
+	.send_data = imx_mu_scu_send_data,
+	.peek_data = imx_mu_scu_peek_data,
+};
+
+int imx_mu_scu_init(struct imx_mu_priv *priv)
+{
+	priv->mbox.dev = priv->dev;
+	priv->mbox.ops = &imx_mu_scu_ops;
+	priv->mbox.chans = &priv->mbox_chans[0];
+	priv->mbox.num_chans = 1;
+	priv->mbox.txdone_irq = false;
+	priv->mbox.txdone_poll = false;
+	priv->mbox.of_xlate = &imx_mu_scu_index_xlate;
+	priv->mbox.chans->con_priv = priv;
+
+	return 0;
+}
+
 static irqreturn_t imx_mu_isr(int irq, void *p)
 {
 	struct mbox_chan *chan = p;
@@ -140,11 +309,11 @@ static int imx_mu_startup(struct mbox_chan *chan)
 	struct imx_mu_con_priv *cp = chan->con_priv;
 	int ret;
 
-	ret = request_irq(cp->irq, imx_mu_isr,
+	ret = request_irq(priv->irq, imx_mu_isr,
 			  IRQF_SHARED, "imx_mu_chan", chan);
 	if (ret) {
 		dev_err(chan->mbox->dev,
-			"Unable to acquire IRQ %d\n", cp->irq);
+			"Unable to acquire IRQ %d\n", priv->irq);
 		return ret;
 	}
 
@@ -161,7 +330,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
 	imx_mu_rmw(priv, IMX_MU_xCR, 0,
 		   IMX_MU_xCR_TIEn(cp->bidx) | IMX_MU_xCR_RIEn(cp->bidx));
 
-	free_irq(cp->irq, chan);
+	free_irq(priv->irq, chan);
 }
 
 static const struct mbox_chan_ops imx_mu_ops = {
@@ -170,24 +339,47 @@ static const struct mbox_chan_ops imx_mu_ops = {
 	.shutdown = imx_mu_shutdown,
 };
 
+int imx_mu_init(struct imx_mu_priv *priv)
+{
+	unsigned int chans;
+	int i;
+
+	priv->dcfg = of_device_get_match_data(priv->dev);
+	if (!priv->dcfg)
+		return -EINVAL;
+
+	chans = min(priv->dcfg->chans, IMX_MU_MAX_CHANS);
+	/* Initialize channel identifiers */
+	for (i = 0; i < chans; i++) {
+		struct imx_mu_con_priv *cp = &priv->con_priv[i];
+
+		cp->bidx = 3 - i;
+		cp->idx = i;
+		priv->mbox_chans[i].con_priv = cp;
+	}
+
+	priv->mbox.dev = priv->dev;
+	priv->mbox.ops = &imx_mu_ops;
+	priv->mbox.chans = priv->mbox_chans;
+	priv->mbox.num_chans = chans;
+	priv->mbox.txdone_irq = true;
+
+	if (priv->dcfg->init_hw)
+		priv->dcfg->init_hw(priv);
+}
+
 static int imx_mu_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct resource *iomem;
 	struct imx_mu_priv *priv;
-	const struct imx_mu_cfg *dcfg;
-	unsigned int i, chans;
-	int irq, ret;
+	u32 val = 0;
+	int ret;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
-	dcfg = of_device_get_match_data(dev);
-	if (!dcfg)
-		return -EINVAL;
-
-	priv->dcfg = dcfg;
 	priv->dev = dev;
 
 	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -195,9 +387,9 @@ static int imx_mu_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq <= 0)
-		return irq < 0 ? irq : -EINVAL;
+	priv->irq = platform_get_irq(pdev, 0);
+	if (priv->irq <= 0)
+		return priv->irq < 0 ? priv->irq : -EINVAL;
 
 	priv->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(priv->clk)) {
@@ -215,28 +407,21 @@ static int imx_mu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	chans = min(dcfg->chans, IMX_MU_MAX_CHANS);
-	/* Initialize channel identifiers */
-	for (i = 0; i < chans; i++) {
-		struct imx_mu_con_priv *cp = &priv->con_priv[i];
+	ret = of_property_read_u32(dev->of_node, "#mbox-cells", &val);
+	if (ret)
+		return ret;
 
-		cp->bidx = 3 - i;
-		cp->idx = i;
-		cp->irq = irq;
-		priv->mbox_chans[i].con_priv = cp;
-	}
+	priv->is_scu_pro = !val;
+	if (priv->is_scu_pro)
+		ret = imx_mu_scu_init(priv);
+	else
+		ret = imx_mu_init(priv);
 
-	priv->mbox.dev = dev;
-	priv->mbox.ops = &imx_mu_ops;
-	priv->mbox.chans = priv->mbox_chans;
-	priv->mbox.num_chans = chans;
-	priv->mbox.txdone_irq = true;
+	if (ret)
+		return ret;
 
 	platform_set_drvdata(pdev, priv);
 
-	if (priv->dcfg->init_hw)
-		priv->dcfg->init_hw(priv);
-
 	return mbox_controller_register(&priv->mbox);
 }
 
@@ -250,7 +435,6 @@ static int imx_mu_remove(struct platform_device *pdev)
 	return 0;
 }
 
-
 static void imx_mu_init_imx7d_a(struct imx_mu_priv *priv)
 {
 	/* Set default config */
@@ -269,6 +453,7 @@ static const struct imx_mu_cfg imx_mu_cfg_imx7d_b = {
 static const struct of_device_id imx_mu_dt_ids[] = {
 	{ .compatible = "fsl,imx7s-mu-a", .data = &imx_mu_cfg_imx7d_a },
 	{ .compatible = "fsl,imx7s-mu-b", .data = &imx_mu_cfg_imx7d_b },
+	{ .compatible = "fsl,imx8qxp-mu", NULL },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, imx_mu_dt_ids);
@@ -284,5 +469,6 @@ static struct platform_driver imx_mu_driver = {
 module_platform_driver(imx_mu_driver);
 
 MODULE_AUTHOR("Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>");
+MODULE_AUTHOR("Dong Aisheng <aisheng.dong@nxp.com>");
 MODULE_DESCRIPTION("Message Unit driver for i.MX");
 MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

* [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support
  2018-07-11 16:32 ` [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support Dong Aisheng
@ 2018-07-12  7:01   ` Sascha Hauer
  2018-07-12  7:31     ` A.s. Dong
                       ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sascha Hauer @ 2018-07-12  7:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 12, 2018 at 12:32:39AM +0800, Dong Aisheng wrote:
> Add SCU protocol support in the generic mailbox driver.
> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  drivers/mailbox/Kconfig       |   6 +-
>  drivers/mailbox/imx-mailbox.c | 252 ++++++++++++++++++++++++++++++++++++------
>  2 files changed, 223 insertions(+), 35 deletions(-)
> 
> +static bool imx_mu_scu_peek_data(struct mbox_chan *chan)
> +{
> +	struct imx_mu_priv *priv = chan->con_priv;
> +	u8 *raw_data;
> +	int i, size;
> +	int ret;
> +
> +	ret = imx_mu_scu_receive_msg(chan, 0, priv->msg);
> +	if (ret)
> +		return false;
> +
> +	raw_data = (u8 *)priv->msg;
> +	size = raw_data[1];
> +
> +	dev_dbg(chan->mbox->dev, "receive data: hdr 0x%x size %d\n",
> +		*priv->msg, size);
> +
> +	for (i = 1; i < size; i++) {
> +		ret = imx_mu_scu_receive_msg(chan, i % 4, priv->msg + i);
> +		if (ret)
> +			return false;
> +	}
> +
> +	mbox_chan_received_data(chan, (void *)priv->msg);
> +
> +	return true;
> +}

Receiving data now works by calling mbox_client_peek_data(). I think
your plan is not to use rx_callback as triggered by mbox_chan_received_data,
but instead exploiting the fact that you have put the receive data into
the same buffer that you previously used to send a message. That way the
client knows where to find the answer.

While that should work I don't think this is very clean. That is the
point where I mentioned I think that the mailbox framework would need a
way to receive messages synchronously.

Jassi, this is for you probably. Do you have an idea how synchronous
receive could be implemented? We'll need some mbox_client_receive_data
function.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support
  2018-07-12  7:01   ` Sascha Hauer
@ 2018-07-12  7:31     ` A.s. Dong
  2018-07-23 15:07     ` Jassi Brar
  2018-07-23 15:09     ` Jassi Brar
  2 siblings, 0 replies; 7+ messages in thread
From: A.s. Dong @ 2018-07-12  7:31 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Sascha,

> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer at pengutronix.de]
> Sent: Thursday, July 12, 2018 3:01 PM
> To: A.s. Dong <aisheng.dong@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org; dongas86 at gmail.com;
> jassisinghbrar at gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> kernel at pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>;
> shawnguo at kernel.org
> Subject: Re: [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol
> support
> 
> On Thu, Jul 12, 2018 at 12:32:39AM +0800, Dong Aisheng wrote:
> > Add SCU protocol support in the generic mailbox driver.
> >
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> >  drivers/mailbox/Kconfig       |   6 +-
> >  drivers/mailbox/imx-mailbox.c | 252
> > ++++++++++++++++++++++++++++++++++++------
> >  2 files changed, 223 insertions(+), 35 deletions(-)
> >
> > +static bool imx_mu_scu_peek_data(struct mbox_chan *chan) {
> > +	struct imx_mu_priv *priv = chan->con_priv;
> > +	u8 *raw_data;
> > +	int i, size;
> > +	int ret;
> > +
> > +	ret = imx_mu_scu_receive_msg(chan, 0, priv->msg);
> > +	if (ret)
> > +		return false;
> > +
> > +	raw_data = (u8 *)priv->msg;
> > +	size = raw_data[1];
> > +
> > +	dev_dbg(chan->mbox->dev, "receive data: hdr 0x%x size %d\n",
> > +		*priv->msg, size);
> > +
> > +	for (i = 1; i < size; i++) {
> > +		ret = imx_mu_scu_receive_msg(chan, i % 4, priv->msg + i);
> > +		if (ret)
> > +			return false;
> > +	}
> > +
> > +	mbox_chan_received_data(chan, (void *)priv->msg);
> > +
> > +	return true;
> > +}
> 
> Receiving data now works by calling mbox_client_peek_data(). I think your
> plan is not to use rx_callback as triggered by mbox_chan_received_data, but
> instead exploiting the fact that you have put the receive data into the same
> buffer that you previously used to send a message. That way the client
> knows where to find the answer.

Yes, exactly.

I just quickly sent out mu part for your review, ipc is still not cleaned up.
The test seemed ok.

The implementation is like:
int sc_call_rpc(sc_ipc_t ipc, sc_rpc_msg_t *msg, bool no_resp)
{
        struct sc_ipc *sc_ipc = (struct sc_ipc *)ipc;
        int ret;

        if (WARN_ON(!sc_ipc || !msg))
                return -EINVAL;

        /* Check size */
        if (msg->size > SC_RPC_MAX_MSG)
                return -EINVAL;

        dev_dbg(sc_ipc->cl.dev, "RPC SVC %u FUNC %u SIZE %u\n", RPC_SVC(msg),
                RPC_FUNC(msg), RPC_SIZE(msg));

        mutex_lock(&sc_ipc->lock);

        ret = mbox_send_message(sc_ipc->chan, msg);
        if (ret < 0) {
                dev_err(sc_ipc->cl.dev, "sc send msg failed: %d\n", ret);
                goto out;
        }

        if (!no_resp) {
                ret = mbox_client_peek_data(sc_ipc->chan);
                if (!ret)
                        return -ETIMEDOUT;
                else
                        ret = 0;
        }

        mbox_client_txdone(sc_ipc->chan, ret);

out:
        mutex_unlock(&sc_ipc->lock);

        dev_dbg(sc_ipc->cl.dev, "RPC SVC done\n");

        return ret;
}

> 
> While that should work I don't think this is very clean. That is the point where
> I mentioned I think that the mailbox framework would need a way to receive
> messages synchronously.
> 
> Jassi, this is for you probably. Do you have an idea how synchronous receive
> could be implemented? We'll need some mbox_client_receive_data
> function.
> 

Jassi seemed to believe .peek_data() works

Regards
Dong Aisheng

> Sascha
> 
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.pengutronix.de%2F&amp;data=02%7C01%7Caisheng.dong%40nxp.com%7
> Cfe3ebeb68c31461b8a9108d5e7c54ca8%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636669756934531262&amp;sdata=dRnijYEto0dFpSN0a2wne
> a%2BoodKIBxskVLg8%2BztKDPM%3D&amp;reserved=0  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support
  2018-07-12  7:01   ` Sascha Hauer
  2018-07-12  7:31     ` A.s. Dong
@ 2018-07-23 15:07     ` Jassi Brar
  2018-07-23 15:09     ` Jassi Brar
  2 siblings, 0 replies; 7+ messages in thread
From: Jassi Brar @ 2018-07-23 15:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 12, 2018 at 12:31 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, Jul 12, 2018 at 12:32:39AM +0800, Dong Aisheng wrote:
>> Add SCU protocol support in the generic mailbox driver.
>>
>> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>> ---
>>  drivers/mailbox/Kconfig       |   6 +-
>>  drivers/mailbox/imx-mailbox.c | 252 ++++++++++++++++++++++++++++++++++++------
>>  2 files changed, 223 insertions(+), 35 deletions(-)
>>
>> +static bool imx_mu_scu_peek_data(struct mbox_chan *chan)
>> +{
>> +     struct imx_mu_priv *priv = chan->con_priv;
>> +     u8 *raw_data;
>> +     int i, size;
>> +     int ret;
>> +
>> +     ret = imx_mu_scu_receive_msg(chan, 0, priv->msg);
>> +     if (ret)
>> +             return false;
>> +
>> +     raw_data = (u8 *)priv->msg;
>> +     size = raw_data[1];
>> +
>> +     dev_dbg(chan->mbox->dev, "receive data: hdr 0x%x size %d\n",
>> +             *priv->msg, size);
>> +
>> +     for (i = 1; i < size; i++) {
>> +             ret = imx_mu_scu_receive_msg(chan, i % 4, priv->msg + i);
>> +             if (ret)
>> +                     return false;
>> +     }
>> +
>> +     mbox_chan_received_data(chan, (void *)priv->msg);
>> +
>> +     return true;
>> +}
>
> Receiving data now works by calling mbox_client_peek_data(). I think
> your plan is not to use rx_callback as triggered by mbox_chan_received_data,
> but instead exploiting the fact that you have put the receive data into
> the same buffer that you previously used to send a message. That way the
> client knows where to find the answer.
>
> While that should work I don't think this is very clean. That is the
> point where I mentioned I think that the mailbox framework would need a
> way to receive messages synchronously.
>
> Jassi, this is for you probably. Do you have an idea how synchronous
> receive could be implemented? We'll need some mbox_client_receive_data
> function.
>
Most platforms call mbox_chan_received_data() from RX interrupt
handler. Which is actually


> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support
  2018-07-12  7:01   ` Sascha Hauer
  2018-07-12  7:31     ` A.s. Dong
  2018-07-23 15:07     ` Jassi Brar
@ 2018-07-23 15:09     ` Jassi Brar
  2 siblings, 0 replies; 7+ messages in thread
From: Jassi Brar @ 2018-07-23 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 12, 2018 at 12:31 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, Jul 12, 2018 at 12:32:39AM +0800, Dong Aisheng wrote:
>> Add SCU protocol support in the generic mailbox driver.
>>
>> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>> ---
>>  drivers/mailbox/Kconfig       |   6 +-
>>  drivers/mailbox/imx-mailbox.c | 252 ++++++++++++++++++++++++++++++++++++------
>>  2 files changed, 223 insertions(+), 35 deletions(-)
>>
>> +static bool imx_mu_scu_peek_data(struct mbox_chan *chan)
>> +{
>> +     struct imx_mu_priv *priv = chan->con_priv;
>> +     u8 *raw_data;
>> +     int i, size;
>> +     int ret;
>> +
>> +     ret = imx_mu_scu_receive_msg(chan, 0, priv->msg);
>> +     if (ret)
>> +             return false;
>> +
>> +     raw_data = (u8 *)priv->msg;
>> +     size = raw_data[1];
>> +
>> +     dev_dbg(chan->mbox->dev, "receive data: hdr 0x%x size %d\n",
>> +             *priv->msg, size);
>> +
>> +     for (i = 1; i < size; i++) {
>> +             ret = imx_mu_scu_receive_msg(chan, i % 4, priv->msg + i);
>> +             if (ret)
>> +                     return false;
>> +     }
>> +
>> +     mbox_chan_received_data(chan, (void *)priv->msg);
>> +
>> +     return true;
>> +}
>
> Receiving data now works by calling mbox_client_peek_data(). I think
> your plan is not to use rx_callback as triggered by mbox_chan_received_data,
> but instead exploiting the fact that you have put the receive data into
> the same buffer that you previously used to send a message. That way the
> client knows where to find the answer.
>
> While that should work I don't think this is very clean. That is the
> point where I mentioned I think that the mailbox framework would need a
> way to receive messages synchronously.
>
> Jassi, this is for you probably. Do you have an idea how synchronous
> receive could be implemented? We'll need some mbox_client_receive_data
> function.
>
Most platforms call mbox_chan_received_data() from RX interrupt
handler. Which is actually calling Client's  rx_callback()

Or I miss your point?

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

end of thread, other threads:[~2018-07-23 15:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-11 16:32 [RFC PATCH 0/2] mailbox: imx-mailbox: add SCU protocol support Dong Aisheng
2018-07-11 16:32 ` [RFC PATCH v6 1/2] dt-bindings: arm: fsl: add mu binding doc Dong Aisheng
2018-07-11 16:32 ` [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support Dong Aisheng
2018-07-12  7:01   ` Sascha Hauer
2018-07-12  7:31     ` A.s. Dong
2018-07-23 15:07     ` Jassi Brar
2018-07-23 15:09     ` Jassi Brar

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).