[PATCH 0/3] mailbox: starfive: Add Support for StarFive Meu Mailbox driver

[PATCH 0/3] mailbox: starfive: Add Support for StarFive Meu Mailbox driver

[Joshua Yeong]

This patch series adds support for StarFive Meu Mailbox driver.

Joshua Yeong (3):
  mailbox: starfive: Add StarFive Meu Mailbox Driver
  dt-bindings: mailbox: starfive: Add StarFive Meu Mailbox Driver
  MAINTAINERS: Add entry for StarFive Mailbox MEU

 .../bindings/mailbox/starfive-meu.yaml        |  66 ++++
 MAINTAINERS                                   |   7 +
 drivers/mailbox/Kconfig                       |  11 +
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/starfive-meu.c                | 334 ++++++++++++++++++
 5 files changed, 420 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
 create mode 100644 drivers/mailbox/starfive-meu.c

--
2.25.1

[PATCH 1/3] mailbox: starfive: Add StarFive Meu Mailbox Driver

[Joshua Yeong]

This patch adds support for the StarFive Meu Mailbox driver. This enables
communication using mailbox doorbell between AP and SCP cores.

Co-developed-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
Signed-off-by: Joshua Yeong <joshua.yeong@starfivetech.com>
Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
---
 drivers/mailbox/Kconfig        |  11 ++
 drivers/mailbox/Makefile       |   2 +
 drivers/mailbox/starfive-meu.c | 334 +++++++++++++++++++++++++++++++++
 3 files changed, 347 insertions(+)
 create mode 100644 drivers/mailbox/starfive-meu.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index bc2e265cb02d..b5e38a50b3d5 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -295,4 +295,15 @@ config QCOM_IPCC
 	  acts as an interrupt controller for receiving interrupts from clients.
 	  Say Y here if you want to build this driver.
 
+config STARFIVE_MEU_MBOX
+	tristate "Starfive MEU Mailbox"
+	depends on OF
+	depends on SOC_STARFIVE || COMPILE_TEST
+	help
+	  Say Y here if you want to build the Starfive MEU mailbox controller
+	  driver.
+
+	  The StarFive mailbox controller has 2 channels. Each channel has a
+	  pair of Tx/Rx link and each link has 31 slots/doorbells.
+
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index fc9376117111..122cc691c256 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -62,3 +62,5 @@ obj-$(CONFIG_SPRD_MBOX)		+= sprd-mailbox.o
 obj-$(CONFIG_QCOM_IPCC)		+= qcom-ipcc.o
 
 obj-$(CONFIG_APPLE_MAILBOX)	+= apple-mailbox.o
