Repository navigation
Upstream: Intel: clarify use of locks (or lack thereof) #657
Description
Feedback from Takashi Iwai
> +static irqreturn_t byt_irq_thread(int irq, void *context)
> +{
> + struct snd_sof_dev *sdev = (struct snd_sof_dev *)context;
> + u64 ipcx, ipcd;
> + u64 imrx;
> +
> + imrx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IMRX);
> + ipcx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCX);
> +
> + /* reply message from DSP */
> + if (ipcx & SHIM_BYT_IPCX_DONE &&
> + !(imrx & SHIM_IMRX_DONE)) {
> + /* Mask Done interrupt before first */
> + snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR,
> + SHIM_IMRX,
> + SHIM_IMRX_DONE,
> + SHIM_IMRX_DONE);
Is it safe to use unlocked call here? It's a question also in general
for other places found in this patch.
Need to add a qualifier for each use of those functions to explain why the unlock mode is safe.
Activity
ranj063 commentedon Feb 28, 2019
@plbossart this looks like a mistake. the irq_thread in hda-ipc.c seems to read/write to the ipc register in the locked context.
plbossart commentedon Feb 28, 2019
@ranj063 we discussed this with @lyakh this morning, it's the same problem on all platforms that access to those IPC bits need to be protected and rethought. Don't spend time on this while Guennadi is already looking into this.
plbossart commentedon Mar 7, 2019
@lyakh can you confirm your PR #695 addresses the comments?
wenqingfu commentedon Apr 8, 2019
@lyakh ping~
plbossart commentedon Jun 10, 2019
closing since fixed a long time ago