From patchwork Sun Jan 23 22:22:59 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Devin Heitmueller X-Patchwork-Id: 499451 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p0NMN4CX008356 for ; Sun, 23 Jan 2011 22:23:04 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752282Ab1AWWXC (ORCPT ); Sun, 23 Jan 2011 17:23:02 -0500 Received: from mail-ey0-f174.google.com ([209.85.215.174]:43959 "EHLO mail-ey0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752248Ab1AWWXB (ORCPT ); Sun, 23 Jan 2011 17:23:01 -0500 Received: by eye27 with SMTP id 27so1625931eye.19 for ; Sun, 23 Jan 2011 14:23:00 -0800 (PST) MIME-Version: 1.0 Received: by 10.213.29.210 with SMTP id r18mr3970225ebc.62.1295821379322; Sun, 23 Jan 2011 14:22:59 -0800 (PST) Received: by 10.213.15.67 with HTTP; Sun, 23 Jan 2011 14:22:59 -0800 (PST) Date: Sun, 23 Jan 2011 17:22:59 -0500 Message-ID: Subject: [RFC PATCH] tuner-core: fix broken G_TUNER call when tuner is in standby From: Devin Heitmueller To: Linux Media Mailing List Cc: Hans Verkuil 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]); Sun, 23 Jan 2011 22:23:28 +0000 (UTC) tuner-core: fix broken G_TUNER call when tuner is in standby From: Devin Heitmueller The logic for determining the supported device modes was combined with the logic which dictates whether the tuner was asleep. This resulted in calls such as G_TUNER returning complete garbage in the event that the tuner was in standby mode (a violation of the V4L2 specification, and causing VLC to be broken for such tuners). This patch reworks the logic so the current tuner mode is maintained separately from whether it is in standby (per Hans Verkuil's suggestion). It also restructures the G_TUNER call such that all the staticly defined information related to the tuner is returned regardless of whether it is in standby (e.g. the supported frequency range, etc). Priority: normal Signed-off-by: Devin Heitmueller Cc: Hans Verkuil --- media_build/linux/drivers/media/video/tuner-core.c 2010-10-24 19:34:59.000000000 -0400 +++ media_build_950qfixes//linux/drivers/media/video/tuner-core.c 2011-01-23 17:18:22.381107568 -0500 @@ -90,6 +90,7 @@ unsigned int mode; unsigned int mode_mask; /* Combination of allowable modes */ + unsigned int in_standby:1; unsigned int type; /* chip type id */ unsigned int config; @@ -700,6 +701,7 @@ static inline int set_mode(struct i2c_client *client, struct tuner *t, int mode, char *cmd) { struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops; + unsigned int orig_mode = t->mode; if (mode == t->mode) return 0; @@ -709,7 +711,8 @@ if (check_mode(t, cmd) == -EINVAL) { tuner_dbg("Tuner doesn't support this mode. " "Putting tuner to sleep\n"); - t->mode = T_STANDBY; + t->mode = orig_mode; + t->in_standby = 1; if (analog_ops->standby) analog_ops->standby(&t->fe); return -EINVAL; @@ -769,7 +772,7 @@ if (check_mode(t, "s_power") == -EINVAL) return 0; - t->mode = T_STANDBY; + t->in_standby = 1; if (analog_ops->standby) analog_ops->standby(&t->fe); return 0; @@ -854,47 +857,54 @@ struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops; struct dvb_tuner_ops *fe_tuner_ops = &t->fe.ops.tuner_ops; - if (check_mode(t, "g_tuner") == -EINVAL) - return 0; switch_v4l2(); + /* First populate everything that doesn't require talking to the + actual hardware */ vt->type = t->mode; - if (analog_ops->get_afc) - vt->afc = analog_ops->get_afc(&t->fe); if (t->mode == V4L2_TUNER_ANALOG_TV) + { vt->capability |= V4L2_TUNER_CAP_NORM; - if (t->mode != V4L2_TUNER_RADIO) { vt->rangelow = tv_range[0] * 16; vt->rangehigh = tv_range[1] * 16; - return 0; + } else { + /* radio mode */ + vt->rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO; + 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; } - /* radio mode */ - vt->rxsubchans = - V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO; - if (fe_tuner_ops->get_status) { - u32 tuner_status; + /* If the hardware is in sleep mode, bail out at this point */ + if (t->in_standby) + return 0; - fe_tuner_ops->get_status(&t->fe, &tuner_status); - vt->rxsubchans = - (tuner_status & TUNER_STATUS_STEREO) ? - V4L2_TUNER_SUB_STEREO : - V4L2_TUNER_SUB_MONO; + /* Now populate the fields that requires the hardware to be alive */ + if (t->mode == V4L2_TUNER_ANALOG_TV) { + if (analog_ops->get_afc) + vt->afc = analog_ops->get_afc(&t->fe); } else { - if (analog_ops->is_stereo) { + if (fe_tuner_ops->get_status) { + u32 tuner_status; + + fe_tuner_ops->get_status(&t->fe, &tuner_status); vt->rxsubchans = - analog_ops->is_stereo(&t->fe) ? + (tuner_status & TUNER_STATUS_STEREO) ? V4L2_TUNER_SUB_STEREO : V4L2_TUNER_SUB_MONO; + } else { + if (analog_ops->is_stereo) { + vt->rxsubchans = + analog_ops->is_stereo(&t->fe) ? + V4L2_TUNER_SUB_STEREO : + V4L2_TUNER_SUB_MONO; + } } + if (analog_ops->has_signal) + vt->signal = analog_ops->has_signal(&t->fe); } - 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; + return 0; } @@ -911,6 +921,11 @@ /* do nothing unless we're a radio tuner */ if (t->mode != V4L2_TUNER_RADIO) return 0; + + /* Should this really fail silently if the device is asleep? */ + if (t->in_standby == 1) + return 0; + t->audmode = vt->audmode; set_radio_freq(client, t->radio_freq); return 0; @@ -1004,14 +1019,11 @@ *tv = NULL; list_for_each_entry(pos, &tuner_list, list) { - int mode_mask; - if (pos->i2c->adapter != adap || strcmp(pos->i2c->driver->driver.name, "tuner")) continue; - mode_mask = pos->mode_mask & ~T_STANDBY; - if (*radio == NULL && mode_mask == T_RADIO) + if (*radio == NULL && pos->mode_mask == T_RADIO) *radio = pos; /* Note: currently TDA9887 is the only demod-only device. If other devices appear then we need to @@ -1063,7 +1075,8 @@ t->i2c->addr) >= 0) { t->type = TUNER_TEA5761; t->mode_mask = T_RADIO; - t->mode = T_STANDBY; + t->mode = T_RADIO; + t->in_standby = 1; /* Sets freq to FM range */ t->radio_freq = 87.5 * 16000; tuner_lookup(t->i2c->adapter, &radio, &tv); @@ -1088,7 +1101,8 @@ t->type = TUNER_TDA9887; t->mode_mask = T_RADIO | T_ANALOG_TV | T_DIGITAL_TV; - t->mode = T_STANDBY; + t->mode = T_ANALOG_TV; + t->in_standby = 1; goto register_client; } break; @@ -1098,7 +1112,8 @@ >= 0) { t->type = TUNER_TEA5767; t->mode_mask = T_RADIO; - t->mode = T_STANDBY; + t->mode = T_RADIO; + t->in_standby = 1; /* Sets freq to FM range */ t->radio_freq = 87.5 * 16000; tuner_lookup(t->i2c->adapter, &radio, &tv);