Messages in this thread Patch in this message | | | From | Kuninori Morimoto <> | Subject | Re: [PATCH v2 9/9] ASoC: simple-card: Handle additional devices | Date | Wed, 24 May 2023 00:08:50 +0000 |
| |
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/Documentation/devicetree/bindings/sound/simple-card.yaml?h=v6.4-rc3#n294
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.
> 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); ... }
Thank you for your help !!
Best regards --- Kuninori Morimoto
| |