All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] [media] dvb: Allow select between DVB-C Annex A and Annex C
@ 2011-11-20 14:56 Mauro Carvalho Chehab
  2011-11-20 14:56 ` [PATCH 2/8] [media] Properly implement ITU-T J.88 Annex C support Mauro Carvalho Chehab
  2011-11-20 20:27 ` [PATCH 1/8] [media] dvb: Allow select between DVB-C Annex A and Annex C Rémi Denis-Courmont
  0 siblings, 2 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-11-20 14:56 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab

DVB-C, as defined by ITU-T J.83 has 3 annexes. The differences between
Annex A and Annex C is that Annex C uses a subset of the modulation
types, and uses a different rolloff factor. A different rolloff means
that the bandwidth required is slicely different, and may affect the
saw filter configuration at the tuners. Also, some demods have different
configurations, depending on using Annex A or Annex C.

So, allow userspace to specify it, by changing the rolloff factor.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 Documentation/DocBook/media/dvb/dvbproperty.xml |    4 ++++
 drivers/media/dvb/dvb-core/dvb_frontend.c       |    2 ++
 include/linux/dvb/frontend.h                    |    2 ++
 3 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/Documentation/DocBook/media/dvb/dvbproperty.xml b/Documentation/DocBook/media/dvb/dvbproperty.xml
index 3bc8a61..6ac8039 100644
--- a/Documentation/DocBook/media/dvb/dvbproperty.xml
+++ b/Documentation/DocBook/media/dvb/dvbproperty.xml
@@ -311,6 +311,8 @@ typedef enum fe_rolloff {
 	ROLLOFF_20,
 	ROLLOFF_25,
 	ROLLOFF_AUTO,
+	ROLLOFF_15, /* DVB-C Annex A */
+	ROLLOFF_13, /* DVB-C Annex C */
 } fe_rolloff_t;
 		</programlisting>
 		</section>
