On Thu, Jan 2, 2025 at 10:56 PM Chen-Yu Tsai [off-list ref] wrote:

On Fri, Jan 3, 2025 at 12:30 PM Jassi Brar [off-list ref] wrote:

quoted

On Sun, Dec 29, 2024 at 9:46 PM Karl.Li [off-list ref] wrote:
....

quoted

+
+static irqreturn_t mtk_apu_mailbox_irq(int irq, void *data)
+{
+       struct mbox_chan *chan = data;
+       struct mtk_apu_mailbox *apu_mbox = get_mtk_apu_mailbox(chan->mbox);
+       struct mbox_chan *link = &apu_mbox->mbox.chans[0];
+       u8 data_cnt = fls(readl(apu_mbox->regs + MTK_APU_MBOX_OUTBOX_IRQ));
+
+       memcpy_fromio(apu_mbox->msgs.data, apu_mbox->regs + MTK_APU_MBOX_OUTBOX,
+                     sizeof(*apu_mbox->msgs.data) * data_cnt);
+
+       mbox_chan_received_data(link, apu_mbox->msgs.data);
+
+       return IRQ_WAKE_THREAD;
+}
+

You don't seem to do anything that 'ack' the irq line. So if you merge
this into mtk_apu_mailbox_irq_thread() and do
 devm_request_threaded_irq(dev, irq,   NULL,
mtk_apu_mailbox_irq_thread,  IRQF_ONESHOT,  ...);
you can avoid adding a new api and keep your client code simple.
mbox_chan_received_data() is strictly between the client and
controller driver. If it is not called from an atomic context, feel
free to do memcpy() from it.

That sounds great. Could we perhaps update the API documentation and
drop the "atomic" requirement, or update it to something you describe
here as "between the client and controller driver".

Not drop, but simply add a line that the client needs to respect the
context in which the controller does the callback.

thanks