[PATCH v2 0/2] mailbox: add Axiado AX3005 mailbox driver

[PATCH v2 0/2] mailbox: add Axiado AX3005 mailbox driver

[Swark Yang]

The Axiado AX3005 SoC integrates a mailbox controller that provides
inter-processor communication channels between the host CPU and the
on-chip co-processor.

The controller exposes 16 fixed-function channels split into two
groups: 8 TX channels and 8 RX channels. Each channel has a
hardware FIFO and a control/status register, while TX and RX
channels reside in separate MMIO register regions.

Only the RX channels expose interrupts to the host CPU.
TX completion is detected by polling the FIFO empty status, so
txdone_poll is used instead of txdone_irq. RX delivery uses a
threaded IRQ per channel because draining a mailbox message word by
word is not appropriate in hard IRQ context.

The ax3005.dtsi mailbox node will follow in a separate patch once
the AX3005 devicetree series currently under review has been merged.

This series consists of:
1. dt-bindings: mailbox: add Axiado AX3005 mailbox
2. mailbox: add Axiado AX3005 mailbox driver

The corresponding MAINTAINERS entry is added incrementally by the
two patches: the binding path in patch 1 and the driver path in
patch 2.

Testing:
Tested on AX3005 using an out-of-tree MCTP-over-mailbox transport
driver, not yet submitted upstream, as the mailbox client. The test
exercised channel 0 for TX and channel 8 for RX.

Verified end to end by issuing a PLDM Firmware Update command from
OpenBMC over MCTP and routing it through this mailbox driver to the
on-chip coprocessor. The firmware update completed successfully,
and the resulting image passed verification.

Signed-off-by: Swark Yang <syang@axiado.com>
---
Changes in v2:
    - dt-bindings: addressed review from Krzysztof Kozlowski
	- dropped the unneeded '|' block-scalar indicator from
	  description fields, dropped the redundant minItems on
	  interrupts, and dropped the default-valued
	  status = "okay" from the example.
    - driver: a further round of internal review turned up a few
      more issues worth folding into this v2 rather than leaving
      them for a v3:
	- Widened the RX wait timeout margin (500us -> 1000us);
	  the previous value had no measured basis for the largest
	  (256-byte) messages.
	- Unified TX/RX message-length validation against a single
	  field (mb->drv_data->msg_size) instead of two fields
	  (chan_msg_size) that always happened to agree.
	- Added a per-channel FIFO error counter exposed via
	  debugfs, alongside the existing rate-limited dev_warn();
	  a ratelimited log alone can silently suppress lines under
	  a burst, losing the true count.
	- Stopped discarding already-queued good data on an
	  overflow/underflow flag alone: clear, log, and count the
	  error, then let the existing length-validated read decide
	  if what's queued is usable. Only an invalid message length
	  or a read timeout now triggers a flush.
	- Only flush the FIFO in startup()/shutdown() when an error
	  is actually detected, not unconditionally -- a blind flush
	  was discarding messages the peer legitimately sent before
	  this side started up.
	- Used devm_kzalloc() instead of devm_kmalloc() for
	  rx_buffer.
	- Fixed a checkpatch --strict alignment warning in the RX
	  wait poll.
	- Dropped the TX/RX channel-type guard added in the previous
	  round, as it prevented standalone loopback testing of the
	  mailbox controller without a coprocessor.
- Link to v1: https://lore.kernel.org/r/20260729-upstream-axiado-ax3005-mailbox-upstream-v1-0-8961b7350338@axiado.com

---
Swark Yang (2):
      dt-bindings: mailbox: add Axiado AX3005 mailbox
      mailbox: add Axiado AX3005 mailbox driver

 .../bindings/mailbox/axiado,ax3005-mailbox.yaml    |  96 +++++
 MAINTAINERS                                        |   8 +
 drivers/mailbox/Kconfig                            |  10 +
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/axiado-mailbox.c                   | 395 +++++++++++++++++++++
 5 files changed, 511 insertions(+)
