diff mbox series

ASoC: soc-pcm: change error message to debug message

Message ID 1612771965-5776-1-git-send-email-shengjiu.wang@nxp.com (mailing list archive)
State Accepted
Commit b6eabd247db8bb2d013fb9a9451ecb04a44ee58f
Headers show
Series ASoC: soc-pcm: change error message to debug message | expand

Commit Message

Shengjiu Wang Feb. 8, 2021, 8:12 a.m. UTC
This log message should be a debug message, because it
doesn't return directly but continue next loop.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/soc-pcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Pierre-Louis Bossart Feb. 8, 2021, 3:06 p.m. UTC | #1
On 2/8/21 2:12 AM, Shengjiu Wang wrote:
> This log message should be a debug message, because it
> doesn't return directly but continue next loop.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>   sound/soc/soc-pcm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 605acec48971..cd9e919d7b99 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1344,8 +1344,8 @@ static int dpcm_add_paths(struct snd_soc_pcm_runtime *fe, int stream,
>   		/* is there a valid BE rtd for this widget */
>   		be = dpcm_get_be(card, widget, stream);
>   		if (!be) {
> -			dev_err(fe->dev, "ASoC: no BE found for %s\n",
> -					widget->name);
> +			dev_dbg(fe->dev, "ASoC: no BE found for %s\n",
> +				widget->name);

Do we really want to do this?

This error message has historically been the means by which we detect 
that userspace didn't set the right mixers (e.g. on Intel Baytrail) or 
the topology was incorrect. And it's really an error in the sense that 
you will not get audio in or out.

If you demote this to dev_dbg, we'll have to ask every single user who 
reports 'sound is broken' to enable dynamic debug traces. I really don't 
see the benefit, this is a clear case of 'fail big and fail early', 
partly concealing the problem doesn't make it go away but harder to 
diagnose.
Shengjiu Wang Feb. 9, 2021, 7:37 a.m. UTC | #2
On Tue, Feb 9, 2021 at 12:39 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
>
> On 2/8/21 2:12 AM, Shengjiu Wang wrote:
> > This log message should be a debug message, because it
> > doesn't return directly but continue next loop.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >   sound/soc/soc-pcm.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> > index 605acec48971..cd9e919d7b99 100644
> > --- a/sound/soc/soc-pcm.c
> > +++ b/sound/soc/soc-pcm.c
> > @@ -1344,8 +1344,8 @@ static int dpcm_add_paths(struct snd_soc_pcm_runtime *fe, int stream,
> >               /* is there a valid BE rtd for this widget */
> >               be = dpcm_get_be(card, widget, stream);
> >               if (!be) {
> > -                     dev_err(fe->dev, "ASoC: no BE found for %s\n",
> > -                                     widget->name);
> > +                     dev_dbg(fe->dev, "ASoC: no BE found for %s\n",
> > +                             widget->name);
>
> Do we really want to do this?
>
> This error message has historically been the means by which we detect
> that userspace didn't set the right mixers (e.g. on Intel Baytrail) or
> the topology was incorrect. And it's really an error in the sense that
> you will not get audio in or out.
>
> If you demote this to dev_dbg, we'll have to ask every single user who
> reports 'sound is broken' to enable dynamic debug traces. I really don't
> see the benefit, this is a clear case of 'fail big and fail early',
> partly concealing the problem doesn't make it go away but harder to
> diagnose.

Thanks for the explanation,  it seems I misunderstood this error message.

Best regards
Wang shengjiu
Mark Brown Feb. 9, 2021, 10:23 p.m. UTC | #3
On Mon, Feb 08, 2021 at 09:06:44AM -0600, Pierre-Louis Bossart wrote:

> If you demote this to dev_dbg, we'll have to ask every single user who
> reports 'sound is broken' to enable dynamic debug traces. I really don't see
> the benefit, this is a clear case of 'fail big and fail early', partly
> concealing the problem doesn't make it go away but harder to diagnose.

Don't you also get the same information out of the DAPM debugfs or did
I misread where the error is generated from?
Pierre-Louis Bossart Feb. 9, 2021, 11:05 p.m. UTC | #4
On 2/9/21 4:23 PM, Mark Brown wrote:
> On Mon, Feb 08, 2021 at 09:06:44AM -0600, Pierre-Louis Bossart wrote:
> 
>> If you demote this to dev_dbg, we'll have to ask every single user who
>> reports 'sound is broken' to enable dynamic debug traces. I really don't see
>> the benefit, this is a clear case of 'fail big and fail early', partly
>> concealing the problem doesn't make it go away but harder to diagnose.
> 
> Don't you also get the same information out of the DAPM debugfs or did
> I misread where the error is generated from?

I re-checked and I will back-pedal on my comment. I confused this error 
message with the classic "ASoC: no backend DAIs enabled for %s".

I didn't find a single occurrence of this "ASoC: no BE found for %s" in 
any bug report or Google search.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Mark Brown Feb. 10, 2021, 8:11 p.m. UTC | #5
On Mon, 8 Feb 2021 16:12:45 +0800, Shengjiu Wang wrote:
> This log message should be a debug message, because it
> doesn't return directly but continue next loop.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: soc-pcm: change error message to debug message
      commit: b6eabd247db8bb2d013fb9a9451ecb04a44ee58f

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
diff mbox series

Patch

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 605acec48971..cd9e919d7b99 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1344,8 +1344,8 @@  static int dpcm_add_paths(struct snd_soc_pcm_runtime *fe, int stream,
 		/* is there a valid BE rtd for this widget */
 		be = dpcm_get_be(card, widget, stream);
 		if (!be) {
-			dev_err(fe->dev, "ASoC: no BE found for %s\n",
-					widget->name);
+			dev_dbg(fe->dev, "ASoC: no BE found for %s\n",
+				widget->name);
 			continue;
 		}