[PATCH 6/9] ASoC: fsl: switch to use snd_soc_daifmt_parse_format/clock_provider()

[Jerome Brunet]

On Tue 08 Jun 2021 at 02:12, Kuninori Morimoto <kuninori.morimoto.gx at renesas.com> wrote:

> From: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
>
> This patch switch to use snd_soc_daifmt_parse_format/clock_provider() from
> snd_soc_of_parse_daifmt().
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> ---
>  sound/soc/fsl/fsl-asoc-card.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> index c62bfd1c3ac7..6a6f098da0dc 100644
> --- a/sound/soc/fsl/fsl-asoc-card.c
> +++ b/sound/soc/fsl/fsl-asoc-card.c
> @@ -540,7 +540,6 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
>  	struct device *codec_dev = NULL;
>  	const char *codec_dai_name;
>  	const char *codec_dev_name;
> -	unsigned int daifmt;
>  	u32 width;
>  	int ret;
>  
> @@ -684,19 +683,12 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Format info from DT is optional. */
> -	daifmt = snd_soc_of_parse_daifmt(np, NULL,
> -					 &bitclkmaster, &framemaster);
> -	daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
> +	snd_soc_daifmt_parse_clock_provider(np, NULL, &bitclkmaster, &framemaster);
>  	if (bitclkmaster || framemaster) {
> -		if (codec_np == bitclkmaster)
> -			daifmt |= (codec_np == framemaster) ?
> -				SND_SOC_DAIFMT_CBM_CFM : SND_SOC_DAIFMT_CBM_CFS;
> -		else
> -			daifmt |= (codec_np == framemaster) ?
> -				SND_SOC_DAIFMT_CBS_CFM : SND_SOC_DAIFMT_CBS_CFS;
> -
>  		/* Override dai_fmt with value from DT */
> -		priv->dai_fmt = daifmt;
> +		priv->dai_fmt = snd_soc_daifmt_parse_format(np, NULL) |
> +			snd_soc_daifmt_clock_provider_pickup(((codec_np == bitclkmaster) << 4) +
> +							      (codec_np == framemaster));

Hi,

I understand you are trying to fold some code but I'm not sure about this
snd_soc_daifmt_clock_provider_pickup().

Instead of repeating the if clause around DAIFMT (which is a bit verbose
but fairly easy to understand and maintain) there is now the calculation
of a made up constant (which is rather opaque as it is), which later
translate to the same type of test around DAIFMT. 

I'd be in favor of dropping the snd_soc_daifmt_clock_provider_pickup()
part for the sake or readability. This apply to the rest of the series,
not just fsl.

The rest looks good, Thx Kuninori.

>  	}
>  
>  	/* Change direction according to format */