On Sunday, June 12, 2011 16:11:30 Mauro Carvalho Chehab wrote:
> Em 12-06-2011 09:23, Hans Verkuil escreveu:
> > That's not unreasonably to do at some point in time, but it doesn't actually
> > answer my question, which is: should the core refuse VIDIOC_S_FREQUENCY calls
> > where the type doesn't match the device node (i.e. radio vs tv)? I think it
> > makes no sense to call VIDIOC_S_FREQUENCY on a radio node with type ANALOG_TV.
>
> No. The core shouldn't do it, otherwise tuner will break. The code doesn't know if
> the opened device is radio or video.
I don't follow this. In v4l2-ioctl.c it is easy to tell if the opened device
is radio or not by looking at vfd->vfl_type.
> >> diff --git a/drivers/media/video/cx18/cx18-ioctl.c b/drivers/media/video/cx18/cx18-ioctl.c
> >> index 1933d4d..5463548 100644
> >> --- a/drivers/media/video/cx18/cx18-ioctl.c
> >> +++ b/drivers/media/video/cx18/cx18-ioctl.c
> >> @@ -611,6 +611,11 @@ static int cx18_g_frequency(struct file *file, void *fh,
> >> if (vf->tuner != 0)
> >> return -EINVAL;
> >>
> >> + if (test_bit(CX18_F_I_RADIO_USER, &cx->i_flags))
> >> + vf->type = V4L2_TUNER_RADIO;
> >> + else
> >> + vf->type = V4L2_TUNER_ANALOG_TV;
> >> +
> >
> > NACK.
> >
> > This sets the type to the current mode. But what we want is to set the type to
> > the current device node. That's what my RFCv4 does (and that patch requires no
> > driver change).
>
> I didn't get your RFCv4 patches here yet, but the fix should be at the driver: it
> needs to set the type before calling g_frequency. G_FREQUENCY shouldn't change the
> device mode, but, instead, to return the frequency and mode currently in usage..
Why bother changing drivers (and probably missing a few) if you can do it
in v4l2-ioctl.c and let drivers just pass it on?
This is the patch in question, BTW:
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 213ba7d..26bf3bf 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -1822,6 +1822,8 @@ static long __video_do_ioctl(struct file *file,
if (!ops->vidioc_g_tuner)
break;
+ p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ?
+ V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
ret = ops->vidioc_g_tuner(file, fh, p);
if (!ret)
dbgarg(cmd, "index=%d, name=%s, type=%d, "
@@ -1840,6 +1842,8 @@ static long __video_do_ioctl(struct file *file,
if (!ops->vidioc_s_tuner)
break;
+ p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ?
+ V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
dbgarg(cmd, "index=%d, name=%s, type=%d, "
"capability=0x%x, rangelow=%d, "
"rangehigh=%d, signal=%d, afc=%d, "
@@ -1858,6 +1862,8 @@ static long __video_do_ioctl(struct file *file,
if (!ops->vidioc_g_frequency)
break;
+ p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ?
+ V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
ret = ops->vidioc_g_frequency(file, fh, p);
if (!ret)
dbgarg(cmd, "tuner=%d, type=%d, frequency=%d\n",
Neither of these three ioctls will change the tuner mode, BTW. With this
code in place drivers that use video_ioctl2 can now rely on the type field
being a sensible value.
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
