* [PATCH v2 1/6] ASoC: audio-graph-card2: use __free(device_node) for device node
2024-12-19 0:34 [PATCH v2 0/6] ASoC: extra format on each DAI Kuninori Morimoto
@ 2024-12-19 0:35 ` Kuninori Morimoto
2024-12-19 0:35 ` [PATCH v2 2/6] ASoC: audio-graph-card: " Kuninori Morimoto
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2024-12-19 0:35 UTC (permalink / raw)
To: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
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 1f5c4e8ff1b90..f4c825b56a7e1 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] 14+ messages in thread* [PATCH v2 2/6] ASoC: audio-graph-card: use __free(device_node) for device node
2024-12-19 0:34 [PATCH v2 0/6] ASoC: extra format on each DAI Kuninori Morimoto
2024-12-19 0:35 ` [PATCH v2 1/6] ASoC: audio-graph-card2: use __free(device_node) for device node Kuninori Morimoto
@ 2024-12-19 0:35 ` Kuninori Morimoto
2024-12-19 0:35 ` [PATCH v2 3/6] ASoC: simple-card: " Kuninori Morimoto
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2024-12-19 0:35 UTC (permalink / raw)
To: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
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] 14+ messages in thread* [PATCH v2 3/6] ASoC: simple-card: use __free(device_node) for device node
2024-12-19 0:34 [PATCH v2 0/6] ASoC: extra format on each DAI Kuninori Morimoto
2024-12-19 0:35 ` [PATCH v2 1/6] ASoC: audio-graph-card2: use __free(device_node) for device node Kuninori Morimoto
2024-12-19 0:35 ` [PATCH v2 2/6] ASoC: audio-graph-card: " Kuninori Morimoto
@ 2024-12-19 0:35 ` Kuninori Morimoto
2024-12-19 0:35 ` [PATCH v2 4/6] ASoC: soc-core: return 0 if np was NULL on snd_soc_daifmt_parse_clock_provider_raw() Kuninori Morimoto
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2024-12-19 0:35 UTC (permalink / raw)
To: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
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] 14+ messages in thread* [PATCH v2 4/6] ASoC: soc-core: return 0 if np was NULL on snd_soc_daifmt_parse_clock_provider_raw()
2024-12-19 0:34 [PATCH v2 0/6] ASoC: extra format on each DAI Kuninori Morimoto
` (2 preceding siblings ...)
2024-12-19 0:35 ` [PATCH v2 3/6] ASoC: simple-card: " Kuninori Morimoto
@ 2024-12-19 0:35 ` Kuninori Morimoto
2024-12-19 0:35 ` [PATCH v2 5/6] ASoC: soc-core: Enable to use extra format on each DAI Kuninori Morimoto
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2024-12-19 0:35 UTC (permalink / raw)
To: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
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 710c278e4f36b..31cf499e3e79d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3390,6 +3390,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] 14+ messages in thread* [PATCH v2 5/6] ASoC: soc-core: Enable to use extra format on each DAI
2024-12-19 0:34 [PATCH v2 0/6] ASoC: extra format on each DAI Kuninori Morimoto
` (3 preceding siblings ...)
2024-12-19 0:35 ` [PATCH v2 4/6] ASoC: soc-core: return 0 if np was NULL on snd_soc_daifmt_parse_clock_provider_raw() Kuninori Morimoto
@ 2024-12-19 0:35 ` Kuninori Morimoto
2024-12-19 0:35 ` [PATCH v2 6/6] ASoC: audio-graph-card2: Use " Kuninori Morimoto
2024-12-20 0:01 ` [PATCH v2 0/6] ASoC: " Stephen Gordon
6 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2024-12-19 0:35 UTC (permalink / raw)
To: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
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
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 31cf499e3e79d..bd16b3fe59c8f 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] 14+ messages in thread* [PATCH v2 6/6] ASoC: audio-graph-card2: Use extra format on each DAI
2024-12-19 0:34 [PATCH v2 0/6] ASoC: extra format on each DAI Kuninori Morimoto
` (4 preceding siblings ...)
2024-12-19 0:35 ` [PATCH v2 5/6] ASoC: soc-core: Enable to use extra format on each DAI Kuninori Morimoto
@ 2024-12-19 0:35 ` Kuninori Morimoto
2024-12-20 0:01 ` [PATCH v2 0/6] ASoC: " Stephen Gordon
6 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2024-12-19 0:35 UTC (permalink / raw)
To: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
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
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 f4c825b56a7e1..2bf7b25e00ed2 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_codec);
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] 14+ messages in thread* Re: [PATCH v2 0/6] ASoC: extra format on each DAI
2024-12-19 0:34 [PATCH v2 0/6] ASoC: extra format on each DAI Kuninori Morimoto
` (5 preceding siblings ...)
2024-12-19 0:35 ` [PATCH v2 6/6] ASoC: audio-graph-card2: Use " Kuninori Morimoto
@ 2024-12-20 0:01 ` Stephen Gordon
2024-12-23 2:01 ` Kuninori Morimoto
6 siblings, 1 reply; 14+ messages in thread
From: Stephen Gordon @ 2024-12-20 0:01 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
[-- Attachment #1: Type: text/plain, Size: 1593 bytes --]
On Thu, 19 Dec 2024 00:34:27 +0000
Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote:
> This patch-set needs Stephen's Tested-by
>
Hi Morimoto-san,
I am attempting to test this by applying your patchset on top of
6.13-rc3. But I am running into a problem, probably because my
graph-card2 DT is wrong. I've attached the DTS file. I used the
Semi-Multi example.
Here is the output of dmesg when I load it:
[ 27.277613] /soc@107c000000/sound/multi: Fixed dependency cycle(s)
with /axi/pcie@120000/rp1/i2s@a4000
[ 27.277635] /axi/pcie@120000/rp1/i2s@a4000: Fixed dependency
cycle(s) with /soc@107c000000/sound/multi
[ 27.277820] /soc@107c000000/sound/multi: Fixed dependency cycle(s)
with /axi/pcie@120000/rp1/i2c@74000/audio-codec@45
[ 27.277841] /axi/pcie@120000/rp1/i2c@74000/audio-codec@45: Fixed
dependency cycle(s) with /soc@107c000000/sound/multi
[ 27.304322] designware-i2s 1f000a4000.i2s: ASoC: Registered DAI
'1f000a4000.i2s'
[ 27.333791] asoc-audio-graph-card2 soc@107c000000:sound:
/axi/pcie@120000/rp1/i2s@a4000/ports/port@0 (Normal)
[ 27.333813] asoc-audio-graph-card2 soc@107c000000:sound: link 1,
dais 3, ccnf 0
[ 27.333822] asoc-audio-graph-card2 soc@107c000000:sound:
/axi/pcie@120000/rp1/i2s@a4000/ports/port@0 (Normal)
[ 27.333830] asoc-audio-graph-card2 soc@107c000000:sound: probe with
driver asoc-audio-graph-card2 failed with error -12
[ 27.346051] pcm3168a 1-0045: ASoC: Registered DAI 'pcm3168a-dac'
[ 27.346055] pcm3168a 1-0045: ASoC: Registered DAI 'pcm3168a-adc'
I would appreciate any advice you can give.
Regards
Stephen
[-- Attachment #2: graph.dts --]
[-- Type: audio/vnd.dts, Size: 4057 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 0/6] ASoC: extra format on each DAI
2024-12-20 0:01 ` [PATCH v2 0/6] ASoC: " Stephen Gordon
@ 2024-12-23 2:01 ` Kuninori Morimoto
2024-12-27 12:21 ` Stephen Gordon
0 siblings, 1 reply; 14+ messages in thread
From: Kuninori Morimoto @ 2024-12-23 2:01 UTC (permalink / raw)
To: Stephen Gordon
Cc: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
Hi Stephen
Thank you for testing
> I am attempting to test this by applying your patchset on top of
> 6.13-rc3. But I am running into a problem, probably because my
> graph-card2 DT is wrong. I've attached the DTS file. I used the
> Semi-Multi example.
Hmm... It seems the DT setting itself is correct. I have copied your
settings to test-cpu / test-codec, but it could probe without any error.
> [ 27.333830] asoc-audio-graph-card2 soc@107c000000:sound: probe with
> driver asoc-audio-graph-card2 failed with error -12
It is -12 = -ENOMEM = kmalloc() error ?
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/6] ASoC: extra format on each DAI
2024-12-23 2:01 ` Kuninori Morimoto
@ 2024-12-27 12:21 ` Stephen Gordon
2024-12-28 11:18 ` Stephen Gordon
0 siblings, 1 reply; 14+ messages in thread
From: Stephen Gordon @ 2024-12-27 12:21 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
On 23/12/2024 1:01 pm, Kuninori Morimoto wrote:
>
> Hi Stephen
>
> Thank you for testing
>
>> I am attempting to test this by applying your patchset on top of
>> 6.13-rc3. But I am running into a problem, probably because my
>> graph-card2 DT is wrong. I've attached the DTS file. I used the
>> Semi-Multi example.
>
> Hmm... It seems the DT setting itself is correct. I have copied your
> settings to test-cpu / test-codec, but it could probe without any error.
>
>> [ 27.333830] asoc-audio-graph-card2 soc@107c000000:sound: probe with
>> driver asoc-audio-graph-card2 failed with error -12
>
> It is -12 = -ENOMEM = kmalloc() error ?
>
>
> Thank you for your help !!
>
> Best regards
> ---
> Kuninori Morimoto
Hi Morimoto-san,
Merry Christmas!
I finally got a chance to look further and after a bit of debugging I
found that graph_parse_node_multi() has a loop iterating through the
nodes. However, when graph_get_next_multi_ep() is called to get the next
endpoint, it returns null. I put a debug statement in there:
if (!ep) {
dev_dbg(dev, "no endpoint");
break;
}
and that statement is shown in dmesg.
After it breaks out of the loop, it just returns the value of 'ret',
which was initialised to -ENOMEM.
That's as far as I got.
Regards
Stephen
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 0/6] ASoC: extra format on each DAI
2024-12-27 12:21 ` Stephen Gordon
@ 2024-12-28 11:18 ` Stephen Gordon
2024-12-30 6:02 ` Stephen Gordon
0 siblings, 1 reply; 14+ messages in thread
From: Stephen Gordon @ 2024-12-28 11:18 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
[-- Attachment #1: Type: text/plain, Size: 2025 bytes --]
On 27/12/2024 11:21 pm, Stephen Gordon wrote:
> On 23/12/2024 1:01 pm, Kuninori Morimoto wrote:
>>
>>
>>> [ 27.333830] asoc-audio-graph-card2 soc@107c000000:sound: probe with
>>> driver asoc-audio-graph-card2 failed with error -12
> However, when graph_get_next_multi_ep() is called to get the next
> endpoint, it returns null.
Hello Morimoto-san.
I reproduced this using the test components with the attached DTS.
I found that this is because the sibling field of the
/soc@107c000000/sound/multi/ports@0/port@0 node is null, which means
that of_graph_get_next_child returns null.
In turn, this is because the linked list of siblings is actually in
reverse order due to the way populate_node() (drivers/of/fdt.c) adds
them. This means that when the driver uses remote-endpoint to navigate
from the endpoint in the cpu node to the endpoint in the multi node, it
is actually arriving at the last child (with address 0). When calling
next_child(), it gets null.
[ 30.196250] asoc-audio-graph-card2 soc@107c000000:sound: ports
parent:/soc@107c000000/sound/multi
child:/soc@107c000000/sound/multi/ports@0/port@2
[ 30.196256] asoc-audio-graph-card2 soc@107c000000:sound: sib #0:
/soc@107c000000/sound/multi/ports@0/port@2
[ 30.196260] asoc-audio-graph-card2 soc@107c000000:sound: sib #1:
/soc@107c000000/sound/multi/ports@0/port@1
[ 30.196264] asoc-audio-graph-card2 soc@107c000000:sound: sib #2:
/soc@107c000000/sound/multi/ports@0/port@0
After finding this, I reversed the order of the port nodes inside
multi/ports (i.e. the cpu link is now last) and the overlay loads
without errors.
I get a playback DAI visible in aplay, however the capture DAI is not
visible in arecord. If I swap the order of the codec ports in
multi/ports, then I get a capture DAI visible in arecord, but the
playback DAI is missing :)
I will keep experimenting to see if I can find a way to make them both
work, but I think some changes are needed to ensure all of the nodes get
processed.
Regards
Stephen
[-- Attachment #2: testgraph.dts --]
[-- Type: text/plain, Size: 2881 bytes --]
//Device tree overlay for multi-channel I2S in clock consumer mode
/dts-v1/;
/plugin/;
/ {
// Pi5 only
compatible = "brcm,bcm2712";
fragment@2 {
target = <&sound>;
__overlay__ {
compatible = "audio-graph-card2";
label = "ezsound";
status = "okay";
links = <&link0>;
multi {
#address-cells = <1>;
#size-cells = <0>;
/*
* +---+
* cpu_i2s <-@------->|X A|-> codec_dac
* | B|-> codec_adc
* +---+
*/
ports@0 {
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
port@0 { reg = <0>; link_cpu: endpoint { remote-endpoint = <&cpu_i2s>; };};
port@1 { reg = <1>; link_dac: endpoint { remote-endpoint = <&codec_dac>; };};
port@2 { reg = <2>; link_adc: endpoint { remote-endpoint = <&codec_adc>; };};
};
};
};
};
// Bring the I2S clock consumer block up
fragment@4 {
target-path = "/";
__overlay__ {
test-cpu {
compatible = "test-cpu-verbose";
#sound-dai-cells = <0>;
status = "okay";
ports {
#address-cells = <1>;
#size-cells = <0>;
link0: port@0 {
reg = <0>;
cpu_i2s: endpoint {
dai-tdm-slot-num = <2>;
dai-tdm-slot-width = <32>;
remote-endpoint = <&link_cpu>;
};
};
};
};
};
};
fragment@5 {
target-path = "/";
__overlay__ {
test-codec {
compatible = "test-codec-verbose";
#sound-dai-cells = <1>;
status = "okay";
ports {
#address-cells = <1>;
#size-cells = <0>;
prefix = "pcm3168a";
port@0 {
reg = <0>;
codec_dac: endpoint {
playback-only;
dai-tdm-slot-num = <2>;
dai-tdm-slot-width = <32>;
remote-endpoint = <&link_dac>;
};
};
port@1 {
reg = <1>;
codec_adc: endpoint {
bitclock-master;
frame-master;
capture-only;
dai-tdm-slot-num = <2>;
dai-tdm-slot-width = <32>;
remote-endpoint = <&link_adc>;
};
};
};
};
};
};
};
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 0/6] ASoC: extra format on each DAI
2024-12-28 11:18 ` Stephen Gordon
@ 2024-12-30 6:02 ` Stephen Gordon
2024-12-30 12:41 ` Stephen Gordon
0 siblings, 1 reply; 14+ messages in thread
From: Stephen Gordon @ 2024-12-30 6:02 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
On 28/12/2024 10:18 pm, Stephen Gordon wrote:
> On 27/12/2024 11:21 pm, Stephen Gordon wrote:
>> On 23/12/2024 1:01 pm, Kuninori Morimoto wrote:
>>>
>>>
>>>> [ 27.333830] asoc-audio-graph-card2 soc@107c000000:sound: probe with
>>>> driver asoc-audio-graph-card2 failed with error -12
>
> In turn, this is because the linked list of siblings is actually in
> reverse order due to the way populate_node() (drivers/of/fdt.c) adds
> them.
It looks like the node order is fixed up later...
https://github.com/torvalds/linux/blob/fc033cf25e612e840e545f8d5ad2edd6ba613ed5/drivers/of/fdt.c#L330
However drivers/ot/dynamic.c uses similar node insertion code, but
without any reordering. I guess this is what is used on my machine
(Raspberry Pi 5) - perhaps your DT is just built differently :)
> I get a playback DAI visible in aplay, however the capture DAI is not
> visible in arecord. If I swap the order of the codec ports in multi/
> ports, then I get a capture DAI visible in arecord, but the playback DAI
> is missing :)
This was because of the playback-only and capture-only flags in my
previous DTS. These flags are actually on the dai_link, so specifying
this flag on a port/endpoint affects the whole link. Since I specified
both, whichever one is processed last takes effect.
I think we should either:
- Make this a flag at the link level
e.g. links = <&link0 PLAYBACK_ONLY>
or
- Document the behaviour
Anyway, once I removed those flags, the DTS works, as long as the CPU
port is listed _last_ inside the 'multi' node. This is equivalent to a
requirement to list the CPU port _first_ on other platforms.
gordoste@rpi5:~/i2smulti $ diff graph-working.dts graph-broken.dts
48,50c48,50
< port@0 { reg = <0>; link_dac: endpoint {
remote-endpoint = <&codec_dac>; };};
< port@1 { reg = <1>; link_adc: endpoint {
remote-endpoint = <&codec_adc>; };};
< port@2 { reg = <2>; link_cpu: endpoint {
remote-endpoint = <&cpu_i2s>; };};
---
> port@0 { reg = <0>; link_cpu: endpoint {
remote-endpoint = <&cpu_i2s>; };};
> port@1 { reg = <1>; link_dac: endpoint {
remote-endpoint = <&codec_dac>; };};
> port@2 { reg = <2>; link_adc: endpoint {
remote-endpoint = <&codec_adc>; };};
Regards
Stephen
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 0/6] ASoC: extra format on each DAI
2024-12-30 6:02 ` Stephen Gordon
@ 2024-12-30 12:41 ` Stephen Gordon
2025-01-06 2:47 ` Kuninori Morimoto
0 siblings, 1 reply; 14+ messages in thread
From: Stephen Gordon @ 2024-12-30 12:41 UTC (permalink / raw)
To: Kuninori Morimoto
Cc: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
On 30/12/2024 5:02 pm, Stephen Gordon wrote:
> Anyway, once I removed those flags, the DTS works, as long as the CPU
> port is listed _last_ inside the 'multi' node. This is equivalent to a
> requirement to list the CPU port _first_ on other platforms.
FYI, I just submitted a patch to avoid reversing the sibling order when
updating the devicetree. This patch makes audio_graph_card2 process the
nodes in the correct order on my machine.
https://lore.kernel.org/linux-devicetree/20241230120315.2490-1-gordoste@iinet.net.au/T/#u
I will stop replying to myself now!
Regards
Stephen
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/6] ASoC: extra format on each DAI
2024-12-30 12:41 ` Stephen Gordon
@ 2025-01-06 2:47 ` Kuninori Morimoto
0 siblings, 0 replies; 14+ messages in thread
From: Kuninori Morimoto @ 2025-01-06 2:47 UTC (permalink / raw)
To: Stephen Gordon
Cc: Jaroslav Kysela, Liam Girdwood, Mark Brown, Takashi Iwai,
linux-sound
Hi Stephen
Thank you for debuging
> > Anyway, once I removed those flags, the DTS works, as long as the CPU
> > port is listed _last_ inside the 'multi' node. This is equivalent to a
> > requirement to list the CPU port _first_ on other platforms.
>
> FYI, I just submitted a patch to avoid reversing the sibling order when
> updating the devicetree. This patch makes audio_graph_card2 process the
> nodes in the correct order on my machine.
Indeed current audio_graph_card2 is not assuming overlay.
I guess it can be solved if graph_get_next_multi_ep() uses
of_graph_get_port_by_id() instead of of_graph_get_next_port().
Will update the patch.
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 14+ messages in thread