At Wed, 6 Nov 2013 11:48:36 +0000, Mark Brown wrote:
[1 <text/plain; us-ascii (quoted-printable)>] On Wed, Nov 06, 2013 at 12:33:14PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
Please fix the comment about BUG_ON() being useless; it does exactly what it's supposed to do.
Well, I can correct the word "useless" with "stupid" in the comment, as BUG_ON() is the most stupid idea to do at such a point. soc-dapm.c is the common code that can be executed from any others. How you can know that the machine *must* be killed suddenly at this point? If you didn't attach the console beforehand, you can't get even any debug logs.
The point is that it's not appropriate in this situation but not useless; if it were useless you should be sending a patch to remove it from the kernel entirely.
OK, feel free to run s/useless/inappropriate/ in the patch. I don't mind wording but only codes.
Using BUG() and BUG_ON() in buggy points without any really serious damage (like heavy memory corruption or data corruption) has to be avoided. Linus stated this in the past, too. (I have no pointer now, unfortunately.)
The ones in DAPM are mostly about memory corruption; if this stuff is getting confused then there's a good chance that the data structures that it's looking at aren't what they're supposed to be.
This isn't so easy to guess. Such a bug is often a memory corruption indeed, but it's not necessarily so. The data can be modified by the device drivers that are calling dapm, so everything is possible. Driver writers can do anything unbelievable you can never imagine...
Practically seen, using BUG_ON() makes such debugging even more difficult.
list_for_each_entry(w, pending, power_list) {
if (WARN_ON(reg != w->reg))continue;We're not using reg here...
Because you must not issue *_POST_* events without *_PRE_* events. *_PRE_* are skipped for these entries in the previous WARN_ON() checks.
No, that's not the case at all - it's very rare for devices to have both events in the first place.
Hrm, are you saying that *_POST events can be issued without the corresponding *_PRE events, as an official dpam design? I didn't expect such a bad consistency in API design level...
Takashi