Re: [PATCH 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 < 20000) {

• 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(5);
  

• }

maybe use human-readable defines for timeout and delay? e.g.

#define TIMEOUT_MS 100 #define DELAY_US 5

• 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 < 20000) {
  

• 	pdm_dma_enable = rn_readl(acp_base +
  

• 				  ACP_WOV_PDM_DMA_ENABLE);
  

• 	if ((pdm_dma_enable & 0x02) == 0x00)
  

• 		return 0;
  

• 	udelay(5);
  

• }
  

• if (timeout == 20000)
  

if this test needed, it'll be always true, no?

• 	return -ETIMEDOUT;
  

• }
• if (pdm_enable == ACP_PDM_ENABLE) {

• pdm_enable = ACP_PDM_DISABLE;
  

• rn_writel(pdm_enable, acp_base + ACP_WOV_PDM_ENABLE);
  

• }
• rn_writel(0x01, acp_base + ACP_WOV_PDM_FIFO_FLUSH);
• return 0;

+}

+static int acp_pdm_dai_hw_params(struct snd_pcm_substream *substream,

• 		 struct snd_pcm_hw_params *params,
  

• 		 struct snd_soc_dai *dai)
  

+{

• struct pdm_stream_instance *rtd;
• unsigned int ch_mask;
• rtd = substream->runtime->private_data;
• switch (params_channels(params)) {
• case TWO_CH:
• default:

• ch_mask = 0x00;
  

• break;
  

• }

the switch doesn't appear very useful at the moment?

• config_pdm_stream_params(ch_mask, rtd->acp_base);
• return 0;

+}