Re: [PATCH 03/19] soundwire: amd: register sdw controller dai ops

[Amadeusz Sławiński]

On 1/11/2023 10:02 AM, Vijendar Mukunda wrote:
> Register dai ops for two controller instances.
>
> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@xxxxxxx>
> ---

> +static int amd_sdwc_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
> +{
> +	struct amd_sdwc_ctrl *ctrl = snd_soc_dai_get_drvdata(dai);
> +	struct sdw_amd_dma_data *dma;
> +	int ret;
> +
> +	dma = snd_soc_dai_get_dma_data(dai, substream);
> +	if (!dma)
> +		return -EIO;
> +
> +	ret = sdw_stream_remove_master(&ctrl->bus, dma->stream);
> +	if (ret < 0) {
> +		dev_err(dai->dev, "remove master from stream %s failed: %d\n",
> +			dma->stream->name, ret);
> +		return ret;
> +	}
> +	dma->hw_params = NULL;
> +	return 0;
> +}
> +
> +static int amd_set_sdw_stream(struct snd_soc_dai *dai, void *stream, int direction)
> +{
> +	struct amd_sdwc_ctrl *ctrl = snd_soc_dai_get_drvdata(dai);
> +	struct sdw_amd_dma_data *dma;
> +
> +	if (stream) {
> +		if (direction == SNDRV_PCM_STREAM_PLAYBACK)
> +			dma = dai->playback_dma_data;
> +		else
> +			dma = dai->capture_dma_data;
> +

The patch itself looks ok, but I have generic ASoC API question. Could 
we perhaps change snd_soc_dai_get_dma_data() definition, so that instead 
of it being:
static inline void *snd_soc_dai_get_dma_data(const struct snd_soc_dai 
*dai, const struct snd_pcm_substream *ss)
it would be something like:
static inline void *snd_soc_dai_get_dma_data(const struct snd_soc_dai 
*dai, int direction)

it would require converting current calls from something like
dma = snd_soc_dai_get_dma_data(dai, substream);
to
dma = snd_soc_dai_get_dma_data(dai, substream->stream);
but would also allow for use in code like above?
It would become just:
dma = snd_soc_dai_get_dma_data(dai, direction);

The more I'm looking at the soc-dai.h header the more I like this idea, 
as other functions in the area seem to pass stream/direction explicitly 
instead of substream.

Mark, what do you think?