At Wed, 6 Nov 2013 16:03:04 +0000, Mark Brown wrote:
On Wed, Nov 06, 2013 at 01:03:41PM +0100, Takashi Iwai wrote:
Mark Brown wrote:
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...
If the driver is modifying the DAPM data then it looses anyway.
Yes, but you cannot know in DAPM level. That's the point. In soc-dapm.c, you know only something is wrong. But you can never know how severely wrong it is.
Practically seen, using BUG_ON() makes such debugging even more difficult.
That's not been my experience at all with these ones, it's much more common for people to ignore soft failures and then wonder why they get breakage further down the line.
This is also partly a function of the usage model for these drivers - if someone is triggering an issue they will almost certainly be doing development on the system. There aren't the problems with random system configurations or unknown hardware that you see with specs like HDA.
My concern here is that dapm isn't in the position to decide whether you press the self destruction switch at all. DAPM, or in general ASoC core, should serve at best it can, should return an error if something looks weird. As repeatedly mentioned, in the layer of DAPM, you cannot know what could be wrong, have too little to judge whether it's an emergency exit or not.
So, basically what such lazy people would need is a knob to make WARN*() triggering panic(). But this shouldn't be done unconditionally for all DAPM usages, or better in all sound driver usages.
BTW, another substantial problem is that this dapm code doesn't handle any errors...
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...
They shouldn't be generated unmatched but it's extremely rare for them both to actually be used by one widget.
So, the unprocessed list item must be filtered out.
To the extent that they're used matched it's with the PRE_PMU matched with POST_PMD and vice versa, though using just one is quite common for inserting delays. It's going to cause more harm than good to actively suppress events.
It won't suppress any events that have been working. Remember that there's been BUG_ON() with the very same condition before the patch. Thus any good-working cases must not hit with this conversion.
Takashi