On Wed, 2012-04-25 at 18:01 -0500, Ricardo Neri wrote:
If so, we need to make sure it's not currently called in an atomic context, because it would break if the function will sleep in the future. And with "make sure" I just mean that we check the code and keep it in mind. Or perhaps adding a comment in the header, that says "audio_enable may sleep, other audio functions do not sleep" or such.
I revisited the ALSA code. Only the audio_start function is atomic. Although ALSA may not be the only user, it makes sense to me to think that they will follow a similar approach in terms of locks.
Hence, based on that and on the reasons you describe (audio_enable potentially taking too long to return), Rephrasing what you stated, a mutex may be used for the enable/disable and config operations. Only start/stop would be protected by a spinlock. This should be described in comments in the header file. Does it make sense to you?
Yes. Well, you don't need to mention the locks, they are internal implementation details. It should be enough to say that start/stop never sleeps and the other functions may sleep.
And, this is obvious but just to be sure, note that you should use spinlocks in all of the functions if possible. We just need to make sure we can use mutex in the future if we need to.
Tomi