---
base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f
change-id: 20260728-upstream-axiado-ax3005-mailbox-upstream-86feaa7edf49

Best regards,
-- 
Swark Yang <syang@axiado.com>

[PATCH v2 1/2] dt-bindings: mailbox: add Axiado AX3005 mailbox

[Swark Yang]

Add a devicetree binding for the Axiado AX3005 mailbox controller.
The controller provides inter-processor communication channels
between the host CPU and the coprocessor, with separate TX and RX
register regions.

Add the corresponding MAINTAINERS entry covering the binding.

Signed-off-by: Swark Yang <syang@axiado.com>
---
 .../bindings/mailbox/axiado,ax3005-mailbox.yaml    | 96 ++++++++++++++++++++++
 MAINTAINERS                                        |  7 ++
 2 files changed, 103 insertions(+)

diff --git a/Documentation/devicetree/bindings/mailbox/axiado,ax3005-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/axiado,ax3005-mailbox.yaml
new file mode 100644
index 000000000000..82bf7d1848c9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/axiado,ax3005-mailbox.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/axiado,ax3005-mailbox.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Axiado AX3005 Mailbox Controller
+
+maintainers:
+  - Krishnakumar Kesavan <kkesavan@axiado.com>
+  - Prasad Bolisetty <pbolisetty@axiado.com>
+  - Swark Yang <syang@axiado.com>
+
+description:
+  The Axiado mailbox controller provides inter-processor communication
+  channels between subsystems within the Axiado SoC. The hardware has
+  separate TX and RX register regions. Only the RX channels (channels 8-15)
+  generate interrupts; TX channels are purely poll/write-based.
+
+properties:
+  compatible:
+    const: axiado,ax3005-mailbox
+
+  reg:
+    items:
+      - description: TX mailbox register region
+      - description: RX mailbox register region
+
+  reg-names:
+    items:
+      - const: tx
+      - const: rx
+
+  interrupts:
+    description:
+      One interrupt per RX channel. The hardware exposes eight RX interrupt
+      lines corresponding to channels 8 through 15.
+    maxItems: 8
+
+  interrupt-names:
+    description:
+      Named interrupt lines for each RX channel, from ch8 to ch15.
+    items:
+      - const: ch8
+      - const: ch9
+      - const: ch10
+      - const: ch11
+      - const: ch12
+      - const: ch13
+      - const: ch14
+      - const: ch15
+
+  "#mbox-cells":
+    description:
+      Number of cells in a mailbox specifier. A single cell encodes the
+      channel index as seen by the client.
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - interrupt-names
+  - "#mbox-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        mailbox@330f2000 {
+            compatible = "axiado,ax3005-mailbox";
+            reg = <0x0 0x330f2000 0x0 0x2000>,
+                  <0x0 0x331b0000 0x0 0x1000>;
+            reg-names = "tx", "rx";
+            /* Only RX channels (8-15) have IRQs */
+            interrupts = <GIC_SPI 242 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 243 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "ch8", "ch9", "ch10", "ch11",
+                              "ch12", "ch13", "ch14", "ch15";
+            interrupt-parent = <&gic500>;
+            #mbox-cells = <1>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a674e36529f7..af6c3571c9f2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4437,6 +4437,13 @@ F:	Documentation/devicetree/bindings/phy/axiado,ax3000-emmc-phy.yaml
 F:	drivers/phy/axiado/Kconfig
 F:	drivers/phy/axiado/phy-axiado-emmc.c
 
+AXIADO MAILBOX DRIVER
+M:	Krishnakumar Kesavan <kkesavan@axiado.com>
+M:	Prasad Bolisetty <pbolisetty@axiado.com>
+M:	Swark Yang <syang@axiado.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/mailbox/axiado,ax3005-mailbox.yaml
+
 AXI SPI ENGINE
 M:	Michael Hennerich <michael.hennerich@analog.com>
 M:	Nuno Sá <nuno.sa@analog.com>

-- 
2.34.1

[PATCH v2 2/2] mailbox: add Axiado AX3005 mailbox driver

