17 Feb
2016
17 Feb
'16
10:07 a.m.
On 02/17/2016 01:31 AM, Russell King - ARM Linux wrote:
On Fri, Jan 22, 2016 at 06:48:28PM +0100, Arnaud Pouliquen wrote:
+static int snd_pcm_iec958_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *uctl)+{
- struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
- if (params->mutex)
mutex_unlock(params->mutex);- uctl->value.iec958.status[0] = params->iec->status[0];
- uctl->value.iec958.status[1] = params->iec->status[1];
- uctl->value.iec958.status[2] = params->iec->status[2];
- uctl->value.iec958.status[3] = params->iec->status[3];
- uctl->value.iec958.status[4] = params->iec->status[4];
- if (params->mutex)
mutex_unlock(params->mutex);I thought I had already commented about the mutex - maybe not. Please don't make these conditional like this: if you need the mutex to eliminate a race condition, then you need it to eliminate the race condition and it can't be "optional". Elimination of race conditions is never optional!
Please get rid of all these conditions and make it mandatory that a mutex is supplied.
Thanks.
Ok, i will suppress condition. I also need to use a spinlock instead of the mutex, to be able to support race condition with atomic callbacks (trigger, pointer, and ack)