+
+obj-$(CONFIG_STARFIVE_MEU_MBOX)	+= starfive-meu.o
diff --git a/drivers/mailbox/starfive-meu.c b/drivers/mailbox/starfive-meu.c
new file mode 100644
index 000000000000..cd439f8c474b
--- /dev/null
+++ b/drivers/mailbox/starfive-meu.c
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 Shanghai StarFive Technology Co., Ltd.
+ *
+ * Author: Jee Heng Sia <jeeheng.sia@starfivetech.com>
+ * Author: Joshua Yeong <joshua.yeong@starfivetech.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#define CHAN_SET_OFFSET		0x0
+#define CHAN_TX_STAT_OFFSET	0x4
+#define CHAN_RX_STAT_OFFSET	0x8
+#define CHAN_CLR_OFFSET		0xc
+#define CHAN_RX_REG_OFFSET	0x40
+
+#define CHAN_RX_DOORBELL	GENMASK(30, 0)
+
+#define MEU0_OFFSET		0x4000
+#define MEU1_OFFSET		0x0
+
+#define MEU_CHANS_GROUP		2
+#define MEU_NUM_DOORBELLS	31
+#define MEU_TOTAL_CHANS		(MEU_CHANS_GROUP * MEU_NUM_DOORBELLS)
+
+struct meu_db_link {
+	unsigned int irq;
+	void __iomem *tx_reg;
+	void __iomem *rx_reg;
+};
+
+struct starfive_meu {
+	void __iomem *base;
+	struct meu_db_link mlink[MEU_CHANS_GROUP];
+	struct mbox_controller mbox;
+	struct device *dev;
+};
+
+/**
+ * StarFive MEU Mailbox allocated channel information
+ *
+ * @meu: Pointer to parent mailbox device
+ * @pchan: Physical channel within which this doorbell resides in
+ * @doorbell: doorbell number pertaining to this channel
+ */
+struct meu_db_channel {
+	struct starfive_meu *meu;
+	unsigned int pchan;
+	unsigned int doorbell;
+};
+
+static inline struct mbox_chan *
+meu_db_mbox_to_channel(struct mbox_controller *mbox, unsigned int pchan,
+		       unsigned int doorbell)
+{
+	struct meu_db_channel *chan_info;
+	int i;
+
+	for (i = 0; i < mbox->num_chans; i++) {
+		chan_info = mbox->chans[i].con_priv;
+		if (chan_info && chan_info->pchan == pchan &&
+		    chan_info->doorbell == doorbell)
+			return &mbox->chans[i];
+	}
+
+	return NULL;
+}
+
+static void meu_db_mbox_clear_irq(struct mbox_chan *chan)
+{
+	struct meu_db_channel *chan_info = chan->con_priv;
+	void __iomem *base = chan_info->meu->mlink[chan_info->pchan].rx_reg;
+
+	writel_relaxed(BIT(chan_info->doorbell), base + CHAN_CLR_OFFSET);
+}
+
+static unsigned int meu_db_mbox_irq_to_pchan_num(struct starfive_meu *meu, int irq)
+{
+	unsigned int pchan;
+
+	for (pchan = 0; pchan < MEU_CHANS_GROUP; pchan++)
+		if (meu->mlink[pchan].irq == irq)
+			break;
+	return pchan;
+}
+
+static struct mbox_chan *
+meu_db_mbox_irq_to_channel(struct starfive_meu *meu, unsigned int pchan)
+{
+	void __iomem *base = meu->mlink[pchan].rx_reg;
+	struct mbox_controller *mbox = &meu->mbox;
+	struct mbox_chan *chan = NULL;
+	unsigned int doorbell;
+	unsigned long bits;
+
+	bits = FIELD_GET(CHAN_RX_DOORBELL, readl_relaxed(base + CHAN_RX_STAT_OFFSET));
+	if (!bits)
+		/* No IRQs fired in specified physical channel */
+		return NULL;
+
+	/* An IRQ has fired, find the associated channel */
+	for (doorbell = 0; bits; doorbell++) {
+		if (!test_and_clear_bit(doorbell, &bits))
+			continue;
+
+		chan = meu_db_mbox_to_channel(mbox, pchan, doorbell);
+		if (chan)
+			break;
+
+		/* Clear IRQ for unregistered doorbell */
+		writel_relaxed(BIT(doorbell), base + CHAN_CLR_OFFSET);
+		dev_err(mbox->dev,
+			"Channel not registered: pchan: %d doorbell: %d\n",
+			pchan, doorbell);
+	}
+
+	return chan;
+}
+
+static irqreturn_t meu_db_mbox_rx_handler(int irq, void *data)
+{
+	struct starfive_meu *meu = data;
+	unsigned int pchan = meu_db_mbox_irq_to_pchan_num(meu, irq);
+	struct mbox_chan *chan;
+
+	while (NULL != (chan = meu_db_mbox_irq_to_channel(meu, pchan))) {
+		mbox_chan_received_data(chan, NULL);
+		meu_db_mbox_clear_irq(chan);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static bool meu_db_last_tx_done(struct mbox_chan *chan)
+{
+	struct meu_db_channel *chan_info = chan->con_priv;
+	void __iomem *base = chan_info->meu->mlink[chan_info->pchan].tx_reg;
+
+	if (readl_relaxed(base + CHAN_TX_STAT_OFFSET) & BIT(chan_info->doorbell))
+		return false;
+
+	return true;
+}
+
+static int meu_db_send_data(struct mbox_chan *chan, void *data)
+{
+	struct meu_db_channel *chan_info = chan->con_priv;
+	void __iomem *base = chan_info->meu->mlink[chan_info->pchan].tx_reg;
+
+	/* Send event to co-processor */
+	writel_relaxed(BIT(chan_info->doorbell), base + CHAN_SET_OFFSET);
+
+	return 0;
+}
+
+static int meu_db_startup(struct mbox_chan *chan)
+{
+	meu_db_mbox_clear_irq(chan);
+	return 0;
+}
+
+static void meu_db_shutdown(struct mbox_chan *chan)
+{
+	struct meu_db_channel *chan_info = chan->con_priv;
+	struct mbox_controller *mbox = &chan_info->meu->mbox;
+	int i;
+
+	for (i = 0; i < mbox->num_chans; i++)
+		if (chan == &mbox->chans[i])
+			break;
+
+	if (mbox->num_chans == i) {
+		dev_warn(mbox->dev, "Request to free non-existent channel\n");
+		return;
+	}
+
+	/* Reset channel */
+	meu_db_mbox_clear_irq(chan);
+	devm_kfree(mbox->dev, chan->con_priv);
+	chan->con_priv = NULL;
+}
+
+static struct mbox_chan *meu_db_mbox_xlate(struct mbox_controller *mbox,
+					   const struct of_phandle_args *spec)
+{
+	struct starfive_meu *meu = dev_get_drvdata(mbox->dev);
+	unsigned int doorbell = spec->args[0] % MEU_NUM_DOORBELLS;
+	unsigned int pchan = spec->args[0] / MEU_NUM_DOORBELLS;
+	struct meu_db_channel *chan_info;
+	struct mbox_chan *chan;
+	int i;
+
+	/* Bounds checking */
+	if (pchan >= MEU_CHANS_GROUP || doorbell >= MEU_NUM_DOORBELLS) {
+		dev_err(mbox->dev,
+			"Invalid channel requested pchan: %d doorbell: %d\n",
+			pchan, doorbell);
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* Is requested channel free? */
+	chan = meu_db_mbox_to_channel(mbox, pchan, doorbell);
+	if (chan) {
+		dev_err(mbox->dev, "Channel in use: pchan: %d doorbell: %d\n",
+			pchan, doorbell);
+		return ERR_PTR(-EBUSY);
+	}
+
+	/* Find the first free slot */
+	for (i = 0; i < mbox->num_chans; i++)
+		if (!mbox->chans[i].con_priv)
+			break;
+
+	if (mbox->num_chans == i) {
+		dev_err(mbox->dev, "No free channels left\n");
+		return ERR_PTR(-EBUSY);
+	}
+
+	chan = &mbox->chans[i];
+
+	chan_info = devm_kzalloc(mbox->dev, sizeof(*chan_info), GFP_KERNEL);
+	if (!chan_info)
+		return ERR_PTR(-ENOMEM);
+
+	chan_info->meu = meu;
+	chan_info->pchan = pchan;
+	chan_info->doorbell = doorbell;
+
+	chan->con_priv = chan_info;
+
+	dev_dbg(mbox->dev, "mbox: created channel phys: %d doorbell: %d\n",
+		pchan, doorbell);
+
+	return chan;
+}
+
+static const struct mbox_chan_ops meu_db_ops = {
+	.send_data = meu_db_send_data,
+	.startup = meu_db_startup,
+	.shutdown = meu_db_shutdown,
+	.last_tx_done = meu_db_last_tx_done,
+};
+
+static int starfive_mbox_probe(struct platform_device *pdev)
+{
+	int meu_reg[MEU_CHANS_GROUP] = {MEU0_OFFSET, MEU1_OFFSET};
+	struct starfive_meu *meu;
+	struct mbox_chan *chans;
+	int i, err;
+
+	meu = devm_kzalloc(&pdev->dev, sizeof(*meu), GFP_KERNEL);
+	if (!meu)
+		return -ENOMEM;
+
+	meu->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(meu->base))
+		return PTR_ERR(meu->base);
+
+	chans = devm_kcalloc(&pdev->dev, MEU_TOTAL_CHANS, sizeof(*chans), GFP_KERNEL);
+	if (!chans)
+		return -ENOMEM;
+
+	meu->dev = &pdev->dev;
+	meu->mbox.dev = &pdev->dev;
+	meu->mbox.chans = chans;
+	meu->mbox.num_chans = MEU_TOTAL_CHANS;
+	meu->mbox.txdone_irq = false;
+	meu->mbox.txdone_poll = true;
+	meu->mbox.txpoll_period = 1;
+	meu->mbox.of_xlate = meu_db_mbox_xlate;
+	meu->mbox.ops = &meu_db_ops;
+
+	platform_set_drvdata(pdev, meu);
+
+	for (i = 0; i < MEU_CHANS_GROUP; i++) {
+		int irq = meu->mlink[i].irq = platform_get_irq(pdev, i);
+
+		if (irq <= 0) {
+			dev_dbg(&pdev->dev, "No IRQ found for Channel %d\n", i);
+			return irq;
+		}
+
+		meu->mlink[i].tx_reg = meu->base + meu_reg[i];
+		meu->mlink[i].rx_reg = meu->mlink[i].tx_reg + CHAN_RX_REG_OFFSET;
+
+		err = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+						meu_db_mbox_rx_handler,
+						IRQF_ONESHOT, "meu_db_link", meu);
+		if (err) {
+			dev_err(&pdev->dev, "Can't claim IRQ %d\n", irq);
+			return err;
+		}
+	}
+
+	err = devm_mbox_controller_register(&pdev->dev, &meu->mbox);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to register mailboxes %d\n", err);
+		return err;
+	}
+
+	dev_info(&pdev->dev, "StarFive MEU Doorbell mailbox registered\n");
+	return 0;
+}
+
+static const struct of_device_id starfive_mbox_of_match[] = {
+	{ .compatible = "starfive,jh8100-meu",},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, starfive_mbox_of_match);
+
+static struct platform_driver starfive_mbox_driver = {
+	.probe	= starfive_mbox_probe,
+	.driver = {
+		.name = "starfive-meu-mailbox",
+		.of_match_table = starfive_mbox_of_match,
+	},
+};
+
+module_platform_driver(starfive_mbox_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("StarFive MEU: communicate between CPU cores and SCP");
+MODULE_AUTHOR("Jee Heng Sia <jeeheng.sia@starfivetech.com>");
+MODULE_AUTHOR("Joshua Yeong <joshua.yeong@starfivetech.com>");
-- 
2.25.1

[PATCH 2/3] dt-bindings: mailbox: starfive: Add StarFive Meu Mailbox Driver

[Joshua Yeong]

The StarFive Meu Mailbox allow communication between AP and SCP cores
through mailbox doorbell.

Co-developed-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
Signed-off-by: Joshua Yeong <joshua.yeong@starfivetech.com>
Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
---
 .../bindings/mailbox/starfive-meu.yaml        | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/starfive-meu.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml b/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
new file mode 100644
index 000000000000..dbc5cfdb90ff
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/starfive-meu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive MEU Mailbox Controller
+
+maintainers:
+  - Jee Heng Sia <jeeheng.sia@starfivetech.com>
+  - Joshua Yeong <joshua.yeong@starfivetech.com>
+
+description: |
+  StarFive's Message-Exchange-Unit (MEU) is a mailbox controller that has 62
+  independent doorbells. Each MEU channel consist of 31 doorbells and consist of
+  a pair of Tx/Rx links that shall communicates with remote processor. The
+  sender set the bit in the SET register to indicate data readiness for the
+  receiver. An interrupt will be raised whenever receiving notification doorbell
+  from remote processor. The receiver will clear the bit in the CLR register
+  upon handling the doorbell notification. The sender should poll the STAT
+  register before starting any transaction to ensure all on-going doorbells are
+  processed.
+
+properties:
+  compatible:
+    enum:
+      - starfive,jh8100-meu
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: mailbox0
+      - description: mailbox1
+
+  '#mbox-cells':
+    description: represents index of the mailbox/doorbell paired channel
+      channel  0 - 30 for mailbox0 doorbell
+      channel 31 - 61 for mailbox1 doorbell
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - '#mbox-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        meu: mailbox@1f370000 {
+            compatible = "starfive,jh8100-meu";
+            reg = <0x0 0x1f370000 0 0x8000>;
+            interrupts = <170>, /* Mailbox0 */
+                         <171>; /* Mailbox1 */
+            #mbox-cells = <1>;
+        };
+    };
+
+...
-- 
2.25.1

