"Leon Ljunggren" <leon.ljunggren@xxxxxxxxx> writes:
> Hi,
> I working on a driver for a embedded system running Linux 2.6.20.3 and
> I'm having a bit of problem getting spin_lock to work as I'd expect it
> to.
>
> The driver reads from an AVR and before it does that one need do a
> write to tell it were to read is to take place. The problem is that I
> have a interupt function that will also perform such a read and if it
> interupts the user induced read after i2c_write but before i2c_read it
> can cause the read to happen in the wrong place.
>
> To ensure that this doesn't happen I'm trying to use
> spin_lock_irqsave, but I'm getting rather unexpected results. First of
> all it doesn't seem to disable the interupt, the interupt function is
> still called and secondly it just seems to ignore the lock. Even if
> avr_read has aquired lock2 and then gets interupted (which it
> shouldn't since interupts should be disabled, right?) avr_interupt
> will not care and still call the write/read functions instead of
> blocking as expected.
>
> Any pointers to what I might be doing wrong?
>
> Thanks
> /Leon Ljunggren
>
> ----- pseudo code ----
>
> spinlock_t lock1 = SPIN_LOCK_UNLOCKED;
> spinlock_t lock2 = SPIN_LOCK_UNLOCKED;
>
> i2c_read()
> {
> spin_lock_irqsave(&lock1, lock1_flag);
> ...
> spin_unlock_irqrestore(&lock2, lock2_flag);
> }
>
> i2c_write()
> {
> spin_lock_irqsave(&lock1, lock1_flag);
> ...
> spin_unlock_irqrestore(&lock2, lock2_flag);
> }
Where do you unlock lock1? And why do you have 2 locks at all?
> i2c_irq_read()
> {
> spin_lock_irqsave(&lock1, lock1_flag);
> ...
> spin_unlock_irqrestore(&lock2, lock2_flag);
> }
>
> i2c_irq_write()
> {
> spin_lock_irqsave(&lock1, lock1_flag);
> ...
> spin_unlock_irqrestore(&lock2, lock2_flag);
> }
>
> avr_read()
> {
> spin_lock_irqsave(&lock2, lock2_flag);
> i2c_write();
After that, lock2 is unlocked, you re-enabled irqs and lock1 is taken.
At this time an interrupt may fire and deadlock on lock1 (because there
is no way, the spinlock is unlocked by the interrupted context). This
makes not much sense to me.
What you basically want is serializing a write->read sequence between
process and interrupt context, right? So you need one lock these two
paths serialize against:
spin_lock_t avr_lock = UNLOCKED
avr_read()
{
spin_lock_irqsave(&avr_lock, flags)
i2c_write()
i2c_read()
spin_unlock_irqrestore(&avr_lock, flags)
}
avr_interrupt()
{
avr_read()
}
where i2c_read/write don't do any extra locking for that matter.
If avr_read() is already serialized by a higher order lock with respect
to concurrent user processes, local_irq_save()/_restore() would also be
enough. But with the spin lock versions you should be on the safe side.
HTH,
Hannes
--
To unsubscribe from this list: send an email with
"unsubscribe kernelnewbies" to ecartis@xxxxxxxxxxxx
Please read the FAQ at http://kernelnewbies.org/FAQ
