[PATCH 0/9] ASoC: tidyup snd_soc_of_parse_daifmt()

[PATCH 0/9] ASoC: tidyup snd_soc_of_parse_daifmt()

[Kuninori Morimoto]

Hi Mark

I want to add new audio-graph-card2 sound card driver,
and this is last part of necessary soc-core cleanup for it.

Current some drivers are using DT, and Then,
snd_soc_of_parse_daifmt() parses daifmt, but bitclock/frame provider
parsing part is one of headache, because we are assuming below both cases.

A)	node {
		bitclock-master;
		frame-master;
		...
	};
    
B)	link {
		bitclock-master = <&xxx>;
		frame-master = <&xxx>;
		...
	};

The original was style A), and style B) was added later.

snd_soc_of_parse_daifmt() parses A) style as original style,
and user need to update to B) style for clock_provider part if needed.
In such case, user need to re-parse it, like below.
    
	daifmt = snd_soc_of_parse_daifmt(..., &bitclkmaster, &framemaster);
	daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
    
	if (codec == bitclkmaster)
		daifmt |= (codec == framemaster) ?
			SND_SOC_DAIFMT_CBM_CFM : SND_SOC_DAIFMT_CBM_CFS;
	else
		daifmt |= (codec == framemaster) ?
			SND_SOC_DAIFMT_CBS_CFM : SND_SOC_DAIFMT_CBS_CFS;

This patch-set adds new functions, and handle these more simply.
Unfortunately, there are too many use-case, do it by 1 function was implessible.

style A)
	bit_frame = snd_soc_daifmt_parse_clock_provider();
	daifmt = snd_soc_daifmt_parse_format(...) |              /* format part */
		snd_soc_daifmt_clock_provider_pickup(bit_frame); /* clock  part */
    
style B)
	snd_soc_daifmt_parse_clock_provider(..., &bit, &frame);
	daifmt = snd_soc_daifmt_parse_format(...) |    /* format part */
		snd_soc_daifmt_clock_provider_pickup(  /* clock  part */
			((codec == bit) << 4) + (codec == frame));


Kuninori Morimoto (9):
  ASoC: soc-core: don't use discriminatory terms on snd_soc_runtime_get_dai_fmt()
  ASoC: soc-core: add snd_soc_daifmt_clock_provider_pickup()
  ASoC: soc-core: add snd_soc_daifmt_clock_provider_fliped()
  ASoC: soc-core: add snd_soc_daifmt_parse_format/clock_provider()
  ASoC: atmel: switch to use snd_soc_daifmt_parse_format/clock_provider()
  ASoC: fsl: switch to use snd_soc_daifmt_parse_format/clock_provider()
  ASoC: meson: switch to use snd_soc_daifmt_parse_format/clock_provider()
  ASoC: simple-card-utils: switch to use snd_soc_daifmt_parse_format/clock_provider()
  ASoC: soc-core: remove snd_soc_of_parse_daifmt()

 include/sound/soc.h                   |  13 +++-
 sound/soc/atmel/mikroe-proto.c        |  18 ++---
 sound/soc/fsl/fsl-asoc-card.c         |  16 +---
 sound/soc/generic/simple-card-utils.c |  19 ++---
 sound/soc/meson/meson-card-utils.c    |  15 ++--
 sound/soc/soc-core.c                  | 103 ++++++++++++++++----------
 6 files changed, 99 insertions(+), 85 deletions(-)

-- 
2.25.1

[PATCH 1/9] ASoC: soc-core: don't use discriminatory terms on snd_soc_runtime_get_dai_fmt()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

snd_soc_runtime_get_dai_fmt() is using discriminatory terms.
This patch fixup it.

