[PATCH 0/5] ASoC: topology: Clean up error messages handling
Series of patches cleaning up error messages when loading topology. In few places instead of logging in place of failure message is logged in caller. Additionally there are places where both caller and failing function log error, leading to unnecessary logs. Clean all of the above up.
Amadeusz Sławiński (5): ASoC: topology: Log control load errors in soc_tplg_control_load() ASoC: topology: Remove redundant logs ASoC: topology: Do not split message string on multiple lines ASoC: topology: Remove redundant log ASoC: topology: Remove redundant log
sound/soc/soc-topology.c | 63 ++++++++++++---------------------------- 1 file changed, 19 insertions(+), 44 deletions(-)
On 2023-05-19 9:56 PM, Amadeusz Sławiński wrote:
Series of patches cleaning up error messages when loading topology. In few places instead of logging in place of failure message is logged in caller. Additionally there are places where both caller and failing function log error, leading to unnecessary logs. Clean all of the above up.
Amadeusz Sławiński (5): ASoC: topology: Log control load errors in soc_tplg_control_load() ASoC: topology: Remove redundant logs ASoC: topology: Do not split message string on multiple lines ASoC: topology: Remove redundant log ASoC: topology: Remove redundant log
Marvelous! And not reviewed by me earlier at all :D
Reviewed-by: Cezary Rojewski cezary.rojewski@intel.com
On 5/19/23 14:56, Amadeusz Sławiński wrote:
Series of patches cleaning up error messages when loading topology. In few places instead of logging in place of failure message is logged in caller. Additionally there are places where both caller and failing function log error, leading to unnecessary logs. Clean all of the above up.
Amadeusz Sławiński (5): ASoC: topology: Log control load errors in soc_tplg_control_load() ASoC: topology: Remove redundant logs ASoC: topology: Do not split message string on multiple lines ASoC: topology: Remove redundant log ASoC: topology: Remove redundant log
Nice cleanup.
For the series:
Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/soc-topology.c | 63 ++++++++++++---------------------------- 1 file changed, 19 insertions(+), 44 deletions(-)
Simplify code by logging any errors in function that does the actual work instead of doing so in its callers.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/soc-topology.c | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 47ab5cf99497..242abbd875fa 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -585,11 +585,15 @@ EXPORT_SYMBOL_GPL(snd_soc_tplg_widget_bind_event); static int soc_tplg_control_load(struct soc_tplg *tplg, struct snd_kcontrol_new *k, struct snd_soc_tplg_ctl_hdr *hdr) { + int ret = 0; + if (tplg->ops && tplg->ops->control_load) - return tplg->ops->control_load(tplg->comp, tplg->index, k, - hdr); + ret = tplg->ops->control_load(tplg->comp, tplg->index, k, hdr);
- return 0; + if (ret) + dev_err(tplg->dev, "ASoC: failed to init %s\n", hdr->name); + + return ret; }
@@ -691,10 +695,8 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, size_t size)
/* pass control to driver for optional further init */ ret = soc_tplg_control_load(tplg, &kc, &be->hdr); - if (ret < 0) { - dev_err(tplg->dev, "ASoC: failed to init %s\n", be->hdr.name); + if (ret < 0) goto err; - }
/* register control here */ ret = soc_tplg_add_kcontrol(tplg, &kc, &sbe->dobj.control.kcontrol); @@ -776,10 +778,8 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, size_t size)
/* pass control to driver for optional further init */ ret = soc_tplg_control_load(tplg, &kc, &mc->hdr); - if (ret < 0) { - dev_err(tplg->dev, "ASoC: failed to init %s\n", mc->hdr.name); + if (ret < 0) goto err; - }
/* register control here */ ret = soc_tplg_add_kcontrol(tplg, &kc, &sm->dobj.control.kcontrol); @@ -945,10 +945,8 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, size_t size)
/* pass control to driver for optional further init */ ret = soc_tplg_control_load(tplg, &kc, &ec->hdr); - if (ret < 0) { - dev_err(tplg->dev, "ASoC: failed to init %s\n", ec->hdr.name); + if (ret < 0) goto err; - }
/* register control here */ ret = soc_tplg_add_kcontrol(tplg, &kc, &se->dobj.control.kcontrol); @@ -1162,11 +1160,8 @@ static int soc_tplg_dapm_widget_dmixer_create(struct soc_tplg *tplg, struct snd_
/* pass control to driver for optional further init */ err = soc_tplg_control_load(tplg, kc, &mc->hdr); - if (err < 0) { - dev_err(tplg->dev, "ASoC: failed to init %s\n", - mc->hdr.name); + if (err < 0) return err; - }
return 0; } @@ -1246,11 +1241,8 @@ static int soc_tplg_dapm_widget_denum_create(struct soc_tplg *tplg, struct snd_k
/* pass control to driver for optional further init */ err = soc_tplg_control_load(tplg, kc, &ec->hdr); - if (err < 0) { - dev_err(tplg->dev, "ASoC: failed to init %s\n", - ec->hdr.name); + if (err < 0) return err; - }
return 0; } @@ -1298,11 +1290,8 @@ static int soc_tplg_dapm_widget_dbytes_create(struct soc_tplg *tplg, struct snd_
/* pass control to driver for optional further init */ err = soc_tplg_control_load(tplg, kc, &be->hdr); - if (err < 0) { - dev_err(tplg->dev, "ASoC: failed to init %s\n", - be->hdr.name); + if (err < 0) return err; - }
return 0; }
soc_tplg_add_kcontrol() logs all the failures in detail already.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/soc-topology.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 242abbd875fa..b5c47b3c63ab 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -700,10 +700,8 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, size_t size)
/* register control here */ ret = soc_tplg_add_kcontrol(tplg, &kc, &sbe->dobj.control.kcontrol); - if (ret < 0) { - dev_err(tplg->dev, "ASoC: failed to add %s\n", be->hdr.name); + if (ret < 0) goto err; - }
list_add(&sbe->dobj.list, &tplg->comp->dobj_list);
@@ -783,10 +781,8 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, size_t size)
/* register control here */ ret = soc_tplg_add_kcontrol(tplg, &kc, &sm->dobj.control.kcontrol); - if (ret < 0) { - dev_err(tplg->dev, "ASoC: failed to add %s\n", mc->hdr.name); + if (ret < 0) goto err; - }
list_add(&sm->dobj.list, &tplg->comp->dobj_list);
@@ -950,10 +946,8 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, size_t size)
/* register control here */ ret = soc_tplg_add_kcontrol(tplg, &kc, &se->dobj.control.kcontrol); - if (ret < 0) { - dev_err(tplg->dev, "ASoC: could not add kcontrol %s\n", ec->hdr.name); + if (ret < 0) goto err; - }
list_add(&se->dobj.list, &tplg->comp->dobj_list);
Kernel coding guidelines recommend to not split string unnecessarily. While at it adapt the other print present in the function to 100 characters line limit.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/soc-topology.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index b5c47b3c63ab..0249e915eafe 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1513,15 +1513,13 @@ static int soc_tplg_dapm_complete(struct soc_tplg *tplg) * If so, just return success. */ if (!snd_soc_card_is_instantiated(card)) { - dev_warn(tplg->dev, "ASoC: Parent card not yet available," - " widget card binding deferred\n"); + dev_warn(tplg->dev, "ASoC: Parent card not yet available, widget card binding deferred\n"); return 0; }
ret = snd_soc_dapm_new_widgets(card); if (ret < 0) - dev_err(tplg->dev, "ASoC: failed to create new widgets %d\n", - ret); + dev_err(tplg->dev, "ASoC: failed to create new widgets %d\n", ret);
return ret; }
soc_tplg_dapm_complete() logs all the failures in detail already.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/soc-topology.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 0249e915eafe..9bb4c6d2a2c9 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -2510,9 +2510,6 @@ static int soc_tplg_process_headers(struct soc_tplg *tplg)
/* signal DAPM we are complete */ ret = soc_tplg_dapm_complete(tplg); - if (ret < 0) - dev_err(tplg->dev, - "ASoC: failed to initialise DAPM from Firmware\n");
return ret; }
soc_tplg_valid_header() logs all the failures in detail already.
Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/soc-topology.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 9bb4c6d2a2c9..20fd46a41cbb 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -2486,11 +2486,8 @@ static int soc_tplg_process_headers(struct soc_tplg *tplg)
/* make sure header is valid before loading */ ret = soc_tplg_valid_header(tplg, hdr); - if (ret < 0) { - dev_err(tplg->dev, - "ASoC: topology: invalid header: %d\n", ret); + if (ret < 0) return ret; - }
/* load the header object */ ret = soc_tplg_load_header(tplg, hdr);
On Fri, 19 May 2023 21:56:06 +0200, Amadeusz Sławiński wrote:
Series of patches cleaning up error messages when loading topology. In few places instead of logging in place of failure message is logged in caller. Additionally there are places where both caller and failing function log error, leading to unnecessary logs. Clean all of the above up.
Amadeusz Sławiński (5): ASoC: topology: Log control load errors in soc_tplg_control_load() ASoC: topology: Remove redundant logs ASoC: topology: Do not split message string on multiple lines ASoC: topology: Remove redundant log ASoC: topology: Remove redundant log
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/5] ASoC: topology: Log control load errors in soc_tplg_control_load() commit: ec5dffcd428f54c117158c7b2cd79a1e14aa5b70 [2/5] ASoC: topology: Remove redundant logs commit: 2316c11fa97779d06bfd7990f45b13a7b6ec1dae [3/5] ASoC: topology: Do not split message string on multiple lines commit: 5308540278d776e10519db144cb0cf3b3dd7ffbf [4/5] ASoC: topology: Remove redundant log commit: f9d1fe7e81b87378e7bb4a0be9c6fb29bbaa73c0 [5/5] ASoC: topology: Remove redundant log commit: f9d1fe7e81b87378e7bb4a0be9c6fb29bbaa73c0
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 (4)
-
Amadeusz Sławiński -
Cezary Rojewski -
Mark Brown -
Pierre-Louis Bossart