Re: [PATCH 2/3] [media] tuner-xc2028: Fix signal strength report

[Mauro Carvalho Chehab <mchehab@redhat.com>]

Em 05-07-2012 11:31, Devin Heitmueller escreveu:
> On Thu, Jul 5, 2012 at 10:25 AM, Devin Heitmueller
> <dheitmueller@kernellabs.com> wrote:
>> On Thu, Jul 5, 2012 at 10:16 AM, Mauro Carvalho Chehab
>>> -       /* Use both frq_lock and signal to generate the result */
>>> -       signal = signal || ((signal & 0x07) << 12);
>>> +       /* Signal level is 3 bits only */
>>> +
>>> +       signal = ((1 << 12) - 1) | ((signal & 0x07) << 12);
>>
>> Are you sure this is correct?   It's entirely possible the original
>> code used a logical or because the signal level isn't valid unless
>> there is a lock.  The author may have been intending to say:
>>
>> if (signal != 0) /* There is a lock, so set the signal level */
>>    signal = (signal & 0x07) << 12;
>> else
>>    signal = 0 /* Leave signal level at zero since there is no lock */
>>
>> I agree that the way the original code was written is confusing, but
>> it may actually be correct.

No, the intention there were to do a bit OR. The idea there was that, 
if a lock was given, some signal would be received. The real signal
level would be identified by the remaining bits.

What it was happening due to the code mistake was:

if (lock)
	return 1;
else
	return 0;

> On second reading, it would have needed to be a logical AND, not an OR
> in order for my suggestion to have been correct.
> 
> That said, empirical results are definitely a stronger argument in
> this case.  You did test this change in cases with no signal, signal
> lock with weak signal, and signal lock with strong signal, right?

Yes, it was tested and the new code works fine: it returns 0 without signal
and it returns a value between 0 and 65535 depending on the signal strength.

Just like the DVB API, the V4L2 API spec is not clear about what type of
range should be applied for the signal (linea range? dB?). It just says
that it should be between 0 and 65555.

In the case of xc2028/3028, there are only 3 bits for signal strengh. The 
levels are in dB, with an 8dB step, where 0 means a signal weaker than 8dB 
and 7 means 56 dB or upper.

The code should now be coherent with that.

Regards,
Mauro