From patchwork Sat Jun 25 14:47:13 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans Verkuil X-Patchwork-Id: 918172 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p5PEloCI022385 for ; Sat, 25 Jun 2011 14:47:53 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751768Ab1FYOrY (ORCPT ); Sat, 25 Jun 2011 10:47:24 -0400 Received: from smtp-vbr2.xs4all.nl ([194.109.24.22]:1911 "EHLO smtp-vbr2.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751452Ab1FYOrX (ORCPT ); Sat, 25 Jun 2011 10:47:23 -0400 Received: from tschai.localnet (215.80-203-102.nextgentel.com [80.203.102.215]) (authenticated bits=0) by smtp-vbr2.xs4all.nl (8.13.8/8.13.8) with ESMTP id p5PElJpB098679 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 25 Jun 2011 16:47:20 +0200 (CEST) (envelope-from hverkuil@xs4all.nl) From: Hans Verkuil To: Mauro Carvalho Chehab Subject: Re: RFC tuner-core: how to set vt->type in g_tuner? Date: Sat, 25 Jun 2011 16:47:13 +0200 User-Agent: KMail/1.13.5 (Linux/3.0.0-rc1-tschai; KDE/4.6.2; x86_64; ; ) Cc: "linux-media" References: <201106251602.45572.hverkuil@xs4all.nl> <4E05EDEE.1000201@redhat.com> In-Reply-To: <4E05EDEE.1000201@redhat.com> MIME-Version: 1.0 Message-Id: <201106251647.13900.hverkuil@xs4all.nl> X-Virus-Scanned: by XS4ALL Virus Scanner Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Sat, 25 Jun 2011 15:00:15 +0000 (UTC) On Saturday, June 25, 2011 16:17:18 Mauro Carvalho Chehab wrote: > Em 25-06-2011 11:02, Hans Verkuil escreveu: > > Hi all, > > > > The tuner-core.c implementation does this at the start of g_tuner: > > > > if (check_mode(t, vt->type) == -EINVAL) > > return 0; > > vt->type = t->mode; > > > > The idea is that the vt->type is set depending on whether the VIDIOC_G_TUNER > > ioctl is called from a radio device node or a video device node. If we have a > > tuner that can do both radio and TV, then the type is set to whatever the > > current tuner mode is. > > > > This seems reasonable, but it will actually run into problems when g_tuner > > is called for audio demodulators like msp3400 or cx25840. These need to know > > the correct vt->type in order to fill in the right fields. > > > > The problem here is that the tuner subdevices are not necessarily called first. > > In fact, the msp3400/cx25840 are actually called first by ivtv. > > > > So the msp3400 will get called with type TV, and later the tuner may change that > > to type RADIO. This causes inconsistencies. This has actually been observed when > > testing with ivtv and a PVR-500. > > > > There are two solutions: > > > > 1) Audit the drivers and ensure that the tuner subdevices are registered first. > > > > 2) Do not allow the tuner to switch the type. > > (2) is the right thing to do. A VIDIOC_GET_foo should not change anything. They are > supposed to be read only access. Great! Can you review this patch? This implements solution 2. I think it's correct, but it's tuner code, so you never know :-) --- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c index 9006c9a..706fc11 100644 --- a/drivers/media/video/tuner-core.c +++ b/drivers/media/video/tuner-core.c @@ -1119,8 +1119,7 @@ static int tuner_g_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *f) if (check_mode(t, f->type) == -EINVAL) return 0; - f->type = t->mode; - if (fe_tuner_ops->get_frequency && !t->standby) { + if (f->type == t->mode && fe_tuner_ops->get_frequency && !t->standby) { u32 abs_freq; fe_tuner_ops->get_frequency(&t->fe, &abs_freq); @@ -1128,7 +1127,7 @@ static int tuner_g_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *f) DIV_ROUND_CLOSEST(abs_freq * 2, 125) : DIV_ROUND_CLOSEST(abs_freq, 62500); } else { - f->frequency = (V4L2_TUNER_RADIO == t->mode) ? + f->frequency = (V4L2_TUNER_RADIO == f->type) ? t->radio_freq : t->tv_freq; } return 0; @@ -1152,32 +1151,33 @@ static int tuner_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt) if (check_mode(t, vt->type) == -EINVAL) return 0; - vt->type = t->mode; - if (analog_ops->get_afc) + if (vt->type == t->mode && analog_ops->get_afc) vt->afc = analog_ops->get_afc(&t->fe); - if (t->mode == V4L2_TUNER_ANALOG_TV) + if (vt->type == V4L2_TUNER_ANALOG_TV) vt->capability |= V4L2_TUNER_CAP_NORM; - if (t->mode != V4L2_TUNER_RADIO) { + if (vt->type != V4L2_TUNER_RADIO) { vt->rangelow = tv_range[0] * 16; vt->rangehigh = tv_range[1] * 16; return 0; } /* radio mode */ - vt->rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO; - if (fe_tuner_ops->get_status) { - u32 tuner_status; - - fe_tuner_ops->get_status(&t->fe, &tuner_status); - vt->rxsubchans = - (tuner_status & TUNER_STATUS_STEREO) ? - V4L2_TUNER_SUB_STEREO : - V4L2_TUNER_SUB_MONO; + if (vt->type == t->mode) { + vt->rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO; + if (fe_tuner_ops->get_status) { + u32 tuner_status; + + fe_tuner_ops->get_status(&t->fe, &tuner_status); + vt->rxsubchans = + (tuner_status & TUNER_STATUS_STEREO) ? + V4L2_TUNER_SUB_STEREO : + V4L2_TUNER_SUB_MONO; + } + if (analog_ops->has_signal) + vt->signal = analog_ops->has_signal(&t->fe); + vt->audmode = t->audmode; } - if (analog_ops->has_signal) - vt->signal = analog_ops->has_signal(&t->fe); vt->capability |= V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO; - vt->audmode = t->audmode; vt->rangelow = radio_range[0] * 16000; vt->rangehigh = radio_range[1] * 16000;