On Thu, 2021-11-25 at 14:23 +0800, Tzung-Bi Shih wrote: > > On Thu, Nov 25, 2021 at 09:51:27AM +0800, allen-kh.cheng wrote: > > > > On Wed, 2021-11-24 at 18:25 +0800, Tzung-Bi Shih wrote: > > > > > > > > On Wed, Nov 24, 2021 at 04:45:13PM +0800, allen- > > > > kh.cheng wrote: > > > > > > > > > > > > +static int mtk_adsp_mbox_send_data(struct > > > > > > mbox_chan *chan, > > > > > > > > > > > > > > > > > > > > void > > > > > > > > > > > > *data) > > > > > > > > > > > > +{ > > > > > > > > > > > > + struct adsp_mbox_ch_info *ch_info = chan- > > > > > > >con_priv; > > > > > > > > > > > > + void __iomem *reg = ch_info->va_reg; > > > > > > > > > > > > + > > > > > > > > > > > > + spin_lock(&ch_info->lock); > > > > > > > > > > > > + writel(ch_info->ipc_op_val, reg + > > > > > > MTK_ADSP_MBOX_IN_CMD); > > > > > > > > > > > > + spin_unlock(&ch_info->lock); > > > > > > > > > > > > > > > > > > > > Why does it need the lock? > > > > > > > > > > > > > > > > Is the write to MTK_ADSP_MBOX_IN_CMD a synchronous > > > > operation? > > > > > > > > - If yes, I failed to understand why does it need the > > > > > > > > lock. Every > > > > > > > > calls to mtk_adsp_mbox_send_data() should wait for the > > > > data > > > > > > > > > > > > > > > transfer > > > > > > > > completion. > > > > > > > > - If no, I also failed to understand > > > > > > > > > > > > > > > why. mtk_adsp_mbox_send_data() > > > > > > > > has no way to be aware of the transfer > > > > completion. Would > > > > > > > > expect a > > > > > > > > loop for checking the completion for the case. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In ADSP MBOX IPC flow, > > > > > > > > Host would call mbox send data when the shared data transfer > > > > completed. > > > > > > > > (mtk_adsp_mbox_send_data will notice client using > > > > MTK_ADSP_MBOX_IN_CMD) > > > > > > > > It’s more like a signal. > > > > > > > > In general case, > > > > > > > > There may be some hosts use the same mbox channel. > > > > > > > > I think it’s better using lock to protect access to > > > > MTK_ADSP_MBOX_IN_CMD registers > > > > > I still failed to understand. What if 2 hosts notify the same > client > > by writing MTK_ADSP_MBOX_IN_CMD at the same time? Hi Tzung-Bi, After I think carefully. There is no need to add lock in mtk_adsp_mbox_send_data. In our dsp ipc design, we only have one host(ap) and one client(dsp). If sof ip message transfer is complete, host will use mbox notice dsp message arrived. (MTK_ADSP_MBOX_IN_CMD is signal to trigger mbox irq) I will remove this in next version.