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]

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
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hans Petter Selasky]

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?

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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html