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: 5663 Return-path: Envelope-to: mchehab@pedra Delivery-date: Sun, 23 Jan 2011 20:36:54 -0200 Received: from mchehab by pedra with local (Exim 4.72) (envelope-from ) id 1Ph8YI-0001UP-3C for mchehab@pedra; Sun, 23 Jan 2011 20:36:54 -0200 Received: from casper.infradead.org [85.118.1.10] by pedra with IMAP (fetchmail-6.3.17) for (single-drop); Sun, 23 Jan 2011 20:36:54 -0200 (BRST) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1Ph8Kv-0004aI-RK; Sun, 23 Jan 2011 22:23:06 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752282Ab1AWWXC (ORCPT + 1 other); 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 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Sender: Hello all, The following patch addresses a V4L2 specification violation where the G_TUNER call would return complete garbage in the event that the tuner is asleep. Per Hans' suggestion, I have split out the tuner operational mode from whether it is in standby, and fixed the G_TUNER call to return as much as possible when the tuner is in standby. Without this change, products that have tuners which support standby mode cannot be tuned from within VLC. I recognize that changes to tuner-core tend to be pretty hairy, so I welcome suggestions/feedback on this patch. Regards, Devin 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);