Re: [PATCH 05/11] ASoC: SOF: Add PCM operations support

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1427 bytes --]

On Thu, Jul 19, 2018 at 07:53:29PM +0100, Liam Girdwood wrote:

> +/* Create DMA buffer page table for DSP */
> +static int create_page_table(struct snd_pcm_substream *substream,
> +			     unsigned char *dma_area, size_t size)
> +{

That's a very generic name!

> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_component *component =
> +		snd_soc_rtdcom_lookup(rtd, DRV_NAME);
> +	struct snd_sof_dev *sdev =
> +		snd_soc_component_get_drvdata(component);
> +	struct snd_sof_pcm *spcm = rtd->sof;
> +	struct snd_dma_buffer *dmab = snd_pcm_get_dma_buf(substream);
> +	int stream = substream->stream;
> +
> +	return snd_sof_create_page_table(sdev, dmab,
> +		spcm->stream[stream].page_table.area, size);
> +}

...for a super thin wrapper.

> +	/* nothing todo for BE */
> +	if (rtd->dai_link->no_pcm)
> +		return 0;

Should these operations be defined for a back end or should back ends
have their own set of operations?  Do we even need to use DPCM here for
that matter?

> +	case SOF_DAI_INTEL_DMIC:
> +		/* DMIC only supports 16 or 32 bit formats */
> +		if (dai->comp_dai.config.frame_fmt == SOF_IPC_FRAME_S24_4LE) {
> +			dev_err(sdev->dev,
> +				"error: invalid fmt %d for DAI type %d\n",
> +				dai->comp_dai.config.frame_fmt,
> +				dai->dai_config.type);
> +		}
> +		/* TODO: add any other DMIC specific fixups */
> +		break;

We print an error but don't actually do anything about it?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]