[Swark Yang]

Add a mailbox controller driver for the Axiado AX3005 SoC.
The controller provides communication channels between
the host CPU and the coprocessor.

The hardware provides 8 TX channels and 8 RX channels
through separate register regions. RX channels use
per-channel interrupts, while TX completion is detected by
polling the FIFO status.

Add the driver path to the existing Axiado mailbox entry in
MAINTAINERS.

Signed-off-by: Swark Yang <syang@axiado.com>
---
 MAINTAINERS                      |   1 +
 drivers/mailbox/Kconfig          |  10 +
 drivers/mailbox/Makefile         |   2 +
 drivers/mailbox/axiado-mailbox.c | 395 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 408 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index af6c3571c9f2..80db69111889 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4443,6 +4443,7 @@ M:	Prasad Bolisetty <pbolisetty@axiado.com>
 M:	Swark Yang <syang@axiado.com>
 S:	Supported
 F:	Documentation/devicetree/bindings/mailbox/axiado,ax3005-mailbox.yaml
+F:	drivers/mailbox/axiado-mailbox.c
 
 AXI SPI ENGINE
 M:	Michael Hennerich <michael.hennerich@analog.com>
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 3062ee352f78..ab869769e278 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -399,4 +399,14 @@ config RISCV_SBI_MPXY_MBOX
 	  or HS-mode hypervisor). Say Y here, unless you are sure you do not
 	  need this.
 
+config AXIADO_MAILBOX
+	tristate "Axiado mailbox driver"
+	depends on ARCH_AXIADO || COMPILE_TEST
+	depends on OF
+	help
+	  Enable support for the Axiado mailbox controller. The driver provides
+	  communication channels between the host CPU and the coprocessor on
+	  Axiado SoCs.
+	  If unsure, say N.
+
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 944d8ea39f34..45dc59bb3e7f 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -84,3 +84,5 @@ obj-$(CONFIG_CIX_MBOX)	+= cix-mailbox.o
 obj-$(CONFIG_BCM74110_MAILBOX)	+= bcm74110-mailbox.o
 
 obj-$(CONFIG_RISCV_SBI_MPXY_MBOX)	+= riscv-sbi-mpxy-mbox.o