[PATCH 3/3] MAINTAINERS: Add entry for StarFive Mailbox MEU

[Joshua Yeong]

Add StarFive Meu Mailbox driver contact point.

Co-developed-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
Signed-off-by: Joshua Yeong <joshua.yeong@starfivetech.com>
Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9104430e148e..dc2db4eca31a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20685,6 +20685,13 @@ F:	Documentation/devicetree/bindings/phy/starfive,jh7110-usb-phy.yaml
 F:	drivers/phy/starfive/phy-jh7110-pcie.c
 F:	drivers/phy/starfive/phy-jh7110-usb.c
 
+STARFIVE MEU DRIVER
+M:	Joshua Yeong <joshua.yeong@starfivetech.com>
+M:	Sia Jee Heng <jeeheng.sia@starfivetech.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
+F:	drivers/mailbox/starfive-meu.c
+
 STATIC BRANCH/CALL
 M:	Peter Zijlstra <peterz@infradead.org>
 M:	Josh Poimboeuf <jpoimboe@kernel.org>
-- 
2.25.1

Re: [PATCH 2/3] dt-bindings: mailbox: starfive: Add StarFive Meu Mailbox Driver

[Conor Dooley]

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

On Mon, Dec 18, 2023 at 02:12:00PM +0800, Joshua Yeong wrote:
> The StarFive Meu Mailbox allow communication between AP and SCP cores
> through mailbox doorbell.
> 
> Co-developed-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> Signed-off-by: Joshua Yeong <joshua.yeong@starfivetech.com>
> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
> ---
>  .../bindings/mailbox/starfive-meu.yaml        | 66 +++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml b/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
> new file mode 100644
> index 000000000000..dbc5cfdb90ff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/starfive-meu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive MEU Mailbox Controller
> +
> +maintainers:
> +  - Jee Heng Sia <jeeheng.sia@starfivetech.com>
> +  - Joshua Yeong <joshua.yeong@starfivetech.com>
> +
> +description: |
> +  StarFive's Message-Exchange-Unit (MEU) is a mailbox controller that has 62
> +  independent doorbells. Each MEU channel consist of 31 doorbells and consist of
> +  a pair of Tx/Rx links that shall communicates with remote processor. The
> +  sender set the bit in the SET register to indicate data readiness for the
> +  receiver. An interrupt will be raised whenever receiving notification doorbell
> +  from remote processor. The receiver will clear the bit in the CLR register
> +  upon handling the doorbell notification. The sender should poll the STAT
> +  register before starting any transaction to ensure all on-going doorbells are
> +  processed.

