Johannes Stezenbach wrote:
> 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?
>
>
The demod uses a private 3 wire interface to communicate with the tuner.
This is basically waiting for the tuner to go ready for the next
available command/byte. If it doesn't, and I saw this once or twice (the
pll seemed to die), it makes sure we never end up in a hanging situation.
It may of been a bad hardware sample, or bad timing generally but I
though the added safety didn't hurt.
We can remove the timeout code if you feel the safety is unnecessary.
Steve