@@ -778,8 +780,10 @@ typedef enum fe_hierarchy {
 			<listitem><para><link linkend="DTV-MODULATION"><constant>DTV_MODULATION</constant></link></para></listitem>
 			<listitem><para><link linkend="DTV-INVERSION"><constant>DTV_INVERSION</constant></link></para></listitem>
 			<listitem><para><link linkend="DTV-SYMBOL-RATE"><constant>DTV_SYMBOL_RATE</constant></link></para></listitem>
+			<listitem><para><link linkend="DTV-ROLLOFF"><constant>DTV_ROLLOFF</constant></link></para></listitem>
 			<listitem><para><link linkend="DTV-INNER-FEC"><constant>DTV_INNER_FEC</constant></link></para></listitem>
 		</itemizedlist>
+		<para>The Rolloff of 0.15 (ROLLOFF_15) is assumed, as ITU-T J.83 Annex A is more common. For Annex C, rolloff should be 0.13 (ROLLOFF_13). All other values are invalid.</para>
 	</section>
 	<section id="dvbc-annex-b-params">
 		<title>DVB-C Annex B delivery system</title>
diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
index 2c0acdb..c849455 100644
--- a/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -876,6 +876,7 @@ static int dvb_frontend_clear_cache(struct dvb_frontend *fe)
 	c->symbol_rate = QAM_AUTO;
 	c->code_rate_HP = FEC_AUTO;
 	c->code_rate_LP = FEC_AUTO;
+	c->rolloff = ROLLOFF_AUTO;
 
 	c->isdbt_partial_reception = -1;
 	c->isdbt_sb_mode = -1;
@@ -1030,6 +1031,7 @@ static void dtv_property_cache_init(struct dvb_frontend *fe,
 		break;
 	case FE_QAM:
 		c->delivery_system = SYS_DVBC_ANNEX_AC;
+		c->rolloff = ROLLOFF_15; /* implied for Annex A */
 		break;
 	case FE_OFDM:
 		c->delivery_system = SYS_DVBT;
diff --git a/include/linux/dvb/frontend.h b/include/linux/dvb/frontend.h
index 1b1094c..d9251df 100644
--- a/include/linux/dvb/frontend.h
+++ b/include/linux/dvb/frontend.h
@@ -329,6 +329,8 @@ typedef enum fe_rolloff {
 	ROLLOFF_20,
 	ROLLOFF_25,
 	ROLLOFF_AUTO,
+	ROLLOFF_15,	/* DVB-C Annex A */
+	ROLLOFF_13,	/* DVB-C Annex C */
 } fe_rolloff_t;
 
 typedef enum fe_delivery_system {
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/8] [media] Properly implement ITU-T J.88 Annex C support
  2011-11-20 14:56 [PATCH 1/8] [media] dvb: Allow select between DVB-C Annex A and Annex C Mauro Carvalho Chehab
@ 2011-11-20 14:56 ` Mauro Carvalho Chehab
  2011-11-20 14:56   ` [PATCH 3/8] [media] em28xx: Fix some Terratec entries (H5 and XS) Mauro Carvalho Chehab
  2011-11-20 20:27 ` [PATCH 1/8] [media] dvb: Allow select between DVB-C Annex A and Annex C Rémi Denis-Courmont
  1 sibling, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-11-20 14:56 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab

The Annex C support were broken with the previous implementation,
as, at xc5000 and tda18271c2dd, it were choosing the wrong bandwidth
for some symbol rates.

At DRX-J, it were always selecting Annex A, even having Annex C
support coded there.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/common/tuners/xc5000.c       |   26 +++++++++++-----------
 drivers/media/dvb/frontends/drxk_hard.c    |    7 ++++++
 drivers/media/dvb/frontends/tda18271c2dd.c |   32 ++++++++++++++-------------
 3 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index aa1b2e8..88b329c 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -628,20 +628,12 @@ static void xc_debug_dump(struct xc5000_priv *priv)
 	dprintk(1, "*** Quality (0:<8dB, 7:>56dB) = %d\n", quality);
 }
 
-/*
- * As defined on EN 300 429, the DVB-C roll-off factor is 0.15.
- * So, the amount of the needed bandwith is given by:
- * 	Bw = Symbol_rate * (1 + 0.15)
- * As such, the maximum symbol rate supported by 6 MHz is given by:
- *	max_symbol_rate = 6 MHz / 1.15 = 5217391 Bauds
- */
-#define MAX_SYMBOL_RATE_6MHz	5217391
-
 static int xc5000_set_params(struct dvb_frontend *fe,
 	struct dvb_frontend_parameters *params)
 {
 	struct xc5000_priv *priv = fe->tuner_priv;
 	int ret;
+	u32 bw;
 
 	if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
 		if (xc_load_fw_and_init_tuner(fe) != XC_RESULT_SUCCESS) {
@@ -707,11 +699,19 @@ static int xc5000_set_params(struct dvb_frontend *fe,
 			dprintk(1, "%s() QAM modulation\n", __func__);
 			priv->rf_mode = XC_RF_MODE_CABLE;
 			/*
-			 * Using a 8MHz bandwidth sometimes fail
-			 * with 6MHz-spaced channels, due to inter-carrier
-			 * interference. So, use DTV6 firmware
+			 * Using a higher bandwidth at the tuner filter may
+			 * allow inter-carrier interference.
+			 * So, determine the minimal channel spacing, in order
+			 * to better adjust the tuner filter.
+			 * According with ITU-T J.83, the bandwidth is given by:
+			 * bw = Simbol Rate * (1 + roll_off), where the roll_off
+			 * is equal to 0.15 for Annex A, and 0.13 for annex C
 			 */
-			if (params->u.qam.symbol_rate <= MAX_SYMBOL_RATE_6MHz) {
+			if (fe->dtv_property_cache.rolloff == ROLLOFF_13)
+				bw = (params->u.qam.symbol_rate * 13) / 10;
+			else
+				bw = (params->u.qam.symbol_rate * 15) / 10;
+			if (bw <= 6000000) {
 				priv->bandwidth = BANDWIDTH_6_MHZ;
 				priv->video_standard = DTV6;
 				priv->freq_hz = params->frequency - 1750000;
diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index f6431ef..dc13fd8 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -6218,6 +6218,13 @@ static int drxk_set_parameters(struct dvb_frontend *fe,
 		return -EINVAL;
 	}
 
+	if (state->m_OperationMode == OM_QAM_ITU_A ||
+	    state->m_OperationMode == OM_QAM_ITU_C) {
+		if (fe->dtv_property_cache.rolloff == ROLLOFF_13)
+			state->m_OperationMode = OM_QAM_ITU_C;
+		else
+			state->m_OperationMode = OM_QAM_ITU_A;
+	}
 
 	if (fe->ops.i2c_gate_ctrl)
 		fe->ops.i2c_gate_ctrl(fe, 1);
diff --git a/drivers/media/dvb/frontends/tda18271c2dd.c b/drivers/media/dvb/frontends/tda18271c2dd.c
index 1b1bf20..de544f6 100644
--- a/drivers/media/dvb/frontends/tda18271c2dd.c
+++ b/drivers/media/dvb/frontends/tda18271c2dd.c
@@ -1123,20 +1123,6 @@ static int release(struct dvb_frontend *fe)
 	return 0;
 }
 
-/*
- * As defined on EN 300 429 Annex A and on ITU-T J.83 annex A, the DVB-C
- * roll-off factor is 0.15.
- * According with the specs, the amount of the needed bandwith is given by:
- *	Bw = Symbol_rate * (1 + 0.15)
- * As such, the maximum symbol rate supported by 6 MHz is
- *	max_symbol_rate = 6 MHz / 1.15 = 5217391 Bauds
- *NOTE: For ITU-T J.83 Annex C, the roll-off factor is 0.13. So:
- *	max_symbol_rate = 6 MHz / 1.13 = 5309735 Baud
- *	That means that an adjustment is needed for Japan,
- *	but, as currently DRX-K is hardcoded to Annex A, let's stick
- *	with 0.15 roll-off factor.
- */
-#define MAX_SYMBOL_RATE_6MHz	5217391
 
 static int set_params(struct dvb_frontend *fe,
 		      struct dvb_frontend_parameters *params)
@@ -1144,6 +1130,7 @@ static int set_params(struct dvb_frontend *fe,
 	struct tda_state *state = fe->tuner_priv;
 	int status = 0;
 	int Standard;
+	u32 bw;
 
 	state->m_Frequency = params->frequency;
 
@@ -1161,8 +1148,23 @@ static int set_params(struct dvb_frontend *fe,
 			break;
 		}
 	else if (fe->ops.info.type == FE_QAM) {
-		if (params->u.qam.symbol_rate <= MAX_SYMBOL_RATE_6MHz)
+		/*
+		 * Using a higher bandwidth at the tuner filter may
+		 * allow inter-carrier interference.
+		 * So, determine the minimal channel spacing, in order
+		 * to better adjust the tuner filter.
+		 * According with ITU-T J.83, the bandwidth is given by:
+		 * bw = Simbol Rate * (1 + roll_off), where the roll_off
+		 * is equal to 0.15 for Annex A, and 0.13 for annex C
+		 */
+		if (fe->dtv_property_cache.rolloff == ROLLOFF_13)
+			bw = (params->u.qam.symbol_rate * 13) / 10;
+		else
+			bw = (params->u.qam.symbol_rate * 15) / 10;
+		if (bw <= 6000000)
 			Standard = HF_DVBC_6MHZ;
+		else if (bw <= 7000000)
+			Standard = HF_DVBC_7MHZ;
 		else
 			Standard = HF_DVBC_8MHZ;
 	} else
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/8] [media] em28xx: Fix some Terratec entries (H5 and XS)
  2011-11-20 14:56 ` [PATCH 2/8] [media] Properly implement ITU-T J.88 Annex C support Mauro Carvalho Chehab
@ 2011-11-20 14:56   ` Mauro Carvalho Chehab
  2011-11-20 14:56     ` [PATCH 4/8] [media] xc5000: Add support for get_if_frequency Mauro Carvalho Chehab
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-11-20 14:56 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab

As reported by  Andreas Oberritter <obi@linuxtv.org>, changeset
33ba28eebc3e1758e6adc1fcec9e1e3151bac453 did the wrong thing.

Fix it to properly reflect the entries for Terratec H5 revs 1 and 2,
and restore Terratec XS entry.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 Documentation/video4linux/CARDLIST.em28xx |    6 +++---
 drivers/media/video/em28xx/em28xx-cards.c |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/video4linux/CARDLIST.em28xx b/Documentation/video4linux/CARDLIST.em28xx
index d8c8544..7734b2c 100644
--- a/Documentation/video4linux/CARDLIST.em28xx
+++ b/Documentation/video4linux/CARDLIST.em28xx
@@ -11,7 +11,7 @@
  10 -> Hauppauge WinTV HVR 900                  (em2880)        [2040:6500]
  11 -> Terratec Hybrid XS                       (em2880)
  12 -> Kworld PVR TV 2800 RF                    (em2820/em2840)
- 13 -> Terratec Prodigy XS                      (em2880)        [0ccd:10ad]
+ 13 -> Terratec Prodigy XS                      (em2880)
  14 -> SIIG AVTuner-PVR / Pixelview Prolink PlayTV USB 2.0 (em2820/em2840)
  15 -> V-Gear PocketTV                          (em2800)
  16 -> Hauppauge WinTV HVR 950                  (em2883)        [2040:6513,2040:6517,2040:651b]
@@ -41,7 +41,7 @@
  40 -> Plextor ConvertX PX-TV100U               (em2861)        [093b:a005]
  41 -> Kworld 350 U DVB-T                       (em2870)        [eb1a:e350]
  42 -> Kworld 355 U DVB-T                       (em2870)        [eb1a:e355,eb1a:e357]
- 43 -> Terratec Cinergy T XS                    (em2870)
+ 43 -> Terratec Cinergy T XS                    (em2870)        [0ccd:0043]
  44 -> Terratec Cinergy T XS (MT2060)           (em2870)
  45 -> Pinnacle PCTV DVB-T                      (em2870)
  46 -> Compro, VideoMate U3                     (em2870)        [185b:2870]
@@ -76,5 +76,5 @@
  76 -> KWorld PlusTV 340U or UB435-Q (ATSC)     (em2870)        [1b80:a340]
  77 -> EM2874 Leadership ISDBT                  (em2874)
  78 -> PCTV nanoStick T2 290e                   (em28174)
- 79 -> Terratec Cinergy H5                      (em2884)        [0ccd:0043,0ccd:10a2]
+ 79 -> Terratec Cinergy H5                      (em2884)        [0ccd:10a2,0ccd:10ad]
  80 -> PCTV DVB-S2 Stick (460e)                 (em28174)
diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c
index 9b747c2..19a5be3 100644
--- a/drivers/media/video/em28xx/em28xx-cards.c
+++ b/drivers/media/video/em28xx/em28xx-cards.c
@@ -1914,11 +1914,11 @@ struct usb_device_id em28xx_id_table[] = {
 	{ USB_DEVICE(0x0ccd, 0x0042),
 			.driver_info = EM2882_BOARD_TERRATEC_HYBRID_XS },
 	{ USB_DEVICE(0x0ccd, 0x0043),
+			.driver_info = EM2870_BOARD_TERRATEC_XS },
+	{ USB_DEVICE(0x0ccd, 0x10a2),	/* H5 Rev. 1 */
 			.driver_info = EM2884_BOARD_TERRATEC_H5 },
-	{ USB_DEVICE(0x0ccd, 0x10a2),	/* Rev. 1 */
+	{ USB_DEVICE(0x0ccd, 0x10ad),	/* H5 Rev. 2 */
 			.driver_info = EM2884_BOARD_TERRATEC_H5 },
-	{ USB_DEVICE(0x0ccd, 0x10ad),	/* Rev. 2 */
-			.driver_info = EM2880_BOARD_TERRATEC_PRODIGY_XS },
 	{ USB_DEVICE(0x0ccd, 0x0084),
 			.driver_info = EM2860_BOARD_TERRATEC_AV350 },
 	{ USB_DEVICE(0x0ccd, 0x0096),
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/8] [media] xc5000: Add support for get_if_frequency
  2011-11-20 14:56   ` [PATCH 3/8] [media] em28xx: Fix some Terratec entries (H5 and XS) Mauro Carvalho Chehab
@ 2011-11-20 14:56     ` Mauro Carvalho Chehab
  2011-11-20 14:56       ` [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again Mauro Carvalho Chehab
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-11-20 14:56 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab

This is needed for devices with DRX-K and xc5000.

Tested with a HVR 930C hardware.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/common/tuners/xc5000.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index 88b329c..ecd1f95 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -968,6 +968,14 @@ static int xc5000_get_frequency(struct dvb_frontend *fe, u32 *freq)
 	return 0;
 }
 
+static int xc5000_get_if_frequency(struct dvb_frontend *fe, u32 *freq)
+{
+	struct xc5000_priv *priv = fe->tuner_priv;
+	dprintk(1, "%s()\n", __func__);
+	*freq = priv->if_khz * 1000;
+	return 0;
+}
+
 static int xc5000_get_bandwidth(struct dvb_frontend *fe, u32 *bw)
 {
 	struct xc5000_priv *priv = fe->tuner_priv;
@@ -1108,6 +1116,7 @@ static const struct dvb_tuner_ops xc5000_tuner_ops = {
 	.set_params	   = xc5000_set_params,
 	.set_analog_params = xc5000_set_analog_params,
 	.get_frequency	   = xc5000_get_frequency,
+	.get_if_frequency  = xc5000_get_if_frequency,
 	.get_bandwidth	   = xc5000_get_bandwidth,
 	.get_status	   = xc5000_get_status
 };
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-11-20 14:56     ` [PATCH 4/8] [media] xc5000: Add support for get_if_frequency Mauro Carvalho Chehab
@ 2011-11-20 14:56       ` Mauro Carvalho Chehab
  2011-11-20 14:56         ` [PATCH 6/8] [media] em28xx: Fix CodingStyle issues introduced by changeset 82e7dbb Mauro Carvalho Chehab
  2011-12-05 18:23         ` [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again Devin Heitmueller
  0 siblings, 2 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-11-20 14:56 UTC (permalink / raw)
  To: linux-media; +Cc: Eddi De Pieri, Mauro Carvalho Chehab

From: Eddi De Pieri <eddi@depieri.net>

With this patch I try again to add initial support for HVR930C.

Tested only DVB-T, since in Italy Analog service is stopped.

Actually "scan -a0 -f1", find only about 50 channel while 400 should
be available.

[mchehab@redhat.com: Tested with DVB-C and fixed a few whitespace issues]
Tested-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Eddi De Pieri <eddi@depieri.net>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/common/tuners/xc5000.c      |    4 +
 drivers/media/dvb/frontends/drxk.h        |    2 +
 drivers/media/dvb/frontends/drxk_hard.c   |    4 +-
 drivers/media/video/em28xx/em28xx-cards.c |   37 ++++++++-
 drivers/media/video/em28xx/em28xx-dvb.c   |  136 ++++++++++++++++++++++++++++-
 drivers/media/video/em28xx/em28xx.h       |    2 +
 6 files changed, 181 insertions(+), 4 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index ecd1f95..048f489 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -1004,6 +1004,8 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
 	struct xc5000_priv *priv = fe->tuner_priv;
 	int ret = 0;
 
+	mutex_lock(&xc5000_list_mutex);
+
 	if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
 		ret = xc5000_fwupload(fe);
 		if (ret != XC_RESULT_SUCCESS)
@@ -1023,6 +1025,8 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
 	/* Default to "CABLE" mode */
 	ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
 
+	mutex_unlock(&xc5000_list_mutex);
+
 	return ret;
 }
 
diff --git a/drivers/media/dvb/frontends/drxk.h b/drivers/media/dvb/frontends/drxk.h
index 58baf41..e6d42e2 100644
--- a/drivers/media/dvb/frontends/drxk.h
+++ b/drivers/media/dvb/frontends/drxk.h
@@ -26,6 +26,8 @@ struct drxk_config {
 	bool	antenna_dvbt;
 	u16	antenna_gpio;
 
+	int    chunk_size;
+
 	const char *microcode_name;
 };
 
diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index dc13fd8..2392092 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -681,7 +681,8 @@ static int init_state(struct drxk_state *state)
 	state->m_hasOOB = false;
 	state->m_hasAudio = false;
 
-	state->m_ChunkSize = 124;
+	if (!state->m_ChunkSize)
+	    state->m_ChunkSize = 124;
 
 	state->m_oscClockFreq = 0;
 	state->m_smartAntInverted = false;
@@ -6430,6 +6431,7 @@ struct dvb_frontend *drxk_attach(const struct drxk_config *config,
 	state->no_i2c_bridge = config->no_i2c_bridge;
 	state->antenna_gpio = config->antenna_gpio;
 	state->antenna_dvbt = config->antenna_dvbt;
+	state->m_ChunkSize = config->chunk_size;
 
 	/* NOTE: as more UIO bits will be used, add them to the mask */
 	state->UIO_mask = config->antenna_gpio;
diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c
index 19a5be3..705aedf 100644
--- a/drivers/media/video/em28xx/em28xx-cards.c
+++ b/drivers/media/video/em28xx/em28xx-cards.c
@@ -336,6 +336,24 @@ static struct em28xx_reg_seq pctv_460e[] = {
 	{             -1,   -1,   -1,  -1},
 };
 
+static struct em28xx_reg_seq hauppauge_930c_gpio[] = {
+// xc5000 reset
+	{EM2874_R80_GPIO,	0x6f,	0xff,	10},
+	{EM2874_R80_GPIO,	0x4f,	0xff,	10},
+	{EM2874_R80_GPIO,	0x6f,	0xff,	10},
+	{EM2874_R80_GPIO,	0x4f,	0xff,	10},
+	{ -1,			-1,	-1,	-1},
+};
+
+#if 0
+static struct em28xx_reg_seq hauppauge_930c_digital[] = {
+	{EM2874_R80_GPIO,	0xf6,	0xff,	10},
+	{EM2874_R80_GPIO,	0xe6,	0xff,	100},
+	{EM2874_R80_GPIO,	0xa6,	0xff,	10},
+	{ -1,			-1,	-1,	-1},
+};
+#endif
+
 /*
  *  Board definitions
  */
@@ -892,6 +910,19 @@ struct em28xx_board em28xx_boards[] = {
 				EM28XX_I2C_CLK_WAIT_ENABLE |
 				EM28XX_I2C_FREQ_400_KHZ,
 	},
+	[EM2884_BOARD_HAUPPAUGE_WINTV_HVR_930C] = {
+		.name         = "Hauppauge WinTV HVR 930C",
+		.has_dvb      = 1,
+//#if 0
+//		.tuner_type   = TUNER_XC5000,
+//		.tuner_addr   = 0x41,
+//		.dvb_gpio     = hauppauge_930c_digital, /* FIXME: probably wrong */
+		.tuner_gpio   = hauppauge_930c_gpio,
+//#endif
+		.i2c_speed    = EM2874_I2C_SECONDARY_BUS_SELECT |
+				EM28XX_I2C_CLK_WAIT_ENABLE |
+				EM28XX_I2C_FREQ_400_KHZ,
+	},
 	[EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900] = {
 		.name         = "Hauppauge WinTV HVR 900",
 		.tda9887_conf = TDA9887_PRESENT,
@@ -1975,6 +2006,8 @@ struct usb_device_id em28xx_id_table[] = {
 			.driver_info = EM28174_BOARD_PCTV_290E },
 	{ USB_DEVICE(0x2013, 0x024c),
 			.driver_info = EM28174_BOARD_PCTV_460E },
+	{ USB_DEVICE(0x2040, 0x1605),
+			.driver_info = EM2884_BOARD_HAUPPAUGE_WINTV_HVR_930C },
 	{ },
 };
 MODULE_DEVICE_TABLE(usb, em28xx_id_table);
@@ -2028,10 +2061,10 @@ int em28xx_tuner_callback(void *ptr, int component, int command, int arg)
 	int rc = 0;
 	struct em28xx *dev = ptr;
 
-	if (dev->tuner_type != TUNER_XC2028)
+	if (dev->tuner_type != TUNER_XC2028 && dev->tuner_type != TUNER_XC5000)
 		return 0;
 
-	if (command != XC2028_TUNER_RESET)
+	if (command != XC2028_TUNER_RESET && command != XC5000_TUNER_RESET)
 		return 0;
 
 	rc = em28xx_gpio_set(dev, dev->board.tuner_gpio);
diff --git a/drivers/media/video/em28xx/em28xx-dvb.c b/drivers/media/video/em28xx/em28xx-dvb.c
index cef7a2d..d19939b 100644
--- a/drivers/media/video/em28xx/em28xx-dvb.c
+++ b/drivers/media/video/em28xx/em28xx-dvb.c
@@ -316,6 +316,14 @@ struct drxk_config terratec_h5_drxk = {
 	.microcode_name = "dvb-usb-terratec-h5-drxk.fw",
 };
 
+struct drxk_config hauppauge_930c_drxk = {
+	.adr = 0x29,
+	.single_master = 1,
+	.no_i2c_bridge = 1,
+	.microcode_name = "dvb-usb-hauppauge-hvr930c-drxk.fw",
+	.chunk_size = 56,
+};
+
 static int drxk_gate_ctrl(struct dvb_frontend *fe, int enable)
 {
 	struct em28xx_dvb *dvb = fe->sec_priv;
@@ -334,6 +342,90 @@ static int drxk_gate_ctrl(struct dvb_frontend *fe, int enable)
 	return status;
 }
 
+static void hauppauge_hvr930c_init(struct em28xx *dev)
+{
+	int i;
+
+	struct em28xx_reg_seq hauppauge_hvr930c_init[] = {
+		{EM2874_R80_GPIO,	0xff,	0xff,	101},  //11111111
+//		{0xd            ,	0xff,	0xff,	101},  //11111111
+		{EM2874_R80_GPIO,	0xfb,	0xff,	50},   //11111011  init bit 3
+		{EM2874_R80_GPIO,	0xff,	0xff,	184},  //11111111
+		{ -1,                   -1,     -1,     -1},
+	};
+	struct em28xx_reg_seq hauppauge_hvr930c_end[] = {
+		{EM2874_R80_GPIO,	0xef,	0xff,	1},    //11101111
+		{EM2874_R80_GPIO,	0xaf,	0xff,	101},  //10101111  init bit 7
+		{EM2874_R80_GPIO,	0xef,	0xff,	118},   //11101111
+
+
+//per il tuner?
+		{EM2874_R80_GPIO,	0xef,	0xff,	1},  //11101111
+		{EM2874_R80_GPIO,	0xcf,	0xff,	11},    //11001111  init bit 6
+		{EM2874_R80_GPIO,	0xef,	0xff,	64},  //11101111
+
+		{EM2874_R80_GPIO,	0xcf,	0xff,	101},  //11001111  init bit 6
+		{EM2874_R80_GPIO,	0xef,	0xff,	101},  //11101111
+		{EM2874_R80_GPIO,	0xcf,	0xff,	11},  //11001111  init bit 6
+		{EM2874_R80_GPIO,	0xef,	0xff,	101},  //11101111
+
+//		{EM2874_R80_GPIO,	0x6f,	0xff,	10},    //01101111
+//		{EM2874_R80_GPIO,	0x6d,	0xff,	100},  //01101101  init bit 2
+		{ -1,                   -1,     -1,     -1},
+	};
+
+	struct em28xx_reg_seq hauppauge_hvr930c_end2[] = {
+//		{EM2874_R80_GPIO,	0x6f,	0xff,	124},  //01101111
+//		{EM2874_R80_GPIO,	0x4f,	0xff,	11},   //01001111  init bit 6
+//		{EM2874_R80_GPIO,	0x6f,	0xff,	1},    //01101111
+//		{EM2874_R80_GPIO,	0x4f,	0xff,	10},   //01001111  init bit 6
+//		{EM2874_R80_GPIO,	0x6f,	0xff,	100},  //01101111
+//		{0xd            ,	0x42,	0xff,	101},  //11111111
+		{ -1,                   -1,     -1,     -1},
+	};
+	struct {
+		unsigned char r[4];
+		int len;
+	} regs[] = {
+		{{ 0x06, 0x02, 0x00, 0x31 }, 4},
+		{{ 0x01, 0x02 }, 2},
+		{{ 0x01, 0x02, 0x00, 0xc6 }, 4},
+		{{ 0x01, 0x00 }, 2},
+		{{ 0x01, 0x00, 0xff, 0xaf }, 4},
+		{{ 0x01, 0x00, 0x03, 0xa0 }, 4},
+		{{ 0x01, 0x00 }, 2},
+		{{ 0x01, 0x00, 0x73, 0xaf }, 4},
+		{{ 0x04, 0x00 }, 2},
+		{{ 0x00, 0x04 }, 2},
+		{{ 0x00, 0x04, 0x00, 0x0a }, 4},
+		{{ 0x04, 0x14 }, 2},
+		{{ 0x04, 0x14, 0x00, 0x00 }, 4},
+	};
+
+	em28xx_gpio_set(dev, hauppauge_hvr930c_init);
+	em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x40);
+	msleep(10);
+	em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
+	msleep(10);
+
+	dev->i2c_client.addr = 0x82 >> 1;
+
+	for (i = 0; i < ARRAY_SIZE(regs); i++)
+		i2c_master_send(&dev->i2c_client, regs[i].r, regs[i].len);
+	em28xx_gpio_set(dev, hauppauge_hvr930c_end);
+
+	msleep(100);
+
+	em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
+	msleep(30);
+
+	em28xx_gpio_set(dev, hauppauge_hvr930c_end2);
+	msleep(10);
+	em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
+	msleep(10);
+
+}
+
 static void terratec_h5_init(struct em28xx *dev)
 {
 	int i;
@@ -788,6 +880,47 @@ static int em28xx_dvb_init(struct em28xx *dev)
 			mfe_shared = 1;
 		}
 		break;
+	case EM2884_BOARD_HAUPPAUGE_WINTV_HVR_930C:
+		hauppauge_hvr930c_init(dev);
+
+		dvb->dont_attach_fe1 = 1;
+
+		dvb->fe[0] = dvb_attach(drxk_attach, &hauppauge_930c_drxk, &dev->i2c_adap, &dvb->fe[1]);
+		if (!dvb->fe[0]) {
+			result = -EINVAL;
+			goto out_free;
+		}
+		/* FIXME: do we need a pll semaphore? */
+		dvb->fe[0]->sec_priv = dvb;
+		sema_init(&dvb->pll_mutex, 1);
+		dvb->gate_ctrl = dvb->fe[0]->ops.i2c_gate_ctrl;
+		dvb->fe[0]->ops.i2c_gate_ctrl = drxk_gate_ctrl;
+		dvb->fe[1]->id = 1;
+
+		/* Attach xc5000 */
+		struct xc5000_config cfg;
+		memset(&cfg, 0, sizeof(cfg));
+		cfg.i2c_address  = 0x61;
+		//cfg.if_khz = 4570; //FIXME
+		cfg.if_khz = 4000; //FIXME (should be ok) read from i2c traffic
+
+		if (dvb->fe[0]->ops.i2c_gate_ctrl)
+			dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1);
+		if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap, &cfg)) {
+			result = -EINVAL;
+			goto out_free;
+		}
+
+		if (dvb->fe[0]->ops.i2c_gate_ctrl)
+			dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 0);
+
+		/* Hack - needed by drxk/tda18271c2dd */
+		dvb->fe[1]->tuner_priv = dvb->fe[0]->tuner_priv;
+		memcpy(&dvb->fe[1]->ops.tuner_ops,
+		       &dvb->fe[0]->ops.tuner_ops,
+		       sizeof(dvb->fe[0]->ops.tuner_ops));
+
+		break;
 	case EM2884_BOARD_TERRATEC_H5:
 		terratec_h5_init(dev);
 
@@ -798,7 +931,6 @@ static int em28xx_dvb_init(struct em28xx *dev)
 			result = -EINVAL;
 			goto out_free;
 		}
-
 		/* FIXME: do we need a pll semaphore? */
 		dvb->fe[0]->sec_priv = dvb;
 		sema_init(&dvb->pll_mutex, 1);
@@ -845,6 +977,8 @@ static int em28xx_dvb_init(struct em28xx *dev)
 	}
 	/* define general-purpose callback pointer */
 	dvb->fe[0]->callback = em28xx_tuner_callback;
+	if (dvb->fe[1])
+	    dvb->fe[1]->callback = em28xx_tuner_callback;
 
 	/* register everything */
 	result = em28xx_register_dvb(dvb, THIS_MODULE, dev, &dev->udev->dev);
diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h
index 2a2cb7e..c16ae8f 100644
--- a/drivers/media/video/em28xx/em28xx.h
+++ b/drivers/media/video/em28xx/em28xx.h
@@ -38,6 +38,7 @@
 #include <media/videobuf-dvb.h>
 #endif
 #include "tuner-xc2028.h"
+#include "xc5000.h"
 #include "em28xx-reg.h"
 
 /* Boards supported by driver */
@@ -121,6 +122,7 @@
 #define EM28174_BOARD_PCTV_290E                   78
 #define EM2884_BOARD_TERRATEC_H5		  79
 #define EM28174_BOARD_PCTV_460E                   80
+#define EM2884_BOARD_HAUPPAUGE_WINTV_HVR_930C	  81
 
 /* Limits minimum and default number of buffers */
 #define EM28XX_MIN_BUF 4
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 6/8] [media] em28xx: Fix CodingStyle issues introduced by changeset 82e7dbb
  2011-11-20 14:56       ` [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again Mauro Carvalho Chehab
@ 2011-11-20 14:56         ` Mauro Carvalho Chehab
  2011-11-20 14:56           ` [PATCH 7/8] [media] em28xx: Add IR support for em2884 Mauro Carvalho Chehab
  2011-12-05 18:23         ` [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again Devin Heitmueller
  1 sibling, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-11-20 14:56 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/dvb/frontends/drxk_hard.c   |    2 +-
 drivers/media/video/em28xx/em28xx-cards.c |   17 +++++----
 drivers/media/video/em28xx/em28xx-dvb.c   |   57 +++++++++++------------------
 3 files changed, 32 insertions(+), 44 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 2392092..95cbc98 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -682,7 +682,7 @@ static int init_state(struct drxk_state *state)
 	state->m_hasAudio = false;
 
 	if (!state->m_ChunkSize)
-	    state->m_ChunkSize = 124;
+		state->m_ChunkSize = 124;
 
 	state->m_oscClockFreq = 0;
 	state->m_smartAntInverted = false;
diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c
index 705aedf..d92e0af 100644
--- a/drivers/media/video/em28xx/em28xx-cards.c
+++ b/drivers/media/video/em28xx/em28xx-cards.c
@@ -337,9 +337,8 @@ static struct em28xx_reg_seq pctv_460e[] = {
 };
 
 static struct em28xx_reg_seq hauppauge_930c_gpio[] = {
-// xc5000 reset
 	{EM2874_R80_GPIO,	0x6f,	0xff,	10},
-	{EM2874_R80_GPIO,	0x4f,	0xff,	10},
+	{EM2874_R80_GPIO,	0x4f,	0xff,	10}, /* xc5000 reset */
 	{EM2874_R80_GPIO,	0x6f,	0xff,	10},
 	{EM2874_R80_GPIO,	0x4f,	0xff,	10},
 	{ -1,			-1,	-1,	-1},
@@ -905,6 +904,8 @@ struct em28xx_board em28xx_boards[] = {
 		.tuner_addr   = 0x41,
 		.dvb_gpio     = terratec_h5_digital, /* FIXME: probably wrong */
 		.tuner_gpio   = terratec_h5_gpio,
+#else
+		.tuner_type   = TUNER_ABSENT,
 #endif
 		.i2c_speed    = EM2874_I2C_SECONDARY_BUS_SELECT |
 				EM28XX_I2C_CLK_WAIT_ENABLE |
@@ -913,12 +914,14 @@ struct em28xx_board em28xx_boards[] = {
 	[EM2884_BOARD_HAUPPAUGE_WINTV_HVR_930C] = {
 		.name         = "Hauppauge WinTV HVR 930C",
 		.has_dvb      = 1,
-//#if 0
-//		.tuner_type   = TUNER_XC5000,
-//		.tuner_addr   = 0x41,
-//		.dvb_gpio     = hauppauge_930c_digital, /* FIXME: probably wrong */
+#if 0 /* FIXME: Add analog support */
+		.tuner_type   = TUNER_XC5000,
+		.tuner_addr   = 0x41,
+		.dvb_gpio     = hauppauge_930c_digital,
 		.tuner_gpio   = hauppauge_930c_gpio,
-//#endif
+#else
+		.tuner_type   = TUNER_ABSENT,
+#endif
 		.i2c_speed    = EM2874_I2C_SECONDARY_BUS_SELECT |
 				EM28XX_I2C_CLK_WAIT_ENABLE |
 				EM28XX_I2C_FREQ_400_KHZ,
diff --git a/drivers/media/video/em28xx/em28xx-dvb.c b/drivers/media/video/em28xx/em28xx-dvb.c
index d19939b..55a9008 100644
--- a/drivers/media/video/em28xx/em28xx-dvb.c
+++ b/drivers/media/video/em28xx/em28xx-dvb.c
@@ -347,42 +347,27 @@ static void hauppauge_hvr930c_init(struct em28xx *dev)
 	int i;
 
 	struct em28xx_reg_seq hauppauge_hvr930c_init[] = {
-		{EM2874_R80_GPIO,	0xff,	0xff,	101},  //11111111
-//		{0xd            ,	0xff,	0xff,	101},  //11111111
-		{EM2874_R80_GPIO,	0xfb,	0xff,	50},   //11111011  init bit 3
-		{EM2874_R80_GPIO,	0xff,	0xff,	184},  //11111111
+		{EM2874_R80_GPIO,	0xff,	0xff,	0x65},
+		{EM2874_R80_GPIO,	0xfb,	0xff,	0x32},
+		{EM2874_R80_GPIO,	0xff,	0xff,	0xb8},
 		{ -1,                   -1,     -1,     -1},
 	};
 	struct em28xx_reg_seq hauppauge_hvr930c_end[] = {
-		{EM2874_R80_GPIO,	0xef,	0xff,	1},    //11101111
-		{EM2874_R80_GPIO,	0xaf,	0xff,	101},  //10101111  init bit 7
-		{EM2874_R80_GPIO,	0xef,	0xff,	118},   //11101111
+		{EM2874_R80_GPIO,	0xef,	0xff,	0x01},
+		{EM2874_R80_GPIO,	0xaf,	0xff,	0x65},
+		{EM2874_R80_GPIO,	0xef,	0xff,	0x76},
+		{EM2874_R80_GPIO,	0xef,	0xff,	0x01},
+		{EM2874_R80_GPIO,	0xcf,	0xff,	0x0b},
+		{EM2874_R80_GPIO,	0xef,	0xff,	0x40},
+
+		{EM2874_R80_GPIO,	0xcf,	0xff,	0x65},
+		{EM2874_R80_GPIO,	0xef,	0xff,	0x65},
+		{EM2874_R80_GPIO,	0xcf,	0xff,	0x0b},
+		{EM2874_R80_GPIO,	0xef,	0xff,	0x65},
 
-
-//per il tuner?
-		{EM2874_R80_GPIO,	0xef,	0xff,	1},  //11101111
-		{EM2874_R80_GPIO,	0xcf,	0xff,	11},    //11001111  init bit 6
-		{EM2874_R80_GPIO,	0xef,	0xff,	64},  //11101111
-
-		{EM2874_R80_GPIO,	0xcf,	0xff,	101},  //11001111  init bit 6
-		{EM2874_R80_GPIO,	0xef,	0xff,	101},  //11101111
-		{EM2874_R80_GPIO,	0xcf,	0xff,	11},  //11001111  init bit 6
-		{EM2874_R80_GPIO,	0xef,	0xff,	101},  //11101111
-
-//		{EM2874_R80_GPIO,	0x6f,	0xff,	10},    //01101111
-//		{EM2874_R80_GPIO,	0x6d,	0xff,	100},  //01101101  init bit 2
 		{ -1,                   -1,     -1,     -1},
 	};
 
-	struct em28xx_reg_seq hauppauge_hvr930c_end2[] = {
-//		{EM2874_R80_GPIO,	0x6f,	0xff,	124},  //01101111
-//		{EM2874_R80_GPIO,	0x4f,	0xff,	11},   //01001111  init bit 6
-//		{EM2874_R80_GPIO,	0x6f,	0xff,	1},    //01101111
-//		{EM2874_R80_GPIO,	0x4f,	0xff,	10},   //01001111  init bit 6
-//		{EM2874_R80_GPIO,	0x6f,	0xff,	100},  //01101111
-//		{0xd            ,	0x42,	0xff,	101},  //11111111
-		{ -1,                   -1,     -1,     -1},
-	};
 	struct {
 		unsigned char r[4];
 		int len;
@@ -419,8 +404,6 @@ static void hauppauge_hvr930c_init(struct em28xx *dev)
 	em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x44);
 	msleep(30);
 
-	em28xx_gpio_set(dev, hauppauge_hvr930c_end2);
-	msleep(10);
 	em28xx_write_reg(dev, EM28XX_R06_I2C_CLK, 0x45);
 	msleep(10);
 
@@ -885,7 +868,9 @@ static int em28xx_dvb_init(struct em28xx *dev)
 
 		dvb->dont_attach_fe1 = 1;
 
-		dvb->fe[0] = dvb_attach(drxk_attach, &hauppauge_930c_drxk, &dev->i2c_adap, &dvb->fe[1]);
+		dvb->fe[0] = dvb_attach(drxk_attach,
+					&hauppauge_930c_drxk, &dev->i2c_adap,
+					&dvb->fe[1]);
 		if (!dvb->fe[0]) {
 			result = -EINVAL;
 			goto out_free;
@@ -901,12 +886,12 @@ static int em28xx_dvb_init(struct em28xx *dev)
 		struct xc5000_config cfg;
 		memset(&cfg, 0, sizeof(cfg));
 		cfg.i2c_address  = 0x61;
-		//cfg.if_khz = 4570; //FIXME
-		cfg.if_khz = 4000; //FIXME (should be ok) read from i2c traffic
+		cfg.if_khz = 4000;
 
 		if (dvb->fe[0]->ops.i2c_gate_ctrl)
 			dvb->fe[0]->ops.i2c_gate_ctrl(dvb->fe[0], 1);
-		if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap, &cfg)) {
+		if (!dvb_attach(xc5000_attach, dvb->fe[0], &dev->i2c_adap,
+				&cfg)) {
 			result = -EINVAL;
 			goto out_free;
 		}
@@ -978,7 +963,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
 	/* define general-purpose callback pointer */
 	dvb->fe[0]->callback = em28xx_tuner_callback;
 	if (dvb->fe[1])
-	    dvb->fe[1]->callback = em28xx_tuner_callback;
+		dvb->fe[1]->callback = em28xx_tuner_callback;
 
 	/* register everything */
 	result = em28xx_register_dvb(dvb, THIS_MODULE, dev, &dev->udev->dev);
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 7/8] [media] em28xx: Add IR support for em2884
  2011-11-20 14:56         ` [PATCH 6/8] [media] em28xx: Fix CodingStyle issues introduced by changeset 82e7dbb Mauro Carvalho Chehab
@ 2011-11-20 14:56           ` Mauro Carvalho Chehab
  2011-11-20 14:56             ` [PATCH 8/8] [media] em28xx: Add IR support for HVR-930C Mauro Carvalho Chehab
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-11-20 14:56 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/video/em28xx/em28xx-input.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/em28xx/em28xx-input.c b/drivers/media/video/em28xx/em28xx-input.c
index 679da48..2630b26 100644
--- a/drivers/media/video/em28xx/em28xx-input.c
+++ b/drivers/media/video/em28xx/em28xx-input.c
@@ -306,7 +306,8 @@ static void em28xx_ir_handle_key(struct em28xx_IR *ir)
 				   poll_result.rc_data[0],
 				   poll_result.toggle_bit);
 
-		if (ir->dev->chip_id == CHIP_ID_EM2874)
+		if (ir->dev->chip_id == CHIP_ID_EM2874 ||
+		    ir->dev->chip_id == CHIP_ID_EM2884)
 			/* The em2874 clears the readcount field every time the
 			   register is read.  The em2860/2880 datasheet says that it
 			   is supposed to clear the readcount, but it doesn't.  So with
@@ -371,13 +372,15 @@ int em28xx_ir_change_protocol(struct rc_dev *rc_dev, u64 rc_type)
 	case CHIP_ID_EM2883:
 		ir->get_key = default_polling_getkey;
 		break;
+	case CHIP_ID_EM2884:
 	case CHIP_ID_EM2874:
 	case CHIP_ID_EM28174:
 		ir->get_key = em2874_polling_getkey;
 		em28xx_write_regs(dev, EM2874_R50_IR_CONFIG, &ir_config, 1);
 		break;
 	default:
-		printk("Unrecognized em28xx chip id: IR not supported\n");
+		printk("Unrecognized em28xx chip id 0x%02x: IR not supported\n",
+			dev->chip_id);
 		rc = -EINVAL;
 	}
 
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 8/8] [media] em28xx: Add IR support for HVR-930C
  2011-11-20 14:56           ` [PATCH 7/8] [media] em28xx: Add IR support for em2884 Mauro Carvalho Chehab
@ 2011-11-20 14:56             ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-11-20 14:56 UTC (permalink / raw)
  To: linux-media; +Cc: Mauro Carvalho Chehab

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/rc/keymaps/rc-hauppauge.c   |   51 +++++++++++++++++++++++++++++
 drivers/media/video/em28xx/em28xx-cards.c |    1 +
 2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/media/rc/keymaps/rc-hauppauge.c b/drivers/media/rc/keymaps/rc-hauppauge.c
index cd3db77..0afb23b 100644
--- a/drivers/media/rc/keymaps/rc-hauppauge.c
+++ b/drivers/media/rc/keymaps/rc-hauppauge.c
@@ -182,6 +182,57 @@ static struct rc_map_table rc5_hauppauge_new[] = {
 	{ 0x1d3f, KEY_HOME },
 
 	/*
+	 * Keycodes for PT# R-005 remote bundled with Haupauge HVR-930C
+	 * Keycodes start with address = 0x1c
+	 */
+	{ 0x1c3b, KEY_GOTO },
+	{ 0x1c3d, KEY_POWER },
+
+	{ 0x1c14, KEY_UP },
+	{ 0x1c15, KEY_DOWN },
+	{ 0x1c16, KEY_LEFT },
+	{ 0x1c17, KEY_RIGHT },
+	{ 0x1c25, KEY_OK },
+
+	{ 0x1c00, KEY_0 },
+	{ 0x1c01, KEY_1 },
+	{ 0x1c02, KEY_2 },
+	{ 0x1c03, KEY_3 },
+	{ 0x1c04, KEY_4 },
+	{ 0x1c05, KEY_5 },
+	{ 0x1c06, KEY_6 },
+	{ 0x1c07, KEY_7 },
+	{ 0x1c08, KEY_8 },
+	{ 0x1c09, KEY_9 },
+
+	{ 0x1c1f, KEY_EXIT },	/* BACK */
+	{ 0x1c0d, KEY_MENU },
+	{ 0x1c1c, KEY_TV },
+
+	{ 0x1c10, KEY_VOLUMEUP },
+	{ 0x1c11, KEY_VOLUMEDOWN },
+
+	{ 0x1c20, KEY_CHANNELUP },
+	{ 0x1c21, KEY_CHANNELDOWN },
+
+	{ 0x1c0f, KEY_MUTE },
+	{ 0x1c12, KEY_PREVIOUS }, /* Prev */
+
+	{ 0x1c36, KEY_STOP },
+	{ 0x1c37, KEY_RECORD },
+
+	{ 0x1c24, KEY_LAST },           /* <|             */
+	{ 0x1c1e, KEY_NEXT },           /* >|             */
+
+	{ 0x1c0a, KEY_TEXT },
+	{ 0x1c0e, KEY_SUBTITLE },	/* CC */
+
+	{ 0x1c32, KEY_REWIND },
+	{ 0x1c30, KEY_PAUSE },
+	{ 0x1c35, KEY_PLAY },
+	{ 0x1c34, KEY_FASTFORWARD },
+
+	/*
 	 * Keycodes for the old Black Remote Controller
 	 * This one also uses RC-5 protocol
 	 * Keycodes start with address = 0x00
diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c
index d92e0af..f63a715 100644
--- a/drivers/media/video/em28xx/em28xx-cards.c
+++ b/drivers/media/video/em28xx/em28xx-cards.c
@@ -922,6 +922,7 @@ struct em28xx_board em28xx_boards[] = {
 #else
 		.tuner_type   = TUNER_ABSENT,
 #endif
+		.ir_codes     = RC_MAP_HAUPPAUGE,
 		.i2c_speed    = EM2874_I2C_SECONDARY_BUS_SELECT |
 				EM28XX_I2C_CLK_WAIT_ENABLE |
 				EM28XX_I2C_FREQ_400_KHZ,
-- 
1.7.7.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/8] [media] dvb: Allow select between DVB-C Annex A and Annex C
  2011-11-20 14:56 [PATCH 1/8] [media] dvb: Allow select between DVB-C Annex A and Annex C Mauro Carvalho Chehab
  2011-11-20 14:56 ` [PATCH 2/8] [media] Properly implement ITU-T J.88 Annex C support Mauro Carvalho Chehab
@ 2011-11-20 20:27 ` Rémi Denis-Courmont
  1 sibling, 0 replies; 22+ messages in thread
From: Rémi Denis-Courmont @ 2011-11-20 20:27 UTC (permalink / raw)
  To: linux-media

Le dimanche 20 novembre 2011 16:56:11 Mauro Carvalho Chehab, vous avez écrit :
> DVB-C, as defined by ITU-T J.83 has 3 annexes. The differences between
> Annex A and Annex C is that Annex C uses a subset of the modulation
> types, and uses a different rolloff factor. A different rolloff means
> that the bandwidth required is slicely different, and may affect the
> saw filter configuration at the tuners. Also, some demods have different
> configurations, depending on using Annex A or Annex C.
> 
> So, allow userspace to specify it, by changing the rolloff factor.

I assume you'll bump the minor DVB version at some point too?

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-11-20 14:56       ` [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again Mauro Carvalho Chehab
  2011-11-20 14:56         ` [PATCH 6/8] [media] em28xx: Fix CodingStyle issues introduced by changeset 82e7dbb Mauro Carvalho Chehab
@ 2011-12-05 18:23         ` Devin Heitmueller
  2011-12-05 18:35           ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 22+ messages in thread
From: Devin Heitmueller @ 2011-12-05 18:23 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, Eddi De Pieri

On Sun, Nov 20, 2011 at 9:56 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
> index ecd1f95..048f489 100644
> --- a/drivers/media/common/tuners/xc5000.c
> +++ b/drivers/media/common/tuners/xc5000.c
> @@ -1004,6 +1004,8 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
>        struct xc5000_priv *priv = fe->tuner_priv;
>        int ret = 0;
>
> +       mutex_lock(&xc5000_list_mutex);
> +
>        if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
>                ret = xc5000_fwupload(fe);
>                if (ret != XC_RESULT_SUCCESS)
> @@ -1023,6 +1025,8 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
>        /* Default to "CABLE" mode */
>        ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
>
> +       mutex_unlock(&xc5000_list_mutex);
> +
>        return ret;
>  }

What's up with this change?  Is this a bugfix for some race condition?
 Why is it jammed into a patch for some particular product?

It seems like a change such as this could significantly change the
timing of tuner initialization if you have multiple xc5000 based
products that might have a slow i2c bus.  Was that intentional?

This patch should be NACK'd and resubmitted as it's own bugfix where
it's implications can be fully understood in the context of all the
other products that use xc5000.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-05 18:23         ` [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again Devin Heitmueller
@ 2011-12-05 18:35           ` Mauro Carvalho Chehab
  2011-12-05 18:46             ` Devin Heitmueller
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-12-05 18:35 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: linux-media, Eddi De Pieri

On 05-12-2011 16:23, Devin Heitmueller wrote:
> On Sun, Nov 20, 2011 at 9:56 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com>  wrote:
>> diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
>> index ecd1f95..048f489 100644
>> --- a/drivers/media/common/tuners/xc5000.c
>> +++ b/drivers/media/common/tuners/xc5000.c
>> @@ -1004,6 +1004,8 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
>>         struct xc5000_priv *priv = fe->tuner_priv;
>>         int ret = 0;
>>
>> +       mutex_lock(&xc5000_list_mutex);
>> +
>>         if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
>>                 ret = xc5000_fwupload(fe);
>>                 if (ret != XC_RESULT_SUCCESS)
>> @@ -1023,6 +1025,8 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
>>         /* Default to "CABLE" mode */
>>         ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
>>
>> +       mutex_unlock(&xc5000_list_mutex);
>> +
>>         return ret;
>>   }
>
> What's up with this change?  Is this a bugfix for some race condition?
>   Why is it jammed into a patch for some particular product?
>
> It seems like a change such as this could significantly change the
> timing of tuner initialization if you have multiple xc5000 based
> products that might have a slow i2c bus.  Was that intentional?
>
> This patch should be NACK'd and resubmitted as it's own bugfix where
> it's implications can be fully understood in the context of all the
> other products that use xc5000.

It is too late for nacking the patch, as there are several other patches
were already applied on the top of it, and we don't rebase the
linux-media.git tree.

Assuming that this is due to some bug that Eddi picked during xc5000
init, what it can be done now is to write a patch that would replace
this xc5000-global mutex lock into a some other per-device locking
schema.

Regards,
Mauro.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-05 18:35           ` Mauro Carvalho Chehab
@ 2011-12-05 18:46             ` Devin Heitmueller
  2011-12-05 20:01               ` Devin Heitmueller
  0 siblings, 1 reply; 22+ messages in thread
From: Devin Heitmueller @ 2011-12-05 18:46 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, Eddi De Pieri

On Mon, Dec 5, 2011 at 1:35 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
>> What's up with this change?  Is this a bugfix for some race condition?
>>  Why is it jammed into a patch for some particular product?
>>
>> It seems like a change such as this could significantly change the
>> timing of tuner initialization if you have multiple xc5000 based
>> products that might have a slow i2c bus.  Was that intentional?
>>
>> This patch should be NACK'd and resubmitted as it's own bugfix where
>> it's implications can be fully understood in the context of all the
>> other products that use xc5000.
>
>
> It is too late for nacking the patch, as there are several other patches
> were already applied on the top of it, and we don't rebase the
> linux-media.git tree.
>
> Assuming that this is due to some bug that Eddi picked during xc5000
> init, what it can be done now is to write a patch that would replace
> this xc5000-global mutex lock into a some other per-device locking
> schema.

At this point we have zero idea why it's there *at all*.  Eddi, can
you comment on what prompted this change?

This patch should not have been accepted in the first place.  It's an
undocumented change on a different driver than is advertised in the
subject line.  Did you review the patch prior to merging?

This change can result in a performance regression for all other
devices using xc5000, and it's not yet clear why it's there in the
first place.  If its use cannot be explained then it should be rolled
back.  If this breaks 930c, then the whole device support series
should be rolled back until somebody can figure out what is going on.

It's crap like this that is the reason that every other week I get
complaints from some user that one of the drivers I wrote support for
worked fine for months/years until they upgraded to the latest kernel.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-05 18:46             ` Devin Heitmueller
@ 2011-12-05 20:01               ` Devin Heitmueller
  2011-12-05 23:32                 ` Eddi De Pieri
  0 siblings, 1 reply; 22+ messages in thread
From: Devin Heitmueller @ 2011-12-05 20:01 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, Eddi De Pieri, Mark Lord

On Mon, Dec 5, 2011 at 1:46 PM, Devin Heitmueller
<dheitmueller@kernellabs.com> wrote:
> On Mon, Dec 5, 2011 at 1:35 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>>> What's up with this change?  Is this a bugfix for some race condition?
>>>  Why is it jammed into a patch for some particular product?
>>>
>>> It seems like a change such as this could significantly change the
>>> timing of tuner initialization if you have multiple xc5000 based
>>> products that might have a slow i2c bus.  Was that intentional?
>>>
>>> This patch should be NACK'd and resubmitted as it's own bugfix where
>>> it's implications can be fully understood in the context of all the
>>> other products that use xc5000.
>>
>>
>> It is too late for nacking the patch, as there are several other patches
>> were already applied on the top of it, and we don't rebase the
>> linux-media.git tree.
>>
>> Assuming that this is due to some bug that Eddi picked during xc5000
>> init, what it can be done now is to write a patch that would replace
>> this xc5000-global mutex lock into a some other per-device locking
>> schema.
>
> At this point we have zero idea why it's there *at all*.  Eddi, can
> you comment on what prompted this change?
>
> This patch should not have been accepted in the first place.  It's an
> undocumented change on a different driver than is advertised in the
> subject line.  Did you review the patch prior to merging?
>
> This change can result in a performance regression for all other
> devices using xc5000, and it's not yet clear why it's there in the
> first place.  If its use cannot be explained then it should be rolled
> back.  If this breaks 930c, then the whole device support series
> should be rolled back until somebody can figure out what is going on.
>
> It's crap like this that is the reason that every other week I get
> complaints from some user that one of the drivers I wrote support for
> worked fine for months/years until they upgraded to the latest kernel.

Speaking of which, Mark Lord just tried out this change (he has an
800i and 950q - both xc5000 based), and now his DVB stack fails to
load.  And yes, he already has the fix to the mutex_unlock()
regression which Dan Carpenter found six days ago and which this patch
introduced.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-05 20:01               ` Devin Heitmueller
@ 2011-12-05 23:32                 ` Eddi De Pieri
  2011-12-05 23:47                   ` Devin Heitmueller
  0 siblings, 1 reply; 22+ messages in thread
From: Eddi De Pieri @ 2011-12-05 23:32 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: Mauro Carvalho Chehab, linux-media, Mark Lord

Sorry,  I think I applied follow patch on my tree while I developed
the driver trying to fix tuner initialization.

http://patchwork.linuxtv.org/patch/6617/

I forgot to remove from my tree after I see that don't solve anything.

Regards
Eddi


On Mon, Dec 5, 2011 at 9:01 PM, Devin Heitmueller
<dheitmueller@kernellabs.com> wrote:
> On Mon, Dec 5, 2011 at 1:46 PM, Devin Heitmueller
> <dheitmueller@kernellabs.com> wrote:
>> On Mon, Dec 5, 2011 at 1:35 PM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>>> What's up with this change?  Is this a bugfix for some race condition?
>>>>  Why is it jammed into a patch for some particular product?
>>>>
>>>> It seems like a change such as this could significantly change the
>>>> timing of tuner initialization if you have multiple xc5000 based
>>>> products that might have a slow i2c bus.  Was that intentional?
>>>>
>>>> This patch should be NACK'd and resubmitted as it's own bugfix where
>>>> it's implications can be fully understood in the context of all the
>>>> other products that use xc5000.
>>>
>>>
>>> It is too late for nacking the patch, as there are several other patches
>>> were already applied on the top of it, and we don't rebase the
>>> linux-media.git tree.
>>>
>>> Assuming that this is due to some bug that Eddi picked during xc5000
>>> init, what it can be done now is to write a patch that would replace
>>> this xc5000-global mutex lock into a some other per-device locking
>>> schema.
>>
>> At this point we have zero idea why it's there *at all*.  Eddi, can
>> you comment on what prompted this change?
>>
>> This patch should not have been accepted in the first place.  It's an
>> undocumented change on a different driver than is advertised in the
>> subject line.  Did you review the patch prior to merging?
>>
>> This change can result in a performance regression for all other
>> devices using xc5000, and it's not yet clear why it's there in the
>> first place.  If its use cannot be explained then it should be rolled
>> back.  If this breaks 930c, then the whole device support series
>> should be rolled back until somebody can figure out what is going on.
>>
>> It's crap like this that is the reason that every other week I get
>> complaints from some user that one of the drivers I wrote support for
>> worked fine for months/years until they upgraded to the latest kernel.
>
> Speaking of which, Mark Lord just tried out this change (he has an
> 800i and 950q - both xc5000 based), and now his DVB stack fails to
> load.  And yes, he already has the fix to the mutex_unlock()
> regression which Dan Carpenter found six days ago and which this patch
> introduced.
>
> Devin
>
> --
> Devin J. Heitmueller - Kernel Labs
> http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-05 23:32                 ` Eddi De Pieri
@ 2011-12-05 23:47                   ` Devin Heitmueller
  2011-12-06 12:51                     ` Mark Lord
  2011-12-07  9:59                     ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 22+ messages in thread
From: Devin Heitmueller @ 2011-12-05 23:47 UTC (permalink / raw)
  To: Eddi De Pieri; +Cc: Mauro Carvalho Chehab, linux-media, Mark Lord

On Mon, Dec 5, 2011 at 6:32 PM, Eddi De Pieri <eddi@depieri.net> wrote:
> Sorry,  I think I applied follow patch on my tree while I developed
> the driver trying to fix tuner initialization.
>
> http://patchwork.linuxtv.org/patch/6617/
>
> I forgot to remove from my tree after I see that don't solve anything.

Ok, great.  At least that explains why it's there (since I couldn't
figure out how on Earth the patch made sense otherwise).

Eddi, could you please submit a patch removing the offending code?

Thanks,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-05 23:47                   ` Devin Heitmueller
@ 2011-12-06 12:51                     ` Mark Lord
  2011-12-06 13:43                       ` Mauro Carvalho Chehab
  2011-12-07  9:59                     ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 22+ messages in thread
From: Mark Lord @ 2011-12-06 12:51 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: Eddi De Pieri, Mauro Carvalho Chehab, linux-media

On 11-12-05 06:47 PM, Devin Heitmueller wrote:
> On Mon, Dec 5, 2011 at 6:32 PM, Eddi De Pieri <eddi@depieri.net> wrote:
>> Sorry,  I think I applied follow patch on my tree while I developed
>> the driver trying to fix tuner initialization.
>>
>> http://patchwork.linuxtv.org/patch/6617/
>>
>> I forgot to remove from my tree after I see that don't solve anything.
> 
> Ok, great.  At least that explains why it's there (since I couldn't
> figure out how on Earth the patch made sense otherwise).
> 
> Eddi, could you please submit a patch removing the offending code?


That's good.

But there definitely still is a race between modules in there somewhere.
The HVR-950Q tuners use several:  xc5000, au8522, au0828, ..
and unless au0828 is loaded *last*, with a delay before/after,
the dongles don't always work.  Preloading all of the modules
before allowing hardware detection seems to help.

Even just changing from a mechanical hard drive to a very fast SSD
is enough to change the behaviour from not-working to working
(and sometimes the other way around).

I tried to track this down a couple of years ago,
and found cross-module calls failing because the
target functions hadn't been loaded yet.
But my lack of notes from 2-3 years ago isn't helpful here.

Here's a similar report from 2 years ago, as valid today as it was then:

  http://www.mythtv.org/pipermail/mythtv-users/2010-January/279912.html

Cheers

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-06 12:51                     ` Mark Lord
@ 2011-12-06 13:43                       ` Mauro Carvalho Chehab
  2011-12-06 13:56                         ` Devin Heitmueller
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-12-06 13:43 UTC (permalink / raw)
  To: Mark Lord; +Cc: Devin Heitmueller, Eddi De Pieri, linux-media

On 06-12-2011 10:51, Mark Lord wrote:
> On 11-12-05 06:47 PM, Devin Heitmueller wrote:
>> On Mon, Dec 5, 2011 at 6:32 PM, Eddi De Pieri<eddi@depieri.net>  wrote:
>>> Sorry,  I think I applied follow patch on my tree while I developed
>>> the driver trying to fix tuner initialization.
>>>
>>> http://patchwork.linuxtv.org/patch/6617/
>>>
>>> I forgot to remove from my tree after I see that don't solve anything.
>>
>> Ok, great.  At least that explains why it's there (since I couldn't
>> figure out how on Earth the patch made sense otherwise).
>>
>> Eddi, could you please submit a patch removing the offending code?
>
>
> That's good.
>
> But there definitely still is a race between modules in there somewhere.
> The HVR-950Q tuners use several:  xc5000, au8522, au0828, ..
> and unless au0828 is loaded *last*, with a delay before/after,
> the dongles don't always work.  Preloading all of the modules
> before allowing hardware detection seems to help.
>
> Even just changing from a mechanical hard drive to a very fast SSD
> is enough to change the behaviour from not-working to working
> (and sometimes the other way around).
>
> I tried to track this down a couple of years ago,
> and found cross-module calls failing because the
> target functions hadn't been loaded yet.
> But my lack of notes from 2-3 years ago isn't helpful here.
>
> Here's a similar report from 2 years ago, as valid today as it was then:
>
>    http://www.mythtv.org/pipermail/mythtv-users/2010-January/279912.html

The driver who binds everything is the bridge driver. In your case, it is
the au0828 driver.

What you're experiencing seems to be some race issue inside it, and not at xc5000.

On a quick look on it, I'm noticing that there's no lock at au0828_usb_probe().

Also, it uses a separate lock for analog and for digital:

	mutex_init(&dev->mutex);
	mutex_init(&dev->dvb.lock);

Probably, the right thing to do would be to use just one lock for both rising
it at usb_probe, lowering it just before return 0. This will avoid any open
operations while the device is not fully initialized. Btw, newer udev's open
the analog part of the driver just after V4L register, in order to get the
device capabilities. This is known to cause race conditions, if the locking
schema is not working properly.

Regards,
Mauro.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-06 13:43                       ` Mauro Carvalho Chehab
@ 2011-12-06 13:56                         ` Devin Heitmueller
  2011-12-06 15:28                           ` Mark Lord
  0 siblings, 1 reply; 22+ messages in thread
From: Devin Heitmueller @ 2011-12-06 13:56 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Mark Lord, Eddi De Pieri, linux-media

On Tue, Dec 6, 2011 at 8:43 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> The driver who binds everything is the bridge driver. In your case, it is
> the au0828 driver.
>
> What you're experiencing seems to be some race issue inside it, and not at
> xc5000.
>
> On a quick look on it, I'm noticing that there's no lock at
> au0828_usb_probe().
>
> Also, it uses a separate lock for analog and for digital:
>
>        mutex_init(&dev->mutex);
>        mutex_init(&dev->dvb.lock);
>
> Probably, the right thing to do would be to use just one lock for both
> rising
> it at usb_probe, lowering it just before return 0. This will avoid any open
> operations while the device is not fully initialized. Btw, newer udev's open
> the analog part of the driver just after V4L register, in order to get the
> device capabilities. This is known to cause race conditions, if the locking
> schema is not working properly.

Just to be clear, we're now talking about a completely different race
condition that has nothing to do with the subject at hand, and this
discussion should probably be moved to a new thread.

That said, yes, there is definitely a race (if not two) in there to be
tracked down.  I know of a couple of users who upgraded to more recent
kernels and started experiencing breakage on module load where there
was none before.  This could obviously be dumb luck in that perhaps
the timing changed slightly, or it could be some change in the core
code which created a new race.  I haven't had the time/energy to dig
into the issue (compounded by the fact that these sorts of issues are
notoriously difficult to debug when it cannot be reproduced locally by
the developer).

The notion that this is something that has been there for over a year
is something I only learned of in the last couple of days.  All the
complaints I had seen thus far were from existing users who were
perfectly happy until they upgraded their kernel a couple of months
ago and then started seeing the problem.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-06 13:56                         ` Devin Heitmueller
@ 2011-12-06 15:28                           ` Mark Lord
  2011-12-06 15:35                             ` Devin Heitmueller
  2011-12-06 15:44                             ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 22+ messages in thread
From: Mark Lord @ 2011-12-06 15:28 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: Mauro Carvalho Chehab, Eddi De Pieri, linux-media

On 11-12-06 08:56 AM, Devin Heitmueller wrote:
> On Tue, Dec 6, 2011 at 8:43 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> The driver who binds everything is the bridge driver. In your case, it is
>> the au0828 driver.
>>
>> What you're experiencing seems to be some race issue inside it, and not at
>> xc5000.
>>
>> On a quick look on it, I'm noticing that there's no lock at
>> au0828_usb_probe().
>>
>> Also, it uses a separate lock for analog and for digital:
>>
>>        mutex_init(&dev->mutex);
>>        mutex_init(&dev->dvb.lock);
>>
>> Probably, the right thing to do would be to use just one lock for both
>> rising
>> it at usb_probe, lowering it just before return 0. This will avoid any open
>> operations while the device is not fully initialized. Btw, newer udev's open
>> the analog part of the driver just after V4L register, in order to get the
>> device capabilities. This is known to cause race conditions, if the locking
>> schema is not working properly.
> 
> Just to be clear, we're now talking about a completely different race
> condition that has nothing to do with the subject at hand, and this
> discussion should probably be moved to a new thread.

If this discussion does change threads, could you folks please copy me
on it?  I'm already subscribed to several other kernel mailing lists
in my roles as developer and maintainer of various bits, but I would
like to avoid having yet another daily deluge added to my inbox.  :)

That said, I can test possible fixes for this stuff,
and am rather interested to see it resolved.
..
> The notion that this is something that has been there for over a year
> is something I only learned of in the last couple of days.  All the
> complaints I had seen thus far were from existing users who were
> perfectly happy until they upgraded their kernel a couple of months
> ago and then started seeing the problem.
..

It's always exhibited races for me here.  I have long since worked around
the issue(s), so my own systems currently behave.   But with the newer
HVR-950Q revision (B4F0), the issue is far more prevalent than before.

I may try Mauro's locking suggestion -- more detail or a patch would be useful.

Mauro?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-06 15:28                           ` Mark Lord
@ 2011-12-06 15:35                             ` Devin Heitmueller
  2011-12-06 15:44                             ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 22+ messages in thread
From: Devin Heitmueller @ 2011-12-06 15:35 UTC (permalink / raw)
  To: Mark Lord; +Cc: Mauro Carvalho Chehab, Eddi De Pieri, linux-media

On Tue, Dec 6, 2011 at 10:28 AM, Mark Lord <kernel@teksavvy.com> wrote:
> It's always exhibited races for me here.  I have long since worked around
> the issue(s), so my own systems currently behave.   But with the newer
> HVR-950Q revision (B4F0), the issue is far more prevalent than before.

I'll ask around and see if I can find out what they changed in the
B4F0 revision.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-06 15:28                           ` Mark Lord
  2011-12-06 15:35                             ` Devin Heitmueller
@ 2011-12-06 15:44                             ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-12-06 15:44 UTC (permalink / raw)
  To: Mark Lord; +Cc: Devin Heitmueller, Eddi De Pieri, linux-media

On 06-12-2011 13:28, Mark Lord wrote:
> On 11-12-06 08:56 AM, Devin Heitmueller wrote:
>> On Tue, Dec 6, 2011 at 8:43 AM, Mauro Carvalho Chehab
>> <mchehab@redhat.com>  wrote:
>>> The driver who binds everything is the bridge driver. In your case, it is
>>> the au0828 driver.
>>>
>>> What you're experiencing seems to be some race issue inside it, and not at
>>> xc5000.
>>>
>>> On a quick look on it, I'm noticing that there's no lock at
>>> au0828_usb_probe().
>>>
>>> Also, it uses a separate lock for analog and for digital:
>>>
>>>         mutex_init(&dev->mutex);
>>>         mutex_init(&dev->dvb.lock);
>>>
>>> Probably, the right thing to do would be to use just one lock for both
>>> rising
>>> it at usb_probe, lowering it just before return 0. This will avoid any open
>>> operations while the device is not fully initialized. Btw, newer udev's open
>>> the analog part of the driver just after V4L register, in order to get the
>>> device capabilities. This is known to cause race conditions, if the locking
>>> schema is not working properly.
>>
>> Just to be clear, we're now talking about a completely different race
>> condition that has nothing to do with the subject at hand, and this
>> discussion should probably be moved to a new thread.
>
> If this discussion does change threads, could you folks please copy me
> on it?  I'm already subscribed to several other kernel mailing lists
> in my roles as developer and maintainer of various bits, but I would
> like to avoid having yet another daily deluge added to my inbox.  :)
>
> That said, I can test possible fixes for this stuff,
> and am rather interested to see it resolved.
> ..
>> The notion that this is something that has been there for over a year
>> is something I only learned of in the last couple of days.  All the
>> complaints I had seen thus far were from existing users who were
>> perfectly happy until they upgraded their kernel a couple of months
>> ago and then started seeing the problem.
> ..
>
> It's always exhibited races for me here.  I have long since worked around
> the issue(s), so my own systems currently behave.   But with the newer
> HVR-950Q revision (B4F0), the issue is far more prevalent than before.
>
> I may try Mauro's locking suggestion -- more detail or a patch would be useful.

You may take a look at the lock changes applied on em28xx driver for some examples.
You basically need to block access to DVB while the device is handling a V4L syscall
and vice-versa.

Changing the locking schema is not trivial, as it may generate dead locks. So,
careful testing is required. It also helps to compile a kernel with the dead lock
detection logic enabled, as it may help you to discover if you did something wrong.

Regards,
Mauro

>
> Mauro?


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again
  2011-12-05 23:47                   ` Devin Heitmueller
  2011-12-06 12:51                     ` Mark Lord
@ 2011-12-07  9:59                     ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2011-12-07  9:59 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: Eddi De Pieri, linux-media, Mark Lord

On 05-12-2011 21:47, Devin Heitmueller wrote:
> On Mon, Dec 5, 2011 at 6:32 PM, Eddi De Pieri<eddi@depieri.net>  wrote:
>> Sorry,  I think I applied follow patch on my tree while I developed
>> the driver trying to fix tuner initialization.
>>
>> http://patchwork.linuxtv.org/patch/6617/
>>
>> I forgot to remove from my tree after I see that don't solve anything.
>
> Ok, great.  At least that explains why it's there (since I couldn't
> figure out how on Earth the patch made sense otherwise).
>
> Eddi, could you please submit a patch removing the offending code?

Ok, As Eddi agreed with this change, I'm adding the enclosed patch to
the development tree, removing the bad code.

I'll do a quick test before pushing it upstream.

Regards,
Mauro

-


[media] xc5000: Remove the global mutex lock at xc5000 firmware init

As reported by Devin Heitmueller <dheitmueller@kernellabs.com>:

> It seems like a change such as this could significantly change the
> timing of tuner initialization if you have multiple xc5000 based
> products that might have a slow i2c bus.  Was that intentional?

After discussed with Eddi de Pierri <eddi@depieri.net>, it was pointed that
the change was not intentional, and it was just a trial while developing
the patches that add support for HVR-930C.

So, remove this hack.

Reported-by: Devin Heitmueller <dheitmueller@kernellabs.com>
Acked by: Eddi de Pierri <eddi@depieri.net>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index 048f489..ecd1f95 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -1004,8 +1004,6 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
  	struct xc5000_priv *priv = fe->tuner_priv;
  	int ret = 0;
  
-	mutex_lock(&xc5000_list_mutex);
-
  	if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
  		ret = xc5000_fwupload(fe);
  		if (ret != XC_RESULT_SUCCESS)
@@ -1025,8 +1023,6 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
  	/* Default to "CABLE" mode */
  	ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
  
-	mutex_unlock(&xc5000_list_mutex);
-
  	return ret;
  }
  



>
> Thanks,
>
> Devin
>


^ permalink raw reply related	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2011-12-07  9:59 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-20 14:56 [PATCH 1/8] [media] dvb: Allow select between DVB-C Annex A and Annex C Mauro Carvalho Chehab
2011-11-20 14:56 ` [PATCH 2/8] [media] Properly implement ITU-T J.88 Annex C support Mauro Carvalho Chehab
2011-11-20 14:56   ` [PATCH 3/8] [media] em28xx: Fix some Terratec entries (H5 and XS) Mauro Carvalho Chehab
2011-11-20 14:56     ` [PATCH 4/8] [media] xc5000: Add support for get_if_frequency Mauro Carvalho Chehab
2011-11-20 14:56       ` [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again Mauro Carvalho Chehab
2011-11-20 14:56         ` [PATCH 6/8] [media] em28xx: Fix CodingStyle issues introduced by changeset 82e7dbb Mauro Carvalho Chehab
2011-11-20 14:56           ` [PATCH 7/8] [media] em28xx: Add IR support for em2884 Mauro Carvalho Chehab
2011-11-20 14:56             ` [PATCH 8/8] [media] em28xx: Add IR support for HVR-930C Mauro Carvalho Chehab
2011-12-05 18:23         ` [PATCH 5/8] [media] em28xx: initial support for HAUPPAUGE HVR-930C again Devin Heitmueller
2011-12-05 18:35           ` Mauro Carvalho Chehab
2011-12-05 18:46             ` Devin Heitmueller
2011-12-05 20:01               ` Devin Heitmueller
2011-12-05 23:32                 ` Eddi De Pieri
2011-12-05 23:47                   ` Devin Heitmueller
2011-12-06 12:51                     ` Mark Lord
2011-12-06 13:43                       ` Mauro Carvalho Chehab
2011-12-06 13:56                         ` Devin Heitmueller
2011-12-06 15:28                           ` Mark Lord
2011-12-06 15:35                             ` Devin Heitmueller
2011-12-06 15:44                             ` Mauro Carvalho Chehab
2011-12-07  9:59                     ` Mauro Carvalho Chehab
2011-11-20 20:27 ` [PATCH 1/8] [media] dvb: Allow select between DVB-C Annex A and Annex C Rémi Denis-Courmont

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.