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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The driver violates I/O accessors guidelines by using register accessors (writel/readl) instead of stream accessors (writesl/readsl) for bulk FIFO transfers.
--

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

mailbox: add Axiado AX3005 mailbox driver

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

> 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);
> +
> +	tail_len = msg_len - offset;
> +	if (tail_len) {
> +		memcpy(tail, buf + offset, tail_len);
> +		writel(get_unaligned_le32(tail), priv->mbox_reg);
> +	}

[Severity: Medium]
Does this code use the correct I/O accessors for FIFO transfers?

According to the I/O accessors guidelines, stream accessors like writesl()
should be used exclusively for FIFOs instead of register accessors like
writel().

While get_unaligned_le32() attempts to cancel out the endianness mapping,
using writel() in a loop introduces unnecessary per-access memory barriers.
Could this be updated to use writesl() for the bulk transfer?

[ ... ]

> +static irqreturn_t axiado_rx_thread(int irq, void *dev_id)
> +{

[ ... ]

> +	for (i = 1; i < num_words; i++) {
> +		word = readl(priv->mbox_reg);
> +		put_unaligned_le32(word,
> +				   buf + i * sizeof(u32));
> +	}

[Severity: Medium]
Similar to the transmit path in axiado_mbox_send_data(), should this loop
use readsl() instead of readl() for reading bulk data from the RX FIFO?

Replacing readl() with readsl() would align with the guidelines for FIFO
data streams and avoid the implicit memory barriers on each read access.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-upstream-axiado-ax3005-mailbox-upstream-v1-0-8961b7350338@axiado.com?part=2