2019-06-26 13:36:56

by Jerome Brunet

[permalink] [raw]
Subject: [PATCH 0/2] ASoC: soc-core: update dai_link init

My initial goal with this patchset was to allow a dai_link to have no
no platform component, instead of having dummy by default.

However, when rebasing, I discovered that Kuninori Morimoto had recently
done that in a different way :)

I am still submitting my change since it should allow multiple platform
components on a dai_link, which is one of the FIXME note in soc-core.

I have also added a check on the codecs component availability to align
on what was done for platforms and cpus

Jerome Brunet (2):
ASoC: soc-core: defer card registration if codec component is missing
ASoC: soc-core: support dai_link with platforms_num != 1

include/sound/soc.h | 6 ++++
sound/soc/soc-core.c | 72 +++++++++++++++++++++-----------------------
2 files changed, 41 insertions(+), 37 deletions(-)

--
2.21.0


2019-06-26 13:37:19

by Jerome Brunet

[permalink] [raw]
Subject: [PATCH 2/2] ASoC: soc-core: support dai_link with platforms_num != 1

Add support platforms_num != 1 in dai_link. Initially, the main purpose of
this change was to make the platform optional in the dai_link, instead of
inserting the dummy platform driver.

This particular case had just been solved by Kuninori Morimoto with
commit 1d7689892878 ("ASoC: soc-core: allow no Platform on dai_link").

However, this change may still be useful for those who need multiple
platform components on a single dai_link (it solves one of the FIXME
note in soc-core)

Cc: Kuninori Morimoto <[email protected]>
Signed-off-by: Jerome Brunet <[email protected]>
---
include/sound/soc.h | 6 ++++
sound/soc/soc-core.c | 66 +++++++++++++++++++-------------------------
2 files changed, 34 insertions(+), 38 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 64405cdab8bb..4e8071269639 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -997,6 +997,12 @@ struct snd_soc_dai_link {
((i) < link->num_codecs) && ((codec) = &link->codecs[i]); \
(i)++)

+#define for_each_link_platforms(link, i, platform) \
+ for ((i) = 0; \
+ ((i) < link->num_platforms) && \
+ ((platform) = &link->platforms[i]); \
+ (i)++)
+
/*
* Sample 1 : Single CPU/Codec/Platform
*
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 002ddbf4e5a3..3053a4a461b3 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -887,7 +887,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card,
struct snd_soc_dai_link *dai_link)
{
struct snd_soc_pcm_runtime *rtd;
- struct snd_soc_dai_link_component *codecs;
+ struct snd_soc_dai_link_component *dlc;
struct snd_soc_component *component;
int i;

@@ -917,13 +917,14 @@ static int soc_bind_dai_link(struct snd_soc_card *card,

/* Find CODEC from registered CODECs */
rtd->num_codecs = dai_link->num_codecs;
- for_each_link_codecs(dai_link, i, codecs) {
- rtd->codec_dais[i] = snd_soc_find_dai(codecs);
+ for_each_link_codecs(dai_link, i, dlc) {
+ rtd->codec_dais[i] = snd_soc_find_dai(dlc);
if (!rtd->codec_dais[i]) {
dev_info(card->dev, "ASoC: CODEC DAI %s not registered\n",
- codecs->dai_name);
+ dlc->dai_name);
goto _err_defer;
}
+
snd_soc_rtdcom_add(rtd, rtd->codec_dais[i]->component);
}

