[alsa-devel] [PATCH 21/31] ASoC: dapm: Use WARN_ON() instead of BUG_ON()

Mark Brown broonie at kernel.org
Thu Nov 7 12:31:45 CET 2013


On Thu, Nov 07, 2013 at 08:31:59AM +0100, Takashi Iwai wrote:

> > But then we're still left with bombing out if the first half of the
> > function ever changes.  The check is there for the benefit of the
> > register handling, the fact that it's affecting events is just a side
> > effect rather than the intention.

> Actually the check reg == w->reg must be always true for all pending
> items, no matter at which point.  Otherwise BUG() would have hit.

Right, but that's not actually what the check is there for.  The loop is
both doing an event check and building up the register value to write,
the check is to tell us if we're confused about the register value since
we'll end up writing to the wrong register during coalescing.

> That being said, maybe a cleaner way would be to run the check over
> the loop once, and quit the function if anything inconsistent found.

Or do two passes, one for the registers and one for the events.  You
could also reorder the loop to pull the events to the top and then do
the check and skip between them and the value update, or simply remove
the BUG().

Or just don't worry about it and leave the second loop alone since none
of this is ideal - possibly the most helpful thing would be to insert a
non-coalesced write for the out of place widget, though depending on why
things are going wrong perhaps that'll just make things worse and we
should be trying to unwind the entire transaction instead.  Unwinding
the entire thing is going to be the generally safest but least helpful
option.

My main concern here is not to confuse the code by having the check for
the register value protecting an event call, that'll be confusing later
on.  If you just want to fix the BUG()s do that alone, change them to
WARN()s or whatever.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20131107/ffe51770/attachment.sig>


More information about the Alsa-devel mailing list