Linux Sound subsystem development
 help / color / mirror / Atom feed
* [PATCH 0/6] ASoC: extra format on each DAI
@ 2024-12-17  1:48 Kuninori Morimoto
  2024-12-17  1:48 ` [PATCH 1/6] ASoC: audio-graph-card2: use __free(device_node) for device node Kuninori Morimoto
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2024-12-17  1:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-sound, Stephen Gordon


Hi Mark
Cc Stephen

Current clock provider/consumer setting is set by dai_link->dai_fmt, and it
is Codec base on Sound Card driver (= SND_SOC_DAIFMT_CBx_CFx).

Current CPU/Codec drivers are already based on its own base
(= SND_SOC_DAIFMT_Bx_Fx). So, Codec clock setting uses dai_link->dai_fmt
as-is, and CPU side clock setting is created from Codec base setting by
flipping. Because of this, we can't set both CPU/Codec clock consumer for
example.

To solve this issue, this patch-set adds new ext_fmt on each DAI.
It can keep compatible with legacy style.

	1. SND_SOC_DAIFMT_FORMAT_MASK
	2. SND_SOC_DAIFMT_CLOCK
	3. SND_SOC_DAIFMT_INV
	4. SND_SOC_DAIFMT_CLOCK_PROVIDER

	dai_fmt : dai_link->dai_fmt = common settings
	ext_fmt : each DAI settings
    
Legacy
	dai_fmt  includes 1, 2, 3, 4
    
New style
	dai_fmt  includes 1, 2, 3
	ext_fmt  includes 4

Audio-Graph-Card2 will use this new style by this patch-set.
By this patch, its default behavior (= no "clock-master / frame-master"
settings on DT) will be changed, but no drivers are having it.

In case of no DAI has "clock-master / frame-master" settings on DT

Legacy
	CPU  : provider (because flipped from Codec)
	Codec: consumer

New style
	CPU  : consumer
	Codec: consumer

One note is that Simple-Card, Audio-Graph-Card don't implement
this new style to keep compatiblily.

2nd note is that [1/6] - [4/6] are normal patch-set,
[5/6] - [6/6] are [RFC]

Kuninori Morimoto (6):
  ASoC: audio-graph-card2: use __free(device_node) for device node
  ASoC: audio-graph-card: use __free(device_node) for device node
  ASoC: simple-card: use __free(device_node) for device node
  ASoC: soc-core: return 0 if np was NULL on snd_soc_daifmt_parse_clock_provider_raw()
  ASoC: soc-core: Enable to use extra format on each DAI
  ASoC: audio-graph-card2: Use extra format on each DAI

 include/sound/soc.h                   |  11 ++
 sound/soc/generic/audio-graph-card.c  |  48 ++---
 sound/soc/generic/audio-graph-card2.c | 248 ++++++++++----------------
 sound/soc/generic/simple-card.c       |  58 ++----
 sound/soc/soc-core.c                  |  30 +++-
 5 files changed, 167 insertions(+), 228 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/6] ASoC: audio-graph-card2: use __free(device_node) for device node
  2024-12-17  1:48 [PATCH 0/6] ASoC: extra format on each DAI Kuninori Morimoto
@ 2024-12-17  1:48 ` Kuninori Morimoto
  2024-12-17  1:48 ` [PATCH 2/6] ASoC: audio-graph-card: " Kuninori Morimoto
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2024-12-17  1:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-sound, Stephen Gordon

audio-graph-card2 handles many type of device_node, thus need to
use of_node_put() in many place. Let's use __free(device_node)
and avoid it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card2.c | 173 ++++++++------------------
 1 file changed, 54 insertions(+), 119 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c
index 5280c1b20d85e..079ae8a7bfff8 100644
--- a/sound/soc/generic/audio-graph-card2.c
+++ b/sound/soc/generic/audio-graph-card2.c
@@ -333,8 +333,7 @@ static int graph_lnk_is_multi(struct device_node *lnk)
 
 static struct device_node *graph_get_next_multi_ep(struct device_node **port)
 {
-	struct device_node *ports = port_to_ports(*port);
-	struct device_node *ep = NULL;
+	struct device_node *ports __free(device_node) = port_to_ports(*port);
 	struct device_node *rep = NULL;
 
 	/*
@@ -354,13 +353,11 @@ static struct device_node *graph_get_next_multi_ep(struct device_node **port)
 	 */
 	*port = of_graph_get_next_port(ports, *port);
 	if (*port) {
-		ep  = of_graph_get_next_port_endpoint(*port, NULL);
+		struct device_node *ep __free(device_node) = of_graph_get_next_port_endpoint(*port, NULL);
+
 		rep = of_graph_get_remote_endpoint(ep);
 	}
 
-	of_node_put(ep);
-	of_node_put(ports);
-
 	return rep;
 }
 
@@ -373,16 +370,13 @@ static const struct snd_soc_ops graph_ops = {
 static void graph_parse_convert(struct device_node *ep,
 				struct simple_dai_props *props)
 {
-	struct device_node *port = ep_to_port(ep);
-	struct device_node *ports = port_to_ports(port);
+	struct device_node *port  __free(device_node) = ep_to_port(ep);
+	struct device_node *ports __free(device_node) = port_to_ports(port);
 	struct simple_util_data *adata = &props->adata;
 
 	simple_util_parse_convert(ports, NULL, adata);
 	simple_util_parse_convert(port, NULL, adata);
 	simple_util_parse_convert(ep,   NULL, adata);
-
-	of_node_put(port);
-	of_node_put(ports);
 }
 
 static int __graph_parse_node(struct simple_util_priv *priv,
@@ -471,14 +465,11 @@ static int __graph_parse_node(struct simple_util_priv *priv,
 	if (!is_cpu && gtype == GRAPH_DPCM) {
 		struct snd_soc_dai_link_component *codecs = snd_soc_link_to_codec(dai_link, idx);
 		struct snd_soc_codec_conf *cconf = simple_props_to_codec_conf(dai_props, idx);
-		struct device_node *rport  = ep_to_port(ep);
-		struct device_node *rports = port_to_ports(rport);
+		struct device_node *rport  __free(device_node) = ep_to_port(ep);
+		struct device_node *rports __free(device_node) = port_to_ports(rport);
 
 		snd_soc_of_parse_node_prefix(rports, cconf, codecs->of_node, "prefix");
 		snd_soc_of_parse_node_prefix(rport,  cconf, codecs->of_node, "prefix");
-
-		of_node_put(rport);
-		of_node_put(rports);
 	}
 
 	if (is_cpu) {
@@ -526,25 +517,21 @@ static int graph_parse_node_multi_nm(struct snd_soc_dai_link *dai_link,
 	 *	};
 	 * };
 	 */
-	struct device_node *mcpu_ep		= of_graph_get_next_port_endpoint(mcpu_port, NULL);
-	struct device_node *mcpu_ports		= port_to_ports(mcpu_port);
-	struct device_node *mcpu_port_top	= of_graph_get_next_port(mcpu_ports, NULL);
-	struct device_node *mcpu_ep_top		= of_graph_get_next_port_endpoint(mcpu_port_top, NULL);
-	struct device_node *mcodec_ep_top	= of_graph_get_remote_endpoint(mcpu_ep_top);
-	struct device_node *mcodec_port_top	= ep_to_port(mcodec_ep_top);
-	struct device_node *mcodec_ports	= port_to_ports(mcodec_port_top);
+	struct device_node *mcpu_ep		__free(device_node) = of_graph_get_next_port_endpoint(mcpu_port, NULL);
+	struct device_node *mcpu_ports		__free(device_node) = port_to_ports(mcpu_port);
+	struct device_node *mcpu_port_top	__free(device_node) = of_graph_get_next_port(mcpu_ports, NULL);
+	struct device_node *mcpu_ep_top		__free(device_node) = of_graph_get_next_port_endpoint(mcpu_port_top, NULL);
+	struct device_node *mcodec_ep_top	__free(device_node) = of_graph_get_remote_endpoint(mcpu_ep_top);
+	struct device_node *mcodec_port_top	__free(device_node) = ep_to_port(mcodec_ep_top);
+	struct device_node *mcodec_ports	__free(device_node) = port_to_ports(mcodec_port_top);
 	int nm_max = max(dai_link->num_cpus, dai_link->num_codecs);
 	int ret = 0;
 
-	if (cpu_idx > dai_link->num_cpus) {
-		ret = -EINVAL;
-		goto mcpu_err;
-	}
+	if (cpu_idx > dai_link->num_cpus)
+		return -EINVAL;
 
 	for_each_of_graph_port_endpoint(mcpu_port, mcpu_ep_n) {
-		struct device_node *mcodec_ep_n;
-		struct device_node *mcodec_port;
-		int codec_idx;
+		int codec_idx = 0;
 
 		/* ignore 1st ep which is for element */
 		if (mcpu_ep_n == mcpu_ep)
@@ -553,16 +540,13 @@ static int graph_parse_node_multi_nm(struct snd_soc_dai_link *dai_link,
 		if (*nm_idx > nm_max)
 			break;
 
-		mcodec_ep_n	= of_graph_get_remote_endpoint(mcpu_ep_n);
-		mcodec_port	= ep_to_port(mcodec_ep_n);
-
-		if (mcodec_ports != port_to_ports(mcodec_port)) {
-			ret = -EINVAL;
-			goto mcpu_err;
-		}
+		struct device_node *mcodec_ep_n __free(device_node) = of_graph_get_remote_endpoint(mcpu_ep_n);
+		struct device_node *mcodec_port __free(device_node) = ep_to_port(mcodec_ep_n);
 
-		codec_idx = 0;
 		ret = -EINVAL;
+		if (mcodec_ports != port_to_ports(mcodec_port))
+			break;
+
 		for_each_of_graph_port(mcodec_ports, mcodec_port_i) {
 
 			/* ignore 1st port which is for pair connection */
@@ -582,18 +566,9 @@ static int graph_parse_node_multi_nm(struct snd_soc_dai_link *dai_link,
 			}
 			codec_idx++;
 		}
-		of_node_put(mcodec_port);
-		of_node_put(mcodec_ep_n);
 		if (ret < 0)
 			break;
 	}
-mcpu_err:
-	of_node_put(mcpu_ep);
-	of_node_put(mcpu_port_top);
-	of_node_put(mcpu_ep_top);
-	of_node_put(mcodec_ep_top);
-	of_node_put(mcodec_port_top);
-	of_node_put(mcodec_ports);
 
 	return ret;
 }
@@ -605,7 +580,6 @@ static int graph_parse_node_multi(struct simple_util_priv *priv,
 {
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
 	struct device *dev = simple_priv_to_dev(priv);
-	struct device_node *ep;
 	int ret = -ENOMEM;
 	int nm_idx = 0;
 	int nm_max = max(dai_link->num_cpus, dai_link->num_codecs);
@@ -640,12 +614,11 @@ static int graph_parse_node_multi(struct simple_util_priv *priv,
 		 *	};
 		 * };
 		 */
-		ep = graph_get_next_multi_ep(&port);
+		struct device_node *ep __free(device_node) = graph_get_next_multi_ep(&port);
 		if (!ep)
 			break;
 
 		ret = __graph_parse_node(priv, gtype, ep, li, is_cpu, idx);
-		of_node_put(ep);
 		if (ret < 0)
 			goto multi_err;
 
@@ -669,12 +642,9 @@ static int graph_parse_node_single(struct simple_util_priv *priv,
 				   struct device_node *port,
 				   struct link_info *li, int is_cpu)
 {
-	struct device_node *ep = of_graph_get_next_port_endpoint(port, NULL);
-	int ret = __graph_parse_node(priv, gtype, ep, li, is_cpu, 0);
-
-	of_node_put(ep);
+	struct device_node *ep __free(device_node) = of_graph_get_next_port_endpoint(port, NULL);
 
-	return ret;
+	return __graph_parse_node(priv, gtype, ep, li, is_cpu, 0);
 }
 
 static int graph_parse_node(struct simple_util_priv *priv,
@@ -751,7 +721,6 @@ static void graph_link_init(struct simple_util_priv *priv,
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
 	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
 	struct device_node *ep_cpu, *ep_codec;
-	struct device_node *ports_cpu, *ports_codec;
 	unsigned int daifmt = 0, daiclk = 0;
 	bool playback_only = 0, capture_only = 0;
 	enum snd_soc_trigger_order trigger_start = SND_SOC_TRIGGER_ORDER_DEFAULT;
@@ -766,7 +735,7 @@ static void graph_link_init(struct simple_util_priv *priv,
 	} else {
 		ep_cpu = of_graph_get_next_port_endpoint(port_cpu, NULL);
 	}
-	ports_cpu = port_to_ports(port_cpu);
+	struct device_node *ports_cpu __free(device_node) = port_to_ports(port_cpu);
 
 	of_node_get(port_codec);
 	if (graph_lnk_is_multi(port_codec)) {
@@ -776,8 +745,7 @@ static void graph_link_init(struct simple_util_priv *priv,
 	} else {
 		ep_codec = of_graph_get_next_port_endpoint(port_codec, NULL);
 	}
-	ports_codec = port_to_ports(port_codec);
-
+	struct device_node *ports_codec __free(device_node) = port_to_ports(port_codec);
 
 	graph_parse_daifmt(ep_cpu,	&daifmt, &bit_frame);
 	graph_parse_daifmt(ep_codec,	&daifmt, &bit_frame);
@@ -832,8 +800,6 @@ static void graph_link_init(struct simple_util_priv *priv,
 	if (priv->ops)
 		dai_link->ops	= priv->ops;
 
-	of_node_put(ports_cpu);
-	of_node_put(ports_codec);
 	of_node_put(port_cpu);
 	of_node_put(port_codec);
 	of_node_put(ep_cpu);
@@ -845,8 +811,8 @@ int audio_graph2_link_normal(struct simple_util_priv *priv,
 			     struct link_info *li)
 {
 	struct device_node *cpu_port = lnk;
-	struct device_node *cpu_ep = of_graph_get_next_port_endpoint(cpu_port, NULL);
-	struct device_node *codec_port = of_graph_get_remote_port(cpu_ep);
+	struct device_node *cpu_ep	__free(device_node) = of_graph_get_next_port_endpoint(cpu_port, NULL);
+	struct device_node *codec_port	__free(device_node) = of_graph_get_remote_port(cpu_ep);
 	int ret;
 
 	/*
@@ -856,19 +822,16 @@ int audio_graph2_link_normal(struct simple_util_priv *priv,
 	 */
 	ret = graph_parse_node(priv, GRAPH_NORMAL, codec_port, li, 0);
 	if (ret < 0)
-		goto err;
+		return ret;
 
 	/*
 	 * call CPU, and set DAI Name
 	 */
 	ret = graph_parse_node(priv, GRAPH_NORMAL, cpu_port, li, 1);
 	if (ret < 0)
-		goto err;
+		return ret;
 
 	graph_link_init(priv, lnk, cpu_port, codec_port, li, 1);
-err:
-	of_node_put(codec_port);
-	of_node_put(cpu_ep);
 
 	return ret;
 }
@@ -878,8 +841,8 @@ int audio_graph2_link_dpcm(struct simple_util_priv *priv,
 			   struct device_node *lnk,
 			   struct link_info *li)
 {
-	struct device_node *ep = of_graph_get_next_port_endpoint(lnk, NULL);
-	struct device_node *rep = of_graph_get_remote_endpoint(ep);
+	struct device_node *ep	__free(device_node) = of_graph_get_next_port_endpoint(lnk, NULL);
+	struct device_node *rep	__free(device_node) = of_graph_get_remote_endpoint(ep);
 	struct device_node *cpu_port = NULL;
 	struct device_node *codec_port = NULL;
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
@@ -963,8 +926,6 @@ int audio_graph2_link_dpcm(struct simple_util_priv *priv,
 
 	graph_link_init(priv, lnk, cpu_port, codec_port, li, is_cpu);
 err:
-	of_node_put(ep);
-	of_node_put(rep);
 	of_node_put(cpu_port);
 	of_node_put(codec_port);
 
@@ -977,9 +938,9 @@ int audio_graph2_link_c2c(struct simple_util_priv *priv,
 			  struct link_info *li)
 {
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
-	struct device_node *port0, *port1, *ports;
-	struct device_node *codec0_port, *codec1_port;
-	struct device_node *ep0, *ep1;
+	struct device_node *port0 = lnk;
+	struct device_node *ports __free(device_node) = port_to_ports(port0);
+	struct device_node *port1 __free(device_node) = of_graph_get_next_port(ports, port0);
 	u32 val = 0;
 	int ret = -EINVAL;
 
@@ -999,10 +960,6 @@ int audio_graph2_link_c2c(struct simple_util_priv *priv,
 	 *	};
 	 * };
 	 */
-	of_node_get(lnk);
-	port0 = lnk;
-	ports = port_to_ports(port0);
-	port1 = of_graph_get_next_port(ports, port0);
 
 	/*
 	 * Card2 can use original Codec2Codec settings if DT has.
@@ -1019,7 +976,7 @@ int audio_graph2_link_c2c(struct simple_util_priv *priv,
 
 		c2c_conf = devm_kzalloc(dev, sizeof(*c2c_conf), GFP_KERNEL);
 		if (!c2c_conf)
-			goto err1;
+			return ret;
 
 		c2c_conf->formats	= SNDRV_PCM_FMTBIT_S32_LE; /* update ME */
 		c2c_conf->rates		= SNDRV_PCM_RATE_8000_384000;
@@ -1032,11 +989,11 @@ int audio_graph2_link_c2c(struct simple_util_priv *priv,
 		dai_link->num_c2c_params	= 1;
 	}
 
-	ep0 = of_graph_get_next_port_endpoint(port0, NULL);
-	ep1 = of_graph_get_next_port_endpoint(port1, NULL);
+	struct device_node *ep0 __free(device_node) = of_graph_get_next_port_endpoint(port0, NULL);
+	struct device_node *ep1 __free(device_node) = of_graph_get_next_port_endpoint(port1, NULL);
 
-	codec0_port = of_graph_get_remote_port(ep0);
-	codec1_port = of_graph_get_remote_port(ep1);
+	struct device_node *codec0_port __free(device_node) = of_graph_get_remote_port(ep0);
+	struct device_node *codec1_port __free(device_node) = of_graph_get_remote_port(ep1);
 
 	/*
 	 * call Codec first.
@@ -1045,25 +1002,16 @@ int audio_graph2_link_c2c(struct simple_util_priv *priv,
 	 */
 	ret = graph_parse_node(priv, GRAPH_C2C, codec1_port, li, 0);
 	if (ret < 0)
-		goto err2;
+		return ret;
 
 	/*
 	 * call CPU, and set DAI Name
 	 */
 	ret = graph_parse_node(priv, GRAPH_C2C, codec0_port, li, 1);
 	if (ret < 0)
-		goto err2;
+		return ret;
 
 	graph_link_init(priv, lnk, codec0_port, codec1_port, li, 1);
-err2:
-	of_node_put(ep0);
-	of_node_put(ep1);
-	of_node_put(codec0_port);
-	of_node_put(codec1_port);
-err1:
-	of_node_put(ports);
-	of_node_put(port0);
-	of_node_put(port1);
 
 	return ret;
 }
@@ -1153,8 +1101,8 @@ static int graph_count_normal(struct simple_util_priv *priv,
 			      struct link_info *li)
 {
 	struct device_node *cpu_port = lnk;
-	struct device_node *cpu_ep = of_graph_get_next_port_endpoint(cpu_port, NULL);
-	struct device_node *codec_port = of_graph_get_remote_port(cpu_ep);
+	struct device_node *cpu_ep	__free(device_node) = of_graph_get_next_port_endpoint(cpu_port, NULL);
+	struct device_node *codec_port	__free(device_node) = of_graph_get_remote_port(cpu_ep);
 
 	/*
 	 *	CPU {
@@ -1171,9 +1119,6 @@ static int graph_count_normal(struct simple_util_priv *priv,
 
 	li->num[li->link].codecs	= graph_counter(codec_port);
 
-	of_node_put(cpu_ep);
-	of_node_put(codec_port);
-
 	return 0;
 }
 
@@ -1181,8 +1126,8 @@ static int graph_count_dpcm(struct simple_util_priv *priv,
 			    struct device_node *lnk,
 			    struct link_info *li)
 {
-	struct device_node *ep = of_graph_get_next_port_endpoint(lnk, NULL);
-	struct device_node *rport = of_graph_get_remote_port(ep);
+	struct device_node *ep		__free(device_node) = of_graph_get_next_port_endpoint(lnk, NULL);
+	struct device_node *rport	__free(device_node) = of_graph_get_remote_port(ep);
 
 	/*
 	 * dpcm {
@@ -1211,9 +1156,6 @@ static int graph_count_dpcm(struct simple_util_priv *priv,
 		li->num[li->link].codecs	= graph_counter(rport); /* BE */
 	}
 
-	of_node_put(ep);
-	of_node_put(rport);
-
 	return 0;
 }
 
@@ -1221,13 +1163,13 @@ static int graph_count_c2c(struct simple_util_priv *priv,
 			   struct device_node *lnk,
 			   struct link_info *li)
 {
-	struct device_node *ports = port_to_ports(lnk);
-	struct device_node *port0 = lnk;
-	struct device_node *port1 = of_graph_get_next_port(ports, of_node_get(port0));
-	struct device_node *ep0 = of_graph_get_next_port_endpoint(port0, NULL);
-	struct device_node *ep1 = of_graph_get_next_port_endpoint(port1, NULL);
-	struct device_node *codec0 = of_graph_get_remote_port(ep0);
-	struct device_node *codec1 = of_graph_get_remote_port(ep1);
+	struct device_node *ports	__free(device_node) = port_to_ports(lnk);
+	struct device_node *port0	= of_node_get(lnk);
+	struct device_node *port1	= of_node_get(of_graph_get_next_port(ports, of_node_get(port0)));
+	struct device_node *ep0		__free(device_node) = of_graph_get_next_port_endpoint(port0, NULL);
+	struct device_node *ep1		__free(device_node) = of_graph_get_next_port_endpoint(port1, NULL);
+	struct device_node *codec0	__free(device_node) = of_graph_get_remote_port(ep0);
+	struct device_node *codec1	__free(device_node) = of_graph_get_remote_port(ep1);
 
 	/*
 	 * codec2codec {
@@ -1247,13 +1189,6 @@ static int graph_count_c2c(struct simple_util_priv *priv,
 
 	li->num[li->link].codecs	= graph_counter(codec1);
 
-	of_node_put(ports);
-	of_node_put(port1);
-	of_node_put(ep0);
-	of_node_put(ep1);
-	of_node_put(codec0);
-	of_node_put(codec1);
-
 	return 0;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/6] ASoC: audio-graph-card: use __free(device_node) for device node
  2024-12-17  1:48 [PATCH 0/6] ASoC: extra format on each DAI Kuninori Morimoto
  2024-12-17  1:48 ` [PATCH 1/6] ASoC: audio-graph-card2: use __free(device_node) for device node Kuninori Morimoto