@@ -931,12 +932,13 @@ static int soc_bind_dai_link(struct snd_soc_card *card,
rtd->codec_dai = rtd->codec_dais[0];

/* Find PLATFORM from registered PLATFORMs */
- for_each_component(component) {
- if (!snd_soc_is_matching_component(dai_link->platforms,
- component))
- continue;
+ for_each_link_platforms(dai_link, i, dlc) {
+ for_each_component(component) {
+ if (!snd_soc_is_matching_component(dlc, component))
+ continue;

- snd_soc_rtdcom_add(rtd, component);
+ snd_soc_rtdcom_add(rtd, component);
+ }
}

soc_add_pcm_runtime(card, rtd);
@@ -1051,22 +1053,22 @@ static int soc_init_dai_link(struct snd_soc_card *card,
struct snd_soc_dai_link *link)
{
int i;
- struct snd_soc_dai_link_component *codec;
+ struct snd_soc_dai_link_component *dlc;

- for_each_link_codecs(link, i, codec) {
+ for_each_link_codecs(link, i, dlc) {
/*
* Codec must be specified by 1 of name or OF node,
* not both or neither.
*/
- if (!!codec->name ==
- !!codec->of_node) {
+ if (!!dlc->name ==
+ !!dlc->of_node) {
dev_err(card->dev, "ASoC: Neither/both codec name/of_node are set for %s\n",
link->name);
return -EINVAL;
}

/* Codec DAI name must be specified */
- if (!codec->dai_name) {
+ if (!dlc->dai_name) {
dev_err(card->dev, "ASoC: codec_dai_name not set for %s\n",
link->name);
return -EINVAL;
@@ -1076,40 +1078,28 @@ static int soc_init_dai_link(struct snd_soc_card *card,
* Defer card registartion if codec component is not added to
* component list.
*/
- if (!soc_find_component(codec))
+ if (!soc_find_component(dlc))
return -EPROBE_DEFER;
}

- /*
- * Platform may be specified by either name or OF node,
- * or no Platform.
- *
- * FIXME
- *
- * We need multi-platform support
- */
- if (link->num_platforms > 0) {
-
- if (link->num_platforms > 1) {
- dev_err(card->dev,
- "ASoC: multi platform is not yet supported %s\n",
- link->name);
- return -EINVAL;
- }
-
- if (link->platforms->name && link->platforms->of_node) {
+ for_each_link_platforms(link, i, dlc) {
+ /*
+ * Platform may be specified by either name or OF node, but it
+ * can be left unspecified, then no components will be inserted
+ * in the rtdcom list
+ */
+ if (!!dlc->name == !!dlc->of_node) {
dev_err(card->dev,
- "ASoC: Both platform name/of_node are set for %s\n",
+ "ASoC: Neither/both platform name/of_node are set for %s\n",
link->name);
return -EINVAL;
}

/*
- * Defer card registartion if platform dai component is not
- * added to component list.
+ * Defer card registartion if platform component is not added to
+ * component list.
*/
- if ((link->platforms->of_node || link->platforms->name) &&
- !soc_find_component(link->platforms))
+ if (!soc_find_component(dlc))
return -EPROBE_DEFER;
}

--
2.21.0

2019-06-26 13:37:31

by Jerome Brunet

[permalink] [raw]
Subject: [PATCH 1/2] ASoC: soc-core: defer card registration if codec component is missing

Like cpus and platforms, defer sound card initialization if the codec
component is missing when initializing the dai_link

Signed-off-by: Jerome Brunet <[email protected]>
---
sound/soc/soc-core.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 358f1fbf9a30..002ddbf4e5a3 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1064,12 +1064,20 @@ static int soc_init_dai_link(struct snd_soc_card *card,
link->name);
return -EINVAL;
}
+
/* Codec DAI name must be specified */
if (!codec->dai_name) {
dev_err(card->dev, "ASoC: codec_dai_name not set for %s\n",
link->name);
return -EINVAL;
}
+
+ /*
+ * Defer card registartion if codec component is not added to
+ * component list.
+ */
+ if (!soc_find_component(codec))
+ return -EPROBE_DEFER;
}

/*
--
2.21.0

2019-06-26 21:21:30

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH 1/2] ASoC: soc-core: defer card registration if codec component is missing

On 6/26/19 3:36 PM, Jerome Brunet wrote:
> Like cpus and platforms, defer sound card initialization if the codec
> component is missing when initializing the dai_link
>
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
> sound/soc/soc-core.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 358f1fbf9a30..002ddbf4e5a3 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1064,12 +1064,20 @@ static int soc_init_dai_link(struct snd_soc_card *card,
> link->name);
> return -EINVAL;
> }
> +
> /* Codec DAI name must be specified */
> if (!codec->dai_name) {
> dev_err(card->dev, "ASoC: codec_dai_name not set for %s\n",
> link->name);
> return -EINVAL;
> }
> +
> + /*
> + * Defer card registartion if codec component is not added to

registration

> + * component list.
> + */
> + if (!soc_find_component(codec))
> + return -EPROBE_DEFER;
> }
>
> /*
>

2019-06-26 23:45:19

by Kuninori Morimoto

[permalink] [raw]
Subject: Re: [PATCH 2/2] ASoC: soc-core: support dai_link with platforms_num != 1


Hi Jerome

Thank you for your patch.
This is nice !

> Add support platforms_num != 1 in dai_link. Initially, the main purpose of
> this change was to make the platform optional in the dai_link, instead of
> inserting the dummy platform driver.
>
> This particular case had just been solved by Kuninori Morimoto with
> commit 1d7689892878 ("ASoC: soc-core: allow no Platform on dai_link").
>
> However, this change may still be useful for those who need multiple
> platform components on a single dai_link (it solves one of the FIXME
> note in soc-core)
>
> Cc: Kuninori Morimoto <[email protected]>
> Signed-off-by: Jerome Brunet <[email protected]>
> ---
(snip)
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 002ddbf4e5a3..3053a4a461b3 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -887,7 +887,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card,
> struct snd_soc_dai_link *dai_link)
> {
> struct snd_soc_pcm_runtime *rtd;
> - struct snd_soc_dai_link_component *codecs;
> + struct snd_soc_dai_link_component *dlc;
> struct snd_soc_component *component;
> int i;

This is nitpick, but having both "codecs" and "platforms" is
easy to read / understand code.

> @@ -1051,22 +1053,22 @@ static int soc_init_dai_link(struct snd_soc_card *card,
> struct snd_soc_dai_link *link)
> {
> int i;
> - struct snd_soc_dai_link_component *codec;
> + struct snd_soc_dai_link_component *dlc;

Same here.

For other parts

Acked-by: Kuninori Morimoto <[email protected]>

Thank you for your help !!
Best regards
---
Kuninori Morimoto