From patchwork Wed Jun 15 19:55:57 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans Verkuil X-Patchwork-Id: 883392 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 p5FJuJgF000816 for ; Wed, 15 Jun 2011 19:56:19 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755025Ab1FOT4M (ORCPT ); Wed, 15 Jun 2011 15:56:12 -0400 Received: from smtp-vbr11.xs4all.nl ([194.109.24.31]:3394 "EHLO smtp-vbr11.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755007Ab1FOT4K (ORCPT ); Wed, 15 Jun 2011 15:56:10 -0400 Received: from tschai.localnet (215.80-203-102.nextgentel.com [80.203.102.215]) (authenticated bits=0) by smtp-vbr11.xs4all.nl (8.13.8/8.13.8) with ESMTP id p5FJtw6o014150 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 15 Jun 2011 21:55:58 +0200 (CEST) (envelope-from hverkuil@xs4all.nl) From: Hans Verkuil To: Devin Heitmueller Subject: Re: [RFCv2 PATCH 0/5] tuner-core: fix s_std and s_tuner Date: Wed, 15 Jun 2011 21:55:57 +0200 User-Agent: KMail/1.13.5 (Linux/3.0.0-rc1-tschai; KDE/4.6.2; x86_64; ; ) Cc: Andy Walls , linux-media@vger.kernel.org, Mauro Carvalho Chehab References: <1307804731-16430-1-git-send-email-hverkuil@xs4all.nl> <201106111902.11384.hverkuil@xs4all.nl> In-Reply-To: MIME-Version: 1.0 Message-Id: <201106152155.57978.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]); Wed, 15 Jun 2011 19:56:20 +0000 (UTC) On Saturday, June 11, 2011 19:08:04 Devin Heitmueller wrote: > On Sat, Jun 11, 2011 at 1:02 PM, Hans Verkuil wrote: > > OK, but how do you get it into standby in the first place? (I must be missing > > something here...) > > The tuner core puts the chip into standby when the last V4L filehandle > is closed. Yes, I realize this violates the V4L spec since you should > be able to make multiple calls with something like v4l2-ctl, but > nobody has ever come up with a better mechanism for knowing when to > put the device to sleep. Why would that violate the spec? If the last filehandle is closed, then you can safely poweroff the tuner. The only exception is when you have a radio tuner whose audio output is hooked up to some line-in: there you can't power off the tuner. > > We've been forced to choose between the purist perspective, which is > properly preserving state, never powering down the tuner and sucking > up 500ma on the USB port when not using the tuner, versus powering > down the tuner when the last party closes the filehandle, which > preserves power but breaks v4l2 conformance and in some cases is > highly noticeable with tuners that require firmware to be reloaded > when being powered back up. Seems fine to me. What isn't fine is that a driver like e.g. em28xx powers off the tuner but doesn't power it on again on the next open. It won't do that until the first S_FREQUENCY/S_TUNER/S_STD call. Devin, Mauro, is there anything wrong with adding support for s_power(1) and calling it in em28xx? A similar power up would be needed in cx23885, au0828, cx88 and cx231xx. Mauro, if you agree with this patch, then I'll add it to the tuner patch series. Tested with em28xx. Regards, Hans --- 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/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c index 7b6461d..6f3f51b 100644 --- a/drivers/media/video/em28xx/em28xx-video.c +++ b/drivers/media/video/em28xx/em28xx-video.c @@ -2111,6 +2111,7 @@ static int em28xx_v4l2_open(struct file *filp) em28xx_wake_i2c(dev); } + v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 1); if (fh->radio) { em28xx_videodbg("video_open: setting radio device\n"); v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_radio); diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c index 9b0d833..9006c9a 100644 --- a/drivers/media/video/tuner-core.c +++ b/drivers/media/video/tuner-core.c @@ -1057,16 +1057,20 @@ static int tuner_s_radio(struct v4l2_subdev *sd) /** * tuner_s_power - controls the power state of the tuner * @sd: pointer to struct v4l2_subdev - * @on: a zero value puts the tuner to sleep + * @on: a zero value puts the tuner to sleep, non-zero wakes it up */ static int tuner_s_power(struct v4l2_subdev *sd, int on) { struct tuner *t = to_tuner(sd); struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops; - /* FIXME: Why this function don't wake the tuner if on != 0 ? */ - if (on) + if (on) { + if (t->standby && set_mode(t, t->mode) == 0) { + tuner_dbg("Waking up tuner\n"); + set_freq(t, 0); + } return 0; + } tuner_dbg("Putting tuner to sleep\n"); t->standby = true;