What is/are the consumer(s) of this mailbox?
Is part of your RPMI implementation?

Cheers,
Conor.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - starfive,jh8100-meu
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: mailbox0
> +      - description: mailbox1
> +
> +  '#mbox-cells':
> +    description: represents index of the mailbox/doorbell paired channel
> +      channel  0 - 30 for mailbox0 doorbell
> +      channel 31 - 61 for mailbox1 doorbell
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - '#mbox-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        meu: mailbox@1f370000 {
> +            compatible = "starfive,jh8100-meu";
> +            reg = <0x0 0x1f370000 0 0x8000>;
> +            interrupts = <170>, /* Mailbox0 */
> +                         <171>; /* Mailbox1 */
> +            #mbox-cells = <1>;
> +        };
> +    };
> +
> +...
> -- 
> 2.25.1
> 

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

Re: [PATCH 1/3] mailbox: starfive: Add StarFive Meu Mailbox Driver

[kernel test robot]

Hi Joshua,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.7-rc6]
[cannot apply to next-20231218]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Joshua-Yeong/mailbox-starfive-Add-StarFive-Meu-Mailbox-Driver/20231218-141510
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20231218061201.98136-2-joshua.yeong%40starfivetech.com
patch subject: [PATCH 1/3] mailbox: starfive: Add StarFive Meu Mailbox Driver
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20231219/202312190442.dVG2haPq-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231219/202312190442.dVG2haPq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312190442.dVG2haPq-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/mailbox/starfive-meu.c:50: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
    * StarFive MEU Mailbox allocated channel information