Fixes: ba9e82a1c891 ("ASoC: soc-core: add snd_soc_runtime_get_dai_fmt()")
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 4daa9b22b33c..44e65f984a5c 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1211,8 +1211,8 @@ static void snd_soc_runtime_get_dai_fmt(struct snd_soc_pcm_runtime *rtd)
 		mask |= SND_SOC_DAIFMT_CLOCK_MASK;
 	if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_INV_MASK))
 		mask |= SND_SOC_DAIFMT_INV_MASK;
-	if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK))
-		mask |= SND_SOC_DAIFMT_MASTER_MASK;
+	if (!(dai_link->dai_fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK))
+		mask |= SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK;
 
 	dai_link->dai_fmt |= (dai_fmt & mask);
 }
-- 
2.25.1

[PATCH 2/9] ASoC: soc-core: add snd_soc_daifmt_clock_provider_pickup()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This patch adds snd_soc_daifmt_clock_provider_pickup() function
to judge clock/frame master.
This is prepare for snd_soc_of_parse_daifmt() cleanup.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  |  1 +
 sound/soc/soc-core.c | 33 +++++++++++++++++++--------------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index e746da996351..e852cfbaf572 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1232,6 +1232,7 @@ void snd_soc_of_parse_audio_prefix(struct snd_soc_card *card,
 int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
 				   const char *propname);
 int snd_soc_of_parse_aux_devs(struct snd_soc_card *card, const char *propname);
+unsigned int snd_soc_daifmt_clock_provider_pickup(unsigned int bit_frame);
 unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 				     const char *prefix,
 				     struct device_node **bitclkmaster,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 44e65f984a5c..2ce73bf77c05 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3017,6 +3017,24 @@ int snd_soc_of_parse_aux_devs(struct snd_soc_card *card, const char *propname)
 }
 EXPORT_SYMBOL_GPL(snd_soc_of_parse_aux_devs);
 
+unsigned int snd_soc_daifmt_clock_provider_pickup(unsigned int bit_frame)
+{
+	/* Codec base */
+	switch (bit_frame) {
+	case 0x11:
+		return SND_SOC_DAIFMT_CBP_CFP;
+	case 0x10:
+		return SND_SOC_DAIFMT_CBP_CFC;
+	case 0x01:
+		return SND_SOC_DAIFMT_CBC_CFP;
+	default:
+		return SND_SOC_DAIFMT_CBC_CFC;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_daifmt_clock_provider_pickup);
+
 unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 				     const char *prefix,
 				     struct device_node **bitclkmaster,
@@ -3115,20 +3133,7 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 	if (frame && framemaster)
 		*framemaster = of_parse_phandle(np, prop, 0);
 
-	switch ((bit << 4) + frame) {
-	case 0x11:
-		format |= SND_SOC_DAIFMT_CBM_CFM;
-		break;
-	case 0x10:
-		format |= SND_SOC_DAIFMT_CBM_CFS;
-		break;
-	case 0x01:
-		format |= SND_SOC_DAIFMT_CBS_CFM;
-		break;
-	default:
-		format |= SND_SOC_DAIFMT_CBS_CFS;
-		break;
-	}
+	format |= snd_soc_daifmt_clock_provider_pickup((bit << 4) + frame);
 
 	return format;
 }
-- 
2.25.1

[PATCH 3/9] ASoC: soc-core: add snd_soc_daifmt_clock_provider_fliped()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Sometimes we want to get CLOCK_PROVIDER fliped dai_fmt.
This patch adds new snd_soc_daifmt_clock_provider_fliped() for it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  |  2 ++
 sound/soc/soc-core.c | 40 +++++++++++++++++++++++++---------------
 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index e852cfbaf572..0f25d4be92ae 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1232,6 +1232,8 @@ void snd_soc_of_parse_audio_prefix(struct snd_soc_card *card,
 int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
 				   const char *propname);
 int snd_soc_of_parse_aux_devs(struct snd_soc_card *card, const char *propname);
