Thread (5 messages) 5 messages, 2 authors, 2020-02-25

RE: [PATCH V2 1/2] mailbox: imx: support SCU channel type

From: Peng Fan <peng.fan@nxp.com>
Date: 2020-02-25 07:59:50
Also in: lkml

Hi Oleksij,
quoted hunk ↗ jump to hunk
Subject: Re: [PATCH V2 1/2] mailbox: imx: support SCU channel type

Hi,

On 25.02.20 04:17, peng.fan@nxp.com wrote:
quoted
From: Peng Fan <peng.fan@nxp.com>

Per i.MX8QXP Reference mannual, Chapter "12.9.2.3.2 Messaging
Examples",
quoted
  Passing short messages: Transmit register(s) can be used to pass
  short messages from one to four words in length. For example, when
  a four-word message is desired, only one of the registers needs to
  have its corresponding interrupt enable bit set at the receiver side;
  the message’s first three words are written to the registers whose
  interrupt is masked, and the fourth word is written to the other
  register (which triggers an interrupt at the receiver side).

i.MX8/8X SCU firmware IPC is an implementation of passing short
messages. But current imx-mailbox driver only support one word
message, i.MX8/8X linux side firmware has to request four TX and four
RX to support IPC to SCU firmware. This is low efficent and more
interrupts triggered compared with one TX and one RX.

To make SCU channel type work,
   - parse the size of msg.
   - Only enable TR0/RR0 interrupt for transmit/receive message.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
  drivers/mailbox/imx-mailbox.c | 46
+++++++++++++++++++++++++++++++++++++++----
quoted
  1 file changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/mailbox/imx-mailbox.c
b/drivers/mailbox/imx-mailbox.c index 2cdcdc5f1119..0b4a33254114
100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -4,6 +4,7 @@
   */

  #include <linux/clk.h>
+#include <linux/firmware/imx/ipc.h>
  #include <linux/interrupt.h>
  #include <linux/io.h>
  #include <linux/kernel.h>
@@ -65,8 +66,14 @@ struct imx_mu_priv {
  	int			irq;

  	bool			side_b;
+	bool			scu;
  };

