[PATCH v2 0/5] ASoC: Harden DAPM route checks and Intel fixes
Set of loosely connected patches. Most impactful change is dropping any permisiveness when snd_soc_dapm_add_routes() fails in soc-topology.c To do it safely, disable route checks for all skylake-driver boards.
Relevant background:
Since commit daa480bde6b3 ("ASoC: soc-core: tidyup for snd_soc_dapm_add_routes()") route checks are no longer permissive. Probe failures for Intel boards have been partially addressed by commit a22ae72b86a4 ("ASoC: soc-core: isable route checks for legacy devices") and its follow up but only skl_nau88l25_ssm4567.c is patched. The rest of the boards still need fixing.
After that, removal of copy-pastas found in ssm4567.c and redundant code in i2s_test.c for avs-boards.
Changes in v2: - glk_rt5682_max98357a.c and skl_hda_dsp_generic.c now disable route checks only for the skylake-drvier - asoc now logs failures of snd_soc_dapm_add_routes() in soc-topology.c
Amadeusz Sławiński (1): ASoC: Intel: avs: i2s_test: Remove redundant dapm routes
Cezary Rojewski (4): ASoC: Intel: Disable route checks for Skylake boards ASoC: topology: Do not ignore route checks when parsing graphs ASoC: Intel: avs: ssm4567: Do not ignore route checks ASoC: Intel: avs: ssm4567: Board cleanup
sound/soc/intel/avs/boards/i2s_test.c | 79 ------------------- sound/soc/intel/avs/boards/ssm4567.c | 5 +- sound/soc/intel/boards/bxt_da7219_max98357a.c | 1 + sound/soc/intel/boards/bxt_rt298.c | 1 + sound/soc/intel/boards/glk_rt5682_max98357a.c | 2 + sound/soc/intel/boards/kbl_da7219_max98357a.c | 1 + sound/soc/intel/boards/kbl_da7219_max98927.c | 4 + sound/soc/intel/boards/kbl_rt5660.c | 1 + sound/soc/intel/boards/kbl_rt5663_max98927.c | 2 + .../intel/boards/kbl_rt5663_rt5514_max98927.c | 1 + sound/soc/intel/boards/skl_hda_dsp_generic.c | 2 + .../soc/intel/boards/skl_nau88l25_max98357a.c | 1 + sound/soc/intel/boards/skl_rt286.c | 1 + sound/soc/soc-topology.c | 11 ++- 14 files changed, 27 insertions(+), 85 deletions(-)
Topology files that are propagated to the world and utilized by the skylake-driver carry shortcomings in their SectionGraphs.
Since commit daa480bde6b3 ("ASoC: soc-core: tidyup for snd_soc_dapm_add_routes()") route checks are no longer permissive. Probe failures for Intel boards have been partially addressed by commit a22ae72b86a4 ("ASoC: soc-core: disable route checks for legacy devices") and its follow up but only skl_nau88l25_ssm4567.c is patched. Fix the problem for the rest of the boards.
Link: https://lore.kernel.org/all/20200309192744.18380-1-pierre-louis.bossart@linu... Fixes: daa480bde6b3 ("ASoC: soc-core: tidyup for snd_soc_dapm_add_routes()") Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/boards/bxt_da7219_max98357a.c | 1 + sound/soc/intel/boards/bxt_rt298.c | 1 + sound/soc/intel/boards/glk_rt5682_max98357a.c | 2 ++ sound/soc/intel/boards/kbl_da7219_max98357a.c | 1 + sound/soc/intel/boards/kbl_da7219_max98927.c | 4 ++++ sound/soc/intel/boards/kbl_rt5660.c | 1 + sound/soc/intel/boards/kbl_rt5663_max98927.c | 2 ++ sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c | 1 + sound/soc/intel/boards/skl_hda_dsp_generic.c | 2 ++ sound/soc/intel/boards/skl_nau88l25_max98357a.c | 1 + sound/soc/intel/boards/skl_rt286.c | 1 + 11 files changed, 17 insertions(+)
diff --git a/sound/soc/intel/boards/bxt_da7219_max98357a.c b/sound/soc/intel/boards/bxt_da7219_max98357a.c index 540f7a29310a..3fe3f38c6cb6 100644 --- a/sound/soc/intel/boards/bxt_da7219_max98357a.c +++ b/sound/soc/intel/boards/bxt_da7219_max98357a.c @@ -768,6 +768,7 @@ static struct snd_soc_card broxton_audio_card = { .dapm_routes = audio_map, .num_dapm_routes = ARRAY_SIZE(audio_map), .fully_routed = true, + .disable_route_checks = true, .late_probe = bxt_card_late_probe, };
diff --git a/sound/soc/intel/boards/bxt_rt298.c b/sound/soc/intel/boards/bxt_rt298.c index c0eb65c14aa9..afc499be8db2 100644 --- a/sound/soc/intel/boards/bxt_rt298.c +++ b/sound/soc/intel/boards/bxt_rt298.c @@ -574,6 +574,7 @@ static struct snd_soc_card broxton_rt298 = { .dapm_routes = broxton_rt298_map, .num_dapm_routes = ARRAY_SIZE(broxton_rt298_map), .fully_routed = true, + .disable_route_checks = true, .late_probe = bxt_card_late_probe,
}; diff --git a/sound/soc/intel/boards/glk_rt5682_max98357a.c b/sound/soc/intel/boards/glk_rt5682_max98357a.c index 657e4658234c..4098b2d32f9b 100644 --- a/sound/soc/intel/boards/glk_rt5682_max98357a.c +++ b/sound/soc/intel/boards/glk_rt5682_max98357a.c @@ -649,6 +649,8 @@ static int geminilake_audio_probe(struct platform_device *pdev) card = &glk_audio_card_rt5682_m98357a; card->dev = &pdev->dev; snd_soc_card_set_drvdata(card, ctx); + if (!snd_soc_acpi_sof_parent(&pdev->dev)) + card->disable_route_checks = true;
/* override platform name, if required */ mach = pdev->dev.platform_data; diff --git a/sound/soc/intel/boards/kbl_da7219_max98357a.c b/sound/soc/intel/boards/kbl_da7219_max98357a.c index a5d8965303a8..9dbc15f9d1c9 100644 --- a/sound/soc/intel/boards/kbl_da7219_max98357a.c +++ b/sound/soc/intel/boards/kbl_da7219_max98357a.c @@ -639,6 +639,7 @@ static struct snd_soc_card kabylake_audio_card_da7219_m98357a = { .dapm_routes = kabylake_map, .num_dapm_routes = ARRAY_SIZE(kabylake_map), .fully_routed = true, + .disable_route_checks = true, .late_probe = kabylake_card_late_probe, };
diff --git a/sound/soc/intel/boards/kbl_da7219_max98927.c b/sound/soc/intel/boards/kbl_da7219_max98927.c index 98c11ec0adc0..e662da5af83b 100644 --- a/sound/soc/intel/boards/kbl_da7219_max98927.c +++ b/sound/soc/intel/boards/kbl_da7219_max98927.c @@ -1036,6 +1036,7 @@ static struct snd_soc_card kbl_audio_card_da7219_m98927 = { .codec_conf = max98927_codec_conf, .num_configs = ARRAY_SIZE(max98927_codec_conf), .fully_routed = true, + .disable_route_checks = true, .late_probe = kabylake_card_late_probe, };
@@ -1054,6 +1055,7 @@ static struct snd_soc_card kbl_audio_card_max98927 = { .codec_conf = max98927_codec_conf, .num_configs = ARRAY_SIZE(max98927_codec_conf), .fully_routed = true, + .disable_route_checks = true, .late_probe = kabylake_card_late_probe, };
@@ -1071,6 +1073,7 @@ static struct snd_soc_card kbl_audio_card_da7219_m98373 = { .codec_conf = max98373_codec_conf, .num_configs = ARRAY_SIZE(max98373_codec_conf), .fully_routed = true, + .disable_route_checks = true, .late_probe = kabylake_card_late_probe, };
@@ -1088,6 +1091,7 @@ static struct snd_soc_card kbl_audio_card_max98373 = { .codec_conf = max98373_codec_conf, .num_configs = ARRAY_SIZE(max98373_codec_conf), .fully_routed = true, + .disable_route_checks = true, .late_probe = kabylake_card_late_probe, };
diff --git a/sound/soc/intel/boards/kbl_rt5660.c b/sound/soc/intel/boards/kbl_rt5660.c index 30e0aca161cd..894d127c482a 100644 --- a/sound/soc/intel/boards/kbl_rt5660.c +++ b/sound/soc/intel/boards/kbl_rt5660.c @@ -518,6 +518,7 @@ static struct snd_soc_card kabylake_audio_card_rt5660 = { .dapm_routes = kabylake_rt5660_map, .num_dapm_routes = ARRAY_SIZE(kabylake_rt5660_map), .fully_routed = true, + .disable_route_checks = true, .late_probe = kabylake_card_late_probe, };
diff --git a/sound/soc/intel/boards/kbl_rt5663_max98927.c b/sound/soc/intel/boards/kbl_rt5663_max98927.c index 9071b1f1cbd0..646e8ff8e961 100644 --- a/sound/soc/intel/boards/kbl_rt5663_max98927.c +++ b/sound/soc/intel/boards/kbl_rt5663_max98927.c @@ -966,6 +966,7 @@ static struct snd_soc_card kabylake_audio_card_rt5663_m98927 = { .codec_conf = max98927_codec_conf, .num_configs = ARRAY_SIZE(max98927_codec_conf), .fully_routed = true, + .disable_route_checks = true, .late_probe = kabylake_card_late_probe, };
@@ -982,6 +983,7 @@ static struct snd_soc_card kabylake_audio_card_rt5663 = { .dapm_routes = kabylake_5663_map, .num_dapm_routes = ARRAY_SIZE(kabylake_5663_map), .fully_routed = true, + .disable_route_checks = true, .late_probe = kabylake_card_late_probe, };
diff --git a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c index 178fe9c37df6..924d5d1de03a 100644 --- a/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c +++ b/sound/soc/intel/boards/kbl_rt5663_rt5514_max98927.c @@ -791,6 +791,7 @@ static struct snd_soc_card kabylake_audio_card = { .codec_conf = max98927_codec_conf, .num_configs = ARRAY_SIZE(max98927_codec_conf), .fully_routed = true, + .disable_route_checks = true, .late_probe = kabylake_card_late_probe, };
diff --git a/sound/soc/intel/boards/skl_hda_dsp_generic.c b/sound/soc/intel/boards/skl_hda_dsp_generic.c index 6e172719c979..4aa7fd2a05e4 100644 --- a/sound/soc/intel/boards/skl_hda_dsp_generic.c +++ b/sound/soc/intel/boards/skl_hda_dsp_generic.c @@ -227,6 +227,8 @@ static int skl_hda_audio_probe(struct platform_device *pdev) ctx->common_hdmi_codec_drv = mach->mach_params.common_hdmi_codec_drv;
hda_soc_card.dev = &pdev->dev; + if (!snd_soc_acpi_sof_parent(&pdev->dev)) + hda_soc_card.disable_route_checks = true;
if (mach->mach_params.dmic_num > 0) { snprintf(hda_soc_components, sizeof(hda_soc_components), diff --git a/sound/soc/intel/boards/skl_nau88l25_max98357a.c b/sound/soc/intel/boards/skl_nau88l25_max98357a.c index 0e7025834594..e4630c33176e 100644 --- a/sound/soc/intel/boards/skl_nau88l25_max98357a.c +++ b/sound/soc/intel/boards/skl_nau88l25_max98357a.c @@ -654,6 +654,7 @@ static struct snd_soc_card skylake_audio_card = { .dapm_routes = skylake_map, .num_dapm_routes = ARRAY_SIZE(skylake_map), .fully_routed = true, + .disable_route_checks = true, .late_probe = skylake_card_late_probe, };
diff --git a/sound/soc/intel/boards/skl_rt286.c b/sound/soc/intel/boards/skl_rt286.c index c59c60e28091..9a8044274908 100644 --- a/sound/soc/intel/boards/skl_rt286.c +++ b/sound/soc/intel/boards/skl_rt286.c @@ -523,6 +523,7 @@ static struct snd_soc_card skylake_rt286 = { .dapm_routes = skylake_rt286_map, .num_dapm_routes = ARRAY_SIZE(skylake_rt286_map), .fully_routed = true, + .disable_route_checks = true, .late_probe = skylake_card_late_probe, };
On 3/8/24 03:04, Cezary Rojewski wrote:
Topology files that are propagated to the world and utilized by the skylake-driver carry shortcomings in their SectionGraphs.
Since commit daa480bde6b3 ("ASoC: soc-core: tidyup for snd_soc_dapm_add_routes()") route checks are no longer permissive. Probe failures for Intel boards have been partially addressed by commit a22ae72b86a4 ("ASoC: soc-core: disable route checks for legacy devices") and its follow up but only skl_nau88l25_ssm4567.c is patched. Fix the problem for the rest of the boards.
Link: https://lore.kernel.org/all/20200309192744.18380-1-pierre-louis.bossart@linu... Fixes: daa480bde6b3 ("ASoC: soc-core: tidyup for snd_soc_dapm_add_routes()") Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
One of the framework responsibilities is to ensure that the enumerated DPCMs are valid i.e.: a valid BE is connected to a valid FE DAI. While the are checks in soc-core.c and soc-pcm.c that verify this, a component driver may attempt to workaround this by loading an invalid graph through the topology file.
Be strict and fail topology loading when invalid graph is encountered.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/soc-topology.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index d6d368837235..90ca37e008b3 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1083,8 +1083,15 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg, break; }
- /* add route, but keep going if some fail */ - snd_soc_dapm_add_routes(dapm, route, 1); + ret = snd_soc_dapm_add_routes(dapm, route, 1); + if (ret) { + if (!dapm->card->disable_route_checks) { + dev_err(tplg->dev, "ASoC: dapm_add_routes failed: %d\n", ret); + break; + } + dev_info(tplg->dev, + "ASoC: disable_route_checks set, ignoring dapm_add_routes errors\n"); + } }
return ret;
On 3/8/24 03:04, Cezary Rojewski wrote:
One of the framework responsibilities is to ensure that the enumerated DPCMs are valid i.e.: a valid BE is connected to a valid FE DAI. While the are checks in soc-core.c and soc-pcm.c that verify this, a component driver may attempt to workaround this by loading an invalid graph through the topology file.
Be strict and fail topology loading when invalid graph is encountered.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
A copy-paste from intel/boards/skl_nau88l25_ssm4567.c made the avs's equivalent disable route checks as well. Such behavior is not desired.
Fixes: 69ea14efe99b ("ASoC: Intel: avs: Add ssm4567 machine board") Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/boards/ssm4567.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/avs/boards/ssm4567.c b/sound/soc/intel/avs/boards/ssm4567.c index 4a0e136835ff..b64be685dc23 100644 --- a/sound/soc/intel/avs/boards/ssm4567.c +++ b/sound/soc/intel/avs/boards/ssm4567.c @@ -172,7 +172,6 @@ static int avs_ssm4567_probe(struct platform_device *pdev) card->dapm_routes = card_base_routes; card->num_dapm_routes = ARRAY_SIZE(card_base_routes); card->fully_routed = true; - card->disable_route_checks = true;
ret = snd_soc_fixup_dai_links_platform_name(card, pname); if (ret)
The card-name suffix and the DP-widgets are an unintended copy-paste from skl_nau88215_ssm4567.c. Both are redundant.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/boards/ssm4567.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/sound/soc/intel/avs/boards/ssm4567.c b/sound/soc/intel/avs/boards/ssm4567.c index b64be685dc23..abb87bb88fff 100644 --- a/sound/soc/intel/avs/boards/ssm4567.c +++ b/sound/soc/intel/avs/boards/ssm4567.c @@ -37,8 +37,6 @@ static const struct snd_kcontrol_new card_controls[] = { static const struct snd_soc_dapm_widget card_widgets[] = { SND_SOC_DAPM_SPK("Left Speaker", NULL), SND_SOC_DAPM_SPK("Right Speaker", NULL), - SND_SOC_DAPM_SPK("DP1", NULL), - SND_SOC_DAPM_SPK("DP2", NULL), };
static const struct snd_soc_dapm_route card_base_routes[] = { @@ -158,7 +156,7 @@ static int avs_ssm4567_probe(struct platform_device *pdev) if (!card) return -ENOMEM;
- card->name = "avs_ssm4567-adi"; + card->name = "avs_ssm4567"; card->dev = dev; card->owner = THIS_MODULE; card->dai_link = dai_link;
From: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com
Remove unnecessary widgets and routes as they are created by snd_soc_dapm_connect_dai_link_widgets() automatically.
Link: https://lore.kernel.org/all/20230612110958.592674-1-brent.lu@intel.com/ Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com --- sound/soc/intel/avs/boards/i2s_test.c | 79 --------------------------- 1 file changed, 79 deletions(-)
diff --git a/sound/soc/intel/avs/boards/i2s_test.c b/sound/soc/intel/avs/boards/i2s_test.c index 28f254eb0d03..282256d18cc6 100644 --- a/sound/soc/intel/avs/boards/i2s_test.c +++ b/sound/soc/intel/avs/boards/i2s_test.c @@ -54,76 +54,13 @@ static int avs_create_dai_link(struct device *dev, const char *platform_name, in return 0; }
-static int avs_create_dapm_routes(struct device *dev, int ssp_port, int tdm_slot, - struct snd_soc_dapm_route **routes, int *num_routes) -{ - struct snd_soc_dapm_route *dr; - const int num_dr = 2; - - dr = devm_kcalloc(dev, num_dr, sizeof(*dr), GFP_KERNEL); - if (!dr) - return -ENOMEM; - - dr[0].sink = devm_kasprintf(dev, GFP_KERNEL, - AVS_STRING_FMT("ssp", "pb", ssp_port, tdm_slot)); - dr[0].source = devm_kasprintf(dev, GFP_KERNEL, - AVS_STRING_FMT("ssp", " Tx", ssp_port, tdm_slot)); - if (!dr[0].sink || !dr[0].source) - return -ENOMEM; - - dr[1].sink = devm_kasprintf(dev, GFP_KERNEL, - AVS_STRING_FMT("ssp", " Rx", ssp_port, tdm_slot)); - dr[1].source = devm_kasprintf(dev, GFP_KERNEL, - AVS_STRING_FMT("ssp", "cp", ssp_port, tdm_slot)); - if (!dr[1].sink || !dr[1].source) - return -ENOMEM; - - *routes = dr; - *num_routes = num_dr; - - return 0; -} - -static int avs_create_dapm_widgets(struct device *dev, int ssp_port, int tdm_slot, - struct snd_soc_dapm_widget **widgets, int *num_widgets) -{ - struct snd_soc_dapm_widget *dw; - const int num_dw = 2; - - dw = devm_kcalloc(dev, num_dw, sizeof(*dw), GFP_KERNEL); - if (!dw) - return -ENOMEM; - - dw[0].id = snd_soc_dapm_hp; - dw[0].reg = SND_SOC_NOPM; - dw[0].name = devm_kasprintf(dev, GFP_KERNEL, - AVS_STRING_FMT("ssp", "pb", ssp_port, tdm_slot)); - if (!dw[0].name) - return -ENOMEM; - - dw[1].id = snd_soc_dapm_mic; - dw[1].reg = SND_SOC_NOPM; - dw[1].name = devm_kasprintf(dev, GFP_KERNEL, - AVS_STRING_FMT("ssp", "cp", ssp_port, tdm_slot)); - if (!dw[1].name) - return -ENOMEM; - - *widgets = dw; - *num_widgets = num_dw; - - return 0; -} - static int avs_i2s_test_probe(struct platform_device *pdev) { - struct snd_soc_dapm_widget *widgets; - struct snd_soc_dapm_route *routes; struct snd_soc_dai_link *dai_link; struct snd_soc_acpi_mach *mach; struct snd_soc_card *card; struct device *dev = &pdev->dev; const char *pname; - int num_routes, num_widgets; int ssp_port, tdm_slot, ret;
mach = dev_get_platdata(dev); @@ -156,26 +93,10 @@ static int avs_i2s_test_probe(struct platform_device *pdev) return ret; }
- ret = avs_create_dapm_routes(dev, ssp_port, tdm_slot, &routes, &num_routes); - if (ret) { - dev_err(dev, "Failed to create dapm routes: %d\n", ret); - return ret; - } - - ret = avs_create_dapm_widgets(dev, ssp_port, tdm_slot, &widgets, &num_widgets); - if (ret) { - dev_err(dev, "Failed to create dapm widgets: %d\n", ret); - return ret; - } - card->dev = dev; card->owner = THIS_MODULE; card->dai_link = dai_link; card->num_links = 1; - card->dapm_routes = routes; - card->num_dapm_routes = num_routes; - card->dapm_widgets = widgets; - card->num_dapm_widgets = num_widgets; card->fully_routed = true;
ret = snd_soc_fixup_dai_links_platform_name(card, pname);
On Fri, 08 Mar 2024 10:04:57 +0100, Cezary Rojewski wrote:
Set of loosely connected patches. Most impactful change is dropping any permisiveness when snd_soc_dapm_add_routes() fails in soc-topology.c To do it safely, disable route checks for all skylake-driver boards.
Relevant background:
Since commit daa480bde6b3 ("ASoC: soc-core: tidyup for snd_soc_dapm_add_routes()") route checks are no longer permissive. Probe failures for Intel boards have been partially addressed by commit a22ae72b86a4 ("ASoC: soc-core: isable route checks for legacy devices") and its follow up but only skl_nau88l25_ssm4567.c is patched. The rest of the boards still need fixing.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/5] ASoC: Intel: Disable route checks for Skylake boards commit: 0cb3b7fd530b8c107443218ce6db5cb6e7b5dbe1 [2/5] ASoC: topology: Do not ignore route checks when parsing graphs commit: 6974857c2b2c7e2d2db59c0e23cc42b0efc58cd8 [3/5] ASoC: Intel: avs: ssm4567: Do not ignore route checks commit: e6719d48ba6329536c459dcee5a571e535687094 [4/5] ASoC: Intel: avs: ssm4567: Board cleanup commit: 3a2be3f05110fa1a5c682ff72a26681ed4f54cef [5/5] ASoC: Intel: avs: i2s_test: Remove redundant dapm routes commit: 60c10c678b582d41532fefa12667d8adca75811b
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (3)
-
Cezary Rojewski -
Mark Brown -
Pierre-Louis Bossart