Re: [PATCH v2 06/16] ASoC: Intel: Replace dpcm_playback/capture to playback/capture_only

["Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>]

On 4/1/2024 2:31 AM, Kuninori Morimoto wrote:
> soc_get_playback_capture() is now handling DPCM and normal comprehensively
> for playback/capture stream. We can use playback/capture_only flag
> instead of using dpcm_playback/capture. This patch replace these.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---

...

> diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
> index b94835448b1b..34a9b2e52451 100644
> --- a/sound/soc/intel/boards/sof_sdw.c
> +++ b/sound/soc/intel/boards/sof_sdw.c
> @@ -1151,8 +1151,8 @@ static void init_dai_link(struct device *dev, struct snd_soc_dai_link *dai_links
>   	dai_links->num_cpus = cpus_num;
>   	dai_links->codecs = codecs;
>   	dai_links->num_codecs = codecs_num;
> -	dai_links->dpcm_playback = playback;
> -	dai_links->dpcm_capture = capture;
> +	dai_links->playback_only = !playback;
> +	dai_links->capture_only = !capture;

Above seems weird? Should probably be:

	dai_links->playback_only = playback && !capture;
	dai_links->capture_only = capture && !playback;


and while at it, I still wonder if it is best way to go about this 
change, because it causes problems like one above due to need to do 
boolean logic to know which direction is enabled. I would just modify 
struct snd_soc_dai_link to have fields like:
int playback_enabled;
int capture_enabled;
which would be far more understandable. And if we don't want to have two 
variables then perhaps something like:
#define ASOC_ENDPOINT_DISABLED BIT(0)
#define ASOC_ENDPOINT_PLAYBACK BIT(1)
#define ASOC_ENDPOINT_CAPTURE BIT(2)
#define ASOC_ENDPOINT_BIDIRECTIONAL (ENDPOINT_PLAYBACK | ENDPOINT_CAPTURE)

struct snd_soc_dai_link {
	(...)
	
	int endpoint_type:2; // see ASOC_ENDPOINT

	(...)
};


I like the idea of removing the duplication of variables, but if we are 
trying to simplify things, let's try to not complicate them at the same 
time.

Thanks,
Amadeusz