[linux-dvb] [PATCH 1/3] Fix a problem during the access to the IER and ISR registers of the SA7146

Oliver Endriss o.endriss at gmx.de
Wed Nov 1 17:34:57 CET 2006


e9hack wrote:
> Oliver Endriss wrote:
> > e9hack wrote:
> >   
> >   
> >>                 SAA7146_IER_ENABLE(dev, MASK_16|MASK_17);
> >>                 saa7146_write(dev, MC2, (MASK_00 | MASK_16));
> >>  
> >> -               wait_event_interruptible(dev->i2c_wq, dev->i2c_op == 0);
> >> -               if (signal_pending (current)) {
> >> -                       /* a signal arrived */
> >> -                       return -ERESTARTSYS;
> >> +               timeout = HZ/100 + 1; /* 10ms */
> >> +               timeout = wait_event_interruptible_timeout(dev->i2c_wq, dev->i2c_op == 0, timeout);
> >> +               if (timeout == -ERESTARTSYS || dev->i2c_op) {
> >> +                       SAA7146_IER_DISABLE(dev, MASK_16|MASK_17);
> >> +                       SAA7146_ISR_CLEAR(dev, MASK_16|MASK_17);
> >> +                       if (timeout == -ERESTARTSYS) {
> >> +                               /* a signal arrived */
> >> +                               return -ERESTARTSYS;
> >> +                       }
> >>     
> >
> > Please remove '{' and '}' - kernel coding style ;-)
> >   
> The original code contains also this brackets. I removed it.

I know. This is old code, written before DVB was part of the kernel.
You can find coding style violations almost everywhere in the drivers.
Anyway, we should fix it on the fly in a piece of code when we are
touching it. ;-)

> >> +                       /* this is normal when probing the bus
> >> +                        * (no answer from nonexisistant device...)
> >> +                        */
> >> +                       DEB_I2C(("saa7146_i2c_writeout: timed out waiting for end of xfer\n"));
> >> +                       return -EIO;
> >>     
> >
> > Are you able to trigger this timeout? Is yes, how?
> >
> > Imho it cannot happen anymore (because of [1]) unless the hardware is
> > broken. Even with nonexistent devices there should be an I2C irq.
> >
> > Anyway, it is a good idea to add this timeout protection.
> >   
> I didn't see such a timeout.
> > If it cannot happen under normal circumstances you should replace
> > DEB_I2C by printk.
> >   
> I changed it.

Ok, I committed both patches to
- http://linuxtv.org/hg/~endriss/v4l-dvb
- http://linuxtv.org/hg/~endriss/v4l-dvb-av7110-refactoring

Oliver

-- 
--------------------------------------------------------
VDR Remote Plugin 0.3.8 available at
http://www.escape-edv.de/endriss/vdr/
--------------------------------------------------------




More information about the linux-dvb mailing list