commit: dpcm: Allow FE to be opened without valid BE routes

commit: dpcm: Allow FE to be opened without valid BE routes

[anish kumar]

Commit 3ac3f5ca91afc03587b1d2d642f126efc5be37ca
ASoC: dpcm: Allow FE to be opened without valid BE routes.

        if (dpcm_path_get(fe, stream, &list) <= 0) {
-               dev_warn(fe->dev, "asoc: %s no valid %s route\n",
+               dev_dbg(fe->dev, "asoc: %s no valid %s route\n",
                        fe->dai_link->name, stream ? "capture" :
"playback");
-                       mutex_unlock(&fe->card->mutex);
-                       return -EINVAL;
        }


As I can see this commit has changed the log level from dev_warn to
dev_dbg but this has caused a subtle problem.Right now we rely on this
warning to find out issues related to "application not sending UCM
commands causing mute issues".Sometimes we have seen that the device
becomes mute due to application not sending UCM command properly and
these issues are hard to reproduce(all we get is dump).So all we have is
this log to find out if the FE(front-end) and BE(back-end) has at-lease
matched or not and most of the time it doesn't and we find out that
using this very log.

With this gone it would be difficult to find out the problem.I don't see
any harm in reverting it back to dev_warn as it will warn us that "still
we have not received UCM command for turning on Back-end" and in case we
receive it then this log hardly matters and just gives us information
that "at that time it was not a valid route".

If everyone agrees then I would be more than happy to send a patch.

Re: commit: dpcm: Allow FE to be opened without valid BE routes

[Liam Girdwood]

On 06/11/12 17:03, anish kumar wrote:
> Commit 3ac3f5ca91afc03587b1d2d642f126efc5be37ca
> ASoC: dpcm: Allow FE to be opened without valid BE routes.
>
>          if (dpcm_path_get(fe, stream, &list) <= 0) {
> -               dev_warn(fe->dev, "asoc: %s no valid %s route\n",
> +               dev_dbg(fe->dev, "asoc: %s no valid %s route\n",
>                          fe->dai_link->name, stream ? "capture" :
> "playback");
> -                       mutex_unlock(&fe->card->mutex);
> -                       return -EINVAL;
>          }
>
>
> As I can see this commit has changed the log level from dev_warn to
> dev_dbg but this has caused a subtle problem.Right now we rely on this
> warning to find out issues related to "application not sending UCM
> commands causing mute issues".Sometimes we have seen that the device
> becomes mute due to application not sending UCM command properly and
> these issues are hard to reproduce(all we get is dump).So all we have is
> this log to find out if the FE(front-end) and BE(back-end) has at-lease
> matched or not and most of the time it doesn't and we find out that
> using this very log.
>
> With this gone it would be difficult to find out the problem.I don't see
> any harm in reverting it back to dev_warn as it will warn us that "still
> we have not received UCM command for turning on Back-end" and in case we
> receive it then this log hardly matters and just gives us information
> that "at that time it was not a valid route".
>
> If everyone agrees then I would be more than happy to send a patch.
>

I did the change since it is quite valid use case to open the FE PCM device 
without a configured route to a BE (as some Android devices do). This was 
filling up the log with lots of warning messages when in fact there was 
nothing wrong.

It sounds like from your description you are doing some debugging and 
dev_dbg() is intended for this.

Liam

Re: commit: dpcm: Allow FE to be opened without valid BE routes

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 849 bytes --]

On Wed, Nov 07, 2012 at 02:03:14AM +0900, anish kumar wrote:

> As I can see this commit has changed the log level from dev_warn to
> dev_dbg but this has caused a subtle problem.Right now we rely on this
> warning to find out issues related to "application not sending UCM

You shouldn't be relying on parsing dmesg for anything, that's never
going to be robust as you've just found.  dmesg is for humans.

> If everyone agrees then I would be more than happy to send a patch.

Please come up with a better way of identifying this; something like
information in debugfs for example or working on getting the media
controller API support merged and then using that to parse out the
routing map from that and notice the misconfiguration.  This would
be much better anyway since the same thing can happen elsewhere in DAPM
and your current mechanism.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: commit: dpcm: Allow FE to be opened without valid BE routes

[anish kumar]

On Wed, 2012-11-07 at 09:48 +0100, Mark Brown wrote:
> On Wed, Nov 07, 2012 at 02:03:14AM +0900, anish kumar wrote:
> 
> > As I can see this commit has changed the log level from dev_warn to
> > dev_dbg but this has caused a subtle problem.Right now we rely on this
> > warning to find out issues related to "application not sending UCM
> 
> You shouldn't be relying on parsing dmesg for anything, that's never
> going to be robust as you've just found.  dmesg is for humans. 
Probably I couldn't explain the problem clearly.Problem is that in our
case we get sometimes(very random) audio mute in android devices due to
UCM commands from the platform not sent properly or sent in such a way
that FE and BE don't get connected.
This issues are very hard to reproduce and only gets reproduced randomly
by the test engineer and when it does happen, test engineer takes crash
dump and provides us the log.So it is almost impossible to know what
caused this audio mute at that time and all we have is this very
log(dev_warn(FE and BE not connected)) to let us know that this mute was
due to platform and not codec or any other problem with the kernel.
Anyway looks like this idea is not liked by the community so I won't
push further.
Thanks for reading.
> 
> > If everyone agrees then I would be more than happy to send a patch.
> 
> Please come up with a better way of identifying this; something like
> information in debugfs for example or working on getting the media
> controller API support merged and then using that to parse out the
> routing map from that and notice the misconfiguration.  This would
> be much better anyway since the same thing can happen elsewhere in DAPM
> and your current mechanism.

Re: commit: dpcm: Allow FE to be opened without valid BE routes

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1318 bytes --]

On Wed, Nov 07, 2012 at 09:56:39PM +0900, anish kumar wrote:
> On Wed, 2012-11-07 at 09:48 +0100, Mark Brown wrote:

> > You shouldn't be relying on parsing dmesg for anything, that's never
> > going to be robust as you've just found.  dmesg is for humans. 

> Probably I couldn't explain the problem clearly.Problem is that in our
> case we get sometimes(very random) audio mute in android devices due to
> UCM commands from the platform not sent properly or sent in such a way
> that FE and BE don't get connected.
> This issues are very hard to reproduce and only gets reproduced randomly
> by the test engineer and when it does happen, test engineer takes crash
> dump and provides us the log.So it is almost impossible to know what
> caused this audio mute at that time and all we have is this very
> log(dev_warn(FE and BE not connected)) to let us know that this mute was
> due to platform and not codec or any other problem with the kernel.
> Anyway looks like this idea is not liked by the community so I won't
> push further.

This really just sounds like a basic debugging issue, you can either add
additional trace locally or work to improve the trace infrastructure -
for example, make sure there's tracepoints showing what DPCM does in the
same way there are tracepoints for DAPM.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]