On 12/16/2013 03:19 PM, Antti Palosaari wrote:
> On 16.12.2013 14:50, Hans Verkuil wrote:
>> On 12/16/2013 01:36 PM, Antti Palosaari wrote:
>>> On 16.12.2013 10:53, Hans Verkuil wrote:
>>>> On 12/14/2013 05:15 PM, Antti Palosaari wrote:
>>>
>>>>> @@ -1288,8 +1288,13 @@ static int v4l_g_frequency(const struct v4l2_ioctl_ops *ops,
>>>>> struct video_device *vfd = video_devdata(file);
>>>>> struct v4l2_frequency *p = arg;
>>>>>
>>>>> - p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ?
>>>>> - V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
>>>>> + if (vfd->vfl_type == VFL_TYPE_SDR) {
>>>>> + if (p->type != V4L2_TUNER_ADC && p->type != V4L2_TUNER_RF)
>>>>> + return -EINVAL;
>>>>
>>>> This is wrong. As you mentioned in patch 1, the type field should always be set by
>>>> the driver. So type is not something that is set by the user.
>>>>
>>>> I would just set type to V4L2_TUNER_ADC here (all SDR devices have at least an ADC
>>>> tuner), and let the driver change it to TUNER_RF if this tuner is really an RF
>>>> tuner.
>>>
>>> I don't think so. It sounds very stupid to handle tuner type with
>>> different meaning in that single case - it sounds just a is a mistake
>>> (and that SDR case mistakes are not needed continue as no regressions
>>> apply). I can say I was very puzzled what is the reason my tuner type is
>>> always changed to wrong, until finally found it was overridden here.
>>>
>>> For me this looks more than it is just forced to "some" suitable value
>>> in a case app does not fill it correctly - not the way driver should
>>> return it to app. Tuner ID and type are here for Kernel driver could
>>> identify not the opposite and that is how it should be without unneeded
>>> exceptions.
>>>
>>> Also, API does not specify that kind of different meaning for tuner type
>>> in a case of g_frequency.
>>>
>>> Have to search some history where that odds is coming from...
>>
>> The application *does not set type* when calling G_FREQUENCY. The driver has
>> to fill that in. So the type field as received from the application is
>> uninitialized. That's the way the spec was defined, and that's the way
>> applications use G_FREQUENCY. There is nothing you can do about that.
>>
>> So drivers have to fill in the type based on vfl_type and the tuner index.
>> Since drivers often didn't do that the vfl_type check has been moved to the
>> v4l2 core. In the case of SDR the type is actually dependent on the tuner
>> index, so the core cannot fully initialize the type field.
>>
>> You can either leave it uninitialized for vfl_type SDR and leave it to the
>> SDR driver to fill in the type, or you can set it to ADC so the driver
>> only has to update the type field if the tuner index corresponds to the
>> RF tuner.
>
>
> commit 227690df75382e46a4f6ea1bbc5df855a674b47f
> Author: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> Date: Sun Jun 12 06:36:41 2011 -0300
>
> [media] v4l2-ioctl.c: prefill tuner type for g_frequency and g/s_tuner
>
> The subdevs are supposed to receive a valid tuner type for the
> g_frequency
This talks about the *subdevs*. The low-level tuner ops implemented by
sub-devices expected a valid type field. That field had to be filled in
by bridge drivers, and they often did not do that, or filled in the wrong
type.
> and g/s_tuner subdev ops. Some drivers do this, others don't. So
> prefill
> this in v4l2-ioctl.c based on whether the device node from which
> this is
> called is a radio node or not.
>
> The spec does not require applications to fill in the type, and if they
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> leave it at 0 then the 'check_mode' call in tuner-core.c will return
> an error and the ioctl does nothing.
>
> Cc: stable@xxxxxxxxxx
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
>
>
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
