Re: [PATCH 10/10] OMAPDSS: HDMI: Implement DSS driver interface for audio

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

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


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]