[PATCH 08/14] ASoC: amd: add ACP PDM DMA driver dai ops

[Pierre-Louis Bossart]

> +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;
> +}
> +