On Fri, Dec 04, 2009 at 10:31:06AM +0100, Tobias Schneider wrote:
Actually I know where the problem in my code is: inside the put callback (see below) I am calling a function (dsp_setoutput) that is sending the signal to the DSP, there I wanted to wait for the reply of the DSP to get a feedback whether the request was successful or not. This was done by a semaphore (going down and waiting for IRQ from DSP that is setting sema up), and thats the point where the posted bug occured.
So I was just wondering that it seems that there is no possibility to wait for a reply of the DSP, to be sure that a given value has been set?
(for the interested ones, here is the code..)
static int snd_mychip_output_set_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_mychip *mychip = snd_kcontrol_chip(kcontrol); int addr = kcontrol->private_value; short change = 0;
spin_lock_irq(&mychip->mixer_lock);
As I said - here you're holding a spinlock ...
if (ucontrol->value.integer.value[0] != mychip->output_set[addr]) { if (dsp_setoutput(addr+1, ucontrol->value.integer.value[0]) >= 0) { mychip->output_set[addr] = ucontrol->value.integer.value[0]; change = 1; } else // error sending signal { change = -EBUSY; // device or resource busy } } spin_unlock_irq(&mychip->mixer_lock); return change; }
later on in the called function... // wait for reply via semaphore while(!end) { if (down_interruptible(&dsp_reply_sema)==-EINTR) // HERE WE GET
And here you allow the CPU to reschedule while waiting for a mutex. That doesn't fly. You should remove the spinlock from the function above and move it to the actual lowlevel I/O function (whatever dsp_setoutput() does eventually).
Think about where locking is actually needed, at which point your function flow must not be interrupted by anything to avoid data corruption. Use locking there, and remove it from other places.
This might also be a good read to get an overview:
http://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/index.htm...
Another point: I figured out that the "ALSA middlelayer" seems to filter the given values of a control. So if the user sets value=1000 where maximum is 10, I will get 10 instead of 1000 in the put callback...so it's not necessary to check values in those callbacks, but unfortunately that's not always good - because sometimes it doesn't make sense to use the max or min values if the user just entered a wrong number...
For integer values, it does make sense to clamp values to their limits. For enums, that might be different. Not sure what the ALSA core does there.
Daniel