snd_soc_remove_pcm_runtime() might be called with rtd == NULL which will
leads to null pointer dereference.
This was reproduced with topology loading and marking a link as ignore
due to missing hardware component on the system.
On module removal the soc_tplg_remove_link() would call
snd_soc_remove_pcm_runtime() with rtd == NULL since the link was ignored,
no runtime was created.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Link: https://patch.msgid.link/20250619084222.559-3-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
No one is using component->id.
One idea is we can re-use it as serial number for component.
But we have no usage, so far. Let's just remove it for now.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://patch.msgid.link/877c1suuna.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
fmt_single_name() requests "ind *id" and not allow NULL for it.
But sometimes we don't need it. Allow NULL.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://patch.msgid.link/878qm8uunz.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
We have below 2 functions, but these are very similar
(A) snd_soc_unregister_component_by_driver()
(B) snd_soc_unregister_component()
(A) void snd_soc_unregister_component_by_driver(...)
{
...
(a) mutex_lock(&client_mutex);
^ (X) component = snd_soc_lookup_component_nolocked(dev, component_driver->name);
| if (!component) ^^^^^^^^^^^^^^^^^^^^^^
| goto out;
(b)
| snd_soc_del_component_unlocked(component);
v
out:
(c) mutex_unlock(&client_mutex);
}
(B) void snd_soc_unregister_component_by_driver(...)
{
(a) mutex_lock(&client_mutex);
^ while (1) {
| (X) struct snd_soc_component *component = snd_soc_lookup_component_nolocked(dev, NULL);
| ^^^^
(b) if (!component)
| break;
|
| snd_soc_del_component_unlocked(component);
v }
(c) mutex_unlock(&client_mutex);
}
Both are calling lock (a), find component and remove it (b), and
unlock (c). The big diff is whether use driver name for lookup() or
not (X).
Merge these into snd_soc_unregister_component_by_driver() (B), and
snd_soc_unregister_component_by_driver() (A) can be macro.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://patch.msgid.link/87h61qy2vn.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Since commit e894efef9a ("ASoC: core: add support to card rebind")
there is a support for card rebind. The support is only partial though.
Let's consider the following scenarios both of which aim to enumerate a
sound card:
1)
snd_soc_add_component(comp1);
(...)
snd_soc_register_card(card1);
2)
snd_soc_register_card(card1);
(...)
snd_soc_add_component(comp1);
For the sake of simplicity, let comp1 be the last dependency needed for
the card1 to enumerate.
Case 1) will end up succeeding whereas 2) is a certain fail -
snd_soc_bind_card() does not honor unbind_card_list so even a non-fatal
return code of EPROBE_DEFER will cause the card to collapse. Given the
typical usecase of platform_device serving as a card->dev and its
probe() ending with:
int carddev_probe(struct platform_device *pdev)
{
(...)
return devm_snd_soc_register_card(dev, card);
}
failure to register card triggers device_unbind_cleanup() -
really_probe() in dd.c.
To allow for card registration to be deferred while being friendly
towards existing users of devm_snd_soc_register_card(), add new
card->devres_dev field, and devm_xxx() variants for card registration:
devm_snd_soc_register_deferrable_card() (external)
devm_snd_soc_bind_card() (internal)
In essence, if requested, devm_snd_soc_bind_card() replaces
snd_soc_bind_card(). The rebind procedure takes care of destroying
old devres before attempting the new bind. This makes sure nothing is
left hanging if binding fails and card becomes unbound but is still
registered to the ASoC framework.
To allow snd_soc_bind_card() to be reused by the deferrable friends,
move 'client_mutex' locking to the function's callers and select between
devm_xxx and non-devm_xxx variants of snd_soc_bind_card() based on
card->devres_dev.
On top of the feature, the refactoring brings two benefits:
a) single lock/unlock of 'client_mutex' in snd_soc_add_component()
instead of ambiguous unlock and immediate lock in
snd_soc_try_rebind_card()
b) all unbind_card_list manipulations done under 'client_mutex'
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Link: https://patch.msgid.link/20250404101622.3673850-1-cezary.rojewski@intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
soc-core.c only calls snd_soc_set_dmi_name(), so we don't need to have
EXPORT_SYMBOL_GPL() for it. Let's makes it local function.
No one uses *flavour parameter, let's remove it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://patch.msgid.link/87v7tfyk7b.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
On R-Car:
OF: /sound: Read of boolean property 'simple-audio-card,bitclock-master' with a value.
OF: /sound: Read of boolean property 'simple-audio-card,frame-master' with a value.
or:
OF: /soc/sound@ec500000/ports/port@0/endpoint: Read of boolean property 'bitclock-master' with a value.
OF: /soc/sound@ec500000/ports/port@0/endpoint: Read of boolean property 'frame-master' with a value.
The use of of_property_read_bool() for non-boolean properties is
deprecated in favor of of_property_present() when testing for property
presence.
Replace testing for presence before calling of_property_read_u32() by
testing for an -EINVAL return value from the latter, to simplify the
code.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://patch.msgid.link/db10e96fbda121e7456d70e97a013cbfc9755f4d.1737533954.git.geert+renesas@glider.be
Signed-off-by: Mark Brown <broonie@kernel.org>
Current ASoC is using dai_link->dai_fmt to set DAI format for both
CPU/Codec. But because it is using same settings, and
SND_SOC_DAIFMT_CLOCK_PROVIDER is flipped for CPU, we can't set both
CPU/Codec as clock consumer, for example.
To solve this issue, this patch enable to use extra format for each
DAI which can keep compatibility with legacy system,
1. SND_SOC_DAIFMT_FORMAT_MASK
2. SND_SOC_DAIFMT_CLOCK
3. SND_SOC_DAIFMT_INV
4. SND_SOC_DAIFMT_CLOCK_PROVIDER
Legacy
dai_fmt includes 1, 2, 3, 4
New idea
dai_fmt includes 1, 2, 3
ext_fmt includes 4
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Stephen Gordon <gordoste@iinet.net.au>
Link: https://patch.msgid.link/87r05go5ja.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
snd_soc_daifmt_parse_clock_provider_raw() might be called with NULL np.
Return 0 in such case.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Stephen Gordon <gordoste@iinet.net.au>
Link: https://patch.msgid.link/87ttaco5jm.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
No driver is using disable_route_checks, let's remove it.
Because snd_soc_dapm_add_routes() itself will indicate detail error
when failed, this patch removes duplicate dev_err() not only
dev_warn() in error case.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Suggested-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Tested-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Link: https://patch.msgid.link/87tta8268e.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The variable ret is being assigned a zero value that is never read,
it is being re-assigned a new value from the return value from the
call to snd_soc_dapm_add_routes. The assignment is redundant and can
be removed.
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Link: https://patch.msgid.link/20241217171151.354550-1-colin.i.king@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
commit a22ae72b86 ("ASoC: soc-core: disable route checks for legacy
devices") added card->disable_route_checks to disable route checks
for legacy devices at soc_probe_component() and snd_soc_bind_card().
And commit 6974857c2b ("ASoC: topology: Do not ignore route checks
when parsing graphs") tidyup soc-topology for same reason.
In snd_soc_bind_card() case, if snd_soc_dapm_add_routes() (A) error,
but has card->disable_route_checks case (B), it will indicate
dev_info() only, and then, next function (C) will be called.
Thus, "ret" will be over written, and it is handled as non-error.
static int snd_soc_bind_card(...)
{
...
(A) ret = snd_soc_dapm_add_routes(...);
if (ret < 0) {
(B) if (card->disable_route_checks) {
dev_info(...);
} else {
...
goto probe_end;
}
}
(C) ret = snd_soc_dapm_add_routes(...);
...
In soc_probe_component() case, if snd_soc_dapm_add_routes() (a)
error, and has card->disable_route_checks case (b), it will indicate
dev_info(). But there is no next function after that, this means ret is
still indicating error, and will not be over written.
So error handline (c) will be handled, and will return error (d)
static int soc_probe_component(...)
{
...
(a) ret = snd_soc_dapm_add_routes(...);
if (ret < 0) {
(b) if (card->disable_route_checks) {
dev_info(...);
} else {
...
goto err_probe;
}
}
/* see for_each_card_components */
list_add(...);
err_probe:
(c) if (ret < 0)
soc_remove_component(...);
(d) return ret;
}
In soc_tplg_dapm_graph_elems_load() case, snd_soc_dapm_add_routes()
is called in for loop (1). if it was error (2), and doesn't have
card->disable_route_checks case flag (3), it will break from loop.
If card has flag, it will indicate dev_info() and handled as non-error.
But ret is still indicating error in this case. If this error happen
in last of loop, if will return error (4).
static int soc_tplg_dapm_graph_elems_load(...)
{
...
(1) for (i = 0; i < count; i++) {
...
(2) ret = snd_soc_dapm_add_routes(...);
if (ret) {
(3) if (!dapm->card->disable_route_checks) {
dev_err(...);
break;
}
dev_info(...);
}
}
(4) return ret;
}
This patch set "ret = 0" for each case.
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
Cc: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://patch.msgid.link/87wmg5tzra.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
some functions had parameter like below
xxx(..., rtd, ..., id);
This "id" is rtd->id. We don't need to have "id" on each functions
because we can get it from "rtd". Let's cleanup it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://patch.msgid.link/87plnqb84p.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
qcom/qdsp6 want to use irregular rtd->id because of its topology.
Current code is calculating it at soc_init_pcm_runtime() which calls
soc_new_pcm(), and it doesn't save it to rtd->id.
Let's calculate and save it to rtd at snd_soc_add_pcm_runtime()
which create rtd and connect related components.
But, this feature should be implemented by using "dai_link" instead
of "component". Add FIXME as comment.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://patch.msgid.link/87r086b84w.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Current rtd has "num". It sounds/looks like size of rtd or something,
but it will be mainly used at snd_pcm_new() as "device index".
This naming is confusable. Let's rename it to "id"
Some drivers are using rtd->num, so let's keep it so far, and remove
it if all user was switched.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://patch.msgid.link/87zfmub85z.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
dpcm_xxx flags are no longer needed. It converts dpcm_xxx flag to
xxx_only if needed.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Link: https://patch.msgid.link/87bjze9vhp.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Use of_property_read_bool() to read boolean properties rather than
of_get_property(). This is part of a larger effort to remove callers
of of_get_property() and similar functions. of_get_property() leaks
the DT property data pointer which is a problem for dynamically
allocated nodes which may be freed.
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
Link: https://patch.msgid.link/20240731191312.1710417-20-robh@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
The struct snd_soc_pcm_runtime has a flex array of snd_soc_component
objects at its end, and the size is kept in num_components field.
We can add __counted_by() annotation for compiler's assistance to
catch array overflows.
A slight additional change is the assignment of rtd->components[];
the array counter has to be incremented at first for avoiding
false-positive reports from compilers.
Also, the allocation size of snd_soc_pcm_runtime is cleaned up with
the standard struct_size() helper, too.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20240726155237.21961-1-tiwai@suse.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Several ASoC functions receive pointers to data which is not modified,
e.g. pointers to 'snd_soc_dai', 'snd_soc_pcm_runtime',
'snd_pcm_hw_params' and 'snd_soc_dai_link'.
All these pointers can be made as a pointer to const. This makes code
safer, serves as clear annotation of function's intentions (no ownership
passed to the function, no modifications) and allows putting pointed
structures in rodata (if ever applicable).
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://msgid.link/r/20240617-n-asoc-const-auto-selectable-formats-v1-3-8004f346ee38@linaro.org
Signed-off-by: Mark Brown <broonie@kernel.org>
At the moment we cannot instantiate two dmaengine_pcms with the same
parent device, as the components will be named the same, leading to
conflicts.
Add 'name' field to the snd_dmaengine_pcm_config, and use that (if
defined) as the component name instead of deriving the component name
from the device.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Link: https://msgid.link/r/20240319-xilinx-dp-audio-v2-1-92d6d3a7ca7e@ideasonboard.com
Signed-off-by: Mark Brown <broonie@kernel.org>
When pcm_runtime is adding platform components it will scan all
registered components. In case of DPCM FE/BE some DAI links will
configure dummy platform. However both dummy codec and dummy platform
are using "snd-soc-dummy" as component->name. Dummy codec should be
skipped when adding platforms otherwise there'll be overflow and UBSAN
complains.
Reported-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
Link: https://msgid.link/r/20240305065606.3778642-1-chancel.liu@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
ASoC machine driver can use snd_soc_{of_}get_dlc() (A) to get DAI name
for dlc (snd_soc_dai_link_component). In this function call
dlc->dai_name is parsed via snd_soc_dai_name_get() (B).
(A) int snd_soc_get_dlc(...)
{
...
(B) dlc->dai_name = snd_soc_dai_name_get(dai);
...
}
(B) has a priority to return dai->name as dlc->dai_name. In most cases
card can probe successfully. However it has an issue that ASoC tries to
rebind card. Here is a simplified flow for example:
| a) Card probes successfully at first
| b) One of the component bound to this card is removed for some
| reason the component->dev is released
| c) That component is re-registered
v d) ASoC calls snd_soc_try_rebind_card()
a) points dlc->dai_name to dai->name. b) releases all resource of the
old DAI. c) creates new DAI structure. In result d) can not use
dlc->dai_name to add new created DAI.
So it's reasonable that prefer to return dai->driver->name in
snd_soc_dai_name_get() because dai->driver is a pre-defined global
variable. Also update snd_soc_is_matching_dai() for alignment.
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
Link: https://msgid.link/r/20240304072128.2845432-1-chancel.liu@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
snd_soc_is_matching_dai() checks DAI name, which is paired function
with snd_soc_dai_name_get().
It checks dlc->dai_name and dai->name (A) or dai->driver_name (B) or
dai->component->name (C)
static int snd_soc_is_matching_dai(...)
{
...
if (strcmp(dlc->dai_name, dai->name) == 0)
~~~~~~~~~~~~~ ^^^^^^^^^(A)
if (...
strcmp(dai->driver->name, dlc->dai_name) == 0)
(B)^^^^^^^^^^^^^^^^ ~~~~~~~~~~~~~
if (...
strcmp(dlc->dai_name, dai->component->name) == 0)
~~~~~~~~~~~~~ ^^^^^^^^^^^^^^^^^^(C)
...
}
But (B) part order is different with (A) and (C) (= ^^^^ and ~~~~).
This is not a big deal, but confusable to read. Fixup it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://msgid.link/r/87wmqxjbcg.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Constify pointer to of_phandle_args in few function arguments, for code
safety and self-documenting code.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://msgid.link/r/20240216145448.224185-1-krzysztof.kozlowski@linaro.org
Signed-off-by: Mark Brown <broonie@kernel.org>
Many ASoC drivers define CPU/Codec/Platform dai_link by below macro.
SND_SOC_DAILINK_DEFS(link,
(A) DAILINK_COMP_ARRAY(COMP_CPU("cpu_dai")),
(B) DAILINK_COMP_ARRAY(COMP_CODEC("codec", "dai1"),
(B) COMP_CODEC("codec", "dai2")),
(C) DAILINK_COMP_ARRAY(COMP_EMPTY()));
In this case, this macro will be converted to like below
[o] = static struct snd_soc_dai_link_component
(A) [o] link_cpus[] = {{ .dai_name = "cpu_dai" }};
(B) [o] link_codecs[] = {{ .dai_name = "dai1", .name = "codec" },
{ .dai_name = "dai2", .name = "codec" }}
(C) [o] link_platforms[] = {{ }};
CPU and Codec info will be filled by COMP_CPU() / COMP_CODEC (= A,B),
and Platform will have empty data by COMP_EMPTY() (= C) in this case.
Platform empty info will be filled when driver probe()
(most of case, CPU info will be copied to use soc-generic-dmaengine-pcm).
For example in case of DPCM FE/BE, it will be like below.
Codec will be dummy Component / DAI in this case (X).
SND_SOC_DAILINK_DEFS(link,
DAILINK_COMP_ARRAY(COMP_CPU(...)),
(X) DAILINK_COMP_ARRAY(COMP_DUMMY()),
DAILINK_COMP_ARRAY(COMP_EMPTY()));
(X) part will converted like below
[o] link_codecs[] = {{ .name = "snd-soc-dummy",
.dai_name = "snd-soc-dummy-dai", }}
Even though we already have common asoc_dummy_dlc for dummy
Component / DAI, this macro will re-create new dummy dlc.
Some drivers defines many dai_link info via SND_SOC_DAILINK_DEFS(),
this means many dummy dlc also will be re-created. This is waste of
memory.
If we can use existing common asoc_dummy_dlc at (X),
we can avoid to re-creating dummy dlc, then, we can save the memory.
At that time, we want to keep existing code as much as possible, because
too many drivers are using this macro. But because of its original style,
using common asoc_dummy_dlc from it is very difficult or impossible.
So let's change the mind. The macro is used like below
SND_SOC_DAILINK_DEFS(link,
DAILINK_COMP_ARRAY(COMP_CPU(...)),
(x) DAILINK_COMP_ARRAY(COMP_DUMMY()),
DAILINK_COMP_ARRAY(COMP_EMPTY()));
static struct snd_soc_dai_link dai_links[] = {
{
.name = ...,
.stream_name = ...,
(y) SND_SOC_DAILINK_REG(link),
},
(y) part will be like below
static struct snd_soc_dai_link dai_links[] = {
{
.name = ...,
.stream_name = ...,
^ ...
| .codecs = link_codecs,
(y) .num_codecs = ARRAY_SIZE(link_codecs),
v ...
}
This patch try to use trick on COMP_DUMMY()
- #define COMP_DUMMY() { .name = "snd-soc-dummy", .dai_name = "snd-soc-dummy-dai", }
+ #define COMP_DUMMY()
By this tric, (x) part will be like below.
before
[o] link_codecs[] = {{ .name = "snd-soc-dummy", .dai_name = "snd-soc-dummy-dai", }}
after
[o] link_codecs[] = { };
This is same as below
[o] link_codecs[0];
This means it has pointer (link_codecs), but the array size is 0.
(y) part will be like below.
static struct snd_soc_dai_link dai_links[] = {
{
...
.codecs = link_codecs,
.num_codecs = 0,
...
},
This is very special settings that normal use usually not do,
but new macro do.
We can find this special settings on soc-core.c and fill it as
"dummy DAI" (= asoc_dummy_dlc). By this tric, we can avoid to re-create
dummy dlc and save the memory.
This patch add tric at COMP_DUMMY() and add snd_soc_fill_dummy_dai()
to fill dummy DAI.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://msgid.link/r/871qbi93qu.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Current ASoC CPU:Codec = N:M connection is using connection mapping idea,
but it is used for N < M case only. We want to use it for any case.
By this patch, not only N:M connection, but all existing connection
(1:1, 1:N, N:N) will use same connection mapping. Then, because it will
use default mapping, no conversion patch is needed to exising drivers.
More over, CPU:Codec = N:M (N > M) also supported in the same time.
ch_maps array will has CPU/Codec index by this patch.
Image
CPU0 <---> Codec0
CPU1 <-+-> Codec1
CPU2 <-/
ch_map
ch_map[0].cpu = 0 ch_map[0].codec = 0
ch_map[1].cpu = 1 ch_map[1].codec = 1
ch_map[2].cpu = 2 ch_map[2].codec = 1
Link: https://lore.kernel.org/r/87fs6wuszr.wl-kuninori.morimoto.gx@renesas.com
Link: https://lore.kernel.org/r/878r7yqeo4.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Tested-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://lore.kernel.org/r/87ttpq4f2c.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
On init we have sequence:
for_each_card_prelinks(card, i, dai_link) {
ret = snd_soc_add_pcm_runtime(card, dai_link);
ret = init_some_other_things(...);
if (ret)
goto probe_end:
for_each_card_rtds(card, rtd) {
ret = soc_init_pcm_runtime(card, rtd);
probe_end:
while on exit:
for_each_card_rtds(card, rtd)
snd_soc_link_exit(rtd);
If init_some_other_things() step fails due to error we end up with
not fully setup rtds and try to call snd_soc_link_exit on them, which
depending on contents on .link_exit handler, can end up dereferencing
NULL pointer.
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Link: https://lore.kernel.org/r/20230929103243.705433-2-amadeuszx.slawinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
When printing log related to component it is useful to know, to which
component it applies to.
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Link: https://lore.kernel.org/r/20230929103243.705433-1-amadeuszx.slawinski@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
ASoC is now unified asoc_xxx() into snd_soc_xxx().
This patch convert asoc_xxx() to snd_soc_xxx().
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87msxsp8tp.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
A recent cleanup of soc_dai_link_sanity_check() is responsible for
generating lots of confusing errors before returning -EPROBE_DEFER:
acp5x_mach acp5x_mach.0: ASoC: Component acp5x_i2s_dma.0 not found for link acp5x-8821-play
[...]
acp5x_mach acp5x_mach.0: ASoC: Component spi-VLV1776:00 not found for link acp5x-CS35L41-Stereo
[...]
acp5x_mach acp5x_mach.0: ASoC: Component spi-VLV1776:01 not found for link acp5x-CS35L41-Stereo
Switch back to the initial behaviour of logging those messages on
KERN_DEBUG level instead of KERN_ERR.
While at it, use the correct form of the verb in 'component_not_find'
label.
Fixes: 0e66a2c694 ("ASoC: soc-core.c: cleanup soc_dai_link_sanity_check()")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com
Link: https://lore.kernel.org/r/20230824193837.369761-1-cristian.ciocaltea@collabora.com
Signed-off-by: Mark Brown <broonie@kernel.org
snd_soc_dai_driver has .ops for call back functions (A), but it also
has other call back functions (B). It is duplicated and confusable.
struct snd_soc_dai_driver {
...
^ int (*probe)(...);
| int (*remove)(...);
(B) int (*compress_new)(...);
| int (*pcm_new)(...);
v ...
(A) const struct snd_soc_dai_ops *ops;
...
}
This patch merges (B) into (A).
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87v8dpb0w6.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
To use multi Component support, we need to check dai_args whether
Card could get DAI from args (CPU/Codec needs set dai_args on DAI driver).
If it could, we need to allocate dai_args for dlc.
This patch adds snd_soc_copy_dai_args() for it.
This is helper function for multi Component support.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87bkgko94e.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Current snd_soc_is_matching_component() checks "of_node" or "dai_args".
Thus coping "of_node" only is not enough to use CPU as Platform.
This patch adds snd_soc_dlc_use_cpu_as_platform() and help it.
This is helper function for multi Component support.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87cz10o94k.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
To enable multi Component, Card driver need to get DAI via dai_args
to identify it. This patch adds snd_soc_get_dai_via_args() for it.
This is helper function for multi Component support.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87edlgo94p.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Current ASoC Card is using dlc (snd_soc_dai_link_component) to find
target DAI / Component to be used.
Current dlc has below 3 items to identify DAI / Component
(a) name for Component
(b) of_node for Component
(c) dai_name for DAI
(a) or (b) is used to identify target Component, and (c) is used
to identify DAI.
One of the biggest issue on it today is dlc needs "name matching"
for "dai_name" (c).
It was not a big deal when we were using platform_device, because we
could specify nessesary "dai_name" via its platform_data.
But we need to find DAI name pointer from whole registered datas and/or
each related driver somehow in case of DT, because we can't specify it.
Therefore, Card driver parses DT and assumes the DAI, and find its name
pointer. How to assume is based on each Component and/or Card.
Next biggest issue is Component node (a)/(b).
Basically, Component is registered when CPU/Codec driver was
probed() (X). Here, 1 Component is possible to have some DAIs.
int xxx_probe(struct platform_device *pdev)
{
...
(X) ret = devm_snd_soc_register_component(pdev->dev,
&component_driver,
&dai_driver, dai_driver_num);
...
}
The image of each data will be like below.
One note here is "driver" is included for later explanation.
+-driver------+
|+-component-+|
|| dai0||
|| dai1||
|| ...||
|+-----------+|
+-------------+
The point here is 1 driver has 1 Component, because basically driver
calles snd_soc_register_component() (= X) once.
Here is the very basic CPU/Codec connection image.
HW image SW image
+-- Board ------------+ +-card--------------------------+
|+-----+ +------+| |+-driver------+ +-driver------+|
|| CPU | <--> |CodecA|| ||+-component-+| |+-component-+||
|+-----+ +------+| ||| dai|<=>|dai |||
+---------------------+ ||+-----------+| |+-----------+||
|+-------------+ +-------------+|
+-------------------------------+
It will be very complex if it has multi DAIs.
Here is intuitive easy to understandable HW / SW example.
HW image SW image
+-- Board ---------------+ +-card--------------------------+
|+--------+ +------+| |+-driver------+ +-driver------+|
|| CPU ch0| <--> |CodecA|| ||+-component-+| |+-component-+||
|| | +------+| ||| ch0 dai|<=>|dai |||
|| | +------+| ||| || |+-----------+||
|| ch1| <--> |CodecB|| ||| || +-------------+|
|+--------+ +------+| ||| || +-driver------+|
+------------------------+ ||| || |+-component-+||
||| ch1 dai|<=>|dai |||
||+-----------+| |+-----------+||
|+-------------+ +-------------+|
+-------------------------------+
It will be handled as multi interface as "one Card".
card0,0: CPU-ch0 - CodecA
card0,1: CPU-ch1 - CodecB
^
But, here is the HW image example which will be more complex
+-- Basic Board ---------+
|+--------+ +------+|
|| CPU ch0| <--> |CodecA||
|| ch1| <-+ +------+|
|+--------+ | |
+-------------|----------+
+-- expansion board -----+
| | +------+|
| +->|CodecB||
| +------+|
+------------------------+
We intuitively think we want to handle these as "2 Sound Cards".
card0,0: CPU-ch0 - CodecA
card1,0: CPU-ch1 - CodecB
^
But below image which we can register today doesn't allow it,
because the same Component will be connected to both Card0/1,
but it will be rejected by (Z).
+-driver------+
|+-component-+|
+-card0-------------------------+
||| || +-driver------+|
||| || |+-component-+||
||| ch0 dai|<=>|dai |||
||| || |+-----------+||
||| || +-------------+|
+-------------------------------+
|| ||
+-card1-------------------------+
||| || +-driver------+|
||| || |+-component-+||
||| ch1 dai|<=>|dai |||
||| || |+-----------+||
||| || +-------------+|
+-------------------------------+
|+-----------+|
+-------------+
static int soc_probe_component()
{
...
if (component->card) {
(Z) if (component->card != card) {
dev_err(component->dev, ...);
return -ENODEV;
}
return 0;
}
...
}
So, how about to call snd_soc_register_component() (= X) multiple times
on probe() to avoid buplicated component->card limitation, to be like
below ?
+-driver------+
+-card0-------------------------+
|| | +-driver------+|
||+-component-+| |+-component-+||
||| ch0 dai|<=>|dai |||
||+-----------+| |+-----------+||
|| | +-------------+|
+-------------------------------+
| |
+-card1-------------------------+
|| | +-driver------+|
||+-component-+| |+-component-+||
||| ch1 dai|<=>|dai |||
||+-----------+| |+-----------+||
|| | +-------------+|
+-------------------------------+
+-------------+
Yes, looks good. But unfortunately it doesn't help us for now.
Let's see soc_component_to_node() and snd_soc_is_matching_component()
static struct device_node
*soc_component_to_node(struct snd_soc_component *component)
{
...
(A) of_node = component->dev->of_node;
...
}
static int snd_soc_is_matching_component(...)
{
...
(B) if (dlc->of_node && component_of_node != dlc->of_node)
...
}
dlc checkes "of_node" to identify target component (B),
but this "of_node" came from component->dev (A) which is added
by snd_soc_register_component() (X) on probe().
This means we can have different "component->card", but have same
"component->dev" in this case.
Even though we calls snd_soc_register_component() (= X) multiple times,
all Components have same driver's dev, thus it is impossible to
identified the Component.
And if it was impossible to identify Component, it is impossible to
identify DAI on current implementation.
So, how to handle above complex HW image today is 2 patterns.
One is handles it as "1 big sound card".
The SW image is like below.
SW image
+-card--------------------------+
|+-driver------+ +-driver------+|
||+-component-+| |+-component-+||
||| ch0 dai|<=>|dai |||
||| || |+-----------+||
||| || +-------------+|
||| || +-driver------+|
||| || |+-component-+||
||| ch1 dai|<->|dai |||
||+-----------+| |+-----------+||
|+-------------+ +-------------+|
+-------------------------------+
But the problem is not intuitive.
We want to handle it as "2 Cards".
2nd pattern is like below.
SW image
+-card0-------------------------+
|+-driver------+ +-driver------+|
||+-component-+| |+-component-+||
||| ch0 dai|<=>|dai |||
||+-----------+| |+-----------+||
|+-------------+ +-------------+|
+-------------------------------+
+-card1-------------------------+
|+-driver------+ +-driver------+|
||+-component-+| |+-component-+||
||| ch1 dai|<=>|dai |||
||+-----------+| |+-----------+||
|+-------------+ +-------------+|
+-------------------------------+
It handles as "2 Cards", but CPU part needs to be probed as 2 drivers.
It is also not intuitive.
To solve this issue, we need to have multi Component support.
In current implementation, we need to identify Component first
to identify DAI, and it is using name matching to identify DAI.
But how about to be enable to directly identify DAI by unique way
instead of name matching ? In such case, we can directly identify DAI,
then it can identify Component from DAI.
For example Simple-Card / Audio-Graph-Card case, it is specifying DAI
via its node.
Simple-Card
sound-dai = <&cpu-sound>;
Audio-Graph-Card
dais = <&cpu-sound>;
If each CPU/Codec driver keeps this property when probing,
we can identify DAI directly from Card.
Being able to identify DAI directly means being able to identify its
Component as well even though Component has same dev (= B).
This patch adds new "dai_node" for it.
To keeping compatibility, it checks "dai_node" first if it has,
otherwise, use existing method (name matching).
Link: https://lore.kernel.org/r/87fskz5yrr.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87fs5wo94v.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Current ASoC is specifying and checking DAI name.
But where it came from and how to check was ambiguous.
This patch adds snd_soc_dai_name_get() / snd_soc_dlc_dai_is_match()
and makes it clear.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87h6qco952.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
dlc->of_node will be set on snd_soc_get_dlc(), but we want
1) protect it by mutex, 2) set only when successed.
This patch do it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/878rc1kerv.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Required CPU/Codec/Platform dlc (snd_soc_dai_link_component) are similar
but not same, and very complex. Current implementation is very confusable
and it will be more complex if multi Component was supported.
This patch cleanup it.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87o7l9blsn.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Current snd_soc_get_dai_id() is initializing dlc *manually*,
but it will might be a problem if dlc had new extra parameter.
This patch uses default initialization, otherwise, non initialized
part will be strange value.
This is prepare for multi Component support.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Link: https://lore.kernel.org/r/87pm5pblst.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Suppress probe deferral error messages when probing link components to
avoid spamming the logs, for example, if a required component has not
yet been registered:
snd-sc8280xp sound: ASoC: failed to instantiate card -517
Note that dev_err_probe() is not used as the card can be unbound and
rebound while the underlying platform device remains bound to its
driver.
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Link: https://lore.kernel.org/r/20230705123018.30903-9-johan+linaro@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
The generic snd_soc_dai_get_dlc() contains a default translation function
for DAI names which has factored out common code in a number of card
drivers, resolving the dai_name and of_node either using a driver provided
translation function or with a generic implementation. Unfortunately the
of_node can't be set by the translation function since it currently doesn't
have an interface to do that but snd_soc_dai_get_dlc() only initialises the
of_node in the case where there is no translation function.
This breaks the Meson support after conversion to use the generic helpers
since the DPCM cards for it check which component of the SoC is connected
to each link by checking the compatible for the component and the Meson
components provide a custom operation so don't use the generic code.
Fix this and potentially other cards by unconditionally storing the node in
the dai_link_component, there shouldn't be a binding specific of_node
selected since that's how we determine the translation function.
Fixes: 2e1dbea1f8 ("ASoC: meson: use snd_soc_{of_}get_dlc()")
Fixes: 3c8b586185 ("ASoC: soc-core.c: add index on snd_soc_of_get_dai_name()")
Link: https://lore.kernel.org/r/Message-Id: <20230623-asoc-fix-meson-probe-v1-1-82b2c2ec5ca4@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
