All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leonard Crestez <leonard.crestez@nxp.com>
To: Shawn Guo <shawnguo@kernel.org>
Cc: Dong Aisheng <aisheng.dong@nxp.com>, Peng Fan <peng.fan@nxp.com>,
	Richard Zhu <hongxing.zhu@nxp.com>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	Oleksij Rempel <o.rempel@pengutronix.de>,
	linux-imx@nxp.com, kernel@pengutronix.de,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Daniel Baluta <daniel.baluta@nxp.com>,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH] firmware: imx: scu: Ensure sequential TX
Date: Thu, 20 Feb 2020 18:10:01 +0200	[thread overview]
Message-ID: <ae051784024f8fcc458437e278c27b4e79c6fe7d.1582214881.git.leonard.crestez@nxp.com> (raw)

SCU requires that all messages words are written sequentially but linux MU
driver implements multiple independent channels for each register so ordering
between different channels must be ensured by SCU API interface.

Wait for tx_done before every send to ensure that no queueing happens at the
mailbox channel level.

Fixes: edbee095fafb ("firmware: imx: add SCU firmware driver support")
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Cc: <stable@vger.kernel.org>
---
 drivers/firmware/imx/imx-scu.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

This manifests as "SCU timeout" message followed by system hang.

This is not a very pretty fix but avoids inserting additional waits
except in extremely rare circumstances.

An alternative would be to implement a new type of mailbox channel which
handles all 4 registers together. Exposing the MU as 4 independent
channels is very awkward.

diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
index 03b43b7a6d1d..f71eaa5bf52d 100644
--- a/drivers/firmware/imx/imx-scu.c
+++ b/drivers/firmware/imx/imx-scu.c
@@ -27,10 +27,11 @@ struct imx_sc_chan {
 	struct imx_sc_ipc *sc_ipc;
 
 	struct mbox_client cl;
 	struct mbox_chan *ch;
 	int idx;
+	struct completion tx_done;
 };
 
 struct imx_sc_ipc {
 	/* SCU uses 4 Tx and 4 Rx channels */
 	struct imx_sc_chan chans[SCU_MU_CHAN_NUM];
@@ -98,10 +99,18 @@ int imx_scu_get_handle(struct imx_sc_ipc **ipc)
 	*ipc = imx_sc_ipc_handle;
 	return 0;
 }
 EXPORT_SYMBOL(imx_scu_get_handle);
 
+/* Callback called when the word of a message is ack-ed, eg read by SCU */
+static void imx_scu_tx_done(struct mbox_client *cl, void *mssg, int r)
+{
+	struct imx_sc_chan *sc_chan = container_of(cl, struct imx_sc_chan, cl);
+
+	complete(&sc_chan->tx_done);
+}
+
 static void imx_scu_rx_callback(struct mbox_client *c, void *msg)
 {
 	struct imx_sc_chan *sc_chan = container_of(c, struct imx_sc_chan, cl);
 	struct imx_sc_ipc *sc_ipc = sc_chan->sc_ipc;
 	struct imx_sc_rpc_msg *hdr;
@@ -147,10 +156,23 @@ static int imx_scu_ipc_write(struct imx_sc_ipc *sc_ipc, void *msg)
 	dev_dbg(sc_ipc->dev, "RPC SVC %u FUNC %u SIZE %u\n", hdr->svc,
 		hdr->func, hdr->size);
 
 	for (i = 0; i < hdr->size; i++) {
 		sc_chan = &sc_ipc->chans[i % 4];
+
+		/*
+		 * SCU requires that all messages words are written
+		 * sequentially but linux MU driver implements multiple
+		 * independent channels for each register so ordering between
+		 * different channels must be ensured by SCU API interface.
+		 *
+		 * Wait for tx_done before every send to ensure that no
+		 * queueing happens at the mailbox channel level.
+		 */
+		wait_for_completion(&sc_chan->tx_done);
+		reinit_completion(&sc_chan->tx_done);
+
 		ret = mbox_send_message(sc_chan->ch, &data[i]);
 		if (ret < 0)
 			return ret;
 	}
 
@@ -245,10 +267,15 @@ static int imx_scu_probe(struct platform_device *pdev)
 		cl->dev = dev;
 		cl->tx_block = false;
 		cl->knows_txdone = true;
 		cl->rx_callback = imx_scu_rx_callback;
 
+		/* Initial tx_done completion as "done" */
+		cl->tx_done = imx_scu_tx_done;
+		init_completion(&sc_chan->tx_done);
+		complete(&sc_chan->tx_done);
+
 		sc_chan->sc_ipc = sc_ipc;
 		sc_chan->idx = i % 4;
 		sc_chan->ch = mbox_request_channel_byname(cl, chan_name);
 		if (IS_ERR(sc_chan->ch)) {
 			ret = PTR_ERR(sc_chan->ch);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

             reply	other threads:[~2020-02-20 16:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-20 16:10 Leonard Crestez [this message]
2020-02-21  1:26 ` [PATCH] firmware: imx: scu: Ensure sequential TX Peng Fan
2020-02-21  5:31 ` Oleksij Rempel
2020-02-24 20:45   ` Leonard Crestez
2020-02-24  7:03 ` Shawn Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae051784024f8fcc458437e278c27b4e79c6fe7d.1582214881.git.leonard.crestez@nxp.com \
    --to=leonard.crestez@nxp.com \
    --cc=aisheng.dong@nxp.com \
    --cc=daniel.baluta@nxp.com \
    --cc=fabio.estevam@nxp.com \
    --cc=hongxing.zhu@nxp.com \
    --cc=jassisinghbrar@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=o.rempel@pengutronix.de \
    --cc=peng.fan@nxp.com \
    --cc=shawnguo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.