+struct imx_sc_rpc_msg_max {
+	struct imx_sc_rpc_msg hdr;
+	u32 data[7];
+} __packed __aligned(4);;
+
  static const struct imx_mu_dcfg imx_mu_cfg_imx6sx = {
  	.xTR	= {0x0, 0x4, 0x8, 0xc},
  	.xRR	= {0x10, 0x14, 0x18, 0x1c},
@@ -123,7 +130,10 @@ static irqreturn_t imx_mu_isr(int irq, void *p)
  	struct mbox_chan *chan = p;
  	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
  	struct imx_mu_con_priv *cp = chan->con_priv;
+	struct imx_sc_rpc_msg_max msg;
+	u32 *p_msg = (u32 *)&msg;
  	u32 val, ctrl, dat;
+	int i;

  	ctrl = imx_mu_read(priv, priv->dcfg->xCR);
  	val = imx_mu_read(priv, priv->dcfg->xSR); @@ -152,8 +162,19 @@
static irqreturn_t imx_mu_isr(int irq, void *p)
  		imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_TIEn(cp->idx));
  		mbox_chan_txdone(chan, 0);
  	} else if (val == IMX_MU_xSR_RFn(cp->idx)) {
-		dat = imx_mu_read(priv, priv->dcfg->xRR[cp->idx]);
-		mbox_chan_received_data(chan, (void *)&dat);
+		if (!priv->scu) {
+			dat = imx_mu_read(priv, priv->dcfg->xRR[cp->idx]);
+			mbox_chan_received_data(chan, (void *)&dat);
+		} else {
+			imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_RIEn(0));
+			*p_msg++ = imx_mu_read(priv, priv->dcfg->xRR[0]);
+			for (i = 1; i < msg.hdr.size; i++) {
+				*p_msg++ = imx_mu_read(priv,
+						       priv->dcfg->xRR[i % 4]);
+			}
+			imx_mu_xcr_rmw(priv, IMX_MU_xCR_RIEn(0), 0);
+			mbox_chan_received_data(chan, (void *)&msg);
+		}
  	} else if (val == IMX_MU_xSR_GIPn(cp->idx)) {
  		imx_mu_write(priv, IMX_MU_xSR_GIPn(cp->idx),
priv->dcfg->xSR);
quoted
  		mbox_chan_received_data(chan, NULL); @@ -169,11 +190,20
@@ static
quoted
int imx_mu_send_data(struct mbox_chan *chan, void *data)
  {
  	struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox);
  	struct imx_mu_con_priv *cp = chan->con_priv;
+	struct imx_sc_rpc_msg_max *msg = data;
  	u32 *arg = data;
+	int i;

  	switch (cp->type) {
  	case IMX_MU_TYPE_TX:
-		imx_mu_write(priv, *arg, priv->dcfg->xTR[cp->idx]);
+		if (priv->scu) {
+			for (i = 0; i < msg->hdr.size; i++) {
+				imx_mu_write(priv, *arg++,
+					     priv->dcfg->xTR[i % 4]);
+			}
+		} else {
+			imx_mu_write(priv, *arg, priv->dcfg->xTR[cp->idx]);
+		}
  		imx_mu_xcr_rmw(priv, IMX_MU_xCR_TIEn(cp->idx), 0);
  		break;
  	case IMX_MU_TYPE_TXDB:
@@ -259,6 +289,7 @@ static const struct mbox_chan_ops imx_mu_ops = {
  static struct mbox_chan * imx_mu_xlate(struct mbox_controller *mbox,
  				       const struct of_phandle_args *sp)
  {
+	struct imx_mu_priv *priv = to_imx_mu_priv(mbox);
  	u32 type, idx, chan;

  	if (sp->args_count != 2) {
@@ -270,7 +301,9 @@ static struct mbox_chan * imx_mu_xlate(struct
mbox_controller *mbox,
quoted
  	idx = sp->args[1]; /* index */
  	chan = type * 4 + idx;

-	if (chan >= mbox->num_chans) {
+	/* For TX/RX SCU, only one channel supported */
+	if ((chan >= mbox->num_chans) ||
+	    (priv->scu && type < 1 && idx >= 1)) {
  		dev_err(mbox->dev, "Not supported channel number: %d.
(type: %d, idx: %d)\n", chan, type, idx);
quoted
  		return ERR_PTR(-EINVAL);
  	}
@@ -341,6 +374,11 @@ static int imx_mu_probe(struct platform_device
*pdev)
quoted
  	}

  	priv->side_b = of_property_read_bool(np, "fsl,mu-side-b");
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx-scu");
This will configure every MU on the SoC as SCU, even it is not communicating
with it.
quoted
+	if (np) {
+		priv->scu = true;
+		of_node_put(np);
+	}
Please take a look at drivers/remoteproc/imx_rproc.c especially the use of
struct imx_rproc_dcfg.
Introduce simialr struct in the imx-mailbox.c
ok, I'll introduce something like that.
It will be some thing like:
struct imx_mu_dcfg {
	unsigned int chans; /* number of supported channels */
	int (*tx)(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp);
	int (*rx)(struct imx_mu_priv *priv, struct imx_mu_con_priv *cp); }

Define functions for generic tx/rx. And for SCU specific tx/rx. Please, add
sanity check in to SCU specific functions. In current implementation an error
on SCU side will couse stack corruption on Linux side.
We expect SCU not crash or have error behavior. Anyway I'll add check in Linux
side.
Use compatible string (some thing like fsl,imx8-mu-scu) to detect proper
metrics of new device.
ok. Thanks for your suggestions.

Thanks,
Peng.
Kind regards,
Oleksij Rempel

--
Pengutronix e.K.                           |
|
Industrial Linux Solutions                 |
https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.p
engutronix.de%2F&amp;data=02%7C01%7Cpeng.fan%40nxp.com%7C96e4b5
fa939d47929c4208d7b9bc26b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
C0%7C0%7C637182090067313481&amp;sdata=y%2BI7j05Cxsz2e67aNiWD3
Tg3%2FCY1JNIIBIfGm4j1bB4%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 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help