+
+unsigned int snd_soc_daifmt_clock_provider_fliped(unsigned int dai_fmt);
 unsigned int snd_soc_daifmt_clock_provider_pickup(unsigned int bit_frame);
 unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 				     const char *prefix,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 2ce73bf77c05..fd5a3b60aeb3 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1249,21 +1249,8 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd,
 	 * Flip the polarity for the "CPU" end of a CODEC<->CODEC link
 	 * the component which has non_legacy_dai_naming is Codec
 	 */
-	inv_dai_fmt = dai_fmt & ~SND_SOC_DAIFMT_MASTER_MASK;
-	switch (dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) {
-	case SND_SOC_DAIFMT_CBM_CFM:
-		inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
-		break;
-	case SND_SOC_DAIFMT_CBM_CFS:
-		inv_dai_fmt |= SND_SOC_DAIFMT_CBS_CFM;
-		break;
-	case SND_SOC_DAIFMT_CBS_CFM:
-		inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFS;
-		break;
-	case SND_SOC_DAIFMT_CBS_CFS:
-		inv_dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
-		break;
-	}
+	inv_dai_fmt = snd_soc_daifmt_clock_provider_fliped(dai_fmt);
+
 	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
 		unsigned int fmt = dai_fmt;
 
@@ -3017,6 +3004,29 @@ int snd_soc_of_parse_aux_devs(struct snd_soc_card *card, const char *propname)
 }
 EXPORT_SYMBOL_GPL(snd_soc_of_parse_aux_devs);
 
+unsigned int snd_soc_daifmt_clock_provider_fliped(unsigned int dai_fmt)
+{
+	unsigned int inv_dai_fmt = dai_fmt & ~SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK;
+
+	switch (dai_fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) {
+	case SND_SOC_DAIFMT_CBP_CFP:
+		inv_dai_fmt |= SND_SOC_DAIFMT_CBC_CFC;
+		break;
+	case SND_SOC_DAIFMT_CBP_CFC:
+		inv_dai_fmt |= SND_SOC_DAIFMT_CBC_CFP;
+		break;
+	case SND_SOC_DAIFMT_CBC_CFP:
+		inv_dai_fmt |= SND_SOC_DAIFMT_CBP_CFC;
+		break;
+	case SND_SOC_DAIFMT_CBC_CFC:
+		inv_dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
+		break;
+	}
+
+	return inv_dai_fmt;
+}
+EXPORT_SYMBOL_GPL(snd_soc_daifmt_clock_provider_fliped);
+
 unsigned int snd_soc_daifmt_clock_provider_pickup(unsigned int bit_frame)
 {
 	/* Codec base */
-- 
2.25.1

[PATCH 4/9] ASoC: soc-core: add snd_soc_daifmt_parse_format/clock_provider()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

snd_soc_of_parse_daifmt() parses daifmt, but bitclock/frame provider
parsing part is one of headache, because we are assuming below both cases.

A)	node {
		bitclock-master;
		frame-master;
		...
	};

B)	link {
		bitclock-master = <&xxx>;
		frame-master = <&xxx>;
		...
	};

The original was style A), and style B) was added later
by commit b3ca11ff59bc ("ASoC: simple-card: Move dai-link level
properties away from dai subnodes").

snd_soc_of_parse_daifmt() parses A) style as original style,
and user need to update to B) style for clock_provider part if needed.
In such case, user need to re-parse it, like below.

	daifmt = snd_soc_of_parse_daifmt(..., &bitclkmaster, &framemaster);
	daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;

	if (codec == bitclkmaster)
		daifmt |= (codec == framemaster) ?
			SND_SOC_DAIFMT_CBM_CFM : SND_SOC_DAIFMT_CBM_CFS;
	else
		daifmt |= (codec == framemaster) ?
			SND_SOC_DAIFMT_CBS_CFM : SND_SOC_DAIFMT_CBS_CFS;

This patch adds new functions which separetes
snd_soc_of_parse_daifmt() helper function into
parsing format         part (= snd_soc_daifmt_parse_format()), and
parsing clock_provider part (= snd_soc_daifmt_parse_clock_provider()).
User can use snd_soc_daifmt_clock_provider_pickup/fliped() helper
function with it.