@ 2024-12-17  1:48 ` Kuninori Morimoto
  2024-12-17  1:48 ` [PATCH 3/6] ASoC: simple-card: " Kuninori Morimoto
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2024-12-17  1:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-sound, Stephen Gordon

audio-graph-card handles many type of device_node, thus need to
use of_node_put() in many place. Let's use __free(device_node)
and avoid it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card.c | 48 ++++++++--------------------
 1 file changed, 14 insertions(+), 34 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 7655425a3deb1..7c422535b01a2 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -81,18 +81,14 @@ static void graph_parse_convert(struct device *dev,
 				struct simple_util_data *adata)
 {
 	struct device_node *top = dev->of_node;
-	struct device_node *port = ep_to_port(ep);
-	struct device_node *ports = port_to_ports(port);
-	struct device_node *node = of_graph_get_port_parent(ep);
+	struct device_node *port  __free(device_node) = ep_to_port(ep);
+	struct device_node *ports __free(device_node) = port_to_ports(port);
+	struct device_node *node  __free(device_node) = of_graph_get_port_parent(ep);
 
 	simple_util_parse_convert(top,   NULL,   adata);
 	simple_util_parse_convert(ports, NULL,   adata);
 	simple_util_parse_convert(port,  NULL,   adata);
 	simple_util_parse_convert(ep,    NULL,   adata);
-
-	of_node_put(port);
-	of_node_put(ports);
-	of_node_put(node);
 }
 
 static int graph_parse_node(struct simple_util_priv *priv,
@@ -140,10 +136,10 @@ static int graph_link_init(struct simple_util_priv *priv,
 	struct device_node *top = dev->of_node;
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
 	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
-	struct device_node *port_cpu = ep_to_port(ep_cpu);
-	struct device_node *port_codec = ep_to_port(ep_codec);
-	struct device_node *ports_cpu = port_to_ports(port_cpu);
-	struct device_node *ports_codec = port_to_ports(port_codec);
+	struct device_node *port_cpu    __free(device_node) = ep_to_port(ep_cpu);
+	struct device_node *port_codec  __free(device_node) = ep_to_port(ep_codec);
+	struct device_node *ports_cpu   __free(device_node) = port_to_ports(port_cpu);
+	struct device_node *ports_codec __free(device_node) = port_to_ports(port_codec);
 	enum snd_soc_trigger_order trigger_start = SND_SOC_TRIGGER_ORDER_DEFAULT;
 	enum snd_soc_trigger_order trigger_stop  = SND_SOC_TRIGGER_ORDER_DEFAULT;
 	bool playback_only = 0, capture_only = 0;
@@ -152,7 +148,7 @@ static int graph_link_init(struct simple_util_priv *priv,
 	ret = simple_util_parse_daifmt(dev, ep_cpu, ep_codec,
 				       NULL, &dai_link->dai_fmt);
 	if (ret < 0)
-		goto init_end;
+		return ret;
 
 	graph_util_parse_link_direction(top,		&playback_only, &capture_only);
 	graph_util_parse_link_direction(port_cpu,	&playback_only, &capture_only);
@@ -187,14 +183,7 @@ static int graph_link_init(struct simple_util_priv *priv,
 	if (priv->ops)
 		dai_link->ops	= priv->ops;
 
-	ret = simple_util_set_dailink_name(dev, dai_link, name);
-init_end:
-	of_node_put(ports_cpu);
-	of_node_put(ports_codec);
-	of_node_put(port_cpu);
-	of_node_put(port_codec);
-
-	return ret;
+	return simple_util_set_dailink_name(dev, dai_link, name);
 }
 
 static int graph_dai_link_of_dpcm(struct simple_util_priv *priv,
@@ -250,8 +239,6 @@ static int graph_dai_link_of_dpcm(struct simple_util_priv *priv,
 	} else {
 		struct snd_soc_codec_conf *cconf = simple_props_to_codec_conf(dai_props, 0);
 		struct snd_soc_dai_link_component *codecs = snd_soc_link_to_codec(dai_link, 0);
-		struct device_node *port;
-		struct device_node *ports;
 
 		/* CPU is dummy */
 
@@ -267,14 +254,12 @@ static int graph_dai_link_of_dpcm(struct simple_util_priv *priv,
 			 "be.%pOFP.%s", codecs->of_node, codecs->dai_name);
 
 		/* check "prefix" from top node */
-		port  = ep_to_port(ep);
-		ports = port_to_ports(port);
+		struct device_node *port  __free(device_node) = ep_to_port(ep);
+		struct device_node *ports __free(device_node) = port_to_ports(port);
+
 		snd_soc_of_parse_node_prefix(top,   cconf, codecs->of_node, "prefix");
 		snd_soc_of_parse_node_prefix(ports, cconf, codecs->of_node, "prefix");
 		snd_soc_of_parse_node_prefix(port,  cconf, codecs->of_node, "prefix");
-
-		of_node_put(ports);
-		of_node_put(port);
 	}
 
 	graph_parse_convert(dev, ep, &dai_props->adata);
@@ -361,8 +346,6 @@ static int __graph_for_each_link(struct simple_util_priv *priv,
 	struct device *dev = simple_priv_to_dev(priv);
 	struct device_node *node = dev->of_node;
 	struct device_node *cpu_port;
-	struct device_node *codec_ep;
-	struct device_node *codec_port;
 	struct device_node *codec_port_old = NULL;
 	struct simple_util_data adata;
 	int rc, ret = 0;
@@ -374,8 +357,8 @@ static int __graph_for_each_link(struct simple_util_priv *priv,
 		/* loop for all CPU endpoint */
 		for_each_of_graph_port_endpoint(cpu_port, cpu_ep) {
 			/* get codec */
-			codec_ep = of_graph_get_remote_endpoint(cpu_ep);
-			codec_port = ep_to_port(codec_ep);
+			struct device_node *codec_ep   __free(device_node) = of_graph_get_remote_endpoint(cpu_ep);
+			struct device_node *codec_port __free(device_node) = ep_to_port(codec_ep);
 
 			/* get convert-xxx property */
 			memset(&adata, 0, sizeof(adata));
@@ -399,9 +382,6 @@ static int __graph_for_each_link(struct simple_util_priv *priv,
 					ret = func_noml(priv, cpu_ep, codec_ep, li);
 			}
 
-			of_node_put(codec_ep);
-			of_node_put(codec_port);
-
 			if (ret < 0)
 				return ret;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/6] ASoC: simple-card: use __free(device_node) for device node
  2024-12-17  1:48 [PATCH 0/6] ASoC: extra format on each DAI Kuninori Morimoto
  2024-12-17  1:48 ` [PATCH 1/6] ASoC: audio-graph-card2: use __free(device_node) for device node Kuninori Morimoto
  2024-12-17  1:48 ` [PATCH 2/6] ASoC: audio-graph-card: " Kuninori Morimoto
@ 2024-12-17  1:48 ` Kuninori Morimoto
  2024-12-17  1:48 ` [PATCH 4/6] ASoC: soc-core: return 0 if np was NULL on snd_soc_daifmt_parse_clock_provider_raw() Kuninori Morimoto
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2024-12-17  1:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-sound, Stephen Gordon

simple-card handles many type of device_node, thus need to
use of_node_put() in many place. Let's use __free(device_node)
and avoid it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/simple-card.c | 58 ++++++++++-----------------------
 1 file changed, 17 insertions(+), 41 deletions(-)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 76a1d05e2ebec..afe7e79ffdbdb 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -120,14 +120,12 @@ static void simple_parse_convert(struct device *dev,
 				 struct simple_util_data *adata)
 {
 	struct device_node *top = dev->of_node;
-	struct device_node *node = of_get_parent(np);
+	struct device_node *node __free(device_node) = of_get_parent(np);
 
 	simple_util_parse_convert(top,  PREFIX, adata);
 	simple_util_parse_convert(node, PREFIX, adata);
 	simple_util_parse_convert(node, NULL,   adata);
 	simple_util_parse_convert(np,   NULL,   adata);
-
-	of_node_put(node);
 }
 
 static int simple_parse_node(struct simple_util_priv *priv,
@@ -176,7 +174,7 @@ static int simple_link_init(struct simple_util_priv *priv,
 	struct device_node *top = dev->of_node;
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
 	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
-	struct device_node *node = of_get_parent(cpu);
+	struct device_node *node __free(device_node) = of_get_parent(cpu);
 	enum snd_soc_trigger_order trigger_start = SND_SOC_TRIGGER_ORDER_DEFAULT;
 	enum snd_soc_trigger_order trigger_stop  = SND_SOC_TRIGGER_ORDER_DEFAULT;
 	bool playback_only = 0, capture_only = 0;
@@ -185,7 +183,7 @@ static int simple_link_init(struct simple_util_priv *priv,
 	ret = simple_util_parse_daifmt(dev, node, codec,
 				       prefix, &dai_link->dai_fmt);
 	if (ret < 0)
-		goto init_end;
+		return ret;
 
 	graph_util_parse_link_direction(top,	&playback_only, &capture_only);
 	graph_util_parse_link_direction(node,	&playback_only, &capture_only);
@@ -215,11 +213,7 @@ static int simple_link_init(struct simple_util_priv *priv,
 	dai_link->init			= simple_util_dai_init;
 	dai_link->ops			= &simple_ops;
 
-	ret = simple_util_set_dailink_name(dev, dai_link, name);
-init_end:
-	of_node_put(node);
-
-	return ret;
+	return simple_util_set_dailink_name(dev, dai_link, name);
 }
 
 static int simple_dai_link_of_dpcm(struct simple_util_priv *priv,
@@ -232,7 +226,7 @@ static int simple_dai_link_of_dpcm(struct simple_util_priv *priv,
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
 	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
 	struct device_node *top = dev->of_node;
-	struct device_node *node = of_get_parent(np);
+	struct device_node *node __free(device_node) = of_get_parent(np);
 	char *prefix = "";
 	char dai_name[64];
 	int ret;
@@ -296,7 +290,6 @@ static int simple_dai_link_of_dpcm(struct simple_util_priv *priv,
 out_put_node:
 	li->link++;
 
-	of_node_put(node);
 	return ret;
 }
 
@@ -312,15 +305,13 @@ static int simple_dai_link_of(struct simple_util_priv *priv,
 	struct snd_soc_dai_link_component *codecs = snd_soc_link_to_codec(dai_link, 0);
 	struct snd_soc_dai_link_component *platforms = snd_soc_link_to_platform(dai_link, 0);
 	struct device_node *cpu = NULL;
-	struct device_node *node = NULL;
-	struct device_node *plat = NULL;
 	char dai_name[64];
 	char prop[128];
 	char *prefix = "";
 	int ret, single_cpu = 0;
 
 	cpu  = np;
-	node = of_get_parent(np);
+	struct device_node *node __free(device_node) = of_get_parent(np);
 
 	dev_dbg(dev, "link_of (%pOF)\n", node);
 
@@ -329,7 +320,7 @@ static int simple_dai_link_of(struct simple_util_priv *priv,
 		prefix = PREFIX;
 
 	snprintf(prop, sizeof(prop), "%splat", prefix);
-	plat = of_get_child_by_name(node, prop);
+	struct device_node *plat __free(device_node)  = of_get_child_by_name(node, prop);
 
 	ret = simple_parse_node(priv, cpu, li, prefix, &single_cpu);
 	if (ret < 0)
@@ -352,9 +343,6 @@ static int simple_dai_link_of(struct simple_util_priv *priv,
 	ret = simple_link_init(priv, cpu, codec, li, prefix, dai_name);
 
 dai_link_of_err:
-	of_node_put(plat);
-	of_node_put(node);
-
 	li->link++;
 
 	return ret;
@@ -374,7 +362,6 @@ static int __simple_for_each_link(struct simple_util_priv *priv,
 	struct device *dev = simple_priv_to_dev(priv);
 	struct device_node *top = dev->of_node;
 	struct device_node *node;
-	struct device_node *add_devs;
 	uintptr_t dpcm_selectable = (uintptr_t)of_device_get_match_data(dev);
 	bool is_top = 0;
 	int ret = 0;
@@ -386,14 +373,11 @@ static int __simple_for_each_link(struct simple_util_priv *priv,
 		is_top = 1;
 	}
 
-	add_devs = of_get_child_by_name(top, PREFIX "additional-devs");
+	struct device_node *add_devs __free(device_node) = of_get_child_by_name(top, PREFIX "additional-devs");
 
 	/* loop for all dai-link */
 	do {
 		struct simple_util_data adata;
-		struct device_node *codec;
-		struct device_node *plat;
-		struct device_node *np;
 		int num = of_get_child_count(node);
 
 		/* Skip additional-devs node */
@@ -403,26 +387,26 @@ static int __simple_for_each_link(struct simple_util_priv *priv,
 		}
 
 		/* get codec */
-		codec = of_get_child_by_name(node, is_top ?
-					     PREFIX "codec" : "codec");
+		struct device_node *codec __free(device_node) =
+			of_get_child_by_name(node, is_top ? PREFIX "codec" : "codec");
 		if (!codec) {
 			ret = -ENODEV;
 			goto error;
 		}
 		/* get platform */
-		plat = of_get_child_by_name(node, is_top ?
-					    PREFIX "plat" : "plat");
+		struct device_node *plat __free(device_node) =
+			of_get_child_by_name(node, is_top ? PREFIX "plat" : "plat");
 
 		/* get convert-xxx property */
 		memset(&adata, 0, sizeof(adata));
-		for_each_child_of_node(node, np) {
+		for_each_child_of_node_scoped(node, np) {
 			if (np == add_devs)
 				continue;
 			simple_parse_convert(dev, np, &adata);
 		}
 
 		/* loop for all CPU/Codec node */
-		for_each_child_of_node(node, np) {
+		for_each_child_of_node_scoped(node, np) {
 			if (plat == np || add_devs == np)
 				continue;
 			/*
@@ -452,22 +436,16 @@ static int __simple_for_each_link(struct simple_util_priv *priv,
 					ret = func_noml(priv, np, codec, li, is_top);
 			}
 
-			if (ret < 0) {
-				of_node_put(codec);
-				of_node_put(plat);
-				of_node_put(np);
+			if (ret < 0)
 				goto error;
-			}
 		}
 
-		of_node_put(codec);
-		of_node_put(plat);
 		node = of_get_next_child(top, node);
 	} while (!is_top && node);
 
  error:
-	of_node_put(add_devs);
 	of_node_put(node);
+
 	return ret;
 }
 
@@ -514,15 +492,13 @@ static void simple_depopulate_aux(void *data)
 static int simple_populate_aux(struct simple_util_priv *priv)
 {
 	struct device *dev = simple_priv_to_dev(priv);
-	struct device_node *node;
+	struct device_node *node __free(device_node) = of_get_child_by_name(dev->of_node, PREFIX "additional-devs");
 	int ret;
 
-	node = of_get_child_by_name(dev->of_node, PREFIX "additional-devs");
 	if (!node)
 		return 0;
 
 	ret = of_platform_populate(node, NULL, NULL, dev);
-	of_node_put(node);
 	if (ret)
 		return ret;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/6] ASoC: soc-core: return 0 if np was NULL on snd_soc_daifmt_parse_clock_provider_raw()
  2024-12-17  1:48 [PATCH 0/6] ASoC: extra format on each DAI Kuninori Morimoto
                   ` (2 preceding siblings ...)
  2024-12-17  1:48 ` [PATCH 3/6] ASoC: simple-card: " Kuninori Morimoto
@ 2024-12-17  1:48 ` Kuninori Morimoto
  2024-12-17  1:49 ` [PATCH 6/6] ASoC: audio-graph-card2: Use extra format on each DAI Kuninori Morimoto
  2024-12-17  1:50 ` [PATCH 5/6] ASoC: soc-core: Enable to use " Kuninori Morimoto
  5 siblings, 0 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2024-12-17  1:48 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-sound, Stephen Gordon

snd_soc_daifmt_parse_clock_provider_raw() might be called with NULL np.
Return 0 in such case.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c8b7f78b02f0a..426f8d8baa38b 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3391,6 +3391,9 @@ unsigned int snd_soc_daifmt_parse_clock_provider_raw(struct device_node *np,
 	char prop[128];
 	unsigned int bit, frame;
 
+	if (!np)
+		return 0;
+
 	if (!prefix)
 		prefix = "";
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/6] ASoC: audio-graph-card2: Use extra format on each DAI
  2024-12-17  1:48 [PATCH 0/6] ASoC: extra format on each DAI Kuninori Morimoto
                   ` (3 preceding siblings ...)
  2024-12-17  1:48 ` [PATCH 4/6] ASoC: soc-core: return 0 if np was NULL on snd_soc_daifmt_parse_clock_provider_raw() Kuninori Morimoto
@ 2024-12-17  1:49 ` Kuninori Morimoto
  2024-12-17 13:06   ` Stephen Gordon
  2024-12-17  1:50 ` [PATCH 5/6] ASoC: soc-core: Enable to use " Kuninori Morimoto
  5 siblings, 1 reply; 11+ messages in thread
From: Kuninori Morimoto @ 2024-12-17  1:49 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-sound, Stephen Gordon

Current ASoC is using dai_link->dai_fmt to set DAI format for both
CPU/Codec. But because it is using same settings, and
SND_SOC_DAIFMT_CLOCK_PROVIDER is flipped for CPU, we can't set both
CPU/Codec as clock consumer, for example.

To solve this issue, this patch uses extra format for each DAI which can
keep compatibility with legacy system,

	1. SND_SOC_DAIFMT_FORMAT_MASK
	2. SND_SOC_DAIFMT_CLOCK
	3. SND_SOC_DAIFMT_INV
	4. SND_SOC_DAIFMT_CLOCK_PROVIDER

Legacy
	dai_fmt  includes 1, 2, 3, 4

New idea
	dai_fmt  includes 1, 2, 3
	ext_fmt  includes 4

Link: https://lore.kernel.org/r/5011ceef-5100-441d-b169-dabba135d27f@iinet.net.au
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/generic/audio-graph-card2.c | 75 +++++++++++++++------------
 1 file changed, 43 insertions(+), 32 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c
index 079ae8a7bfff8..8c7c449dc6498 100644
--- a/sound/soc/generic/audio-graph-card2.c
+++ b/sound/soc/generic/audio-graph-card2.c
@@ -658,8 +658,7 @@ static int graph_parse_node(struct simple_util_priv *priv,
 		return graph_parse_node_single(priv, gtype, port, li, is_cpu);
 }
 
-static void graph_parse_daifmt(struct device_node *node,
-			       unsigned int *daifmt, unsigned int *bit_frame)
+static void graph_parse_daifmt(struct device_node *node, unsigned int *daifmt)
 {
 	unsigned int fmt;
 
@@ -684,16 +683,6 @@ static void graph_parse_daifmt(struct device_node *node,
 	 * };
 	 */
 
-	/*
-	 * clock_provider:
-	 *
-	 * It can be judged it is provider
-	 * if (A) or (B) or (C) has bitclock-master / frame-master flag.
-	 *
-	 * use "or"
-	 */
-	*bit_frame |= snd_soc_daifmt_parse_clock_provider_as_bitmap(node, NULL);
-
 #define update_daifmt(name)					\
 	if (!(*daifmt & SND_SOC_DAIFMT_##name##_MASK) &&	\
 		 (fmt & SND_SOC_DAIFMT_##name##_MASK))		\
@@ -711,6 +700,17 @@ static void graph_parse_daifmt(struct device_node *node,
 	update_daifmt(INV);
 }
 
+static unsigned int graph_parse_bitframe(struct device_node *ep)
+{
+	struct device_node *port  __free(device_node) = ep_to_port(ep);
+	struct device_node *ports __free(device_node) = port_to_ports(port);
+
+	return	snd_soc_daifmt_clock_provider_from_bitmap(
+			snd_soc_daifmt_parse_clock_provider_as_bitmap(ep,    NULL) |
+			snd_soc_daifmt_parse_clock_provider_as_bitmap(port,  NULL) |
+			snd_soc_daifmt_parse_clock_provider_as_bitmap(ports, NULL));
+}
+
 static void graph_link_init(struct simple_util_priv *priv,
 			    struct device_node *lnk,
 			    struct device_node *port_cpu,
@@ -721,15 +721,18 @@ static void graph_link_init(struct simple_util_priv *priv,
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
 	struct simple_dai_props *dai_props = simple_priv_to_props(priv, li->link);
 	struct device_node *ep_cpu, *ep_codec;
-	unsigned int daifmt = 0, daiclk = 0;
+	struct device_node *multi_cpu_ports = NULL, *multi_codec_ports = NULL;
+	struct snd_soc_dai_link_component *dlc;
+	unsigned int daifmt = 0;
 	bool playback_only = 0, capture_only = 0;
 	enum snd_soc_trigger_order trigger_start = SND_SOC_TRIGGER_ORDER_DEFAULT;
 	enum snd_soc_trigger_order trigger_stop  = SND_SOC_TRIGGER_ORDER_DEFAULT;
-	unsigned int bit_frame = 0;
+	int i;
 
 	of_node_get(port_cpu);
 	if (graph_lnk_is_multi(port_cpu)) {
-		ep_cpu = graph_get_next_multi_ep(&port_cpu);
+		multi_cpu_ports = port_cpu;
+		ep_cpu = graph_get_next_multi_ep(&multi_cpu_ports);
 		of_node_put(port_cpu);
 		port_cpu = ep_to_port(ep_cpu);
 	} else {
@@ -739,7 +742,8 @@ static void graph_link_init(struct simple_util_priv *priv,
 
 	of_node_get(port_codec);
 	if (graph_lnk_is_multi(port_codec)) {
-		ep_codec = graph_get_next_multi_ep(&port_codec);
+		multi_codec_ports = port_codec;
+		ep_codec = graph_get_next_multi_ep(&multi_codec_ports);
 		of_node_put(port_cpu);
 		port_codec = ep_to_port(ep_codec);
 	} else {
@@ -747,13 +751,13 @@ static void graph_link_init(struct simple_util_priv *priv,
 	}
 	struct device_node *ports_codec __free(device_node) = port_to_ports(port_codec);
 
-	graph_parse_daifmt(ep_cpu,	&daifmt, &bit_frame);
-	graph_parse_daifmt(ep_codec,	&daifmt, &bit_frame);
-	graph_parse_daifmt(port_cpu,	&daifmt, &bit_frame);
-	graph_parse_daifmt(port_codec,	&daifmt, &bit_frame);
-	graph_parse_daifmt(ports_cpu,	&daifmt, &bit_frame);
-	graph_parse_daifmt(ports_codec,	&daifmt, &bit_frame);
-	graph_parse_daifmt(lnk,		&daifmt, &bit_frame);
+	graph_parse_daifmt(ep_cpu,	&daifmt);
+	graph_parse_daifmt(ep_codec,	&daifmt);
+	graph_parse_daifmt(port_cpu,	&daifmt);
+	graph_parse_daifmt(port_codec,	&daifmt);
+	graph_parse_daifmt(ports_cpu,	&daifmt);
+	graph_parse_daifmt(ports_codec,	&daifmt);
+	graph_parse_daifmt(lnk,		&daifmt);
 
 	graph_util_parse_link_direction(lnk,		&playback_only, &capture_only);
 	graph_util_parse_link_direction(ports_cpu,	&playback_only, &capture_only);
@@ -779,14 +783,21 @@ static void graph_link_init(struct simple_util_priv *priv,
 	graph_util_parse_trigger_order(priv, ep_cpu,		&trigger_start, &trigger_stop);
 	graph_util_parse_trigger_order(priv, ep_codec,		&trigger_start, &trigger_stop);
 
-	/*
-	 * convert bit_frame
-	 * We need to flip clock_provider if it was CPU node,
-	 * because it is Codec base.
-	 */
-	daiclk = snd_soc_daifmt_clock_provider_from_bitmap(bit_frame);
-	if (is_cpu_node)
-		daiclk = snd_soc_daifmt_clock_provider_flipped(daiclk);
+	for_each_link_cpus(dai_link, i, dlc) {
+		dlc->ext_fmt = graph_parse_bitframe(ep_cpu);
+
+		if (multi_cpu_ports)
+			ep_cpu = graph_get_next_multi_ep(&multi_cpu_ports);
+	}
+
+	for_each_link_codecs(dai_link, i, dlc) {
+		dlc->ext_fmt = graph_parse_bitframe(ep_codec);
+
+		if (multi_codec_ports)
+			ep_codec = graph_get_next_multi_ep(&multi_codec_ports);
+	}
+
+	/*** Don't use port_cpu / port_codec after here ***/
 
 	dai_link->playback_only	= playback_only;
 	dai_link->capture_only	= capture_only;
@@ -794,7 +805,7 @@ static void graph_link_init(struct simple_util_priv *priv,
 	dai_link->trigger_start	= trigger_start;
 	dai_link->trigger_stop	= trigger_stop;
 
-	dai_link->dai_fmt	= daifmt | daiclk;
+	dai_link->dai_fmt	= daifmt;
 	dai_link->init		= simple_util_dai_init;
 	dai_link->ops		= &graph_ops;
 	if (priv->ops)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/6] ASoC: soc-core: Enable to use extra format on each DAI
  2024-12-17  1:48 [PATCH 0/6] ASoC: extra format on each DAI Kuninori Morimoto
                   ` (4 preceding siblings ...)
  2024-12-17  1:49 ` [PATCH 6/6] ASoC: audio-graph-card2: Use extra format on each DAI Kuninori Morimoto
@ 2024-12-17  1:50 ` Kuninori Morimoto
  5 siblings, 0 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2024-12-17  1:50 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-sound, Stephen Gordon

Current ASoC is using dai_link->dai_fmt to set DAI format for both
CPU/Codec. But because it is using same settings, and
SND_SOC_DAIFMT_CLOCK_PROVIDER is flipped for CPU, we can't set both
CPU/Codec as clock consumer, for example.

To solve this issue, this patch enable to use extra format for each
DAI which can keep compatibility with legacy system,

	1. SND_SOC_DAIFMT_FORMAT_MASK
	2. SND_SOC_DAIFMT_CLOCK
	3. SND_SOC_DAIFMT_INV
	4. SND_SOC_DAIFMT_CLOCK_PROVIDER

	Legacy
		dai_fmt  includes 1, 2, 3, 4

	New idea
		dai_fmt  includes 1, 2, 3
		ext_fmt  includes 4

Link: https://lore.kernel.org/r/5011ceef-5100-441d-b169-dabba135d27f@iinet.net.au
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  | 11 +++++++++++
 sound/soc/soc-core.c | 27 +++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 4f5d411e3823f..61eee40da12ad 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -681,6 +681,17 @@ struct snd_soc_dai_link_component {
 	struct device_node *of_node;
 	const char *dai_name;
 	const struct of_phandle_args *dai_args;
+
+	/*
+	 * Extra format = SND_SOC_DAIFMT_Bx_Fx
+	 *
+	 * [Note] it is Bx_Fx base, not CBx_CFx
+	 *
+	 * It will be used with dai_link->dai_fmt
+	 * see
+	 *	snd_soc_runtime_set_dai_fmt()
+	 */
+	unsigned int ext_fmt;
 };
 
 /*
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 426f8d8baa38b..e067366f2a315 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1449,23 +1449,46 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd,
 {
 	struct snd_soc_dai *cpu_dai;
 	struct snd_soc_dai *codec_dai;
+	unsigned int ext_fmt;
 	unsigned int i;
 	int ret;
 
 	if (!dai_fmt)
 		return 0;
 
+	/*
+	 * dai_fmt has 4 types
+	 *	1. SND_SOC_DAIFMT_FORMAT_MASK
+	 *	2. SND_SOC_DAIFMT_CLOCK
+	 *	3. SND_SOC_DAIFMT_INV
+	 *	4. SND_SOC_DAIFMT_CLOCK_PROVIDER
+	 *
+	 * 4. CLOCK_PROVIDER is set from Codec perspective in dai_fmt. So it will be flipped
+	 * when this function calls set_fmt() for CPU (CBx_CFx -> Bx_Cx). see below.
+	 * This mean, we can't set CPU/Codec both are clock consumer for example.
+	 * New idea handles 4. in each dai->ext_fmt. It can keep compatibility.
+	 *
+	 * Legacy
+	 *	dai_fmt  includes 1, 2, 3, 4
+	 *
+	 * New idea
+	 *	dai_fmt  includes 1, 2, 3
+	 *	ext_fmt  includes 4
+	 */
 	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		ret = snd_soc_dai_set_fmt(codec_dai, dai_fmt);
+		ext_fmt = rtd->dai_link->codecs[i].ext_fmt;
+		ret = snd_soc_dai_set_fmt(codec_dai, dai_fmt | ext_fmt);
 		if (ret != 0 && ret != -ENOTSUPP)
 			return ret;
 	}
 
 	/* Flip the polarity for the "CPU" end of link */
+	/* Will effect only for 4. SND_SOC_DAIFMT_CLOCK_PROVIDER */
 	dai_fmt = snd_soc_daifmt_clock_provider_flipped(dai_fmt);
 
 	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		ret = snd_soc_dai_set_fmt(cpu_dai, dai_fmt);
+		ext_fmt = rtd->dai_link->cpus[i].ext_fmt;
+		ret = snd_soc_dai_set_fmt(cpu_dai, dai_fmt | ext_fmt);
 		if (ret != 0 && ret != -ENOTSUPP)
 			return ret;
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/6] ASoC: audio-graph-card2: Use extra format on each DAI
  2024-12-17  1:49 ` [PATCH 6/6] ASoC: audio-graph-card2: Use extra format on each DAI Kuninori Morimoto
@ 2024-12-17 13:06   ` Stephen Gordon
  2024-12-18  6:29     ` Kuninori Morimoto
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen Gordon @ 2024-12-17 13:06 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Mark Brown, linux-sound


On Tue, 17 Dec 2024 01:49:30 +0000
Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote:

> Current ASoC is using dai_link->dai_fmt to set DAI format for both
> CPU/Codec. But because it is using same settings, and
> SND_SOC_DAIFMT_CLOCK_PROVIDER is flipped for CPU, we can't set both
> CPU/Codec as clock consumer, for example.
> 

Hi Morimoto-san,

This patch (#6 of the set) was generated against a tree which didn't
have the port_cpu -> port_codec correction applied
(687630aa582acf674120c87350beb01d836c837c) and it fails when applied
against HEAD of
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/

Not sure if this is a problem...

Stephen





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/6] ASoC: audio-graph-card2: Use extra format on each DAI
  2024-12-17 13:06   ` Stephen Gordon
@ 2024-12-18  6:29     ` Kuninori Morimoto
  2024-12-18 13:26       ` Mark Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Kuninori Morimoto @ 2024-12-18  6:29 UTC (permalink / raw)
  To: Stephen Gordon; +Cc: Mark Brown, linux-sound


Hi Stephen, Mark

> This patch (#6 of the set) was generated against a tree which didn't
> have the port_cpu -> port_codec correction applied
> (687630aa582acf674120c87350beb01d836c837c) and it fails when applied
> against HEAD of
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/

Ah, it seems above patch is...

	asoc/for-6.13 :     included
	asoc/for-6.14 : not included

Mark, is it possible to merge for-6.13 into for-6.14 ?
It can ignore conflict issue. I can post v2 patch-set on top of it.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/6] ASoC: audio-graph-card2: Use extra format on each DAI
  2024-12-18  6:29     ` Kuninori Morimoto
@ 2024-12-18 13:26       ` Mark Brown
  2024-12-19  0:37         ` Kuninori Morimoto
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Brown @ 2024-12-18 13:26 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Stephen Gordon, linux-sound

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

On Wed, Dec 18, 2024 at 06:29:51AM +0000, Kuninori Morimoto wrote:

> Ah, it seems above patch is...

> 	asoc/for-6.13 :     included
> 	asoc/for-6.14 : not included

> Mark, is it possible to merge for-6.13 into for-6.14 ?
> It can ignore conflict issue. I can post v2 patch-set on top of it.

Yes, I tend to do that if there's dependency issues.  Just base on
for-next for now, I'll do a merge into for-6.14 before applying if I've
not already merged up mainline (I'm holding off on doing the latter for
now since the KVM people broke things again).

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 6/6] ASoC: audio-graph-card2: Use extra format on each DAI
  2024-12-18 13:26       ` Mark Brown
@ 2024-12-19  0:37         ` Kuninori Morimoto
  0 siblings, 0 replies; 11+ messages in thread
From: Kuninori Morimoto @ 2024-12-19  0:37 UTC (permalink / raw)
  To: Mark Brown; +Cc: Stephen Gordon, linux-sound


Hi Mark

> > 	asoc/for-6.13 :     included
> > 	asoc/for-6.14 : not included
> 
> > Mark, is it possible to merge for-6.13 into for-6.14 ?
> > It can ignore conflict issue. I can post v2 patch-set on top of it.
> 
> Yes, I tend to do that if there's dependency issues.  Just base on
> for-next for now, I'll do a merge into for-6.14 before applying if I've
> not already merged up mainline (I'm holding off on doing the latter for
> now since the KVM people broke things again).

Thanks a lot !
I have updated based on for-next branch, and pushed.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-12-19  0:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-17  1:48 [PATCH 0/6] ASoC: extra format on each DAI Kuninori Morimoto
2024-12-17  1:48 ` [PATCH 1/6] ASoC: audio-graph-card2: use __free(device_node) for device node Kuninori Morimoto
2024-12-17  1:48 ` [PATCH 2/6] ASoC: audio-graph-card: " Kuninori Morimoto
2024-12-17  1:48 ` [PATCH 3/6] ASoC: simple-card: " Kuninori Morimoto
2024-12-17  1:48 ` [PATCH 4/6] ASoC: soc-core: return 0 if np was NULL on snd_soc_daifmt_parse_clock_provider_raw() Kuninori Morimoto
2024-12-17  1:49 ` [PATCH 6/6] ASoC: audio-graph-card2: Use extra format on each DAI Kuninori Morimoto
2024-12-17 13:06   ` Stephen Gordon
2024-12-18  6:29     ` Kuninori Morimoto
2024-12-18 13:26       ` Mark Brown
2024-12-19  0:37         ` Kuninori Morimoto
2024-12-17  1:50 ` [PATCH 5/6] ASoC: soc-core: Enable to use " Kuninori Morimoto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox