[1/9] ASoC: core: allow a dt node to provide several components
diff mbox series

Message ID 20200213155159.3235792-2-jbrunet@baylibre.com
State New
Headers show
Series
  • ASoC: meson: gx: add audio output support
Related show

Commit Message

Jerome Brunet Feb. 13, 2020, 3:51 p.m. UTC
At the moment, querying the dai_name will stop of the first component
matching the dt node. This does not allow a device (single dt node) to
provide several ASoC components which could then be used through DT.

This change let the search go on if the xlate function of the component
returns an error, giving the possibility to another component to match
and return the dai_name.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 sound/soc/soc-core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Mark Brown Feb. 13, 2020, 5:18 p.m. UTC | #1
On Thu, Feb 13, 2020 at 04:51:51PM +0100, Jerome Brunet wrote:

> At the moment, querying the dai_name will stop of the first component
> matching the dt node. This does not allow a device (single dt node) to
> provide several ASoC components which could then be used through DT.

> This change let the search go on if the xlate function of the component
> returns an error, giving the possibility to another component to match
> and return the dai_name.

My first question here would be why you'd want to do that rather than
combine everything into a single component since the hardware seems to
be doing that anyway.  Hopefully the rest of the series will answer this
but it'd be good in the changelog here.
Jerome Brunet Feb. 13, 2020, 5:37 p.m. UTC | #2
On Thu 13 Feb 2020 at 18:18, Mark Brown <broonie@kernel.org> wrote:

> On Thu, Feb 13, 2020 at 04:51:51PM +0100, Jerome Brunet wrote:
>
>> At the moment, querying the dai_name will stop of the first component
>> matching the dt node. This does not allow a device (single dt node) to
>> provide several ASoC components which could then be used through DT.
>
>> This change let the search go on if the xlate function of the component
>> returns an error, giving the possibility to another component to match
>> and return the dai_name.
>
> My first question here would be why you'd want to do that rather than
> combine everything into a single component since the hardware seems to
> be doing that anyway.  Hopefully the rest of the series will answer this
> but it'd be good in the changelog here.

Hi Mark,

Sorry if I was not clear enough.

This HW is messy. It is indeed one monolithic device which
provides several functions/sub-devices/components

I tried several approaches:

* Just 1 component: This was ugly because the part that is present only on 1
SoC variant, I needed to reconstruct the dai, widget, route and control
table which involved a fair amount of useless copies.

* A lot of devices (and components) with syscon: This ended up being even
  uglier, difficult to work with since it did not really reflected the
  actual HW.

The solution proposed here is just one device with 3 possible
components (groups):
* The CPU producers a associated path
* The HDMI control
* The Internal DAC control

The impact on ASoC is rather small, the driver reflect quite well what
the HW is and, with a sound-dai-cell=2, it fairly simple in DT as well.

Do you think there is something wrong with a linux device providing
several ASoC components ?
Mark Brown Feb. 13, 2020, 5:40 p.m. UTC | #3
On Thu, Feb 13, 2020 at 06:37:41PM +0100, Jerome Brunet wrote:

> > My first question here would be why you'd want to do that rather than
> > combine everything into a single component since the hardware seems to
> > be doing that anyway.  Hopefully the rest of the series will answer this
> > but it'd be good in the changelog here.

> Do you think there is something wrong with a linux device providing
> several ASoC components ?

I don't know that it's actively wrong, it's more a comment about the
changelog only describing the what of the change and not the why - the
original idea for a component was that there should be a 1:1 mapping
between components and devices but as you say it's not actually a big
change to let things get split up more.

Patch
diff mbox series

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 068d809c349a..03b87427faa7 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3102,6 +3102,14 @@  int snd_soc_get_dai_name(struct of_phandle_args *args,
 			*dai_name = dai->driver->name;
 			if (!*dai_name)
 				*dai_name = pos->name;
+		} else if (ret) {
+			/*
+			 * if another error than ENOTSUPP is returned go on and
+			 * check if another component is provided with the same
+			 * node. This may happen if a device provides several
+			 * components
+			 */
+			continue;
 		}
 
 		break;