style A)
	bit_frame = snd_soc_daifmt_parse_clock_provider();
	daifmt = snd_soc_daifmt_parse_format(...) |               /* format part */
		 snd_soc_daifmt_clock_provider_pickup(bit_frame); /* clock  part */

style B)
	snd_soc_daifmt_parse_clock_provider(..., &bit, &frame);
	daifmt = snd_soc_daifmt_parse_format(...) |     /* format part */
		 snd_soc_daifmt_clock_provider_pickup(  /* clock  part */
			((codec == bit) << 4) + (codec == frame));

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  |   6 +++
 sound/soc/soc-core.c | 114 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 0f25d4be92ae..f402b259a255 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1235,6 +1235,12 @@ int snd_soc_of_parse_aux_devs(struct snd_soc_card *card, const char *propname);
 
 unsigned int snd_soc_daifmt_clock_provider_fliped(unsigned int dai_fmt);
 unsigned int snd_soc_daifmt_clock_provider_pickup(unsigned int bit_frame);
+unsigned int snd_soc_daifmt_parse_format(struct device_node *np,
+					 const char *prefix);
+unsigned int snd_soc_daifmt_parse_clock_provider(struct device_node *np,
+						 const char *prefix,
+						 struct device_node **bitclkmaster,
+						 struct device_node **framemaster);
 unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 				     const char *prefix,
 				     struct device_node **bitclkmaster,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index fd5a3b60aeb3..a9cb39c3d8c5 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3045,6 +3045,120 @@ unsigned int snd_soc_daifmt_clock_provider_pickup(unsigned int bit_frame)
 }
 EXPORT_SYMBOL_GPL(snd_soc_daifmt_clock_provider_pickup);
 