vim +50 drivers/mailbox/starfive-meu.c

    48	
    49	/**
  > 50	 * StarFive MEU Mailbox allocated channel information
    51	 *
    52	 * @meu: Pointer to parent mailbox device
    53	 * @pchan: Physical channel within which this doorbell resides in
    54	 * @doorbell: doorbell number pertaining to this channel
    55	 */
    56	struct meu_db_channel {
    57		struct starfive_meu *meu;
    58		unsigned int pchan;
    59		unsigned int doorbell;
    60	};
    61	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/3] dt-bindings: mailbox: starfive: Add StarFive Meu Mailbox Driver

[Joshua Yeong]

On 18/12/2023 7:17 PM, Conor Dooley wrote:
> On Mon, Dec 18, 2023 at 02:12:00PM +0800, Joshua Yeong wrote:
>> The StarFive Meu Mailbox allow communication between AP and SCP cores
>> through mailbox doorbell.
>>
>> Co-developed-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
>> Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
>> Signed-off-by: Joshua Yeong <joshua.yeong@starfivetech.com>
>> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
>> ---
>>   .../bindings/mailbox/starfive-meu.yaml        | 66 +++++++++++++++++++
>>   1 file changed, 66 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml b/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
>> new file mode 100644
>> index 000000000000..dbc5cfdb90ff
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
>> @@ -0,0 +1,66 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mailbox/starfive-meu.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive MEU Mailbox Controller
>> +
>> +maintainers:
>> +  - Jee Heng Sia <jeeheng.sia@starfivetech.com>
>> +  - Joshua Yeong <joshua.yeong@starfivetech.com>
>> +
>> +description: |
>> +  StarFive's Message-Exchange-Unit (MEU) is a mailbox controller that has 62
>> +  independent doorbells. Each MEU channel consist of 31 doorbells and consist of
>> +  a pair of Tx/Rx links that shall communicates with remote processor. The
>> +  sender set the bit in the SET register to indicate data readiness for the
>> +  receiver. An interrupt will be raised whenever receiving notification doorbell
>> +  from remote processor. The receiver will clear the bit in the CLR register
>> +  upon handling the doorbell notification. The sender should poll the STAT
>> +  register before starting any transaction to ensure all on-going doorbells are
>> +  processed.
> What is/are the consumer(s) of this mailbox?
> Is part of your RPMI implementation?
>
> Cheers,
> Conor.

