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

Message ID 20231218061201.98136-2-joshua.yeong@starfivetech.com
State New
Headers
Series mailbox: starfive: Add Support for StarFive Meu Mailbox driver |

Commit Message

Joshua Yeong Dec. 18, 2023, 6:11 a.m. UTC
  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
  

Comments

kernel test robot Dec. 18, 2023, 9:20 p.m. UTC | #1
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
  
Arnd Bergmann Dec. 19, 2023, 5:38 p.m. UTC | #2
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
  

Patch

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>");