Re: [BUG] [PATCH 10/21] radio-tea5764: some cleanups and clamp frequency when out-of-range AND [PATCH 15/21] tef6862: clamp frequency.

[Hans Verkuil <hverkuil@xs4all.nl>]

Hi Hans,

On 10/28/2013 12:45 PM, Hans Petter Selasky wrote:
> On 05/31/13 12:02, Hans Verkuil wrote:
>>   		return -EINVAL;
>> +	}
>> +	clamp(freq, FREQ_MIN * FREQ_MUL, FREQ_MAX * FREQ_MUL);
>>   	tea5764_power_up(radio);
>> -	tea5764_tune(radio, (f->frequency * 125) / 2);
>> +	tea5764_tune(radio, (freq * 125) / 2);
>>   	return 0;
> 
> Hi Hans,
> 
> Should the part quoted above part perhaps read:
> 
> freq = clamp(freq, FREQ_MIN * FREQ_MUL, FREQ_MAX * FREQ_MUL);
> 
> Or did "#define clamp() change" recently?

Nope, that's a bug. Thanks for spotting this!

I'll make a patch for this.

Regards,

	Hans

> 
> http://lxr.free-electrons.com/source/include/linux/kernel.h
> 
>> 698 /**
>> 699  * clamp - return a value clamped to a given range with strict typechecking
>> 700  * @val: current value
>> 701  * @min: minimum allowable value
>> 702  * @max: maximum allowable value
>> 703  *
>> 704  * This macro does strict typechecking of min/max to make sure they are of the
>> 705  * same type as val.  See the unnecessary pointer comparisons.
>> 706  */
>> 707 #define clamp(val, min, max) ({                 \
>> 708         typeof(val) __val = (val);              \
>> 709         typeof(min) __min = (min);              \
>> 710         typeof(max) __max = (max);              \
>> 711         (void) (&__val == &__min);              \
>> 712         (void) (&__val == &__max);              \
>> 713         __val = __val < __min ? __min: __val;   \
>> 714         __val > __max ? __max: __val; })
> 
> Thank you!
> 
> Same spotted in:
> 
>>> media_tree/drivers/media/radio/radio-tea5764.c: In function 'vidioc_s_frequency':
>>> media_tree/drivers/media/radio/radio-tea5764.c:359: warning: statement with no effect
>>
>>> media_tree/drivers/media/radio/tef6862.c: In function 'tef6862_s_frequency':
>>> media_tree/drivers/media/radio/tef6862.c:115: warning: statement with no effect
> 
> Keep up the good work!
> 
> --HPS
>