Yes, it would be part of StarFive RPMI implementation.

Regards,
Joshua

>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - starfive,jh8100-meu
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    items:
>> +      - description: mailbox0
>> +      - description: mailbox1
>> +
>> +  '#mbox-cells':
>> +    description: represents index of the mailbox/doorbell paired channel
>> +      channel  0 - 30 for mailbox0 doorbell
>> +      channel 31 - 61 for mailbox1 doorbell
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - '#mbox-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        meu: mailbox@1f370000 {
>> +            compatible = "starfive,jh8100-meu";
>> +            reg = <0x0 0x1f370000 0 0x8000>;
>> +            interrupts = <170>, /* Mailbox0 */
>> +                         <171>; /* Mailbox1 */
>> +            #mbox-cells = <1>;
>> +        };
>> +    };
>> +
>> +...
>> -- 
>> 2.25.1
>>

Re: [PATCH 2/3] dt-bindings: mailbox: starfive: Add StarFive Meu Mailbox Driver

[Conor Dooley]

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

On Tue, Dec 19, 2023 at 01:52:53PM +0800, Joshua Yeong wrote:
> On 18/12/2023 7:17 PM, Conor Dooley wrote:
> > On Mon, Dec 18, 2023 at 02:12:00PM +0800, Joshua Yeong wrote:
> > > The StarFive Meu Mailbox allow communication between AP and SCP cores
> > > through mailbox doorbell.
> > > 
> > > Co-developed-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> > > Signed-off-by: Sia Jee Heng <jeeheng.sia@starfivetech.com>
> > > Signed-off-by: Joshua Yeong <joshua.yeong@starfivetech.com>
> > > Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com>
> > > ---
> > >   .../bindings/mailbox/starfive-meu.yaml        | 66 +++++++++++++++++++
> > >   1 file changed, 66 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml b/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
> > > new file mode 100644
> > > index 000000000000..dbc5cfdb90ff
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mailbox/starfive-meu.yaml
> > > @@ -0,0 +1,66 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mailbox/starfive-meu.yaml#

