Re: [PATCH v2 05/10] mailbox: add MediaTek GPUEB IPI mailbox

Chia-I Wu
2025-09-12T22:11:21Z

On Fri, Sep 12, 2025 at 11:38 AM Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
<snipped>
> +static irqreturn_t mtk_gpueb_mbox_thread(int irq, void *data)
> +{
> + struct mtk_gpueb_mbox_chan *ch = data;
> + int status;
> +
> + status = atomic_cmpxchg(&ch->rx_status,
> + MBOX_FULL | MBOX_CLOGGED, MBOX_FULL);
> + if (status == (MBOX_FULL | MBOX_CLOGGED)) {
> + mtk_gpueb_mbox_read_rx(ch);
> + writel(BIT(ch->num), ch->ebm->mbox_ctl + MBOX_CTL_IRQ_CLR);
> + mbox_chan_received_data(&ch->ebm->mbox.chans[ch->num],
> + ch->rx_buf);
Given what other drivers do, and how mtk_mfg consumes the data, we should

char buf[MAX_OF_RX_LEN]; // MAX_OF_RX_LEN is 32; we can also
allocate it during probe
mtk_gpueb_mbox_read_rx(ch);
mbox_chan_received_data(..., buf);

mtx_mfg makes a copy eventually anyway. We don't need to maintain any
extra copy.

Then we might not need rx_status.

> + atomic_set(&ch->rx_status, 0);
> + return IRQ_HANDLED;
> + }
> +
> + return IRQ_NONE;
> +}
> +
> +static int mtk_gpueb_mbox_send_data(struct mbox_chan *chan, void *data)
> +{
> + struct mtk_gpueb_mbox_chan *ch = chan->con_priv;
> + int i;
> + u32 *values = data;
> +
> + if (atomic_read(&ch->rx_status))
> + return -EBUSY;
> +
> + /*
> + * We don't want any fancy nonsense, just write the 32-bit values in
> + * order. memcpy_toio/__iowrite32_copy don't work here, because fancy.
> + */
> + for (i = 0; i < ch->c->tx_len; i += 4)
> + writel(values[i / 4], ch->ebm->mbox_mmio + ch->c->tx_offset + i);
> +
> + writel(BIT(ch->num), ch->ebm->mbox_ctl + MBOX_CTL_IRQ_SET);
> +
> + return 0;
> +}
> +
> +static int mtk_gpueb_mbox_startup(struct mbox_chan *chan)
> +{
> + struct mtk_gpueb_mbox_chan *ch = chan->con_priv;
> + int ret;
> +
> + atomic_set(&ch->rx_status, 0);
> +
> + ret = clk_enable(ch->ebm->clk);
> + if (ret) {
> + dev_err(ch->ebm->dev, "Failed to enable EB clock: %pe\n",
> + ERR_PTR(ret));
> + goto err_clog;
> + }
> +
> + writel(BIT(ch->num), ch->ebm->mbox_ctl + MBOX_CTL_IRQ_CLR);
> +
> + ret = devm_request_threaded_irq(ch->ebm->dev, ch->ebm->irq, mtk_gpueb_mbox_isr,
> + mtk_gpueb_mbox_thread, IRQF_SHARED | IRQF_ONESHOT,
> + ch->full_name, ch);
I don't think this warrants a per-channel irq thread.

mbox_chan_received_data is atomic. I think wecan start simple with
just a devm_request_irq for all channels. mtk_gpueb_mbox_isr can

read bits from MBOX_CTL_RX_STS
for each bit set:
read data from rx
mbox_chan_received_data
write bits to MBOX_CTL_IRQ_CLR