On Mon, Nov 28, 2005 at 08:56:33PM -0500, Steve Toth wrote:
>
> >>There's some dubious timeout code in cx24123_pll_writereg(), I guess
> >>this should go. Is this copied from the Windoze driver?
> >>
> >
> >I didn't receive an answer to this. Do you care to comment?
> >Can we just drop the timeout cruft?
> >
> Windows code, no. What's dubious about it?
>
> It's supposed to avoid situations where the pll isn't working correctly.
> It returns an error and avoids hanging situations. Which I've personally
> witnessed. If you don't like safety, it can be removed.
Let's quote from your patch:
+ timeout=0;
+ /* write the msb 8 bits, wait for the send to be completed */
+ cx24123_writereg(state, 0x22, (data>>16) & 0xff);
+ while ( ( cx24123_readreg(state, 0x20) & 0x40 ) == 0 )
+ {
+ /* Safety - No reason why the write should not complete, and we never get here, avoid hang */
+ if (timeout++ >= 4) {
+ printk("%s: demodulator is no longer responding, aborting.\n",__FUNCTION__);
+ return -EREMOTEIO;
+ }
+ msleep(500);
+ }
"No reason why the write should not complete, and we never get here"?
500 msec delay?
I haven't got the data sheet so I can only guess what the code is doing,
but it looks fishy. ;-)
What is (cx24123_readreg(state, 0x20) & 0x40) testing for?
Johannes