Filename should match the compatible.

> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: StarFive MEU Mailbox Controller
> > > +
> > > +maintainers:
> > > +  - Jee Heng Sia <jeeheng.sia@starfivetech.com>
> > > +  - Joshua Yeong <joshua.yeong@starfivetech.com>
> > > +
> > > +description: |

This | is not needed, there's no formatting here to preserve.

> > > +  StarFive's Message-Exchange-Unit (MEU) is a mailbox controller that has 62
> > > +  independent doorbells. Each MEU channel consist of 31 doorbells and consist of
> > > +  a pair of Tx/Rx links that shall communicates with remote processor. The
> > > +  sender set the bit in the SET register to indicate data readiness for the
> > > +  receiver. An interrupt will be raised whenever receiving notification doorbell
> > > +  from remote processor. The receiver will clear the bit in the CLR register
> > > +  upon handling the doorbell notification. The sender should poll the STAT
> > > +  register before starting any transaction to ensure all on-going doorbells are
> > > +  processed.
> > What is/are the consumer(s) of this mailbox?
> > Is part of your RPMI implementation?
> > 
> > Cheers,
> > Conor.
> 
> Yes, it would be part of StarFive RPMI implementation.

I see. Seems a bit weird to submit the mailbox provider without a
consumer, since it'll be dead code until a provider arrives, but that's
for the mailbox driver maintainer to decide if he has a problem with.

> 
> Regards,
> Joshua
> 
> > 
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - starfive,jh8100-meu

I would just make this const: starfive,jh8100-meu unless you already
expect your other SoCs to use a different programming model.

> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    items:
> > > +      - description: mailbox0
> > > +      - description: mailbox1
> > > +
> > > +  '#mbox-cells':
> > > +    description: represents index of the mailbox/doorbell paired channel

> > > +      channel  0 - 30 for mailbox0 doorbell
> > > +      channel 31 - 61 for mailbox1 doorbell

I'd probably move this channel to doorbell interrupt mapping into the
main description and leave this as just

'#mbox-cells':
  const: 1

> > > +    const: 1
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - '#mbox-cells'
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    soc {
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +
> > > +        meu: mailbox@1f370000 {

Drop the label here, it has no user.

> > > +            compatible = "starfive,jh8100-meu";
> > > +            reg = <0x0 0x1f370000 0 0x8000>;
> > > +            interrupts = <170>, /* Mailbox0 */
> > > +                         <171>; /* Mailbox1 */

I'd also probably drop the comments here, since the binding enforces
this ordering.

Cheers,
Conor.

> > > +            #mbox-cells = <1>;
> > > +        };
> > > +    };
> > > +
> > > +...
> > > -- 
> > > 2.25.1
> > > 

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

Re: [PATCH 1/3] mailbox: starfive: Add StarFive Meu Mailbox Driver

[Arnd Bergmann]

On Mon, Dec 18, 2023, at 06:11, Joshua Yeong wrote:
> This patch adds support for the StarFive Meu Mailbox driver. This enables
> communication using mailbox doorbell between AP and SCP cores.