+unsigned int snd_soc_daifmt_parse_format(struct device_node *np,
+					 const char *prefix)
+{
+	int ret, i;
+	char prop[128];
+	unsigned int format = 0;
+	int bit, frame;
+	const char *str;
+	struct {
+		char *name;
+		unsigned int val;
+	} of_fmt_table[] = {
+		{ "i2s",	SND_SOC_DAIFMT_I2S },
+		{ "right_j",	SND_SOC_DAIFMT_RIGHT_J },
+		{ "left_j",	SND_SOC_DAIFMT_LEFT_J },
+		{ "dsp_a",	SND_SOC_DAIFMT_DSP_A },
+		{ "dsp_b",	SND_SOC_DAIFMT_DSP_B },
+		{ "ac97",	SND_SOC_DAIFMT_AC97 },
+		{ "pdm",	SND_SOC_DAIFMT_PDM},
+		{ "msb",	SND_SOC_DAIFMT_MSB },
+		{ "lsb",	SND_SOC_DAIFMT_LSB },
+	};
+
+	if (!prefix)
+		prefix = "";
+
+	/*
+	 * check "dai-format = xxx"
+	 * or    "[prefix]format = xxx"
+	 * SND_SOC_DAIFMT_FORMAT_MASK area
+	 */
+	ret = of_property_read_string(np, "dai-format", &str);
+	if (ret < 0) {
+		snprintf(prop, sizeof(prop), "%sformat", prefix);
+		ret = of_property_read_string(np, prop, &str);
+	}
+	if (ret == 0) {
+		for (i = 0; i < ARRAY_SIZE(of_fmt_table); i++) {
+			if (strcmp(str, of_fmt_table[i].name) == 0) {
+				format |= of_fmt_table[i].val;
+				break;
+			}
+		}
+	}
+
+	/*
+	 * check "[prefix]continuous-clock"
+	 * SND_SOC_DAIFMT_CLOCK_MASK area
+	 */
+	snprintf(prop, sizeof(prop), "%scontinuous-clock", prefix);
+	if (of_property_read_bool(np, prop))
+		format |= SND_SOC_DAIFMT_CONT;
+	else
+		format |= SND_SOC_DAIFMT_GATED;
+
+	/*
+	 * check "[prefix]bitclock-inversion"
+	 * check "[prefix]frame-inversion"
+	 * SND_SOC_DAIFMT_INV_MASK area
+	 */
+	snprintf(prop, sizeof(prop), "%sbitclock-inversion", prefix);
+	bit = !!of_get_property(np, prop, NULL);
+
+	snprintf(prop, sizeof(prop), "%sframe-inversion", prefix);
+	frame = !!of_get_property(np, prop, NULL);
+
+	switch ((bit << 4) + frame) {
+	case 0x11:
+		format |= SND_SOC_DAIFMT_IB_IF;
+		break;
+	case 0x10:
+		format |= SND_SOC_DAIFMT_IB_NF;
+		break;
+	case 0x01:
+		format |= SND_SOC_DAIFMT_NB_IF;
+		break;
+	default:
+		/* SND_SOC_DAIFMT_NB_NF is default */
+		break;
+	}
+
+	return format;
+}
+EXPORT_SYMBOL_GPL(snd_soc_daifmt_parse_format);
+
+unsigned int snd_soc_daifmt_parse_clock_provider(struct device_node *np,
+						 const char *prefix,
+						 struct device_node **bitclkmaster,
+						 struct device_node **framemaster)
+{
+	char prop[128];
+	unsigned int bit, frame;
+
+	if (!prefix)
+		prefix = "";
+
+	/*
+	 * check "[prefix]bitclock-master"
+	 * check "[prefix]frame-master"
+	 */
+	snprintf(prop, sizeof(prop), "%sbitclock-master", prefix);
+	bit = !!of_get_property(np, prop, NULL);
+	if (bit && bitclkmaster)
+		*bitclkmaster = of_parse_phandle(np, prop, 0);
+
+	snprintf(prop, sizeof(prop), "%sframe-master", prefix);
+	frame = !!of_get_property(np, prop, NULL);
+	if (frame && framemaster)
+		*framemaster = of_parse_phandle(np, prop, 0);
+
+	return (bit << 4) + frame;
+}
+EXPORT_SYMBOL_GPL(snd_soc_daifmt_parse_clock_provider);
+
 unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 				     const char *prefix,
 				     struct device_node **bitclkmaster,
-- 
2.25.1

[PATCH 5/9] ASoC: atmel: switch to use snd_soc_daifmt_parse_format/clock_provider()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@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@renesas.com>
---
 sound/soc/atmel/mikroe-proto.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/sound/soc/atmel/mikroe-proto.c b/sound/soc/atmel/mikroe-proto.c
index f9a85fd01b79..eeb2effed1e4 100644
--- a/sound/soc/atmel/mikroe-proto.c
+++ b/sound/soc/atmel/mikroe-proto.c
@@ -69,6 +69,7 @@ static int snd_proto_probe(struct platform_device *pdev)
 	struct device_node *bitclkmaster = NULL;
 	struct device_node *framemaster = NULL;
 	unsigned int dai_fmt;
