-----Original Message----- From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Sent: Tuesday, May 12, 2020 3:42 AM To: Alex Deucher alexdeucher@gmail.com; alsa-devel@alsa-project.org; broonie@kernel.org; Mukunda, Vijendar Vijendar.Mukunda@amd.com; tiwai@suse.de Cc: Deucher, Alexander Alexander.Deucher@amd.com Subject: Re: [PATCH v2 08/14] ASoC: amd: add ACP PDM DMA driver dai ops
+static int start_pdm_dma(void __iomem *acp_base) +{
- u32 pdm_enable;
- u32 pdm_dma_enable;
- int timeout;
- pdm_enable = 0x01;
- pdm_dma_enable = 0x01;
- enable_pdm_clock(acp_base);
- rn_writel(pdm_enable, acp_base + ACP_WOV_PDM_ENABLE);
- rn_writel(pdm_dma_enable, acp_base +
ACP_WOV_PDM_DMA_ENABLE);
- pdm_dma_enable = 0x00;
- timeout = 0;
- while (++timeout < ACP_COUNTER) {
pdm_dma_enable = rn_readl(acp_base +
ACP_WOV_PDM_DMA_ENABLE);
if ((pdm_dma_enable & 0x02) ==
ACP_PDM_DMA_EN_STATUS)
return 0;
udelay(DELAY_US);
- }
- return -ETIMEDOUT;
+}
+static int stop_pdm_dma(void __iomem *acp_base) +{
- u32 pdm_enable, pdm_dma_enable, pdm_fifo_flush;
- int timeout;
- pdm_enable = 0x00;
- pdm_dma_enable = 0x00;
- pdm_fifo_flush = 0x00;
- pdm_enable = rn_readl(acp_base + ACP_WOV_PDM_ENABLE);
- pdm_dma_enable = rn_readl(acp_base +
ACP_WOV_PDM_DMA_ENABLE);
- if (pdm_dma_enable & 0x01) {
pdm_dma_enable = 0x02;
rn_writel(pdm_dma_enable, acp_base +
ACP_WOV_PDM_DMA_ENABLE);
pdm_dma_enable = 0x00;
timeout = 0;
while (++timeout < ACP_COUNTER) {
pdm_dma_enable = rn_readl(acp_base +
ACP_WOV_PDM_DMA_ENABLE);
if ((pdm_dma_enable & 0x02) == 0x00)
break;
udelay(DELAY_US);
}
if (timeout == ACP_COUNTER)
if you reach this point, timeout is by construction equal to ACP_COUNTER so this test is useless
Here we are checking current dma status. If DMA enable status bit is set to zero , it will exit the while loop. It will continue rest of the registers programming to disable PDM decoder and flushing fifo.
When this condition "if ((pdm_dma_enable & 0x02) == 0x00)" is true, "timeout" counter won't be exhausted i.e it won't cross ACP_COUNTER value.
We have to return time out error only when DMA enable status bit is not cleared and timeout reached max value i.e ACP_COUNTER. i.e This check " if (timeout == ACP_COUNTER) " required to know the while loop exit condition.
You could also use a helper where you check if the value is ACP_PDM_DMA_EN_STATUS or zero so that you don't copy the same logic twice.
I believe it's only a single register check, still its hold to be good.