Hi Kuninori,
On Wed, 24 May 2023 00:08:50 +0000 Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
Hi
An additional-devs subnode can be present in the simple-card top node. This subnode is used to declared some "virtual" additional devices.
Create related devices from this subnode and avoid this subnode presence to interfere with the already supported subnodes analysis.
Signed-off-by: Herve Codina herve.codina@bootlin.com
simple-card is used in many boards, but is old. Adding new feature on audio-graph-card/audio-graph-card2 instead of simple-card is my ideal, but it is OK.
simple-card is possible to handle multiple DAI links by using "dai-link" node on 1 Sound Card. see
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Is this "additional-devs" available only one per 1 Card ? If it is possible to use 1 additional-devs per 1 DAI link, I think this patch want to care "dai-link". Or adding temporally NOTE or FIXME message like /* NOTE: it doesn't support dai-link so far */ is good idea.
As you said on your other mail 1 "additional-devs" per 1 Card. And further more, I think that "additional-devs" has nothing to do with DAI link. These additional devices are really related to the the Card itself and not DAI links.
simple_populate_aux() needs to be called once per Card.
static int asoc_simple_probe(struct platform_device *pdev) { struct asoc_simple_priv *priv; @@ -688,6 +731,11 @@ static int asoc_simple_probe(struct platform_device *pdev) return ret;
if (np && of_device_is_available(np)) {
ret = simple_populate_aux(priv);if (ret < 0) {dev_err_probe(dev, ret, "populate aux error\n");goto err;}ret = simple_parse_of(priv, li); if (ret < 0) {
-- 2.40.1
Calling simple_populate_aux() before calling simple_parse_of() is possible, but looks strange for me. see below
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 5a5e4ecd0f61..4992ab433d6a 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c
(snip)
@@ -359,6 +360,8 @@ static int __simple_for_each_link(struct asoc_simple_priv *priv, is_top = 1; }
- add_devs = of_get_child_by_name(top, PREFIX "additional-devs");
I think better position to call simple_populate_aux() is here. But __simple_for_each_link() will be called multiple times for CPU and for Codec. So maybe you want to calling it under CPU turn.
/* NOTE: it doesn't support dai-link so far */add_devs = of_get_child_by_name(top, PREFIX "additional-devs"); if (add_devs && li->cpu) { ret = simple_populate_aux(priv); ... }
So, IMHO, calling simple_populate_aux() from __simple_for_each_link() is not correct as it has nothing to do with DAI links and must be call once per Card.
simple_populate_aux() could be called by simple_parse_of() itself or after simple_parse_of() call.
I would prefer calling it before snd_soc_of_parse_aux_devs() because this function parses aux-devs phandles and these phandles can refer an auxiliary device present in the additional-devs node.
The current code has no issue with this but some evolution can lead to EPROBE_DEFER if the device related to the phandle was not probed. If simple_populate_aux() is called after snd_soc_of_parse_aux_devs(), an EPROBE_DEFER related to the missing probe() call has no chance to be resolved.
Tell be what you prefer: a) Call before simple_parse_of() (no changes) or b) Call at the very beginning of simple_parse_of() or c) Other suggestion ...
Thanks a lot for your review. Best regards, Hervé
Thank you for your help !!
Best regards
Kuninori Morimoto