+	unsigned int bit_frame;
 	int ret = 0;
 
 	if (!np) {
@@ -120,19 +121,18 @@ static int snd_proto_probe(struct platform_device *pdev)
 	dai->cpus->of_node = cpu_np;
 	dai->platforms->of_node = cpu_np;
 
-	dai_fmt = snd_soc_of_parse_daifmt(np, NULL,
-					  &bitclkmaster, &framemaster);
+	bit_frame = snd_soc_daifmt_parse_clock_provider(np, NULL, &bitclkmaster, &framemaster);
 	if (bitclkmaster != framemaster) {
 		dev_err(&pdev->dev, "Must be the same bitclock and frame master\n");
 		return -EINVAL;
 	}
-	if (bitclkmaster) {
-		dai_fmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
-		if (codec_np == bitclkmaster)
-			dai_fmt |= SND_SOC_DAIFMT_CBM_CFM;
-		else
-			dai_fmt |= SND_SOC_DAIFMT_CBS_CFS;
-	}
+	if (bitclkmaster)
+		bit_frame = ((codec_np == bitclkmaster) << 4) +
+			     (codec_np == framemaster);
+
+	dai_fmt = snd_soc_daifmt_parse_format(np, NULL) |
+		  snd_soc_daifmt_clock_provider_pickup(bit_frame);
+
 	of_node_put(bitclkmaster);
 	of_node_put(framemaster);
 	dai->dai_fmt = dai_fmt;
-- 
2.25.1

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

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@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@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));
 	}
 
 	/* Change direction according to format */
-- 
2.25.1

[PATCH 7/9] ASoC: meson: switch to use snd_soc_daifmt_parse_format/clock_provider()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@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@renesas.com>
---
 sound/soc/meson/meson-card-utils.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/sound/soc/meson/meson-card-utils.c b/sound/soc/meson/meson-card-utils.c
