e9hack wrote:
> Hi,
>
> it exist some macros to access the IER and ISR registers of the SAA7146. This
> macros are using a read and a write
> operation and this macros are executed inside of the interrupt handler of the
> SAA7146 and outside of it. It exist a
> reentrant problem. The interrupt handler may intercept the execution of such
> a macro and may also access the IER and/or
> ISR registers. The access to the IER and ISR register must be protect by a
> locking primitive. The attached patch does
> fix this problem.
>
> The patch does not fix the stradis driver. This driver has the same problem.
Now I found some time to look at your patch:
> Protect the access to the IER/ISR register of the SAA7146 by the device
> spinlock.
Imho it is not necessary to protect write operations to the ISR because
it is a single write-only operation.
SAA7146_IER_DISABLE/SAA7146_IER_ENABLE must be protected by a spinlock
because it is a read-modify-write operation.
So your patch could be replaced by the attached one.
What do you think?
Oliver
--
--------------------------------------------------------
VDR Remote Plugin 0.3.8 available at
http://www.escape-edv.de/endriss/vdr/
--------------------------------------------------------
diff -r c9cc116948c3 linux/include/media/saa7146.h
--- a/linux/include/media/saa7146.h Sun Oct 29 21:41:06 2006 +0100
+++ b/linux/include/media/saa7146.h Tue Oct 31 06:27:48 2006 +0100
@@ -54,10 +54,20 @@ extern unsigned int saa7146_debug;
#define DEB_INT(x) if (0!=(DEBUG_VARIABLE&0x20)) { DEBUG_PROLOG; printk x; } /* interrupt debug messages */
#define DEB_CAP(x) if (0!=(DEBUG_VARIABLE&0x40)) { DEBUG_PROLOG; printk x; } /* capture debug messages */
-#define SAA7146_IER_DISABLE(x,y) \
- saa7146_write(x, IER, saa7146_read(x, IER) & ~(y));
-#define SAA7146_IER_ENABLE(x,y) \
- saa7146_write(x, IER, saa7146_read(x, IER) | (y));
+#define SAA7146_IER_DISABLE(x,y) \
+ do { \
+ unsigned long flags; \
+ spin_lock_irqsave(&x->slock, flags); \
+ saa7146_write(x, IER, saa7146_read(x, IER) & ~(y)); \
+ spin_unlock_irqrestore(&x->slock, flags); \
+ } while(0)
+#define SAA7146_IER_ENABLE(x,y) \
+ do { \
+ unsigned long flags; \
+ spin_lock_irqsave(&x->slock, flags); \
+ saa7146_write(x, IER, saa7146_read(x, IER) | (y)); \
+ spin_unlock_irqrestore(&x->slock, flags); \
+ } while(0)
#define SAA7146_ISR_CLEAR(x,y) \
saa7146_write(x, ISR, (y));
_______________________________________________
linux-dvb mailing list
[email protected]
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
