[alsa-devel] [PATCH 0/7][RFC] ASoC: modern style CPU
Hi Mark
These are RFC of modern style CPU support.
modern style dai_link needs CPU/Codec/Platform array and its size, but it will be very ugly code. To avoid it, 3) adds macro.
4) - 7) are sample code for switching to modern style dai_link. 4), 5) are good sample for single CPU/Codec/Platform. 6), 7) are good sample for multi Codec.
Almost all sound cards will get similar patch. I think these can work, but I can't test these. So, it is very useful if someone can test these. If these are all OK (= test/review), I can post patch-set for all sound cards.
1) ASoC: soc-core: use snd_soc_dai_link_component for CPU 2) ASoC: simple-card: support snd_soc_dai_link_component style for cpu 3) ASoC: soc.h: add sound dai_link connection macro 4) ASoC: samsung: smdk_wm8580: use modern dai_link style 5) ASoC: samsung: smdk_wm8994: use modern dai_link style 6) ASoC: mediatek: mt8173-rt5650-rt5676: use modern dai_link style 7) ASoC: mediatek: mt8173-rt5650-rt5514: use modern dai_link style
Thank you for your help !!
Best regards --- Kuninori Morimoto
current ALSA SoC is starting to support modern style dai_linke (= struct snd_soc_dai_link_component) which is mainly used for multipul DAI/component connection. Now Codec has full multi-codec support, Platform is using modern style but still for single Platform. Only CPU is not yet supporting modern style yet. If we could support it for CPU, we can switch to modern style dai_link on all CPU/Codec/Platform, and remove legacy style from ALSA SoC.
Multi-CPU will be supported in the future. This patch is initial support for modern style for CPU
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 4 +++ sound/soc/soc-core.c | 83 +++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 73 insertions(+), 14 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 1e2be35..23463ba 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -922,6 +922,9 @@ struct snd_soc_dai_link { */ const char *cpu_dai_name;
+ struct snd_soc_dai_link_component *cpus; + unsigned int num_cpus; + /* * codec_name * codec_of_node @@ -1030,6 +1033,7 @@ struct snd_soc_dai_link { * drivers should not modify this value. */ unsigned int legacy_platform:1; + unsigned int legacy_cpu:1;
struct list_head list; /* DAI link list of the soc card */ struct snd_soc_dobj dobj; /* For topology */ diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 75f6a80..4c1fdad 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -876,7 +876,6 @@ static int soc_bind_dai_link(struct snd_soc_card *card, { struct snd_soc_pcm_runtime *rtd; struct snd_soc_dai_link_component *codecs; - struct snd_soc_dai_link_component cpu_dai_component; struct snd_soc_component *component; struct snd_soc_dai **codec_dais; int i; @@ -896,13 +895,11 @@ static int soc_bind_dai_link(struct snd_soc_card *card, if (!rtd) return -ENOMEM;
- cpu_dai_component.name = dai_link->cpu_name; - cpu_dai_component.of_node = dai_link->cpu_of_node; - cpu_dai_component.dai_name = dai_link->cpu_dai_name; - rtd->cpu_dai = snd_soc_find_dai(&cpu_dai_component); + /* FIXME: we need multi CPU support in the future */ + rtd->cpu_dai = snd_soc_find_dai(dai_link->cpus); if (!rtd->cpu_dai) { dev_info(card->dev, "ASoC: CPU DAI %s not registered\n", - dai_link->cpu_dai_name); + dai_link->cpus->dai_name); goto _err_defer; } snd_soc_rtdcom_add(rtd, rtd->cpu_dai->component); @@ -1038,6 +1035,46 @@ static void soc_remove_dai_links(struct snd_soc_card *card) } }
+static int snd_soc_init_cpu(struct snd_soc_card *card, + struct snd_soc_dai_link *dai_link) +{ + struct snd_soc_dai_link_component *cpu = dai_link->cpus; + + /* + * REMOVE ME + * + * This is glue code for Legacy vs Modern dai_link. + * This function will be removed if all derivers are switched to + * modern style dai_link. + * Driver shouldn't use both legacy and modern style in the same time. + * see + * soc.h :: struct snd_soc_dai_link + */ + /* convert Legacy platform link */ + if (!cpu) { + cpu = devm_kzalloc(card->dev, + sizeof(struct snd_soc_dai_link_component), + GFP_KERNEL); + if (!cpu) + return -ENOMEM; + + dai_link->cpus = cpu; + dai_link->num_cpus = 1; + dai_link->legacy_cpu = 1; + + cpu->name = dai_link->cpu_name; + cpu->of_node = dai_link->cpu_of_node; + cpu->dai_name = dai_link->cpu_dai_name; + } + + if (!dai_link->cpus) { + dev_err(card->dev, "ASoC: DAI link has no CPUs\n"); + return -EINVAL; + } + + return 0; +} + static int snd_soc_init_platform(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link) { @@ -1077,7 +1114,7 @@ static int snd_soc_init_platform(struct snd_soc_card *card, return 0; }
-static void soc_cleanup_platform(struct snd_soc_card *card) +static void soc_cleanup_legacy(struct snd_soc_card *card) { struct snd_soc_dai_link *link; int i; @@ -1092,6 +1129,10 @@ static void soc_cleanup_platform(struct snd_soc_card *card) link->legacy_platform = 0; link->platforms = NULL; } + if (link->legacy_cpu) { + link->legacy_cpu = 0; + link->cpus = NULL; + } } }
@@ -1139,6 +1180,12 @@ static int soc_init_dai_link(struct snd_soc_card *card, int i, ret; struct snd_soc_dai_link_component *codec;
+ ret = snd_soc_init_cpu(card, link); + if (ret) { + dev_err(card->dev, "ASoC: failed to init cpu\n"); + return ret; + } + ret = snd_soc_init_platform(card, link); if (ret) { dev_err(card->dev, "ASoC: failed to init multiplatform\n"); @@ -1197,12 +1244,20 @@ static int soc_init_dai_link(struct snd_soc_card *card, !soc_find_component(link->platforms->of_node, link->platforms->name)) return -EPROBE_DEFER;
+ /* FIXME */ + if (link->num_cpus > 1) { + dev_err(card->dev, + "ASoC: multi cpu is not yet supported %s\n", + link->name); + return -EINVAL; + } + /* * CPU device may be specified by either name or OF node, but * can be left unspecified, and will be matched based on DAI * name alone.. */ - if (link->cpu_name && link->cpu_of_node) { + if (link->cpus->name && link->cpus->of_node) { dev_err(card->dev, "ASoC: Neither/both cpu name/of_node are set for %s\n", link->name); @@ -1213,16 +1268,16 @@ static int soc_init_dai_link(struct snd_soc_card *card, * Defer card registartion if cpu dai component is not added to * component list. */ - if ((link->cpu_of_node || link->cpu_name) && - !soc_find_component(link->cpu_of_node, link->cpu_name)) + if ((link->cpus->of_node || link->cpus->name) && + !soc_find_component(link->cpus->of_node, link->cpus->name)) return -EPROBE_DEFER;
/* * At least one of CPU DAI name or CPU device name/node must be * specified */ - if (!link->cpu_dai_name && - !(link->cpu_name || link->cpu_of_node)) { + if (!link->cpus->dai_name && + !(link->cpus->name || link->cpus->of_node)) { dev_err(card->dev, "ASoC: Neither cpu_dai_name nor cpu_name/of_node are set for %s\n", link->name); @@ -2043,7 +2098,7 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card) /* remove and free each DAI */ soc_remove_dai_links(card); soc_remove_pcm_runtimes(card); - soc_cleanup_platform(card); + soc_cleanup_legacy(card);
/* remove auxiliary devices */ soc_remove_aux_devices(card); @@ -2800,7 +2855,7 @@ int snd_soc_register_card(struct snd_soc_card *card)
ret = soc_init_dai_link(card, link); if (ret) { - soc_cleanup_platform(card); + soc_cleanup_legacy(card); dev_err(card->dev, "ASoC: failed to init link %s\n", link->name); mutex_unlock(&client_mutex);
ASoC supports modern style dai_link (= snd_soc_dai_link_component) for CPU. legacy style dai_link (= cpu_dai_name, cpu_name, cpu_of_node) are no longer needed. This patch switches to modern style.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/simple_card_utils.h | 20 ++++++------------ sound/soc/generic/audio-graph-card.c | 30 +++++++-------------------- sound/soc/generic/simple-card-utils.c | 21 ++++++------------- sound/soc/generic/simple-card.c | 39 +++++++++++++---------------------- 4 files changed, 34 insertions(+), 76 deletions(-)
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index 67dc3ee..dd19374 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -42,6 +42,7 @@ struct asoc_simple_priv { struct simple_dai_props { struct asoc_simple_dai *cpu_dai; struct asoc_simple_dai *codec_dai; + struct snd_soc_dai_link_component cpus; /* single cpu */ struct snd_soc_dai_link_component codecs; /* single codec */ struct snd_soc_dai_link_component platforms; struct asoc_simple_data adata; @@ -80,16 +81,12 @@ int asoc_simple_parse_card_name(struct snd_soc_card *card, char *prefix);
#define asoc_simple_parse_clk_cpu(dev, node, dai_link, simple_dai) \ - asoc_simple_parse_clk(dev, node, dai_link->cpu_of_node, simple_dai, \ - dai_link->cpu_dai_name, NULL) + asoc_simple_parse_clk(dev, node, simple_dai, dai_link->cpus) #define asoc_simple_parse_clk_codec(dev, node, dai_link, simple_dai) \ - asoc_simple_parse_clk(dev, node, dai_link->codec_of_node, simple_dai,\ - dai_link->codec_dai_name, dai_link->codecs) + asoc_simple_parse_clk(dev, node, simple_dai, dai_link->codecs) int asoc_simple_parse_clk(struct device *dev, struct device_node *node, - struct device_node *dai_of_node, struct asoc_simple_dai *simple_dai, - const char *dai_name, struct snd_soc_dai_link_component *dlc); int asoc_simple_startup(struct snd_pcm_substream *substream); void asoc_simple_shutdown(struct snd_pcm_substream *substream); @@ -100,16 +97,11 @@ int asoc_simple_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params);
#define asoc_simple_parse_cpu(node, dai_link, is_single_link) \ - asoc_simple_parse_dai(node, NULL, \ - &dai_link->cpu_of_node, \ - &dai_link->cpu_dai_name, is_single_link) + asoc_simple_parse_dai(node, dai_link->cpus, is_single_link) #define asoc_simple_parse_codec(node, dai_link) \ - asoc_simple_parse_dai(node, dai_link->codecs, \ - &dai_link->codec_of_node, \ - &dai_link->codec_dai_name, NULL) + asoc_simple_parse_dai(node, dai_link->codecs, NULL) #define asoc_simple_parse_platform(node, dai_link) \ - asoc_simple_parse_dai(node, dai_link->platforms, \ - &dai_link->platform_of_node, NULL, NULL) + asoc_simple_parse_dai(node, dai_link->platforms, NULL)
#define asoc_simple_parse_tdm(np, dai) \ snd_soc_of_parse_tdm_slot(np, &(dai)->tx_slot_mask, \ diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c index ec7e673..e438011 100644 --- a/sound/soc/generic/audio-graph-card.c +++ b/sound/soc/generic/audio-graph-card.c @@ -111,29 +111,14 @@ static int graph_get_dai_id(struct device_node *ep)
static int asoc_simple_parse_dai(struct device_node *ep, struct snd_soc_dai_link_component *dlc, - struct device_node **dai_of_node, - const char **dai_name, int *is_single_link) { struct device_node *node; struct of_phandle_args args; int ret;
- /* - * Use snd_soc_dai_link_component instead of legacy style. - * It is only for codec, but cpu will be supported in the future. - * see - * soc-core.c :: snd_soc_init_multicodec() - */ - if (dlc) { - dai_name = &dlc->dai_name; - dai_of_node = &dlc->of_node; - } - if (!ep) return 0; - if (!dai_name) - return 0;
node = of_graph_get_port_parent(ep);
@@ -142,11 +127,11 @@ static int asoc_simple_parse_dai(struct device_node *ep, args.args[0] = graph_get_dai_id(ep); args.args_count = (of_graph_get_endpoint_count(node) > 1);
- ret = snd_soc_get_dai_name(&args, dai_name); + ret = snd_soc_get_dai_name(&args, &dlc->dai_name); if (ret < 0) return ret;
- *dai_of_node = node; + dlc->of_node = node;
if (is_single_link) *is_single_link = of_graph_get_endpoint_count(node) == 1; @@ -207,6 +192,7 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv, struct device_node *ports; struct device_node *node; struct asoc_simple_dai *dai; + struct snd_soc_dai_link_component *cpus = dai_link->cpus; struct snd_soc_dai_link_component *codecs = dai_link->codecs; int ret;
@@ -251,7 +237,7 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv,
ret = asoc_simple_set_dailink_name(dev, dai_link, "fe.%s", - dai_link->cpu_dai_name); + cpus->dai_name); if (ret < 0) return ret;
@@ -261,9 +247,9 @@ static int graph_dai_link_of_dpcm(struct asoc_simple_priv *priv, struct snd_soc_codec_conf *cconf;
/* FE is dummy */ - dai_link->cpu_of_node = NULL; - dai_link->cpu_dai_name = "snd-soc-dummy-dai"; - dai_link->cpu_name = "snd-soc-dummy"; + cpus->of_node = NULL; + cpus->dai_name = "snd-soc-dummy-dai"; + cpus->name = "snd-soc-dummy";
/* BE settings */ dai_link->no_pcm = 1; @@ -383,7 +369,7 @@ static int graph_dai_link_of(struct asoc_simple_priv *priv,
ret = asoc_simple_set_dailink_name(dev, dai_link, "%s-%s", - dai_link->cpu_dai_name, + dai_link->cpus->dai_name, dai_link->codecs->dai_name); if (ret < 0) return ret; diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index db1458a..e756e70 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -159,24 +159,13 @@ static void asoc_simple_clk_disable(struct asoc_simple_dai *dai)
int asoc_simple_parse_clk(struct device *dev, struct device_node *node, - struct device_node *dai_of_node, struct asoc_simple_dai *simple_dai, - const char *dai_name, struct snd_soc_dai_link_component *dlc) { struct clk *clk; u32 val;
/* - * Use snd_soc_dai_link_component instead of legacy style. - * It is only for codec, but cpu will be supported in the future. - * see - * soc-core.c :: snd_soc_init_multicodec() - */ - if (dlc) - dai_of_node = dlc->of_node; - - /* * Parse dai->sysclk come from "clocks = <&xxx>" * (if system has common clock) * or "system-clock-frequency = <xxx>" @@ -190,7 +179,7 @@ int asoc_simple_parse_clk(struct device *dev, } else if (!of_property_read_u32(node, "system-clock-frequency", &val)) { simple_dai->sysclk = val; } else { - clk = devm_get_clk_from_child(dev, dai_of_node, NULL); + clk = devm_get_clk_from_child(dev, dlc->of_node, NULL); if (!IS_ERR(clk)) simple_dai->sysclk = clk_get_rate(clk); } @@ -359,7 +348,7 @@ void asoc_simple_canonicalize_platform(struct snd_soc_dai_link *dai_link) { /* Assumes platform == cpu */ if (!dai_link->platforms->of_node) - dai_link->platforms->of_node = dai_link->cpu_of_node; + dai_link->platforms->of_node = dai_link->cpus->of_node; } EXPORT_SYMBOL_GPL(asoc_simple_canonicalize_platform);
@@ -376,7 +365,7 @@ void asoc_simple_canonicalize_cpu(struct snd_soc_dai_link *dai_link, * fmt_multiple_name() */ if (is_single_links) - dai_link->cpu_dai_name = NULL; + dai_link->cpus->dai_name = NULL; } EXPORT_SYMBOL_GPL(asoc_simple_canonicalize_cpu);
@@ -386,7 +375,7 @@ int asoc_simple_clean_reference(struct snd_soc_card *card) int i;
for_each_card_prelinks(card, i, dai_link) { - of_node_put(dai_link->cpu_of_node); + of_node_put(dai_link->cpus->of_node); of_node_put(dai_link->codecs->of_node); } return 0; @@ -519,6 +508,8 @@ int asoc_simple_init_priv(struct asoc_simple_priv *priv, * simple-card-utils.c :: asoc_simple_canonicalize_platform() */ for (i = 0; i < li->link; i++) { + dai_link[i].cpus = &dai_props[i].cpus; + dai_link[i].num_cpus = 1; dai_link[i].codecs = &dai_props[i].codecs; dai_link[i].num_codecs = 1; dai_link[i].platforms = &dai_props[i].platforms; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 335ead0..200a9c9 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -30,8 +30,6 @@ static const struct snd_soc_ops simple_ops = {
static int asoc_simple_parse_dai(struct device_node *node, struct snd_soc_dai_link_component *dlc, - struct device_node **dai_of_node, - const char **dai_name, int *is_single_link) { struct of_phandle_args args; @@ -41,17 +39,6 @@ static int asoc_simple_parse_dai(struct device_node *node, return 0;
/* - * Use snd_soc_dai_link_component instead of legacy style. - * It is only for codec, but cpu will be supported in the future. - * see - * soc-core.c :: snd_soc_init_multicodec() - */ - if (dlc) { - dai_name = &dlc->dai_name; - dai_of_node = &dlc->of_node; - } - - /* * Get node via "sound-dai = <&phandle port>" * it will be used as xxx_of_node on soc_bind_dai_link() */ @@ -60,13 +47,11 @@ static int asoc_simple_parse_dai(struct device_node *node, return ret;
/* Get dai->name */ - if (dai_name) { - ret = snd_soc_of_get_dai_name(node, dai_name); - if (ret < 0) - return ret; - } + ret = snd_soc_of_get_dai_name(node, &dlc->dai_name); + if (ret < 0) + return ret;
- *dai_of_node = args.np; + dlc->of_node = args.np;
if (is_single_link) *is_single_link = !args.args_count; @@ -119,6 +104,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_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 asoc_simple_dai *dai; + struct snd_soc_dai_link_component *cpus = dai_link->cpus; struct snd_soc_dai_link_component *codecs = dai_link->codecs; struct device_node *top = dev->of_node; struct device_node *node = of_get_parent(np); @@ -169,7 +155,7 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv,
ret = asoc_simple_set_dailink_name(dev, dai_link, "fe.%s", - dai_link->cpu_dai_name); + cpus->dai_name); if (ret < 0) return ret;
@@ -178,9 +164,9 @@ static int simple_dai_link_of_dpcm(struct asoc_simple_priv *priv, struct snd_soc_codec_conf *cconf;
/* FE is dummy */ - dai_link->cpu_of_node = NULL; - dai_link->cpu_dai_name = "snd-soc-dummy-dai"; - dai_link->cpu_name = "snd-soc-dummy"; + cpus->of_node = NULL; + cpus->dai_name = "snd-soc-dummy-dai"; + cpus->name = "snd-soc-dummy";
/* BE settings */ dai_link->no_pcm = 1; @@ -320,7 +306,7 @@ static int simple_dai_link_of(struct asoc_simple_priv *priv,
ret = asoc_simple_set_dailink_name(dev, dai_link, "%s-%s", - dai_link->cpu_dai_name, + dai_link->cpus->dai_name, dai_link->codecs->dai_name); if (ret < 0) goto dai_link_of_err; @@ -642,6 +628,7 @@ static int simple_probe(struct platform_device *pdev)
} else { struct asoc_simple_card_info *cinfo; + struct snd_soc_dai_link_component *cpus; struct snd_soc_dai_link_component *codecs; struct snd_soc_dai_link_component *platform; struct snd_soc_dai_link *dai_link = priv->dai_link; @@ -667,6 +654,9 @@ static int simple_probe(struct platform_device *pdev) dai_props->cpu_dai = &priv->dais[dai_idx++]; dai_props->codec_dai = &priv->dais[dai_idx++];
+ cpus = dai_link->cpus; + cpus->dai_name = cinfo->cpu_dai.name; + codecs = dai_link->codecs; codecs->name = cinfo->codec; codecs->dai_name = cinfo->codec_dai.name; @@ -677,7 +667,6 @@ static int simple_probe(struct platform_device *pdev) card->name = (cinfo->card) ? cinfo->card : cinfo->name; dai_link->name = cinfo->name; dai_link->stream_name = cinfo->name; - dai_link->cpu_dai_name = cinfo->cpu_dai.name; dai_link->dai_fmt = cinfo->daifmt; dai_link->init = asoc_simple_dai_init; memcpy(dai_props->cpu_dai, &cinfo->cpu_dai,
Modern style dai_link requests CPU/Codec/Platform component pointer array and its size, but it will be very verbose code. To avoid such scene, this patch adds dai_link connection macro.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 23463ba..fdde69a 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1043,6 +1043,63 @@ struct snd_soc_dai_link { ((i) < link->num_codecs) && ((codec) = &link->codecs[i]); \ (i)++)
+/* + * Sample 1 : Single CPU/Codec/Platform + * + * SND_SOC_DAI_LINK_CCP(single_dsp, + * SND_SOC_DAI_LINK(SDL_CPU("xxx")), + * SND_SOC_DAI_LINK(SDL_CODEC("xxx", "xxx")), + * SND_SOC_DAI_LINK(SDL_PLATFORM("xxx"))); + * + * static struct snd_soc_dai_link xxx = { + * ... + * SND_SOC_LINK_CCP(single_dsp), + * ... + * }; + * + * Sample 2 : Multi CPU/Codec, no Platform + * + * SND_SOC_DAI_LINK_CC(multi_dsp, + * SND_SOC_DAI_LINK(SDL_CPU("xxx"), + * SDL_CPU("xxx")), + * SND_SOC_DAI_LINK(SDL_CODEC("xxx", "xxx"), + * SDL_CODEC("xxx", "xxx"))) + * + * static struct snd_soc_dai_link xxx = { + * ... + * SND_SOC_LINK_CC(multi_dsp), + * ... + * }; + */ +#define SND_SOC_DAI_LINK_CC(name, cpu, codec)\ + static struct snd_soc_dai_link_component name##_cpu[] = cpu;\ + static struct snd_soc_dai_link_component name##_codec[] = codec + +#define SND_SOC_DAI_LINK_CCP(name, cpu, codec, platform)\ + static struct snd_soc_dai_link_component name##_cpu[] = cpu;\ + static struct snd_soc_dai_link_component name##_codec[] = codec;\ + static struct snd_soc_dai_link_component name##_platform[] = platform + +#define SND_SOC_DAI_LINK(param...) { param } +#define SDL_EMPTY() { } +#define SDL_CPU(_dai) { .dai_name = _dai, } +#define SDL_CODEC(_name, _dai) { .name = _name, .dai_name = _dai, } +#define SDL_PLATFORM(_name) { .name = _name } + +#define SND_SOC_LINK_CC(name) \ + .cpus = name##_cpu, \ + .num_cpus = ARRAY_SIZE(name##_cpu), \ + .codecs = name##_codec, \ + .num_codecs = ARRAY_SIZE(name##_codec) + +#define SND_SOC_LINK_CCP(name) \ + .cpus = name##_cpu, \ + .num_cpus = ARRAY_SIZE(name##_cpu), \ + .codecs = name##_codec, \ + .num_codecs = ARRAY_SIZE(name##_codec), \ + .platforms = name##_platform, \ + .num_platforms = ARRAY_SIZE(name##_platform) + struct snd_soc_codec_conf { /* * specify device either by device name, or by
ASoC is now supporting modern style dai_link (= snd_soc_dai_link_component) for CPU/Codec/Platform. This patch switches to use it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/samsung/smdk_wm8580.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/sound/soc/samsung/smdk_wm8580.c b/sound/soc/samsung/smdk_wm8580.c index 6e4dfa7..74cfd8a 100644 --- a/sound/soc/samsung/smdk_wm8580.c +++ b/sound/soc/samsung/smdk_wm8580.c @@ -147,27 +147,31 @@ enum { #define SMDK_DAI_FMT (SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | \ SND_SOC_DAIFMT_CBM_CFM)
+SND_SOC_DAI_LINK_CCP(paif_rx, + SND_SOC_DAI_LINK(SDL_CPU("samsung-i2s.2")), + SND_SOC_DAI_LINK(SDL_CODEC("wm8580.0-001b", "wm8580-hifi-playback")), + SND_SOC_DAI_LINK(SDL_PLATFORM("samsung-i2s.0"))); + +SND_SOC_DAI_LINK_CCP(paif_tx, + SND_SOC_DAI_LINK(SDL_CPU("samsung-i2s.2")), + SND_SOC_DAI_LINK(SDL_CODEC("wm8580.0-001b", "wm8580-hifi-capture")), + SND_SOC_DAI_LINK(SDL_PLATFORM("samsung-i2s.0"))); + static struct snd_soc_dai_link smdk_dai[] = { [PRI_PLAYBACK] = { /* Primary Playback i/f */ .name = "WM8580 PAIF RX", .stream_name = "Playback", - .cpu_dai_name = "samsung-i2s.2", - .codec_dai_name = "wm8580-hifi-playback", - .platform_name = "samsung-i2s.0", - .codec_name = "wm8580.0-001b", .dai_fmt = SMDK_DAI_FMT, .ops = &smdk_ops, + SND_SOC_LINK_CCP(paif_rx), }, [PRI_CAPTURE] = { /* Primary Capture i/f */ .name = "WM8580 PAIF TX", .stream_name = "Capture", - .cpu_dai_name = "samsung-i2s.2", - .codec_dai_name = "wm8580-hifi-capture", - .platform_name = "samsung-i2s.0", - .codec_name = "wm8580.0-001b", .dai_fmt = SMDK_DAI_FMT, .init = smdk_wm8580_init_paiftx, .ops = &smdk_ops, + SND_SOC_LINK_CCP(paif_tx), }, };
ASoC is now supporting modern style dai_link (= snd_soc_dai_link_component) for CPU/Codec/Platform. This patch switches to use it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/samsung/smdk_wm8994.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/sound/soc/samsung/smdk_wm8994.c b/sound/soc/samsung/smdk_wm8994.c index ff57b19..f090a08 100644 --- a/sound/soc/samsung/smdk_wm8994.c +++ b/sound/soc/samsung/smdk_wm8994.c @@ -107,28 +107,32 @@ static int smdk_wm8994_init_paiftx(struct snd_soc_pcm_runtime *rtd) return 0; }
+SND_SOC_DAI_LINK_CCP(aif1, + SND_SOC_DAI_LINK(SDL_CPU("samsung-i2s.0")), + SND_SOC_DAI_LINK(SDL_CODEC("wm8994-codec", "wm8994-aif1")), + SND_SOC_DAI_LINK(SDL_PLATFORM("samsung-i2s.0"))); + +SND_SOC_DAI_LINK_CCP(fifo_tx, + SND_SOC_DAI_LINK(SDL_CPU("samsung-i2s-sec")), + SND_SOC_DAI_LINK(SDL_CODEC("wm8994-codec", "wm8994-aif1")), + SND_SOC_DAI_LINK(SDL_PLATFORM("samsung-i2s-sec"))); + static struct snd_soc_dai_link smdk_dai[] = { { /* Primary DAI i/f */ .name = "WM8994 AIF1", .stream_name = "Pri_Dai", - .cpu_dai_name = "samsung-i2s.0", - .codec_dai_name = "wm8994-aif1", - .platform_name = "samsung-i2s.0", - .codec_name = "wm8994-codec", .init = smdk_wm8994_init_paiftx, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM, .ops = &smdk_ops, + SND_SOC_LINK_CCP(aif1), }, { /* Sec_Fifo Playback i/f */ .name = "Sec_FIFO TX", .stream_name = "Sec_Dai", - .cpu_dai_name = "samsung-i2s-sec", - .codec_dai_name = "wm8994-aif1", - .platform_name = "samsung-i2s-sec", - .codec_name = "wm8994-codec", .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM, .ops = &smdk_ops, + SND_SOC_LINK_CCP(fifo_tx), }, };
@@ -160,17 +164,17 @@ static int smdk_audio_probe(struct platform_device *pdev) return -ENOMEM;
if (np) { - smdk_dai[0].cpu_dai_name = NULL; - smdk_dai[0].cpu_of_node = of_parse_phandle(np, + smdk_dai[0].cpus->dai_name = NULL; + smdk_dai[0].cpus->of_node = of_parse_phandle(np, "samsung,i2s-controller", 0); - if (!smdk_dai[0].cpu_of_node) { + if (!smdk_dai[0].cpus->of_node) { dev_err(&pdev->dev, "Property 'samsung,i2s-controller' missing or invalid\n"); ret = -EINVAL; }
- smdk_dai[0].platform_name = NULL; - smdk_dai[0].platform_of_node = smdk_dai[0].cpu_of_node; + smdk_dai[0].platforms->name = NULL; + smdk_dai[0].platforms->of_node = smdk_dai[0].cpus->of_node; }
id = of_match_device(of_match_ptr(samsung_wm8994_of_match), &pdev->dev);
ASoC is now supporting modern style dai_link (= snd_soc_dai_link_component) for CPU/Codec/Platform. This patch switches to use it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c | 85 +++++++++++++----------- 1 file changed, 48 insertions(+), 37 deletions(-)
diff --git a/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c b/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c index d83cd03..81cbb96 100644 --- a/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c +++ b/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c @@ -111,14 +111,6 @@ static int mt8173_rt5650_rt5676_init(struct snd_soc_pcm_runtime *runtime) &mt8173_rt5650_rt5676_jack); }
-static struct snd_soc_dai_link_component mt8173_rt5650_rt5676_codecs[] = { - { - .dai_name = "rt5645-aif1", - }, - { - .dai_name = "rt5677-aif1", - }, -};
enum { DAI_LINK_PLAYBACK, @@ -129,47 +121,69 @@ enum { DAI_LINK_INTERCODEC };
+SND_SOC_DAI_LINK_CCP(playback, + SND_SOC_DAI_LINK(SDL_CPU("DL1")), + SND_SOC_DAI_LINK(SDL_DUMMY()), + SND_SOC_DAI_LINK(SDL_EMPTY())); + +SND_SOC_DAI_LINK_CCP(capture, + SND_SOC_DAI_LINK(SDL_CPU("VUL")), + SND_SOC_DAI_LINK(SDL_DUMMY()), + SND_SOC_DAI_LINK(SDL_EMPTY())); + +SND_SOC_DAI_LINK_CCP(hdmi_pcm, + SND_SOC_DAI_LINK(SDL_CPU("HDMI")), + SND_SOC_DAI_LINK(SDL_DUMMY()), + SND_SOC_DAI_LINK(SDL_EMPTY())); + +SND_SOC_DAI_LINK_CCP(codec, + SND_SOC_DAI_LINK(SDL_CPU("I2S")), + SND_SOC_DAI_LINK(SDL_CODEC(NULL, "rt5645-aif1"), + SDL_CODEC(NULL, "rt5677-aif1")), + SND_SOC_DAI_LINK(SDL_EMPTY())); + +SND_SOC_DAI_LINK_CCP(hdmi_be, + SND_SOC_DAI_LINK(SDL_CPU("HDMIO")), + SND_SOC_DAI_LINK(SDL_CODEC(NULL, "i2s-hifi")), + SND_SOC_DAI_LINK(SDL_EMPTY())); + +SND_SOC_DAI_LINK_CCP(intercodec, + SND_SOC_DAI_LINK(SDL_DUMMY()), + SND_SOC_DAI_LINK(SDL_CODEC(NULL, "rt5677-aif2")), + SND_SOC_DAI_LINK(SDL_DUMMY())); + /* Digital audio interface glue - connects codec <---> CPU */ static struct snd_soc_dai_link mt8173_rt5650_rt5676_dais[] = { /* Front End DAI links */ [DAI_LINK_PLAYBACK] = { .name = "rt5650_rt5676 Playback", .stream_name = "rt5650_rt5676 Playback", - .cpu_dai_name = "DL1", - .codec_name = "snd-soc-dummy", - .codec_dai_name = "snd-soc-dummy-dai", .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, .dynamic = 1, .dpcm_playback = 1, + SND_SOC_LINK_CCP(playback), }, [DAI_LINK_CAPTURE] = { .name = "rt5650_rt5676 Capture", .stream_name = "rt5650_rt5676 Capture", - .cpu_dai_name = "VUL", - .codec_name = "snd-soc-dummy", - .codec_dai_name = "snd-soc-dummy-dai", .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, .dynamic = 1, .dpcm_capture = 1, + SND_SOC_LINK_CCP(capture), }, [DAI_LINK_HDMI] = { .name = "HDMI", .stream_name = "HDMI PCM", - .cpu_dai_name = "HDMI", - .codec_name = "snd-soc-dummy", - .codec_dai_name = "snd-soc-dummy-dai", .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, .dynamic = 1, .dpcm_playback = 1, + SND_SOC_LINK_CCP(hdmi_pcm), },
/* Back End DAI links */ [DAI_LINK_CODEC_I2S] = { .name = "Codec", - .cpu_dai_name = "I2S", .no_pcm = 1, - .codecs = mt8173_rt5650_rt5676_codecs, - .num_codecs = 2, .init = mt8173_rt5650_rt5676_init, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, @@ -177,26 +191,23 @@ static struct snd_soc_dai_link mt8173_rt5650_rt5676_dais[] = { .ignore_pmdown_time = 1, .dpcm_playback = 1, .dpcm_capture = 1, + SND_SOC_LINK_CCP(codec), }, [DAI_LINK_HDMI_I2S] = { .name = "HDMI BE", - .cpu_dai_name = "HDMIO", .no_pcm = 1, - .codec_dai_name = "i2s-hifi", .dpcm_playback = 1, + SND_SOC_LINK_CCP(hdmi_be), }, /* rt5676 <-> rt5650 intercodec link: Sets rt5676 I2S2 as master */ [DAI_LINK_INTERCODEC] = { .name = "rt5650_rt5676 intercodec", .stream_name = "rt5650_rt5676 intercodec", - .cpu_dai_name = "snd-soc-dummy-dai", - .platform_name = "snd-soc-dummy", .no_pcm = 1, - .codec_dai_name = "rt5677-aif2", .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM, + SND_SOC_LINK_CCP(intercodec), }, - };
static struct snd_soc_codec_conf mt8173_rt5650_rt5676_codec_conf[] = { @@ -235,34 +246,34 @@ static int mt8173_rt5650_rt5676_dev_probe(struct platform_device *pdev) }
for_each_card_prelinks(card, i, dai_link) { - if (dai_link->platform_name) + if (dai_link->platforms->name) continue; - dai_link->platform_of_node = platform_node; + dai_link->platforms->of_node = platform_node; }
- mt8173_rt5650_rt5676_codecs[0].of_node = + mt8173_rt5650_rt5676_dais[DAI_LINK_CODEC_I2S].codecs[0].of_node = of_parse_phandle(pdev->dev.of_node, "mediatek,audio-codec", 0); - if (!mt8173_rt5650_rt5676_codecs[0].of_node) { + if (!mt8173_rt5650_rt5676_dais[DAI_LINK_CODEC_I2S].codecs[0].of_node) { dev_err(&pdev->dev, "Property 'audio-codec' missing or invalid\n"); return -EINVAL; } - mt8173_rt5650_rt5676_codecs[1].of_node = + mt8173_rt5650_rt5676_dais[DAI_LINK_CODEC_I2S].codecs[1].of_node = of_parse_phandle(pdev->dev.of_node, "mediatek,audio-codec", 1); - if (!mt8173_rt5650_rt5676_codecs[1].of_node) { + if (!mt8173_rt5650_rt5676_dais[DAI_LINK_CODEC_I2S].codecs[1].of_node) { dev_err(&pdev->dev, "Property 'audio-codec' missing or invalid\n"); return -EINVAL; } mt8173_rt5650_rt5676_codec_conf[0].of_node = - mt8173_rt5650_rt5676_codecs[1].of_node; + mt8173_rt5650_rt5676_dais[DAI_LINK_CODEC_I2S].codecs[1].of_node;
- mt8173_rt5650_rt5676_dais[DAI_LINK_INTERCODEC].codec_of_node = - mt8173_rt5650_rt5676_codecs[1].of_node; + mt8173_rt5650_rt5676_dais[DAI_LINK_INTERCODEC].codecs->of_node = + mt8173_rt5650_rt5676_dais[DAI_LINK_CODEC_I2S].codecs[1].of_node;
- mt8173_rt5650_rt5676_dais[DAI_LINK_HDMI_I2S].codec_of_node = + mt8173_rt5650_rt5676_dais[DAI_LINK_HDMI_I2S].codecs->of_node = of_parse_phandle(pdev->dev.of_node, "mediatek,audio-codec", 2); - if (!mt8173_rt5650_rt5676_dais[DAI_LINK_HDMI_I2S].codec_of_node) { + if (!mt8173_rt5650_rt5676_dais[DAI_LINK_HDMI_I2S].codecs->of_node) { dev_err(&pdev->dev, "Property 'audio-codec' missing or invalid\n"); return -EINVAL;
ASoC is now supporting modern style dai_link (= snd_soc_dai_link_component) for CPU/Codec/Platform. This patch switches to use it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/mediatek/mt8173/mt8173-rt5650-rt5514.c | 51 ++++++++++++------------ 1 file changed, 26 insertions(+), 25 deletions(-)
diff --git a/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5514.c b/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5514.c index da5b58c..ccd2262 100644 --- a/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5514.c +++ b/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5514.c @@ -98,51 +98,51 @@ static int mt8173_rt5650_rt5514_init(struct snd_soc_pcm_runtime *runtime) &mt8173_rt5650_rt5514_jack); }
-static struct snd_soc_dai_link_component mt8173_rt5650_rt5514_codecs[] = { - { - .dai_name = "rt5645-aif1", - }, - { - .dai_name = "rt5514-aif1", - }, -}; - enum { DAI_LINK_PLAYBACK, DAI_LINK_CAPTURE, DAI_LINK_CODEC_I2S, };
+SND_SOC_DAI_LINK_CCP(playback, + SND_SOC_DAI_LINK(SDL_CPU("DL1")), + SND_SOC_DAI_LINK(SDL_DUMMY()), + SND_SOC_DAI_LINK(SDL_EMPTY())); + +SND_SOC_DAI_LINK_CCP(capture, + SND_SOC_DAI_LINK(SDL_CPU("VUL")), + SND_SOC_DAI_LINK(SDL_DUMMY()), + SND_SOC_DAI_LINK(SDL_EMPTY())); + +SND_SOC_DAI_LINK_CCP(codec, + SND_SOC_DAI_LINK(SDL_CPU("I2S")), + SND_SOC_DAI_LINK(SDL_CODEC(NULL, "rt5645-aif1"), + SDL_CODEC(NULL, "rt5514-aif1")), + SND_SOC_DAI_LINK(SDL_EMPTY())); + /* Digital audio interface glue - connects codec <---> CPU */ static struct snd_soc_dai_link mt8173_rt5650_rt5514_dais[] = { /* Front End DAI links */ [DAI_LINK_PLAYBACK] = { .name = "rt5650_rt5514 Playback", .stream_name = "rt5650_rt5514 Playback", - .cpu_dai_name = "DL1", - .codec_name = "snd-soc-dummy", - .codec_dai_name = "snd-soc-dummy-dai", .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, .dynamic = 1, .dpcm_playback = 1, + SND_SOC_LINK_CCP(playback), }, [DAI_LINK_CAPTURE] = { .name = "rt5650_rt5514 Capture", .stream_name = "rt5650_rt5514 Capture", - .cpu_dai_name = "VUL", - .codec_name = "snd-soc-dummy", - .codec_dai_name = "snd-soc-dummy-dai", .trigger = {SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST}, .dynamic = 1, .dpcm_capture = 1, + SND_SOC_LINK_CCP(capture), }, /* Back End DAI links */ [DAI_LINK_CODEC_I2S] = { .name = "Codec", - .cpu_dai_name = "I2S", .no_pcm = 1, - .codecs = mt8173_rt5650_rt5514_codecs, - .num_codecs = 2, .init = mt8173_rt5650_rt5514_init, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, @@ -150,6 +150,7 @@ static struct snd_soc_dai_link mt8173_rt5650_rt5514_dais[] = { .ignore_pmdown_time = 1, .dpcm_playback = 1, .dpcm_capture = 1, + SND_SOC_LINK_CCP(codec), }, };
@@ -189,27 +190,27 @@ static int mt8173_rt5650_rt5514_dev_probe(struct platform_device *pdev) }
for_each_card_prelinks(card, i, dai_link) { - if (dai_link->platform_name) + if (dai_link->platforms->name) continue; - dai_link->platform_of_node = platform_node; + dai_link->platforms->of_node = platform_node; }
- mt8173_rt5650_rt5514_codecs[0].of_node = + mt8173_rt5650_rt5514_dais[DAI_LINK_CODEC_I2S].codecs[0].of_node = of_parse_phandle(pdev->dev.of_node, "mediatek,audio-codec", 0); - if (!mt8173_rt5650_rt5514_codecs[0].of_node) { + if (!mt8173_rt5650_rt5514_dais[DAI_LINK_CODEC_I2S].codecs[0].of_node) { dev_err(&pdev->dev, "Property 'audio-codec' missing or invalid\n"); return -EINVAL; } - mt8173_rt5650_rt5514_codecs[1].of_node = + mt8173_rt5650_rt5514_dais[DAI_LINK_CODEC_I2S].codecs[1].of_node = of_parse_phandle(pdev->dev.of_node, "mediatek,audio-codec", 1); - if (!mt8173_rt5650_rt5514_codecs[1].of_node) { + if (!mt8173_rt5650_rt5514_dais[DAI_LINK_CODEC_I2S].codecs[1].of_node) { dev_err(&pdev->dev, "Property 'audio-codec' missing or invalid\n"); return -EINVAL; } mt8173_rt5650_rt5514_codec_conf[0].of_node = - mt8173_rt5650_rt5514_codecs[1].of_node; + mt8173_rt5650_rt5514_dais[DAI_LINK_CODEC_I2S].codecs[1].of_node;
card->dev = &pdev->dev;
On Mon, Apr 08, 2019 at 11:31:01AM +0900, Kuninori Morimoto wrote:
Hi Mark
These are RFC of modern style CPU support.
modern style dai_link needs CPU/Codec/Platform array and its size, but it will be very ugly code. To avoid it, 3) adds macro.
- are sample code for switching to modern style dai_link.
4), 5) are good sample for single CPU/Codec/Platform. 6), 7) are good sample for multi Codec.
Almost all sound cards will get similar patch. I think these can work, but I can't test these. So, it is very useful if someone can test these. If these are all OK (= test/review), I can post patch-set for all sound cards.
This looks basically fine to me, hopefully someone will be able to help out with the testing - thanks for your persistence with this work!
Hi Mark
These are RFC of modern style CPU support.
modern style dai_link needs CPU/Codec/Platform array and its size, but it will be very ugly code. To avoid it, 3) adds macro.
- are sample code for switching to modern style dai_link.
4), 5) are good sample for single CPU/Codec/Platform. 6), 7) are good sample for multi Codec.
Almost all sound cards will get similar patch. I think these can work, but I can't test these. So, it is very useful if someone can test these. If these are all OK (= test/review), I can post patch-set for all sound cards.
This looks basically fine to me, hopefully someone will be able to help out with the testing - thanks for your persistence with this work!
Thanks. I will post these patches at this or next week
Thank you for your help !!
Best regards --- Kuninori Morimoto
Hi Pierre-Louis
I have 1 question about modern style
These are RFC of modern style CPU support.
modern style dai_link needs CPU/Codec/Platform array and its size, but it will be very ugly code. To avoid it, 3) adds macro.
- are sample code for switching to modern style dai_link.
4), 5) are good sample for single CPU/Codec/Platform. 6), 7) are good sample for multi Codec.
Almost all sound cards will get similar patch. I think these can work, but I can't test these. So, it is very useful if someone can test these. If these are all OK (= test/review), I can post patch-set for all sound cards.
This looks basically fine to me, hopefully someone will be able to help out with the testing - thanks for your persistence with this work!
I know you are already working for it for Intel
https://github.com/plbossart/sound/commits/fix/codec-legacy-dai
and, it will be conflict to my posted patch style.
https://patchwork.kernel.org/patch/10888741/ https://patchwork.kernel.org/patch/10888745/
So, I can avoid to posting patch for Intel, then, you need to update it again for modern style CPU. Or, I can update Intel by using above macro. What is your opinion ?
Thank you for your help !!
Best regards --- Kuninori Morimoto
I know you are already working for it for Intel
https://github.com/plbossart/sound/commits/fix/codec-legacy-dai
and, it will be conflict to my posted patch style.
https://patchwork.kernel.org/patch/10888741/ https://patchwork.kernel.org/patch/10888745/
So, I can avoid to posting patch for Intel, then, you need to update it again for modern style CPU. Or, I can update Intel by using above macro. What is your opinion ?
I must admit I didn't look at the new style. With all due respect I am not sold, it's becoming a lot less intuitive and readable, e.g.:
+SND_SOC_DAI_LINK_CCP(aif1, + SND_SOC_DAI_LINK(SDL_CPU("samsung-i2s.0")), + SND_SOC_DAI_LINK(SDL_CODEC("wm8994-codec", "wm8994-aif1")), + SND_SOC_DAI_LINK(SDL_PLATFORM("samsung-i2s.0"))); + +SND_SOC_DAI_LINK_CCP(fifo_tx, + SND_SOC_DAI_LINK(SDL_CPU("samsung-i2s-sec")), + SND_SOC_DAI_LINK(SDL_CODEC("wm8994-codec", "wm8994-aif1")), + SND_SOC_DAI_LINK(SDL_PLATFORM("samsung-i2s-sec"))); + static struct snd_soc_dai_link smdk_dai[] = { { /* Primary DAI i/f */ .name = "WM8994 AIF1", .stream_name = "Pri_Dai", - .cpu_dai_name = "samsung-i2s.0", - .codec_dai_name = "wm8994-aif1", - .platform_name = "samsung-i2s.0", - .codec_name = "wm8994-codec", .init = smdk_wm8994_init_paiftx, .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM, .ops = &smdk_ops, + SND_SOC_LINK_CCP(aif1),
is this really the new direction? Even the acronyms are not simple, it took me 15mn to figure out that CCP stood for CPU/Codec/Platform and I couldn't figure out what SDL means. The multiple repetitions of SND_SOC_DAI_LINK is also misleading, it's just a property of the *same* dailink that you handle.
I am even more nervous since we have a need to explicitly some cpu and codec dai names depending on quirks, with the additional abstraction it'll become plain unreadable - or we need new helpers.
Hi xxx
Hi Pierre-Louis
Thank you for your feedback
+SND_SOC_DAI_LINK_CCP(aif1,
- SND_SOC_DAI_LINK(SDL_CPU("samsung-i2s.0")),
- SND_SOC_DAI_LINK(SDL_CODEC("wm8994-codec", "wm8994-aif1")),
- SND_SOC_DAI_LINK(SDL_PLATFORM("samsung-i2s.0")));
(snip)
static struct snd_soc_dai_link smdk_dai[] = { { /* Primary DAI i/f */ .name = "WM8994 AIF1", .stream_name = "Pri_Dai",
.cpu_dai_name = "samsung-i2s.0",.codec_dai_name = "wm8994-aif1",.platform_name = "samsung-i2s.0",.codec_name = "wm8994-codec",
SND_SOC_LINK_CCP(aif1),is this really the new direction? Even the acronyms are not simple, it took me 15mn to figure out that CCP stood for CPU/Codec/Platform and I couldn't figure out what SDL means.
The reason of "CCP" (= CPU/CODEC/PLATFORM) was to avoid long naming. and SDL is acronyms of "Snd soc Dai Link". but yes, it is un-understandable. It should be more understandable (and possibly short) naming.
The multiple repetitions of SND_SOC_DAI_LINK is also misleading, it's just a property of the *same* dailink that you handle.
it is needed to handle both single/multi CPU/Codec/Platform. But yes, it has naming issue.
I am even more nervous since we have a need to explicitly some cpu and codec dai names depending on quirks, with the additional abstraction it'll become plain unreadable - or we need new helpers.
Hmm, OK. I will reconsider about macro. I'm happy if you can review it again.
Thank you for your help !!
Best regards --- Kuninori Morimoto
On Wed, Apr 17, 2019 at 03:26:48PM +0900, Kuninori Morimoto wrote:
+SND_SOC_DAI_LINK_CCP(aif1,
- SND_SOC_DAI_LINK(SDL_CPU("samsung-i2s.0")),
- SND_SOC_DAI_LINK(SDL_CODEC("wm8994-codec", "wm8994-aif1")),
- SND_SOC_DAI_LINK(SDL_PLATFORM("samsung-i2s.0")));
is this really the new direction? Even the acronyms are not simple, it took me 15mn to figure out that CCP stood for CPU/Codec/Platform and I couldn't figure out what SDL means.
The reason of "CCP" (= CPU/CODEC/PLATFORM) was to avoid long naming. and SDL is acronyms of "Snd soc Dai Link". but yes, it is un-understandable. It should be more understandable (and possibly short) naming.
I think you're right to want a shorter name but Pierre's probably right that that particular name isn't the best... I'm thinking it might be good to look at the fully expanded definitions and then work back from there, or perhaps just stick with them for a while (since the compact forms are hopefully not a requirement for legibility, even if they are a useful improvement).
Hi Mark
I think you're right to want a shorter name but Pierre's probably right that that particular name isn't the best... I'm thinking it might be good to look at the fully expanded definitions and then work back from there, or perhaps just stick with them for a while (since the compact forms are hopefully not a requirement for legibility, even if they are a useful improvement).
OK. I'm happy to consider about it.
Thank you for your help !!
Best regards --- Kuninori Morimoto
Hi Mark, Pierre-Louis
I think you're right to want a shorter name but Pierre's probably right that that particular name isn't the best... I'm thinking it might be good to look at the fully expanded definitions and then work back from there, or perhaps just stick with them for a while (since the compact forms are hopefully not a requirement for legibility, even if they are a useful improvement).
These are proposal of modern style macro v2. Any opinions are welcome.
v1 -> v2 - Naming fixup - No difference between with / without Platform (= no more xx_CC and xx_CCP) - It can handle each CPU/Codec/Platform manually (if you want)
/* Macro */ #define SND_DAILINK_REG(cpu, codec, platform) \ .cpus = cpu, \ .num_cpus = ARRAY_SIZE(cpu), \ .codecs = codec, \ .num_codecs = ARRAY_SIZE(codec), \ .platforms = platform, \ .num_platforms = ARRAY_SIZE(platform)
#define SND_DAILINK_REGS(name) \ .cpus = name##_cpus, \ .num_cpus = ARRAY_SIZE(name##_cpus), \ .codecs = name##_codecs, \ .num_codecs = ARRAY_SIZE(name##_codecs), \ .platforms = name##_platforms, \ .num_platforms = ARRAY_SIZE(name##_platforms)
#define SND_DAILINK_DEF(name, def...) \ static struct snd_soc_dai_link_component name[] = def
#define SND_DAILINK_DEFS(name, cpu, codec, platform) \ SND_DAILINK_DEF(name##_cpus, cpu); \ SND_DAILINK_DEF(name##_codecs, codec); \ SND_DAILINK_DEF(name##_platforms, platform)
#define DAILINK_COMPONENT_ARRAY(param...) { param } #define CPU_COMPONENT(_dai) { .dai_name = _dai, } #define CODEC_COMPONENT(_name, _dai) { .name = _name, .dai_name = _dai, } #define PLATFORM_COMPONENT(_name) { .name = _name } #define NO_DAILINK_COMPONENT() { }
struct snd_soc_dai_link_component null_dailink_component[0];
/* Sample1. Single CPU/Codec/Platform */
SND_DAILINK_DEFS(test, DAILINK_COMPONENT_ARRAY(CPU_COMPONENT("cpu_dai")), DAILINK_COMPONENT_ARRAY(CODEC_COMPONENT("codec", "codec_dai")), DAILINK_COMPONENT_ARRAY(PLATFORM_COMPONENT("platform")));
struct snd_soc_dai_link link = { ... SND_DAILINK_REGS(test), };
/* Sample2. Multi CPU/Codec, No Platform */
SND_DAILINK_DEFS(test, DAILINK_COMPONENT_ARRAY(CPU_COMPONENT("cpu_dai1"), CPU_COMPONENT("cpu_dai2")), DAILINK_COMPONENT_ARRAY(CODEC_COMPONENT("codec1", "codec_dai1"), CODEC_COMPONENT("codec2", "codec_dai2")), NO_DAILINK_COMPONENT());
struct snd_soc_dai_link link = { ... SND_DAILINK_REGS(test), };
/* Sample3. Define each CPU/Codec/Platform manually */
SND_DAILINK_DEF(test_cpu, DAILINK_COMPONENT_ARRAY(CPU_COMPONENT("cpu_dai1"), CPU_COMPONENT("cpu_dai2"))); SND_DAILINK_DEF(test_codec, DAILINK_COMPONENT_ARRAY(CODEC_COMPONENT("codec1", "codec_dai1"), CODEC_COMPONENT("codec2", "codec_dai2"))); SND_DAILINK_DEF(test_platform, DAILINK_COMPONENT_ARRAY(PLATFORM_COMPONENT("platform")));
struct snd_soc_dai_link link = { ... SND_DAILINK_REG(test_cpu, test_codec, test_platform), };
/* Sample4. Sample3 + no platform */
struct snd_soc_dai_link link = { ... SND_DAILINK_REG(test_cpu, test_codec, null_dailink_component), };
Thank you for your help !!
Best regards --- Kuninori Morimoto
Hi Pierre-Louis
Thank you for your feedback
+SND_SOC_DAI_LINK_CCP(aif1,
- SND_SOC_DAI_LINK(SDL_CPU("samsung-i2s.0")),
- SND_SOC_DAI_LINK(SDL_CODEC("wm8994-codec", "wm8994-aif1")),
- SND_SOC_DAI_LINK(SDL_PLATFORM("samsung-i2s.0")));
(snip)
static struct snd_soc_dai_link smdk_dai[] = { { /* Primary DAI i/f */ .name = "WM8994 AIF1", .stream_name = "Pri_Dai",
.cpu_dai_name = "samsung-i2s.0",.codec_dai_name = "wm8994-aif1",.platform_name = "samsung-i2s.0",.codec_name = "wm8994-codec",
SND_SOC_LINK_CCP(aif1),is this really the new direction? Even the acronyms are not simple, it took me 15mn to figure out that CCP stood for CPU/Codec/Platform and I couldn't figure out what SDL means.
The reason of "CCP" (= CPU/CODEC/PLATFORM) was to avoid long naming. and SDL is acronyms of "Snd soc Dai Link". but yes, it is un-understandable. It should be more understandable (and possibly short) naming.
The multiple repetitions of SND_SOC_DAI_LINK is also misleading, it's just a property of the *same* dailink that you handle.
it is needed to handle both single/multi CPU/Codec/Platform. But yes, it has naming issue.
I am even more nervous since we have a need to explicitly some cpu and codec dai names depending on quirks, with the additional abstraction it'll become plain unreadable - or we need new helpers.
Hmm, OK. I will reconsider about macro. I'm happy if you can review it again.
Thank you for your help !!
Best regards --- Kuninori Morimoto
participants (3)
-
Kuninori Morimoto -
Mark Brown -
Pierre-Louis Bossart