index 300ac8be46ef..60959e2c71b8 100644
--- a/sound/soc/meson/meson-card-utils.c
+++ b/sound/soc/meson/meson-card-utils.c
@@ -119,18 +119,13 @@ unsigned int meson_card_parse_daifmt(struct device_node *node,
 	struct device_node *framemaster = NULL;
 	unsigned int daifmt;
 
-	daifmt = snd_soc_of_parse_daifmt(node, "",
-					 &bitclkmaster, &framemaster);
-	daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
+	snd_soc_daifmt_parse_clock_provider(node, "", &bitclkmaster, &framemaster);
 
 	/* If no master is provided, default to cpu master */
-	if (!bitclkmaster || bitclkmaster == cpu_node) {
-		daifmt |= (!framemaster || framemaster == cpu_node) ?
-			SND_SOC_DAIFMT_CBS_CFS : SND_SOC_DAIFMT_CBS_CFM;
-	} else {
-		daifmt |= (!framemaster || framemaster == cpu_node) ?
-			SND_SOC_DAIFMT_CBM_CFS : SND_SOC_DAIFMT_CBM_CFM;
-	}
+	daifmt = snd_soc_daifmt_parse_format(node, "") |
+		 snd_soc_daifmt_clock_provider_pickup(
+			((bitclkmaster && bitclkmaster != cpu_node) << 4) +
+			 (framemaster  && framemaster  != cpu_node));
 
 	of_node_put(bitclkmaster);
 	of_node_put(framemaster);
-- 
2.25.1

[PATCH 8/9] ASoC: simple-card-utils: switch to use snd_soc_daifmt_parse_format/clock_provider()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@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@renesas.com>
---
 sound/soc/generic/simple-card-utils.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index fa1247f0dda1..de23e6e7f3f3 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -60,10 +60,9 @@ int asoc_simple_parse_daifmt(struct device *dev,
 	struct device_node *bitclkmaster = NULL;
 	struct device_node *framemaster = NULL;
 	unsigned int daifmt;
+	unsigned int bit_frame;
 
-	daifmt = snd_soc_of_parse_daifmt(node, prefix,
-					 &bitclkmaster, &framemaster);
-	daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
+	snd_soc_daifmt_parse_clock_provider(node, prefix, &bitclkmaster, &framemaster);
 
 	if (!bitclkmaster && !framemaster) {
 		/*
@@ -73,17 +72,15 @@ int asoc_simple_parse_daifmt(struct device *dev,
 		 */
 		dev_dbg(dev, "Revert to legacy daifmt parsing\n");
 
-		daifmt = snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
-			(daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
+		bit_frame = snd_soc_daifmt_parse_clock_provider(codec, NULL, NULL, NULL);
 	} else {
-		if (codec == bitclkmaster)
-			daifmt |= (codec == framemaster) ?
-				SND_SOC_DAIFMT_CBM_CFM : SND_SOC_DAIFMT_CBM_CFS;
-		else
-			daifmt |= (codec == framemaster) ?
-				SND_SOC_DAIFMT_CBS_CFM : SND_SOC_DAIFMT_CBS_CFS;
+		bit_frame = ((codec == bitclkmaster) << 4) +
+			     (codec == framemaster);
 	}
 
+	daifmt = snd_soc_daifmt_parse_format(node, prefix) |
+		 snd_soc_daifmt_clock_provider_pickup(bit_frame);
+
 	of_node_put(bitclkmaster);
 	of_node_put(framemaster);
 
-- 
2.25.1

[PATCH 9/9] ASoC: soc-core: remove snd_soc_of_parse_daifmt()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

No driver is using snd_soc_of_parse_daifmt().
This patch removes it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  |   4 --
 sound/soc/soc-core.c | 104 -------------------------------------------
 2 files changed, 108 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index f402b259a255..75ad899fd84f 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1241,10 +1241,6 @@ unsigned int snd_soc_daifmt_parse_clock_provider(struct device_node *np,
 						 const char *prefix,
 						 struct device_node **bitclkmaster,
 						 struct device_node **framemaster);
-unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
-				     const char *prefix,
-				     struct device_node **bitclkmaster,
-				     struct device_node **framemaster);
 int snd_soc_get_dai_id(struct device_node *ep);
 int snd_soc_get_dai_name(const struct of_phandle_args *args,
 			 const char **dai_name);
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index a9cb39c3d8c5..3009a315a4a6 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3159,110 +3159,6 @@ unsigned int snd_soc_daifmt_parse_clock_provider(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(snd_soc_daifmt_parse_clock_provider);
 
-unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
-				     const char *prefix,
-				     struct device_node **bitclkmaster,
-				     struct device_node **framemaster)
-{
-	int ret, i;
-	char prop[128];
-	unsigned int format = 0;
-	int bit, frame;
-	const char *str;
-	struct {
-		char *name;
-		unsigned int val;
-	} of_fmt_table[] = {
-		{ "i2s",	SND_SOC_DAIFMT_I2S },
-		{ "right_j",	SND_SOC_DAIFMT_RIGHT_J },
-		{ "left_j",	SND_SOC_DAIFMT_LEFT_J },
-		{ "dsp_a",	SND_SOC_DAIFMT_DSP_A },
-		{ "dsp_b",	SND_SOC_DAIFMT_DSP_B },
-		{ "ac97",	SND_SOC_DAIFMT_AC97 },
-		{ "pdm",	SND_SOC_DAIFMT_PDM},
-		{ "msb",	SND_SOC_DAIFMT_MSB },
-		{ "lsb",	SND_SOC_DAIFMT_LSB },
-	};
-
-	if (!prefix)
-		prefix = "";
-
-	/*
-	 * check "dai-format = xxx"
-	 * or    "[prefix]format = xxx"
-	 * SND_SOC_DAIFMT_FORMAT_MASK area
-	 */
-	ret = of_property_read_string(np, "dai-format", &str);
-	if (ret < 0) {
-		snprintf(prop, sizeof(prop), "%sformat", prefix);
-		ret = of_property_read_string(np, prop, &str);
-	}
-	if (ret == 0) {
-		for (i = 0; i < ARRAY_SIZE(of_fmt_table); i++) {
-			if (strcmp(str, of_fmt_table[i].name) == 0) {
-				format |= of_fmt_table[i].val;
-				break;
-			}
-		}
-	}
-
-	/*
-	 * check "[prefix]continuous-clock"
-	 * SND_SOC_DAIFMT_CLOCK_MASK area
-	 */
-	snprintf(prop, sizeof(prop), "%scontinuous-clock", prefix);
-	if (of_property_read_bool(np, prop))
-		format |= SND_SOC_DAIFMT_CONT;
-	else
-		format |= SND_SOC_DAIFMT_GATED;
-
-	/*
-	 * check "[prefix]bitclock-inversion"
-	 * check "[prefix]frame-inversion"
-	 * SND_SOC_DAIFMT_INV_MASK area
-	 */
-	snprintf(prop, sizeof(prop), "%sbitclock-inversion", prefix);
-	bit = !!of_get_property(np, prop, NULL);
-
-	snprintf(prop, sizeof(prop), "%sframe-inversion", prefix);
-	frame = !!of_get_property(np, prop, NULL);
-
-	switch ((bit << 4) + frame) {
-	case 0x11:
-		format |= SND_SOC_DAIFMT_IB_IF;
-		break;
-	case 0x10:
-		format |= SND_SOC_DAIFMT_IB_NF;
-		break;
-	case 0x01:
-		format |= SND_SOC_DAIFMT_NB_IF;
-		break;
-	default:
-		/* SND_SOC_DAIFMT_NB_NF is default */
-		break;
-	}
-
-	/*
-	 * check "[prefix]bitclock-master"
-	 * check "[prefix]frame-master"
-	 * SND_SOC_DAIFMT_MASTER_MASK area
-	 */
-	snprintf(prop, sizeof(prop), "%sbitclock-master", prefix);
-	bit = !!of_get_property(np, prop, NULL);
-	if (bit && bitclkmaster)
-		*bitclkmaster = of_parse_phandle(np, prop, 0);
-
-	snprintf(prop, sizeof(prop), "%sframe-master", prefix);
-	frame = !!of_get_property(np, prop, NULL);
-	if (frame && framemaster)
-		*framemaster = of_parse_phandle(np, prop, 0);
-
-	format |= snd_soc_daifmt_clock_provider_pickup((bit << 4) + frame);
-
-	return format;
-}
-EXPORT_SYMBOL_GPL(snd_soc_of_parse_daifmt);
-
 int snd_soc_get_dai_id(struct device_node *ep)
 {
 	struct snd_soc_component *component;
-- 
2.25.1

Re: [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@renesas.com> wrote:

> From: Kuninori Morimoto <kuninori.morimoto.gx@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@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 */

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

On Tue, Jun 08, 2021 at 09:50:52AM +0200, Jerome Brunet wrote:

> 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.

Yeah, I'm also just finding the _pickup() name unclear and can't really
immediately think how to make it clearer - I think the bit being
factored out needs to be at least as complex/unclear as the interface
being introduced to do so.

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

Re: (subset) [PATCH 0/9] ASoC: tidyup snd_soc_of_parse_daifmt()

[Mark Brown]

On 08 Jun 2021 09:11:17 +0900, Kuninori Morimoto wrote:
> I want to add new audio-graph-card2 sound card driver,
> and this is last part of necessary soc-core cleanup for it.
> 
> Current some drivers are using DT, and Then,
> snd_soc_of_parse_daifmt() parses daifmt, but bitclock/frame provider
> parsing part is one of headache, because we are assuming below both cases.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/9] ASoC: soc-core: don't use discriminatory terms on snd_soc_runtime_get_dai_fmt()
      commit: 640eac4c849d6390f272862ba8db14f28d423670

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

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

[Kuninori Morimoto]

Hi Jerome,  Mark

Thank you for your feedback

> > 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.
> 
> Yeah, I'm also just finding the _pickup() name unclear and can't really
> immediately think how to make it clearer - I think the bit being
> factored out needs to be at least as complex/unclear as the interface
> being introduced to do so.

Because there are many kind of use case,
it was difficult to do this thing by 1 function.
And indeed the naming was unclear.

Main purpose of this patch-set was that I want to have
clock_provider related functions.
But indeed it was a little overkill to force it to use it to
existing drivers.

I will fixup it in v2

Thank you for your help !!

Best regards
---
Kuninori Morimoto