On Tue 08 Jun 2021 at 02:12, Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> ---
> 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 */