I think the naming here is a bit confusing. As far as I can tell, this
is not a mailbox at all but just a doorbell, so maybe clarify that in the
wording.

It could be turned into a mailbox if you add a shared memory segment of
course, but neither the driver nor the binding specifies how to use that.

> +config STARFIVE_MEU_MBOX
> +	tristate "Starfive MEU Mailbox"

Please spell out MEU here, I don't think that is a well understood acronynm,
at least I have never heard of it.

> +	depends on OF
> +	depends on SOC_STARFIVE || COMPILE_TEST
> +	help
> +	  Say Y here if you want to build the Starfive MEU mailbox controller
> +	  driver.
> +
> +	  The StarFive mailbox controller has 2 channels. Each channel has a
> +	  pair of Tx/Rx link and each link has 31 slots/doorbells.

What is the significance of the "channels" as opposed to "slots"?

The binding seems to indicate that there are just 62 fully equal
doorbell channels as far as consumers are concerned. For efficiency
one might want to of course only use one doorbell per channel here
to avoid the interrupt sharing.

> +static inline struct mbox_chan *
> +meu_db_mbox_to_channel(struct mbox_controller *mbox, unsigned int 
> pchan,
> +		       unsigned int doorbell)
> +{
> +	struct meu_db_channel *chan_info;
> +	int i;
> +
> +	for (i = 0; i < mbox->num_chans; i++) {
> +		chan_info = mbox->chans[i].con_priv;
> +		if (chan_info && chan_info->pchan == pchan &&
> +		    chan_info->doorbell == doorbell)
> +			return &mbox->chans[i];
> +	}
> +
> +	return NULL;
> +}

If the number of doorbells is known in advance, maybe use
use it as the array index here?

> +static void meu_db_mbox_clear_irq(struct mbox_chan *chan)
> +{
> +	struct meu_db_channel *chan_info = chan->con_priv;
> +	void __iomem *base = chan_info->meu->mlink[chan_info->pchan].rx_reg;
> +
> +	writel_relaxed(BIT(chan_info->doorbell), base + CHAN_CLR_OFFSET);
> +}

Any reason to use writel_relaxed() instead of writel() here? Please
add a comment if this is necessary, otherwise this risks adding
races if someone tries to use the doorbell in combination with
shared memory.


> +static struct mbox_chan *
> +meu_db_mbox_irq_to_channel(struct starfive_meu *meu, unsigned int 
> pchan)
> +{
> +	void __iomem *base = meu->mlink[pchan].rx_reg;
> +	struct mbox_controller *mbox = &meu->mbox;
> +	struct mbox_chan *chan = NULL;
> +	unsigned int doorbell;
> +	unsigned long bits;
> +
> +	bits = FIELD_GET(CHAN_RX_DOORBELL, readl_relaxed(base + 
> CHAN_RX_STAT_OFFSET));
> +	if (!bits)
> +		/* No IRQs fired in specified physical channel */
> +		return NULL;
> +
> +	/* An IRQ has fired, find the associated channel */
> +	for (doorbell = 0; bits; doorbell++) {
> +		if (!test_and_clear_bit(doorbell, &bits))
> +			continue;

There is no need to use atomic updates on stack variables. Just
use for_each_set_bit() to do the loop here.

> +		chan = meu_db_mbox_to_channel(mbox, pchan, doorbell);
> +		if (chan)
> +			break;
> +
> +		/* Clear IRQ for unregistered doorbell */
> +		writel_relaxed(BIT(doorbell), base + CHAN_CLR_OFFSET);

Again, add a comment about the use of _relaxed() here.


> +static bool meu_db_last_tx_done(struct mbox_chan *chan)
> +{
> +	struct meu_db_channel *chan_info = chan->con_priv;
> +	void __iomem *base = chan_info->meu->mlink[chan_info->pchan].tx_reg;
> +
> +	if (readl_relaxed(base + CHAN_TX_STAT_OFFSET) & 
> BIT(chan_info->doorbell))

The readl_relaxed() again prevents the use of shared memory, so
change it to a normal readl() or add a comment about how ordering
is maintained.

     Arnd