Re: [PATCH 1/3] Fix a problem during the access to the IER and ISR registers of the SA7146

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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/
--------------------------------------------------------


_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux