| Message ID | 20210309082328.38388-1-daniel.baluta@oss.nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: core: Don't set platform name when of_node is set | expand |
On Tue, Mar 09, 2021 at 10:23:28AM +0200, Daniel Baluta wrote: > From: Daniel Baluta <daniel.baluta@nxp.com> > > Platform may be specified by either name or OF node but not > both. > > For OF node platforms (e.g i.MX) we end up with both platform name > and of_node set and sound card registration will fail with the error: > > asoc-simple-card sof-sound-wm8960: ASoC: Neither/both > platform name/of_node are set for sai1-wm8960-hifi This doesn't actually say what the change does. > - dai_link->platforms->name = component->name; > + > + if (!dai_link->platforms->of_node) > + dai_link->platforms->name = component->name; Why would we prefer the node name over something explicitly configured?
On Tue, Mar 9, 2021 at 5:38 PM Mark Brown <broonie@kernel.org> wrote: > > On Tue, Mar 09, 2021 at 10:23:28AM +0200, Daniel Baluta wrote: > > From: Daniel Baluta <daniel.baluta@nxp.com> > > > > Platform may be specified by either name or OF node but not > > both. > > > > For OF node platforms (e.g i.MX) we end up with both platform name > > and of_node set and sound card registration will fail with the error: > > > > asoc-simple-card sof-sound-wm8960: ASoC: Neither/both > > platform name/of_node are set for sai1-wm8960-hifi > > This doesn't actually say what the change does. Will send v2 with a better explanation. > > > - dai_link->platforms->name = component->name; > > + > > + if (!dai_link->platforms->of_node) > > + dai_link->platforms->name = component->name; > > Why would we prefer the node name over something explicitly configured? Not sure I follow your question. I think the difference stands in the way we treat OF vs non-OF platforms. With OF-platforms, dai_link->platforms->of_node is always set! So we no longer need to set dai->platforms->name. Actually setting both of_node and name will make sound card registration fail! In this is the case I'm trying to fix here.
On Fri, Mar 12, 2021 at 12:50 PM Mark Brown <broonie@kernel.org> wrote: > > On Fri, Mar 12, 2021 at 10:32:54AM +0200, Daniel Baluta wrote: > > On Tue, Mar 9, 2021 at 5:38 PM Mark Brown <broonie@kernel.org> wrote: > > > > > + if (!dai_link->platforms->of_node) > > > > + dai_link->platforms->name = component->name; > > > > Why would we prefer the node name over something explicitly configured? > > > Not sure I follow your question. I think the difference stands in the > > way we treat OF vs non-OF platforms. > > If an explicit name has been provided why would we override it with an > autogenerated one? Wait, are you asking why the initial code: dai_link->platforms->name = component->name; is there in the initial code?
On Fri, Mar 12, 2021 at 4:24 PM Mark Brown <broonie@kernel.org> wrote: > > On Fri, Mar 12, 2021 at 02:37:30PM +0200, Daniel Baluta wrote: > > On Fri, Mar 12, 2021 at 1:59 PM Mark Brown <broonie@kernel.org> wrote: > > > > No, just the opposite! If there's an explict name configured why do you > > > want to ignore it? > > > Because the initial assignment: > > > dai_link->platforms->name = component->name; > > > doesn't take into consideration that dai_link->platform->of_node is > > also explicitly configured. > > But why should we take that into consideration here? > > > dai->link->platforms->of_node > > configured and we hit this error: > > > > soc_dai_link_sanity_check: > > /* > > * 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 (!!platform->name == !!platform->of_node) { > > dev_err(card->dev, > > "ASoC: Neither/both platform name/of_node are set for %s\n", link->name); > > return -EINVAL; > > } > > OK, but then does this check actually make sense? The code has been > that way since the initial DT introduction since we disallow matching by > both name and OF node in order to avoid confusion when building the card > so I think it does but it's only with this mail that I get the > information to figure out that this is something we actually check for > then go find the reason why we check. I will enhance the commit message and send v2. Hope to catch all the inner details.
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 16ba54eb8164..76ab42fa9461 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1660,7 +1660,9 @@ static void soc_check_tplg_fes(struct snd_soc_card *card) dev_err(card->dev, "init platform error"); continue; } - dai_link->platforms->name = component->name; + + if (!dai_link->platforms->of_node) + dai_link->platforms->name = component->name; /* convert non BE into BE */ if (!dai_link->no_pcm) {