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

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".

Disclosure: I've been doing pre-reviews for this series and we were
operating under the assumption that mbox_chan_received_data() and
its callbacks must work under atomic contexts.


Thanks
ChenYu