+
+obj-$(CONFIG_AXIADO_MAILBOX)	+= axiado-mailbox.o
diff --git a/drivers/mailbox/axiado-mailbox.c b/drivers/mailbox/axiado-mailbox.c
new file mode 100644
index 000000000000..3a3cf7421863
--- /dev/null
+++ b/drivers/mailbox/axiado-mailbox.c
@@ -0,0 +1,395 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021-2026 Axiado Corporation (or its affiliates).
+ */
+
+#include <linux/atomic.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/bitfield.h>
+#include <linux/debugfs.h>
+#include <linux/iopoll.h>
+#include <linux/mailbox_controller.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/unaligned.h>
+
+#define AXIADO_MBOX_TX_CHANS		8	/* 0-7 */
+#define AXIADO_MBOX_RX_CHANS		8	/* 8-15 */
+#define AXIADO_MBOX_CHAN_STRIDE	0x4
+#define AXIADO_MBOX_TX_REG_STRIDE	0x40
+#define AXIADO_MBOX_RX_REG_STRIDE	0x30
+#define AXIADO_MBOX_RX_POLL_US		10
+#define AXIADO_MBOX_RX_TIMEOUT_US	1000
+
+/* Mailbox CSR bit definitions */
+#define AXIADO_MBOX_CSR_EMPTY		BIT(0) /* 1 = FIFO empty; 0 = data available */
+#define AXIADO_MBOX_CSR_OVERFLOW	BIT(2) /* write 1 to clear (W1C) */
+#define AXIADO_MBOX_CSR_UNDERFLOW	BIT(3) /* write 1 to clear (W1C) */
+#define AXIADO_MBOX_CSR_FLUSH		BIT(4) /* W1TRG flush; startup/shutdown */
+#define AXIADO_MBOX_CSR_LEVEL		GENMASK(11, 5)
+
+#define AXIADO_MBOX_CSR_ERRORS		(AXIADO_MBOX_CSR_OVERFLOW | \
+					 AXIADO_MBOX_CSR_UNDERFLOW)
+
+struct axiado_mbox_data {
+	u8 num_chans;
+	u16 msg_size;
+};
+
+struct axiado_channel_data {
+	void __iomem *mbox_reg;
+	void __iomem *csr_reg;
+	void *rx_buffer;
+	u8 channel_num;
+	int irq;
+	struct mbox_chan *chan;
+	bool active;
+	atomic_t fifo_errors;
+};
+
+/* mailbox side */
+struct axiado_mbox {
+	struct mbox_controller mbox;
+	const struct axiado_mbox_data *drv_data;
+	void __iomem *tx_base;
+	void __iomem *rx_base;
+	struct dentry *debugfs_dir;
+};
+
+/*
+ * Checks OVERFLOW/UNDERFLOW, logs and W1C-clears them if set. Shared by the
+ * TX and RX paths; RX additionally flushes the FIFO afterwards to resync
+ * framing, which TX must not do since it could discard data the peer has
+ * not read yet.
+ */
+static bool axiado_mbox_clear_fifo_errors(struct axiado_channel_data *priv)
+{
+	struct device *dev = priv->chan->mbox->dev;
+	u32 errors;
+
+	errors = readl(priv->csr_reg) & AXIADO_MBOX_CSR_ERRORS;
+	if (!errors)
+		return false;
+
+	writel(errors, priv->csr_reg);
+	atomic_inc(&priv->fifo_errors);
+
+	dev_warn_ratelimited(dev, "Channel %u FIFO error: %#x\n",
+			     priv->channel_num, errors);
+
+	return true;
+}
+
+static int axiado_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+	struct axiado_mbox *mb = dev_get_drvdata(chan->mbox->dev);
+	struct axiado_channel_data *priv = chan->con_priv;
+	unsigned int idx = priv->channel_num;
+	u8 tail[sizeof(u32)] = { 0 };
+	const u8 *buf = data;
+	unsigned int tail_len;
+	unsigned int offset;
+	u32 msg_len;
+
+	if (!data)
+		return -EINVAL;
+
+	/*
+	 * The Axiado mailbox message ABI stores the total message length,
+	 * including this length word, as a little-endian byte count in the
+	 * first word of every message.
+	 */
+	msg_len = get_unaligned_le32(data);
+	if (msg_len < sizeof(u32) || msg_len > mb->drv_data->msg_size)
+		return -EINVAL;
+
+	/* Only touch the hardware after validating the message. */
+	axiado_mbox_clear_fifo_errors(priv);
+
+	if (!(readl(priv->csr_reg) & AXIADO_MBOX_CSR_EMPTY)) {
+		dev_warn_ratelimited(mb->mbox.dev, "Channel %u is busy\n", idx);
+		return -EBUSY;
+	}
+
+	for (offset = 0; offset + sizeof(u32) <= msg_len;
+	     offset += sizeof(u32))
+		writel(get_unaligned_le32(buf + offset), priv->mbox_reg);
+
+	tail_len = msg_len - offset;
+	if (tail_len) {
+		memcpy(tail, buf + offset, tail_len);
+		writel(get_unaligned_le32(tail), priv->mbox_reg);
+	}
+
+	dev_dbg(mb->mbox.dev, "%s: Ch-%u sent\n", __func__, idx);
+
+	return 0;
+}
+
+static irqreturn_t axiado_rx_thread(int irq, void *dev_id)
+{
+	struct axiado_channel_data *priv = dev_id;
+	struct mbox_chan *chan = priv->chan;
+	struct axiado_mbox *mb = dev_get_drvdata(chan->mbox->dev);
+	u8 *buf = priv->rx_buffer;
+	unsigned int num_words;
+	unsigned int remaining;
+	unsigned int i;
+	u32 msg_len;
+	u32 word;
+	u32 csr;
+	int ret;
+
+	/*
+	 * Clear and log/count any pending errors, but don't discard data
+	 * on their account alone: a rejected overflow write doesn't
+	 * corrupt what was already safely queued ahead of it, so let the
+	 * length-based read below decide whether what's here is usable.
+	 */
+	axiado_mbox_clear_fifo_errors(priv);
+
+	csr = readl(priv->csr_reg);
+	if (csr & AXIADO_MBOX_CSR_EMPTY)
+		return IRQ_NONE;
+
+	/*
+	 * The first word contains the total message length in bytes,
+	 * including the length word itself.
+	 */
+	word = readl(priv->mbox_reg);
+	msg_len = word;
+	put_unaligned_le32(word, buf);
+
+	if (msg_len < sizeof(u32) || msg_len > mb->drv_data->msg_size)
+		goto invalid_message;
+
+	num_words = DIV_ROUND_UP(msg_len, sizeof(u32));
+	remaining = num_words - 1;
+
+	/*
+	 * The not-empty interrupt may occur as soon as the first DW enters
+	 * the FIFO. Wait until all remaining DWs of this message arrive.
+	 */
+	if (remaining) {
+		ret = readl_poll_timeout(priv->csr_reg, csr,
+					 (csr & AXIADO_MBOX_CSR_ERRORS) ||
+					  FIELD_GET(AXIADO_MBOX_CSR_LEVEL, csr) >=
+					  remaining,
+					  AXIADO_MBOX_RX_POLL_US,
+					  AXIADO_MBOX_RX_TIMEOUT_US);
+		if (ret)
+			goto incomplete_message;
+
+		axiado_mbox_clear_fifo_errors(priv);
+	}
+
+	for (i = 1; i < num_words; i++) {
+		word = readl(priv->mbox_reg);
+		put_unaligned_le32(word, buf + i * sizeof(u32));
+	}
+
+	axiado_mbox_clear_fifo_errors(priv);
+
+	if (READ_ONCE(priv->active))
+		mbox_chan_received_data(chan, priv->rx_buffer);
+
+	return IRQ_HANDLED;
+
+incomplete_message:
+	dev_warn_ratelimited(chan->mbox->dev,
+			     "Channel %u received an incomplete message\n",
+			     priv->channel_num);
+
+invalid_message:
+	writel(AXIADO_MBOX_CSR_FLUSH, priv->csr_reg);
+
+	return IRQ_HANDLED;
+}
+
+static int axiado_mbox_startup(struct mbox_chan *chan)
+{
+	struct axiado_channel_data *priv = chan->con_priv;
+
+	/*
+	 * Only flush on a genuine error. A blind flush here would discard
+	 * a message the peer legitimately sent before this side started
+	 * up (e.g. during normal boot sequencing), which is not corrupt
+	 * and does not need resyncing.
+	 */
+	if (axiado_mbox_clear_fifo_errors(priv))
+		writel(AXIADO_MBOX_CSR_FLUSH, priv->csr_reg);
+
+	WRITE_ONCE(priv->active, true);
+
+	if (priv->channel_num >= AXIADO_MBOX_TX_CHANS)
+		enable_irq(priv->irq);
+
+	return 0;
+}
+
+static void axiado_mbox_shutdown(struct mbox_chan *chan)
+{
+	struct axiado_channel_data *priv = chan->con_priv;
+
+	WRITE_ONCE(priv->active, false);
+
+	if (priv->channel_num >= AXIADO_MBOX_TX_CHANS)
+		disable_irq(priv->irq);
+
+	if (axiado_mbox_clear_fifo_errors(priv))
+		writel(AXIADO_MBOX_CSR_FLUSH, priv->csr_reg);
+}
+
+static bool axiado_mbox_last_tx_done(struct mbox_chan *chan)
+{
+	struct axiado_channel_data *priv = chan->con_priv;
+
+	return !!(readl(priv->csr_reg) & AXIADO_MBOX_CSR_EMPTY);
+}
+
+static const struct mbox_chan_ops axiado_mbox_chan_ops = {
+	.send_data	= axiado_mbox_send_data,
+	.startup	= axiado_mbox_startup,
+	.shutdown	= axiado_mbox_shutdown,
+	.last_tx_done	= axiado_mbox_last_tx_done,
+};
+
+static void axiado_mbox_debugfs_remove(void *dentry)
+{
+	debugfs_remove_recursive(dentry);
+}
+
+static int axiado_mbox_probe(struct platform_device *pdev)
+{
+	const struct axiado_mbox_data *drv_data;
+	struct axiado_channel_data *ch_data;
+	struct device *dev = &pdev->dev;
+	struct axiado_mbox *mb;
+	unsigned int irq_idx = 0;
+	unsigned int rx_chan;
+	unsigned int i;
+	int ret;
+
+	drv_data = device_get_match_data(dev);
+	if (!drv_data)
+		return -ENODEV;
+
+	mb = devm_kzalloc(dev, sizeof(*mb), GFP_KERNEL);
+	if (!mb)
+		return -ENOMEM;
+
+	mb->mbox.dev = dev;
+	mb->mbox.num_chans = drv_data->num_chans;
+
+	mb->mbox.chans = devm_kcalloc(&pdev->dev,
+				      drv_data->num_chans,
+				      sizeof(*mb->mbox.chans),
+				      GFP_KERNEL);
+	if (!mb->mbox.chans)
+		return -ENOMEM;
+
+	mb->tx_base = devm_platform_ioremap_resource_byname(pdev, "tx");
+	if (IS_ERR(mb->tx_base))
+		return PTR_ERR(mb->tx_base);
+
+	mb->rx_base = devm_platform_ioremap_resource_byname(pdev, "rx");
+	if (IS_ERR(mb->rx_base))
+		return PTR_ERR(mb->rx_base);
+
+	ch_data = devm_kcalloc(&pdev->dev,
+			       drv_data->num_chans,
+			       sizeof(*ch_data),
+			       GFP_KERNEL);
+	if (!ch_data)
+		return -ENOMEM;
+
+	mb->debugfs_dir = debugfs_create_dir(dev_name(dev), NULL);
+	ret = devm_add_action_or_reset(dev, axiado_mbox_debugfs_remove,
+				       mb->debugfs_dir);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < drv_data->num_chans; i++) {
+		char name[16];
+
+		ch_data[i].channel_num = i;
+		ch_data[i].chan = &mb->mbox.chans[i];
+
+		mb->mbox.chans[i].con_priv = &ch_data[i];
+
+		snprintf(name, sizeof(name), "chan%u-errors", i);
+		debugfs_create_atomic_t(name, 0444, mb->debugfs_dir,
+					&ch_data[i].fifo_errors);
+
+		if (i < AXIADO_MBOX_TX_CHANS) {
+			ch_data[i].mbox_reg = mb->tx_base + (i * AXIADO_MBOX_CHAN_STRIDE);
+			ch_data[i].csr_reg = mb->tx_base + AXIADO_MBOX_TX_REG_STRIDE +
+					       (i * AXIADO_MBOX_CHAN_STRIDE);
+			ch_data[i].irq = -1;
+			continue;
+		}
+
+		ch_data[i].rx_buffer = devm_kzalloc(dev, drv_data->msg_size,
+						    GFP_KERNEL);
+		if (!ch_data[i].rx_buffer)
+			return -ENOMEM;
+
+		rx_chan = i - AXIADO_MBOX_TX_CHANS;
+		ch_data[i].mbox_reg = mb->rx_base +
+				       (rx_chan * AXIADO_MBOX_CHAN_STRIDE);
+		ch_data[i].csr_reg = mb->rx_base + AXIADO_MBOX_RX_REG_STRIDE +
+				      (rx_chan * AXIADO_MBOX_CHAN_STRIDE);
+		ch_data[i].irq = platform_get_irq(pdev, irq_idx++);
+		if (ch_data[i].irq < 0)
+			return dev_err_probe(dev, ch_data[i].irq,
+					     "Failed to get IRQ for channel %u\n", i);
+
+		ret = devm_request_threaded_irq(dev, ch_data[i].irq, NULL,
+						axiado_rx_thread,
+						IRQF_ONESHOT | IRQF_NO_AUTOEN,
+						dev_name(dev), &ch_data[i]);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to request IRQ for channel %u\n", i);
+
+		dev_dbg(dev, "RX chan %u -> irq %d\n", i, ch_data[i].irq);
+	}
+
+	platform_set_drvdata(pdev, mb);
+	mb->drv_data = drv_data;
+	mb->mbox.ops = &axiado_mbox_chan_ops;
+	mb->mbox.txdone_irq = false;
+	mb->mbox.txdone_poll = true;
+	mb->mbox.txpoll_period = 5;
+
+	return devm_mbox_controller_register(dev, &mb->mbox);
+}
+
+static const struct axiado_mbox_data axiado_drv_data = {
+	.num_chans = AXIADO_MBOX_TX_CHANS + AXIADO_MBOX_RX_CHANS,
+	.msg_size = 256,
+};
+
+static const struct of_device_id axiado_mbox_of_match[] = {
+	{ .compatible = "axiado,ax3005-mailbox", .data = &axiado_drv_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, axiado_mbox_of_match);
+
+static struct platform_driver axiado_mbox_driver = {
+	.driver = {
+		.name = "axiado-mailbox",
+		.of_match_table = axiado_mbox_of_match,
+	},
+	.probe = axiado_mbox_probe,
+};
+module_platform_driver(axiado_mbox_driver);
+
+MODULE_AUTHOR("Axiado Corporation");
+MODULE_DESCRIPTION("Axiado Mailbox driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v2 2/2] mailbox: add Axiado AX3005 mailbox driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The RX thread blindly reads data from the FIFO if the poll loop breaks early due to an error, leading to hardware underflows and garbage data delivery.
- [High] Returning IRQ_NONE after successfully clearing FIFO errors can trigger the spurious interrupt detector and permanently disable the IRQ.
- [High] TOCTOU race in RX thread between checking the active flag and dereferencing the channel client, leading to a NULL pointer dereference.
- [Medium] The driver uses register I/O accessors (`writel`/`readl`) for FIFO bulk data transfers instead of the mandated stream I/O accessors (`writesl`/`readsl`).
--

--- Patch [2]: [PATCH v2 2/2] mailbox: add Axiado AX3005 mailbox driver ---
commit e614bfa4ee11362f9c00a58fdcc67351b8f3046c
Author: Swark Yang <syang@axiado.com>

mailbox: add Axiado AX3005 mailbox driver

This commit adds a new mailbox controller driver for the Axiado AX3005 SoC.
It supports 8 TX and 8 RX channels using separate register regions, with
TX status via polling and RX via per-channel interrupts.

> diff --git a/drivers/mailbox/axiado-mailbox.c b/drivers/mailbox/axiado-mailbox.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/mailbox/axiado-mailbox.c

[ ... ]

> +static int axiado_mbox_send_data(struct mbox_chan *chan, void *data)
> +{

[ ... ]

> +	for (offset = 0; offset + sizeof(u32) <= msg_len;
> +	     offset += sizeof(u32))
> +		writel(get_unaligned_le32(buf + offset), priv->mbox_reg);

[Severity: Medium]
Since this loop writes bulk data to a FIFO, should it use the stream I/O
accessor writesl() instead of writel()?

The io-accessors subsystem guidelines mandate using stream accessors for
FIFOs to avoid unnecessary per-word memory barriers and redundant endianness
conversions.

> +
> +	tail_len = msg_len - offset;
> +	if (tail_len) {
> +		memcpy(tail, buf + offset, tail_len);
> +		writel(get_unaligned_le32(tail), priv->mbox_reg);
> +	}
> +
> +	dev_dbg(mb->mbox.dev, "%s: Ch-%u sent\n", __func__, idx);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t axiado_rx_thread(int irq, void *dev_id)
> +{
> +	struct axiado_channel_data *priv = dev_id;
> +	struct mbox_chan *chan = priv->chan;
> +	struct axiado_mbox *mb = dev_get_drvdata(chan->mbox->dev);

[ ... ]

> +	/*
> +	 * Clear and log/count any pending errors, but don't discard data
> +	 * on their account alone: a rejected overflow write doesn't
> +	 * corrupt what was already safely queued ahead of it, so let the
> +	 * length-based read below decide whether what's here is usable.
> +	 */
> +	axiado_mbox_clear_fifo_errors(priv);
> +
> +	csr = readl(priv->csr_reg);
> +	if (csr & AXIADO_MBOX_CSR_EMPTY)
> +		return IRQ_NONE;

[Severity: High]
If the interrupt was asserted solely due to a hardware error (like an
underflow), axiado_mbox_clear_fifo_errors() successfully clears the condition.
If the FIFO is empty afterwards, this check returns IRQ_NONE.

Does this risk permanently disabling the IRQ line? Returning IRQ_NONE after
successfully servicing a hardware event can trigger the core spurious
interrupt detector.

> +
> +	/*
> +	 * The first word contains the total message length in bytes,
> +	 * including the length word itself.
> +	 */
> +	word = readl(priv->mbox_reg);
> +	msg_len = word;
> +	put_unaligned_le32(word, buf);
> +
> +	if (msg_len < sizeof(u32) || msg_len > mb->drv_data->msg_size)
> +		goto invalid_message;
> +
> +	num_words = DIV_ROUND_UP(msg_len, sizeof(u32));
> +	remaining = num_words - 1;
> +
> +	/*
> +	 * The not-empty interrupt may occur as soon as the first DW enters
> +	 * the FIFO. Wait until all remaining DWs of this message arrive.
> +	 */
> +	if (remaining) {
> +		ret = readl_poll_timeout(priv->csr_reg, csr,
> +					 (csr & AXIADO_MBOX_CSR_ERRORS) ||
> +					  FIELD_GET(AXIADO_MBOX_CSR_LEVEL, csr) >=
> +					  remaining,

[Severity: High]
The poll condition succeeds if AXIADO_MBOX_CSR_ERRORS is set, terminating
early without verifying the FIFO actually reached the required level.

If the loop exits early due to an error rather than the level check, the code
continues to the for-loop below and blindly reads remaining words from the
FIFO. Will this cause hardware underflows and pass garbage data to the
mailbox client?

> +					  AXIADO_MBOX_RX_POLL_US,
> +					  AXIADO_MBOX_RX_TIMEOUT_US);
> +		if (ret)
> +			goto incomplete_message;
> +
> +		axiado_mbox_clear_fifo_errors(priv);
> +	}
> +
> +	for (i = 1; i < num_words; i++) {
> +		word = readl(priv->mbox_reg);

[Severity: Medium]
Similar to the TX path, does this bulk FIFO read need to use readsl() instead
of readl() to avoid the per-word memory barriers and endianness swaps?

> +		put_unaligned_le32(word, buf + i * sizeof(u32));
> +	}
> +
> +	axiado_mbox_clear_fifo_errors(priv);
> +
> +	if (READ_ONCE(priv->active))
> +		mbox_chan_received_data(chan, priv->rx_buffer);

[Severity: High]
Can mbox_free_channel() nullify the channel client pointer while this thread
is preempted?

If a mailbox client frees the channel, mbox_free_channel() clears chan->cl
under a lock, releases the lock, and calls axiado_mbox_shutdown(). If this
threaded IRQ handler evaluates READ_ONCE(priv->active) as true just before
being preempted, the shutdown routine will block in disable_irq().

When this handler resumes, will mbox_chan_received_data() dereference a NULL
chan->cl pointer?

> +
> +	return IRQ_HANDLED;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-upstream-axiado-ax3005-mailbox-upstream-v2-0-25e713eccff3@axiado.com?part=2