[PATCH 00/24] Various HVR-950q and xc5000 fixes

[PATCH 00/24] Various HVR-950q and xc5000 fixes

[Devin Heitmueller]

This patch series contains fixes for a variety of problems found in the
HVR-950q as well as the xc5000 driver.

Details can be found in the individual patches, but it is worth mentioning
specifically that this addresses the MythTV problem causing BUG() to occur,
firmware loading is now significantly improved, and we now have a
redistributable version for the xc5000c firmware.

Devin Heitmueller (24):
  au8522: fix intermittent lockup of analog video decoder
  au8522: Fix off-by-one in SNR table for QAM256
  au8522: properly recover from the au8522 delivering misaligned TS
    streams
  au0828: Make the s_reg and g_reg advanced debug calls work against
    the bridge
  xc5000: properly show quality register values
  xc5000: add support for showing the SNR and gain in the debug output
  xc5000: properly report i2c write failures
  au0828: fix race condition that causes xc5000 to not bind for digital
  au0828: make sure video standard is setup in tuner-core
  au8522: fix regression in logging introduced by separation of modules
  xc5000: don't invoke auto calibration unless we really did reset
    tuner
  au0828: prevent i2c gate from being kept open while in analog mode
  au0828: fix case where STREAMOFF being called on stopped stream
    causes BUG()
  au0828: speed up i2c clock when doing xc5000 firmware load
  au0828: remove control buffer from send_control_msg
  au0828: tune retry interval for i2c interaction
  au0828: fix possible race condition in usage of dev->ctrlmsg
  xc5000: reset device if encountering PLL lock failure
  xc5000: add support for firmware load check and init status
  au0828: tweak workaround for i2c clock stretching bug
  xc5000: show debug version fields in decimal instead of hex
  au0828: fix a couple of missed edge cases for i2c gate with analog
  au0828: make xc5000 firmware speedup apply to the xc5000c as well
  xc5000: change filename to production/redistributable xc5000c
    firmware

 drivers/media/common/tuners/xc5000.c         |  161 +++++++++++++++++++++-----
 drivers/media/dvb/frontends/au8522_common.c  |   22 +++-
 drivers/media/dvb/frontends/au8522_decoder.c |   11 +-
 drivers/media/dvb/frontends/au8522_dig.c     |   98 ++++++++--------
 drivers/media/dvb/frontends/au8522_priv.h    |   29 ++++-
 drivers/media/video/au0828/au0828-cards.c    |    4 +-
 drivers/media/video/au0828/au0828-core.c     |   59 ++++------
 drivers/media/video/au0828/au0828-dvb.c      |   54 ++++++++-
 drivers/media/video/au0828/au0828-i2c.c      |   21 +++-
 drivers/media/video/au0828/au0828-reg.h      |    1 +
 drivers/media/video/au0828/au0828-video.c    |   76 +++++++++---
 drivers/media/video/au0828/au0828.h          |    2 +
 12 files changed, 379 insertions(+), 159 deletions(-)

[PATCH 01/24] au8522: fix intermittent lockup of analog video decoder

[Devin Heitmueller]

It turns up the autodetection for the video standard in the au8522 is prone
to hanging the chip until a reset is performed.  This condition is trivial
to reproduce simply by tuning to a station and then rapidly unplugging/
replugging the coax feed.

Because we've never claimed to support anything other than NTSC-M, just
disable the video-standard autodetection logic and force it to always be
NTSC-M.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/dvb/frontends/au8522_decoder.c |    6 +++-
 drivers/media/dvb/frontends/au8522_priv.h    |   28 +++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb/frontends/au8522_decoder.c b/drivers/media/dvb/frontends/au8522_decoder.c
index 55b6390..f2e786b 100644
--- a/drivers/media/dvb/frontends/au8522_decoder.c
+++ b/drivers/media/dvb/frontends/au8522_decoder.c
@@ -257,9 +257,11 @@ static void setup_decoder_defaults(struct au8522_state *state, u8 input_mode)
 	au8522_writereg(state, AU8522_TVDED_DBG_MODE_REG060H,
 			AU8522_TVDED_DBG_MODE_REG060H_CVBS);
 	au8522_writereg(state, AU8522_TVDEC_FORMAT_CTRL1_REG061H,
-			AU8522_TVDEC_FORMAT_CTRL1_REG061H_CVBS13);
+			AU8522_TVDEC_FORMAT_CTRL1_REG061H_FIELD_LEN_525 |
+			AU8522_TVDEC_FORMAT_CTRL1_REG061H_LINE_LEN_63_492 |
+			AU8522_TVDEC_FORMAT_CTRL1_REG061H_SUBCARRIER_NTSC_MN);
 	au8522_writereg(state, AU8522_TVDEC_FORMAT_CTRL2_REG062H,
-			AU8522_TVDEC_FORMAT_CTRL2_REG062H_CVBS13);
+			AU8522_TVDEC_FORMAT_CTRL2_REG062H_STD_NTSC);
 	au8522_writereg(state, AU8522_TVDEC_VCR_DET_LLIM_REG063H,
 			AU8522_TVDEC_VCR_DET_LLIM_REG063H_CVBS);
 	au8522_writereg(state, AU8522_TVDEC_VCR_DET_HLIM_REG064H,
diff --git a/drivers/media/dvb/frontends/au8522_priv.h b/drivers/media/dvb/frontends/au8522_priv.h
index 6e4a438..9f44a7b 100644
--- a/drivers/media/dvb/frontends/au8522_priv.h
+++ b/drivers/media/dvb/frontends/au8522_priv.h
@@ -325,6 +325,31 @@ int au8522_led_ctrl(struct au8522_state *state, int led);
 
 /**************************************************************/
 
+/* Format control 1 */
+
+/* VCR Mode 7-6 */
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_VCR_MODE_YES		0x80
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_VCR_MODE_NO		0x40
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_VCR_MODE_AUTO		0x00
+/* Field len 5-4 */
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_FIELD_LEN_625		0x20
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_FIELD_LEN_525		0x10
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_FIELD_LEN_AUTO	0x00
+/* Line len (us) 3-2 */
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_LINE_LEN_64_000	0x0b
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_LINE_LEN_63_492	0x08
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_LINE_LEN_63_556	0x04
+/* Subcarrier freq 1-0 */
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_SUBCARRIER_NTSC_AUTO	0x03
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_SUBCARRIER_NTSC_443	0x02
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_SUBCARRIER_NTSC_MN	0x01
+#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_SUBCARRIER_NTSC_50	0x00
+
+/* Format control 2 */
+#define AU8522_TVDEC_FORMAT_CTRL2_REG062H_STD_AUTODETECT	0x00
+#define AU8522_TVDEC_FORMAT_CTRL2_REG062H_STD_NTSC		0x01
+
+
 #define AU8522_INPUT_CONTROL_REG081H_ATSC               	0xC4
 #define AU8522_INPUT_CONTROL_REG081H_ATVRF			0xC4
 #define AU8522_INPUT_CONTROL_REG081H_ATVRF13			0xC4
@@ -385,9 +410,6 @@ int au8522_led_ctrl(struct au8522_state *state, int led);
 #define AU8522_TVDEC_COMB_MODE_REG015H_CVBS			0x00
 #define AU8522_REG016H_CVBS					0x00
 #define AU8522_TVDED_DBG_MODE_REG060H_CVBS			0x00
-#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_CVBS			0x0B
-#define AU8522_TVDEC_FORMAT_CTRL1_REG061H_CVBS13		0x03
-#define AU8522_TVDEC_FORMAT_CTRL2_REG062H_CVBS13		0x00
 #define AU8522_TVDEC_VCR_DET_LLIM_REG063H_CVBS			0x19
 #define AU8522_REG0F9H_AUDIO					0x20
 #define AU8522_TVDEC_VCR_DET_HLIM_REG064H_CVBS			0xA7
-- 
1.7.1

[PATCH 02/24] au8522: Fix off-by-one in SNR table for QAM256

[Devin Heitmueller]

The table of valid SNR values for QAM 256 is off by one, and as a result if
the SNR is oscillating between 40.0 and 39.9 dB, tools like azap show it
going back and forth between 40.0 and 0 (misleading some people, including
myself, to think signal lock is being lost or there is a problem with register
reads).

Fix the table so that 40.0 dB is properly represented.

Cc: Steven Toth <stoth@kernellabs.com>
Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/dvb/frontends/au8522_dig.c |   96 +++++++++++++++---------------
 1 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/drivers/media/dvb/frontends/au8522_dig.c b/drivers/media/dvb/frontends/au8522_dig.c
index 5fc70d6..ee8cf81 100644
--- a/drivers/media/dvb/frontends/au8522_dig.c
+++ b/drivers/media/dvb/frontends/au8522_dig.c
@@ -157,54 +157,54 @@ static struct mse2snr_tab qam64_mse2snr_tab[] = {
 
 /* QAM256 SNR lookup table */
 static struct mse2snr_tab qam256_mse2snr_tab[] = {
-	{  16,   0 },
-	{  17, 400 },
-	{  18, 398 },
-	{  19, 396 },
-	{  20, 394 },
-	{  21, 392 },
-	{  22, 390 },
-	{  23, 388 },
-	{  24, 386 },
-	{  25, 384 },
-	{  26, 382 },
-	{  27, 380 },
-	{  28, 379 },
-	{  29, 378 },
-	{  30, 377 },
-	{  31, 376 },
-	{  32, 375 },
-	{  33, 374 },
-	{  34, 373 },
-	{  35, 372 },
-	{  36, 371 },
-	{  37, 370 },
-	{  38, 362 },
-	{  39, 354 },
-	{  40, 346 },
-	{  41, 338 },
-	{  42, 330 },
-	{  43, 328 },
-	{  44, 326 },
-	{  45, 324 },
-	{  46, 322 },
-	{  47, 320 },
-	{  48, 319 },
-	{  49, 318 },
-	{  50, 317 },
-	{  51, 316 },
-	{  52, 315 },
-	{  53, 314 },
-	{  54, 313 },
-	{  55, 312 },
-	{  56, 311 },
-	{  57, 310 },
-	{  58, 308 },
-	{  59, 306 },
-	{  60, 304 },
-	{  61, 302 },
-	{  62, 300 },
-	{  63, 298 },
+	{  15,   0 },
+	{  16, 400 },
+	{  17, 398 },
+	{  18, 396 },
+	{  19, 394 },
+	{  20, 392 },
+	{  21, 390 },
+	{  22, 388 },
+	{  23, 386 },
+	{  24, 384 },
+	{  25, 382 },
+	{  26, 380 },
+	{  27, 379 },
+	{  28, 378 },
+	{  29, 377 },
+	{  30, 376 },
+	{  31, 375 },
+	{  32, 374 },
+	{  33, 373 },
+	{  34, 372 },
+	{  35, 371 },
+	{  36, 370 },
+	{  37, 362 },
+	{  38, 354 },
+	{  39, 346 },
+	{  40, 338 },
+	{  41, 330 },
+	{  42, 328 },
+	{  43, 326 },
+	{  44, 324 },
+	{  45, 322 },
+	{  46, 320 },
+	{  47, 319 },
+	{  48, 318 },
+	{  49, 317 },
+	{  50, 316 },
+	{  51, 315 },
+	{  52, 314 },
+	{  53, 313 },
+	{  54, 312 },
+	{  55, 311 },
+	{  56, 310 },
+	{  57, 308 },
+	{  58, 306 },
+	{  59, 304 },
+	{  60, 302 },
+	{  61, 300 },
+	{  62, 298 },
 	{  65, 295 },
 	{  68, 294 },
 	{  70, 293 },
-- 
1.7.1

[PATCH 03/24] au8522: properly recover from the au8522 delivering misaligned TS streams

[Devin Heitmueller]

There is an apparent bug in the au8522 TS clocking which can result in it
delivering a TS payload to the au0828 that is shifted by some number of bits.
For example, the device will announce a packet containing "FA 38 FF F8" which
if you shift left one bit is "1F 47 1F FF F0..."

This presents itself as no TS stream being delivered from the kernel to
userland, since the kernel demux will drop every packet.

In the event that this condition occurs, restart the DVB stream.

Also, this patch includes a couple of lines of cleanup to not change the
FIFO configuration while the FIFO is running (which can screw up the state
machine), and dequeue the buffers before turning off the FIFO.  This puts the
logic in sync with the Windows driver.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-dvb.c |   54 +++++++++++++++++++++++++++---
 drivers/media/video/au0828/au0828.h     |    1 +
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-dvb.c b/drivers/media/video/au0828/au0828-dvb.c
index 39ece8e..b328f65 100644
--- a/drivers/media/video/au0828/au0828-dvb.c
+++ b/drivers/media/video/au0828/au0828-dvb.c
@@ -101,11 +101,14 @@ static struct tda18271_config hauppauge_woodbury_tunerconfig = {
 	.gate    = TDA18271_GATE_DIGITAL,
 };
 
+static void au0828_restart_dvb_streaming(struct work_struct *work);
+
 /*-------------------------------------------------------------------*/
 static void urb_completion(struct urb *purb)
 {
 	struct au0828_dev *dev = purb->context;
 	int ptype = usb_pipetype(purb->pipe);
+	unsigned char *ptr;
 
 	dprintk(2, "%s()\n", __func__);
 
@@ -121,6 +124,16 @@ static void urb_completion(struct urb *purb)
 		return;
 	}
 
+	/* See if the stream is corrupted (to work around a hardware
+	   bug where the stream gets misaligned */
+	ptr = purb->transfer_buffer;
+	if (purb->actual_length > 0 && ptr[0] != 0x47) {
+		dprintk(1, "Need to restart streaming %02x len=%d!\n",
+			ptr[0], purb->actual_length);
+		schedule_work(&dev->restart_streaming);
+		return;
+	}
+
 	/* Feed the transport payload into the kernel demux */
 	dvb_dmx_swfilter_packets(&dev->dvb.demux,
 		purb->transfer_buffer, purb->actual_length / 188);
@@ -138,14 +151,13 @@ static int stop_urb_transfer(struct au0828_dev *dev)
 
 	dprintk(2, "%s()\n", __func__);
 
+	dev->urb_streaming = 0;
 	for (i = 0; i < URB_COUNT; i++) {
 		usb_kill_urb(dev->urbs[i]);
 		kfree(dev->urbs[i]->transfer_buffer);
 		usb_free_urb(dev->urbs[i]);
 	}
 
-	dev->urb_streaming = 0;
-
 	return 0;
 }
 
@@ -246,11 +258,8 @@ static int au0828_dvb_stop_feed(struct dvb_demux_feed *feed)
 		mutex_lock(&dvb->lock);
 		if (--dvb->feeding == 0) {
 			/* Stop transport */
-			au0828_write(dev, 0x608, 0x00);
-			au0828_write(dev, 0x609, 0x00);
-			au0828_write(dev, 0x60a, 0x00);
-			au0828_write(dev, 0x60b, 0x00);
 			ret = stop_urb_transfer(dev);
+			au0828_write(dev, 0x60b, 0x00);
 		}
 		mutex_unlock(&dvb->lock);
 	}
@@ -258,6 +267,37 @@ static int au0828_dvb_stop_feed(struct dvb_demux_feed *feed)
 	return ret;
 }
 
+static void au0828_restart_dvb_streaming(struct work_struct *work)
+{
+	struct au0828_dev *dev = container_of(work, struct au0828_dev,
+					      restart_streaming);
+	struct au0828_dvb *dvb = &dev->dvb;
+	int ret;
+
+	if (dev->urb_streaming == 0)
+		return;
+
+	dprintk(1, "Restarting streaming...!\n");
+
+	mutex_lock(&dvb->lock);
+
+	/* Stop transport */
+	ret = stop_urb_transfer(dev);
+	au0828_write(dev, 0x608, 0x00);
+	au0828_write(dev, 0x609, 0x00);
+	au0828_write(dev, 0x60a, 0x00);
+	au0828_write(dev, 0x60b, 0x00);
+
+	/* Start transport */
+	au0828_write(dev, 0x608, 0x90);
+	au0828_write(dev, 0x609, 0x72);
+	au0828_write(dev, 0x60a, 0x71);
+	au0828_write(dev, 0x60b, 0x01);
+	ret = start_urb_transfer(dev);
+
+	mutex_unlock(&dvb->lock);
+}
+
 static int dvb_register(struct au0828_dev *dev)
 {
 	struct au0828_dvb *dvb = &dev->dvb;
@@ -265,6 +305,8 @@ static int dvb_register(struct au0828_dev *dev)
 
 	dprintk(1, "%s()\n", __func__);
 
+	INIT_WORK(&dev->restart_streaming, au0828_restart_dvb_streaming);
+
 	/* register adapter */
 	result = dvb_register_adapter(&dvb->adapter, DRIVER_NAME, THIS_MODULE,
 				      &dev->usbdev->dev, adapter_nr);
diff --git a/drivers/media/video/au0828/au0828.h b/drivers/media/video/au0828/au0828.h
index 9cde353..61cd63e 100644
--- a/drivers/media/video/au0828/au0828.h
+++ b/drivers/media/video/au0828/au0828.h
@@ -197,6 +197,7 @@ struct au0828_dev {
 
 	/* Digital */
 	struct au0828_dvb		dvb;
+	struct work_struct              restart_streaming;
 
 	/* Analog */
 	struct v4l2_device v4l2_dev;
-- 
1.7.1

[PATCH 04/24] au0828: Make the s_reg and g_reg advanced debug calls work against the bridge

[Devin Heitmueller]

The g_reg and s_reg calls worked properly if acting on subdev registers (such
as the au8522), but didn't work against the au0828 itself.  Copy the logic
over from em28xx.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-video.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-video.c b/drivers/media/video/au0828/au0828-video.c
index ac3dd73..6e30c09 100644
--- a/drivers/media/video/au0828/au0828-video.c
+++ b/drivers/media/video/au0828/au0828-video.c
@@ -1717,8 +1717,12 @@ static int vidioc_g_register(struct file *file, void *priv,
 		v4l2_device_call_all(&dev->v4l2_dev, 0, core, g_register, reg);
 		return 0;
 	default:
-		return -EINVAL;
+		if (!v4l2_chip_match_host(&reg->match))
+			return -EINVAL;
 	}
+
+	reg->val = au0828_read(dev, reg->reg);
+	return 0;
 }
 
 static int vidioc_s_register(struct file *file, void *priv,
@@ -1732,9 +1736,10 @@ static int vidioc_s_register(struct file *file, void *priv,
 		v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_register, reg);
 		return 0;
 	default:
-		return -EINVAL;
+		if (!v4l2_chip_match_host(&reg->match))
+			return -EINVAL;
 	}
-	return 0;
+	return au0828_writereg(dev, reg->reg, reg->val);
 }
 #endif
 
-- 
1.7.1

[PATCH 05/24] xc5000: properly show quality register values

[Devin Heitmueller]

The quality register only has relevant data in bits 2-0, so discard the
other bits (which results in a value being printed that is consistent with
the expected 0-7 range).

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/common/tuners/xc5000.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index dcca42c..c41f2b9 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -684,7 +684,7 @@ static void xc_debug_dump(struct xc5000_priv *priv)
 	dprintk(1, "*** Frame lines = %d\n", frame_lines);
 
 	xc_get_quality(priv,  &quality);
-	dprintk(1, "*** Quality (0:<8dB, 7:>56dB) = %d\n", quality);
+	dprintk(1, "*** Quality (0:<8dB, 7:>56dB) = %d\n", quality & 0x07);
 }
 
 static int xc5000_set_params(struct dvb_frontend *fe)
-- 
1.7.1

[PATCH 06/24] xc5000: add support for showing the SNR and gain in the debug output

[Devin Heitmueller]

When debugging is enabled, also show the analog SNR and the total gain
status values.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/common/tuners/xc5000.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index c41f2b9..f660e33 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -111,6 +111,7 @@ struct xc5000_priv {
 #define XREG_PRODUCT_ID   0x08
 #define XREG_BUSY         0x09
 #define XREG_BUILD        0x0D
+#define XREG_TOTALGAIN    0x0F
 
 /*
    Basic firmware description. This will remain with
@@ -539,6 +540,16 @@ static int xc_get_quality(struct xc5000_priv *priv, u16 *quality)
 	return xc5000_readreg(priv, XREG_QUALITY, quality);
 }
 
+static int xc_get_analogsnr(struct xc5000_priv *priv, u16 *snr)
+{
+	return xc5000_readreg(priv, XREG_SNR, snr);
+}
+
+static int xc_get_totalgain(struct xc5000_priv *priv, u16 *totalgain)
+{
+	return xc5000_readreg(priv, XREG_TOTALGAIN, totalgain);
+}
+
 static u16 WaitForLock(struct xc5000_priv *priv)
 {
 	u16 lockState = 0;
@@ -650,6 +661,8 @@ static void xc_debug_dump(struct xc5000_priv *priv)
 	u32 hsync_freq_hz = 0;
 	u16 frame_lines;
 	u16 quality;
+	u16 snr;
+	u16 totalgain;
 	u8 hw_majorversion = 0, hw_minorversion = 0;
 	u8 fw_majorversion = 0, fw_minorversion = 0;
 	u16 fw_buildversion = 0;
@@ -685,6 +698,13 @@ static void xc_debug_dump(struct xc5000_priv *priv)
 
 	xc_get_quality(priv,  &quality);
 	dprintk(1, "*** Quality (0:<8dB, 7:>56dB) = %d\n", quality & 0x07);
+
+	xc_get_analogsnr(priv,  &snr);
+	dprintk(1, "*** Unweighted analog SNR = %d dB\n", snr & 0x3f);
+
+	xc_get_totalgain(priv,  &totalgain);
+	dprintk(1, "*** Total gain = %d.%d dB\n", totalgain / 256,
+		(totalgain % 256) * 100 / 256);
 }
 
 static int xc5000_set_params(struct dvb_frontend *fe)
-- 
1.7.1

[PATCH 07/24] xc5000: properly report i2c write failures

[Devin Heitmueller]

The logic as written would *never* actually return an error condition, since
the loop would run until the counter hit zero but the check was for a value
less than zero.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/common/tuners/xc5000.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index f660e33..a7fa17e 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -341,7 +341,7 @@ static int xc_write_reg(struct xc5000_priv *priv, u16 regAddr, u16 i2cData)
 			}
 		}
 	}
-	if (WatchDogTimer < 0)
+	if (WatchDogTimer <= 0)
 		result = XC_RESULT_I2C_WRITE_FAILURE;
 
 	return result;
-- 
1.7.1

[PATCH 08/24] au0828: fix race condition that causes xc5000 to not bind for digital

[Devin Heitmueller]

In some cases users would see the xc5000_attach() call failing for the
digital side of the tuner on initialization.  This is because of udev
running v4l-id while the digital side of the board is still coming up.

This is the exact same race condition which was present in em28xx (not
surprising since I copied all the locking logic from that driver when I
added analog support).  Reproduce Mauro's fix from the em28xx driver in
au0828.

Reported-by: Rick Harding <rharding@mitechie.com>
Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-core.c  |    5 +++++
 drivers/media/video/au0828/au0828-video.c |   21 ++++++++-------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-core.c b/drivers/media/video/au0828/au0828-core.c
index 1e4ce50..b2c4254 100644
--- a/drivers/media/video/au0828/au0828-core.c
+++ b/drivers/media/video/au0828/au0828-core.c
@@ -205,6 +205,8 @@ static int au0828_usb_probe(struct usb_interface *interface,
 		return -ENOMEM;
 	}
 
+	mutex_init(&dev->lock);
+	mutex_lock(&dev->lock);
 	mutex_init(&dev->mutex);
 	mutex_init(&dev->dvb.lock);
 	dev->usbdev = usbdev;
@@ -215,6 +217,7 @@ static int au0828_usb_probe(struct usb_interface *interface,
 	if (retval) {
 		printk(KERN_ERR "%s() v4l2_device_register failed\n",
 		       __func__);
+		mutex_unlock(&dev->lock);
 		kfree(dev);
 		return -EIO;
 	}
@@ -245,6 +248,8 @@ static int au0828_usb_probe(struct usb_interface *interface,
 	printk(KERN_INFO "Registered device AU0828 [%s]\n",
 		dev->board.name == NULL ? "Unset" : dev->board.name);
 
+	mutex_unlock(&dev->lock);
+
 	return 0;
 }
 
diff --git a/drivers/media/video/au0828/au0828-video.c b/drivers/media/video/au0828/au0828-video.c
index 6e30c09..b1f8d18 100644
--- a/drivers/media/video/au0828/au0828-video.c
+++ b/drivers/media/video/au0828/au0828-video.c
@@ -864,17 +864,15 @@ static int res_get(struct au0828_fh *fh, unsigned int bit)
 		return 1;
 
 	/* is it free? */
-	mutex_lock(&dev->lock);
 	if (dev->resources & bit) {
 		/* no, someone else uses it */
-		mutex_unlock(&dev->lock);
 		return 0;
 	}
 	/* it's free, grab it */
 	fh->resources  |= bit;
 	dev->resources |= bit;
 	dprintk(1, "res: get %d\n", bit);
-	mutex_unlock(&dev->lock);
+
 	return 1;
 }
 
@@ -894,11 +892,9 @@ static void res_free(struct au0828_fh *fh, unsigned int bits)
 
 	BUG_ON((fh->resources & bits) != bits);
 
-	mutex_lock(&dev->lock);
 	fh->resources  &= ~bits;
 	dev->resources &= ~bits;
 	dprintk(1, "res: put %d\n", bits);
-	mutex_unlock(&dev->lock);
 }
 
 static int get_ressource(struct au0828_fh *fh)
@@ -1023,7 +1019,8 @@ static int au0828_v4l2_open(struct file *filp)
 				    NULL, &dev->slock,
 				    V4L2_BUF_TYPE_VIDEO_CAPTURE,
 				    V4L2_FIELD_INTERLACED,
-				    sizeof(struct au0828_buffer), fh, NULL);
+				    sizeof(struct au0828_buffer), fh,
+				    &dev->lock);
 
 	/* VBI Setup */
 	dev->vbi_width = 720;
@@ -1032,8 +1029,8 @@ static int au0828_v4l2_open(struct file *filp)
 				    NULL, &dev->slock,
 				    V4L2_BUF_TYPE_VBI_CAPTURE,
 				    V4L2_FIELD_SEQ_TB,
-				    sizeof(struct au0828_buffer), fh, NULL);
-
+				    sizeof(struct au0828_buffer), fh,
+				    &dev->lock);
 	return ret;
 }
 
@@ -1312,8 +1309,6 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 	if (rc < 0)
 		return rc;
 
-	mutex_lock(&dev->lock);
-
 	if (videobuf_queue_is_busy(&fh->vb_vidq)) {
 		printk(KERN_INFO "%s queue busy\n", __func__);
 		rc = -EBUSY;
@@ -1322,7 +1317,6 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 
 	rc = au0828_set_format(dev, VIDIOC_S_FMT, f);
 out:
-	mutex_unlock(&dev->lock);
 	return rc;
 }
 
@@ -1832,7 +1826,7 @@ static struct v4l2_file_operations au0828_v4l_fops = {
 	.read       = au0828_v4l2_read,
 	.poll       = au0828_v4l2_poll,
 	.mmap       = au0828_v4l2_mmap,
-	.ioctl      = video_ioctl2,
+	.unlocked_ioctl = video_ioctl2,
 };
 
 static const struct v4l2_ioctl_ops video_ioctl_ops = {
@@ -1922,7 +1916,6 @@ int au0828_analog_register(struct au0828_dev *dev,
 
 	init_waitqueue_head(&dev->open);
 	spin_lock_init(&dev->slock);
-	mutex_init(&dev->lock);
 
 	/* init video dma queues */
 	INIT_LIST_HEAD(&dev->vidq.active);
@@ -1963,11 +1956,13 @@ int au0828_analog_register(struct au0828_dev *dev,
 	/* Fill the video capture device struct */
 	*dev->vdev = au0828_video_template;
 	dev->vdev->parent = &dev->usbdev->dev;
+	dev->vdev->lock = &dev->lock;
 	strcpy(dev->vdev->name, "au0828a video");
 
 	/* Setup the VBI device */
 	*dev->vbi_dev = au0828_video_template;
 	dev->vbi_dev->parent = &dev->usbdev->dev;
+	dev->vbi_dev->lock = &dev->lock;
 	strcpy(dev->vbi_dev->name, "au0828a vbi");
 
 	/* Register the v4l2 device */
-- 
1.7.1

[PATCH 09/24] au0828: make sure video standard is setup in tuner-core

[Devin Heitmueller]

If the user performs a tuning attempt without explicitly calling the s_std
ioctl(), a value of zero is sent from tuner-core to xc5000.  This causes
the xc5000 driver to leave the standard unchanged.  The problem was masked
by the fact that the xc5000 driver defaulted to NTSC, but if you happened to
perform an ATSC/ClearQAM tuning attempt and then do an analog tune, the net
effect is an analog tune with the standard still set to DTV6.

Keep track of whether the standard has ever been sent to tuner-core.  We
don't make an s_std subdev call explicitly during probe because that will
cause a firmware load (which is very time consuming on the 950q).  With the
logic in this patch, the s_std call will occur automatically on the s_freq
call if it hasn't already been set.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-video.c |   10 ++++++++++
 drivers/media/video/au0828/au0828.h       |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-video.c b/drivers/media/video/au0828/au0828-video.c
index b1f8d18..f3e6e3f 100644
--- a/drivers/media/video/au0828/au0828-video.c
+++ b/drivers/media/video/au0828/au0828-video.c
@@ -1330,6 +1330,7 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id * norm)
 	   buffer, which is currently hardcoded at 720x480 */
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, *norm);
+	dev->std_set_in_tuner_core = 1;
 	return 0;
 }
 
@@ -1540,6 +1541,15 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 
 	dev->ctrl_freq = freq->frequency;
 
+	if (dev->std_set_in_tuner_core == 0) {
+	  /* If we've never sent the standard in tuner core, do so now.  We
+	     don't do this at device probe because we don't want to incur
+	     the cost of a firmware load */
+	  v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std,
+			       dev->vdev->tvnorms);
+	  dev->std_set_in_tuner_core = 1;
+	}
+
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, freq);
 
 	au0828_analog_stream_reset(dev);
diff --git a/drivers/media/video/au0828/au0828.h b/drivers/media/video/au0828/au0828.h
index 61cd63e..66a56ef 100644
--- a/drivers/media/video/au0828/au0828.h
+++ b/drivers/media/video/au0828/au0828.h
@@ -225,6 +225,7 @@ struct au0828_dev {
 	unsigned int frame_count;
 	int ctrl_freq;
 	int input_type;
+	int std_set_in_tuner_core;
 	unsigned int ctrl_input;
 	enum au0828_dev_state dev_state;
 	enum au0828_stream_state stream_state;
-- 
1.7.1

[PATCH 10/24] au8522: fix regression in logging introduced by separation of modules

[Devin Heitmueller]

The au8522 driver was broken into three modules (dig, decoder, common), and
as a result the debug modprobe option doesn't work for any of the common
functions.

Copy the module macros over to the common module so that the debug option
works again.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/dvb/frontends/au8522_common.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb/frontends/au8522_common.c b/drivers/media/dvb/frontends/au8522_common.c
index 5cfe151..8b4da40 100644
--- a/drivers/media/dvb/frontends/au8522_common.c
+++ b/drivers/media/dvb/frontends/au8522_common.c
@@ -26,8 +26,6 @@
 #include "dvb_frontend.h"
 #include "au8522_priv.h"
 
-MODULE_LICENSE("GPL");
-
 static int debug;
 
 #define dprintk(arg...)\
@@ -257,3 +255,10 @@ int au8522_sleep(struct dvb_frontend *fe)
 	return 0;
 }
 EXPORT_SYMBOL(au8522_sleep);
+
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "Enable verbose debug messages");
+
+MODULE_DESCRIPTION("Auvitek AU8522 QAM-B/ATSC Demodulator driver");
+MODULE_AUTHOR("Steven Toth");
+MODULE_LICENSE("GPL");
-- 
1.7.1

[PATCH 11/24] xc5000: don't invoke auto calibration unless we really did reset tuner

[Devin Heitmueller]

The current code invokes the auto calibration of the tuner whenever the
init routine is called (whenever the DVB frontend opens the device).
However we should really only be invoking the calibration if we actually
did reset the device and reload the firmware.

Rework the routine to only do calibration if reset and firmware load was
performed.  Also because the called function is now a no-op if the firmware
is already loaded, the caller no longer needs to invoke is_firmware_loaded().

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/common/tuners/xc5000.c |   40 +++++++++++++++------------------
 1 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index a7fa17e..1cfaa7c 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -715,11 +715,9 @@ static int xc5000_set_params(struct dvb_frontend *fe)
 	u32 freq = fe->dtv_property_cache.frequency;
 	u32 delsys  = fe->dtv_property_cache.delivery_system;
 
-	if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
-		if (xc_load_fw_and_init_tuner(fe) != XC_RESULT_SUCCESS) {
-			dprintk(1, "Unable to load firmware and init tuner\n");
-			return -EINVAL;
-		}
+	if (xc_load_fw_and_init_tuner(fe) != XC_RESULT_SUCCESS) {
+		dprintk(1, "Unable to load firmware and init tuner\n");
+		return -EINVAL;
 	}
 
 	dprintk(1, "%s() frequency=%d (Hz)\n", __func__, freq);
@@ -994,11 +992,9 @@ static int xc5000_set_analog_params(struct dvb_frontend *fe,
 	if (priv->i2c_props.adap == NULL)
 		return -EINVAL;
 
-	if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
-		if (xc_load_fw_and_init_tuner(fe) != XC_RESULT_SUCCESS) {
-			dprintk(1, "Unable to load firmware and init tuner\n");
-			return -EINVAL;
-		}
+	if (xc_load_fw_and_init_tuner(fe) != XC_RESULT_SUCCESS) {
+		dprintk(1, "Unable to load firmware and init tuner\n");
+		return -EINVAL;
 	}
 
 	switch (params->mode) {
@@ -1057,26 +1053,26 @@ static int xc5000_get_status(struct dvb_frontend *fe, u32 *status)
 static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
 {
 	struct xc5000_priv *priv = fe->tuner_priv;
-	int ret = 0;
+	int ret = XC_RESULT_SUCCESS;
 
 	if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
 		ret = xc5000_fwupload(fe);
 		if (ret != XC_RESULT_SUCCESS)
 			return ret;
-	}
 
-	/* Start the tuner self-calibration process */
-	ret |= xc_initialize(priv);
+		/* Start the tuner self-calibration process */
+		ret |= xc_initialize(priv);
 
-	/* Wait for calibration to complete.
-	 * We could continue but XC5000 will clock stretch subsequent
-	 * I2C transactions until calibration is complete.  This way we
-	 * don't have to rely on clock stretching working.
-	 */
-	xc_wait(100);
+		/* Wait for calibration to complete.
+		 * We could continue but XC5000 will clock stretch subsequent
+		 * I2C transactions until calibration is complete.  This way we
+		 * don't have to rely on clock stretching working.
+		 */
+		xc_wait(100);
 
-	/* Default to "CABLE" mode */
-	ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
+		/* Default to "CABLE" mode */
+		ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
+	}
 
 	return ret;
 }
-- 
1.7.1

[PATCH 12/24] au0828: prevent i2c gate from being kept open while in analog mode

[Devin Heitmueller]

The original implementation of the analog support would use an i2c_gate_ctrl
function when using the digital side of the au8522, but on the analog side
we would always just force the gate open and leave it open all the time.

This can have adverse effects on the xc5000 given the tuner is receiving all
the spurious i2c traffic (a problem which can be exaggerated due to bugs in
the au0828 i2c hardware implementation).

Rework the existing hack to only open/close the gate when actually talking
to the tuner.

This logic might need to be reworked a bit if anybody ever tries to add
support for a board that has the au0828/au8522 but doesn't have digital
support implemented (because the i2c_gate_ctrl callback is being set in
the DVB attach).  However given how few different models are in circulation,
this can be deferred until such a situation arises (if ever).

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/dvb/frontends/au8522_common.c  |   13 +++++++++++++
 drivers/media/dvb/frontends/au8522_decoder.c |    5 -----
 drivers/media/dvb/frontends/au8522_dig.c     |    2 ++
 drivers/media/dvb/frontends/au8522_priv.h    |    1 +
 drivers/media/video/au0828/au0828-video.c    |    6 ++++++
 5 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb/frontends/au8522_common.c b/drivers/media/dvb/frontends/au8522_common.c
index 8b4da40..3559ff2 100644
--- a/drivers/media/dvb/frontends/au8522_common.c
+++ b/drivers/media/dvb/frontends/au8522_common.c
@@ -99,6 +99,19 @@ int au8522_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
 }
 EXPORT_SYMBOL(au8522_i2c_gate_ctrl);
 
+int au8522_analog_i2c_gate_ctrl(struct dvb_frontend *fe, int enable)
+{
+	struct au8522_state *state = fe->demodulator_priv;
+
+	dprintk("%s(%d)\n", __func__, enable);
+
+	if (enable)
+		return au8522_writereg(state, 0x106, 1);
+	else
+		return au8522_writereg(state, 0x106, 0);
+}
+EXPORT_SYMBOL(au8522_analog_i2c_gate_ctrl);
+
 /* Reset the demod hardware and reset all of the configuration registers
    to a default state. */
 int au8522_get_state(struct au8522_state **state, struct i2c_adapter *i2c,
diff --git a/drivers/media/dvb/frontends/au8522_decoder.c b/drivers/media/dvb/frontends/au8522_decoder.c
index f2e786b..5243ba6 100644
--- a/drivers/media/dvb/frontends/au8522_decoder.c
+++ b/drivers/media/dvb/frontends/au8522_decoder.c
@@ -659,11 +659,6 @@ static int au8522_s_video_routing(struct v4l2_subdev *sd,
 
 	au8522_reset(sd, 0);
 
-	/* Jam open the i2c gate to the tuner.  We do this here to handle the
-	   case where the user went into digital mode (causing the gate to be
-	   closed), and then came back to analog mode */
-	au8522_writereg(state, 0x106, 1);
-
 	if (input == AU8522_COMPOSITE_CH1) {
 		au8522_setup_cvbs_mode(state);
 	} else if (input == AU8522_SVIDEO_CH13) {
diff --git a/drivers/media/dvb/frontends/au8522_dig.c b/drivers/media/dvb/frontends/au8522_dig.c
index ee8cf81..a68974f 100644
--- a/drivers/media/dvb/frontends/au8522_dig.c
+++ b/drivers/media/dvb/frontends/au8522_dig.c
@@ -777,6 +777,8 @@ struct dvb_frontend *au8522_attach(const struct au8522_config *config,
 	       sizeof(struct dvb_frontend_ops));
 	state->frontend.demodulator_priv = state;
 
+	state->frontend.ops.analog_ops.i2c_gate_ctrl = au8522_analog_i2c_gate_ctrl;
+
 	if (au8522_init(&state->frontend) != 0) {
 		printk(KERN_ERR "%s: Failed to initialize correctly\n",
 			__func__);
diff --git a/drivers/media/dvb/frontends/au8522_priv.h b/drivers/media/dvb/frontends/au8522_priv.h
index 9f44a7b..0529699 100644
--- a/drivers/media/dvb/frontends/au8522_priv.h
+++ b/drivers/media/dvb/frontends/au8522_priv.h
@@ -82,6 +82,7 @@ int au8522_get_state(struct au8522_state **state, struct i2c_adapter *i2c,
 		     u8 client_address);
 void au8522_release_state(struct au8522_state *state);
 int au8522_i2c_gate_ctrl(struct dvb_frontend *fe, int enable);
+int au8522_analog_i2c_gate_ctrl(struct dvb_frontend *fe, int enable);
 int au8522_led_ctrl(struct au8522_state *state, int led);
 
 /* REGISTERS */
diff --git a/drivers/media/video/au0828/au0828-video.c b/drivers/media/video/au0828/au0828-video.c
index f3e6e3f..df92322 100644
--- a/drivers/media/video/au0828/au0828-video.c
+++ b/drivers/media/video/au0828/au0828-video.c
@@ -1541,6 +1541,9 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 
 	dev->ctrl_freq = freq->frequency;
 
+	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
+		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 1);
+
 	if (dev->std_set_in_tuner_core == 0) {
 	  /* If we've never sent the standard in tuner core, do so now.  We
 	     don't do this at device probe because we don't want to incur
@@ -1552,6 +1555,9 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, freq);
 
+	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
+		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 0);
+
 	au0828_analog_stream_reset(dev);
 
 	return 0;
-- 
1.7.1

[PATCH 13/24] au0828: fix case where STREAMOFF being called on stopped stream causes BUG()

[Devin Heitmueller]

We weren't checking whether the resource was in use before calling res_free(),
so applications which called STREAMOFF on a v4l2 device that wasn't already
streaming would cause a BUG() to be hit (MythTV).

Reported-by: Larry Finger <larry.finger@lwfinger.net>
Reported-by: Jay Harbeston <jharbestonus@gmail.com>
Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-video.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-video.c b/drivers/media/video/au0828/au0828-video.c
index df92322..4d5b670 100644
--- a/drivers/media/video/au0828/au0828-video.c
+++ b/drivers/media/video/au0828/au0828-video.c
@@ -1702,14 +1702,18 @@ static int vidioc_streamoff(struct file *file, void *priv,
 			(AUVI_INPUT(i).audio_setup)(dev, 0);
 		}
 
-		videobuf_streamoff(&fh->vb_vidq);
-		res_free(fh, AU0828_RESOURCE_VIDEO);
+		if (res_check(fh, AU0828_RESOURCE_VIDEO)) {
+			videobuf_streamoff(&fh->vb_vidq);
+			res_free(fh, AU0828_RESOURCE_VIDEO);
+		}
 	} else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
 		dev->vbi_timeout_running = 0;
 		del_timer_sync(&dev->vbi_timeout);
 
-		videobuf_streamoff(&fh->vb_vbiq);
-		res_free(fh, AU0828_RESOURCE_VBI);
+		if (res_check(fh, AU0828_RESOURCE_VBI)) {
+			videobuf_streamoff(&fh->vb_vbiq);
+			res_free(fh, AU0828_RESOURCE_VBI);
+		}
 	}
 
 	return 0;
-- 
1.7.1

[PATCH 14/24] au0828: speed up i2c clock when doing xc5000 firmware load

[Devin Heitmueller]

Put a hack in place to speed up the firmware load in the case that the
xc5000 has just been reset.  The chip can safely do 400 KHz in this mode,
while in normal operation it can only do 100 KHz.

This reduces the firmware load time from 6.9 seconds to 4.2.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-i2c.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-i2c.c b/drivers/media/video/au0828/au0828-i2c.c
index 05c299f..d454555 100644
--- a/drivers/media/video/au0828/au0828-i2c.c
+++ b/drivers/media/video/au0828/au0828-i2c.c
@@ -26,7 +26,7 @@
 #include <linux/io.h>
 
 #include "au0828.h"
-
+#include "media/tuner.h"
 #include <media/v4l2-common.h>
 
 static int i2c_scan;
@@ -147,8 +147,18 @@ static int i2c_sendbytes(struct i2c_adapter *i2c_adap,
 	au0828_write(dev, AU0828_I2C_MULTIBYTE_MODE_2FF, 0x01);
 
 	/* Set the I2C clock */
-	au0828_write(dev, AU0828_I2C_CLK_DIVIDER_202,
-		     dev->board.i2c_clk_divider);
+	if ((dev->board.tuner_type == TUNER_XC5000) &&
+	    (dev->board.tuner_addr == msg->addr) &&
+	    (msg->len == 64)) {
+		/* Hack to speed up firmware load.  The xc5000 lets us do up
+		   to 400 KHz when in firmware download mode */
+		au0828_write(dev, AU0828_I2C_CLK_DIVIDER_202,
+			     AU0828_I2C_CLK_250KHZ);
+	} else {
+		/* Use the i2c clock speed in the board configuration */
+		au0828_write(dev, AU0828_I2C_CLK_DIVIDER_202,
+			     dev->board.i2c_clk_divider);
+	}
 
 	/* Hardware needs 8 bit addresses */
 	au0828_write(dev, AU0828_I2C_DEST_ADDR_203, msg->addr << 1);
-- 
1.7.1

[PATCH 15/24] au0828: remove control buffer from send_control_msg

[Devin Heitmueller]

There are no cases where a control message is ever sent to the au0828 with
an actual buffer defined.  Remove the reference to dev->ctrlmsg, which
currently requires us to hold a mutex since it is shared with the read
function.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-core.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-core.c b/drivers/media/video/au0828/au0828-core.c
index b2c4254..65914bc 100644
--- a/drivers/media/video/au0828/au0828-core.c
+++ b/drivers/media/video/au0828/au0828-core.c
@@ -46,7 +46,7 @@ MODULE_PARM_DESC(disable_usb_speed_check,
 #define _BULKPIPESIZE 0xffff
 
 static int send_control_msg(struct au0828_dev *dev, u16 request, u32 value,
-	u16 index, unsigned char *cp, u16 size);
+			    u16 index);
 static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
 	u16 index, unsigned char *cp, u16 size);
 
@@ -64,8 +64,7 @@ u32 au0828_readreg(struct au0828_dev *dev, u16 reg)
 u32 au0828_writereg(struct au0828_dev *dev, u16 reg, u32 val)
 {
 	dprintk(8, "%s(0x%04x, 0x%02x)\n", __func__, reg, val);
-	return send_control_msg(dev, CMD_REQUEST_OUT, val, reg,
-				dev->ctrlmsg, 0);
+	return send_control_msg(dev, CMD_REQUEST_OUT, val, reg);
 }
 
 static void cmd_msg_dump(struct au0828_dev *dev)
@@ -87,10 +86,10 @@ static void cmd_msg_dump(struct au0828_dev *dev)
 }
 
 static int send_control_msg(struct au0828_dev *dev, u16 request, u32 value,
-	u16 index, unsigned char *cp, u16 size)
+	u16 index)
 {
 	int status = -ENODEV;
-	mutex_lock(&dev->mutex);
+
 	if (dev->usbdev) {
 
 		/* cp must be memory that has been allocated by kmalloc */
@@ -99,8 +98,7 @@ static int send_control_msg(struct au0828_dev *dev, u16 request, u32 value,
 				request,
 				USB_DIR_OUT | USB_TYPE_VENDOR |
 					USB_RECIP_DEVICE,
-				value, index,
-				cp, size, 1000);
+				value, index, NULL, 0, 1000);
 
 		status = min(status, 0);
 
@@ -110,7 +108,7 @@ static int send_control_msg(struct au0828_dev *dev, u16 request, u32 value,
 		}
 
 	}
-	mutex_unlock(&dev->mutex);
+
 	return status;
 }
 
-- 
1.7.1

[PATCH 16/24] au0828: tune retry interval for i2c interaction

[Devin Heitmueller]

Adjust the retry timeout and number of retries to speed up xc5000 firmware
download.  With this change it goes from 4.2 seconds to 2.9.  The net time
waited is pretty much the same, but we just poll more often.

Tested at 250 KHz as well as 30 KHz.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-i2c.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-i2c.c b/drivers/media/video/au0828/au0828-i2c.c
index d454555..3bc76df 100644
--- a/drivers/media/video/au0828/au0828-i2c.c
+++ b/drivers/media/video/au0828/au0828-i2c.c
@@ -33,8 +33,8 @@ static int i2c_scan;
 module_param(i2c_scan, int, 0444);
 MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
 
-#define I2C_WAIT_DELAY 512
-#define I2C_WAIT_RETRY 64
+#define I2C_WAIT_DELAY 25
+#define I2C_WAIT_RETRY 1000
 
 static inline int i2c_slave_did_write_ack(struct i2c_adapter *i2c_adap)
 {
-- 
1.7.1

[PATCH 17/24] au0828: fix possible race condition in usage of dev->ctrlmsg

[Devin Heitmueller]

The register read function is referencing the dev->ctrlmsg structure outside
of the dev->mutex lock, which can cause corruption of the value if multiple
callers are invoking au0828_readreg() simultaneously.

Use a stack variable to hold the result, and copy the buffer returned by
usb_control_msg() to that variable.

In reality, the whole recv_control_msg() function can probably be collapsed
into au0288_readreg() since it is the only caller.

Also get rid of cmd_msg_dump() since the only case in which the function is
ever called only is ever passed a single byte for the response (and it is
already logged).

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-core.c |   40 +++++++++---------------------
 1 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-core.c b/drivers/media/video/au0828/au0828-core.c
index 65914bc..745a80a 100644
--- a/drivers/media/video/au0828/au0828-core.c
+++ b/drivers/media/video/au0828/au0828-core.c
@@ -56,9 +56,12 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
 
 u32 au0828_readreg(struct au0828_dev *dev, u16 reg)
 {
-	recv_control_msg(dev, CMD_REQUEST_IN, 0, reg, dev->ctrlmsg, 1);
-	dprintk(8, "%s(0x%04x) = 0x%02x\n", __func__, reg, dev->ctrlmsg[0]);
-	return dev->ctrlmsg[0];
+	u8 result = 0;
+
+	recv_control_msg(dev, CMD_REQUEST_IN, 0, reg, &result, 1);
+	dprintk(8, "%s(0x%04x) = 0x%02x\n", __func__, reg, result);
+
+	return result;
 }
 
 u32 au0828_writereg(struct au0828_dev *dev, u16 reg, u32 val)
@@ -67,24 +70,6 @@ u32 au0828_writereg(struct au0828_dev *dev, u16 reg, u32 val)
 	return send_control_msg(dev, CMD_REQUEST_OUT, val, reg);
 }
 
-static void cmd_msg_dump(struct au0828_dev *dev)
-{
-	int i;
-
-	for (i = 0; i < sizeof(dev->ctrlmsg); i += 16)
-		dprintk(2, "%s() %02x %02x %02x %02x %02x %02x %02x %02x "
-				"%02x %02x %02x %02x %02x %02x %02x %02x\n",
-			__func__,
-			dev->ctrlmsg[i+0], dev->ctrlmsg[i+1],
-			dev->ctrlmsg[i+2], dev->ctrlmsg[i+3],
-			dev->ctrlmsg[i+4], dev->ctrlmsg[i+5],
-			dev->ctrlmsg[i+6], dev->ctrlmsg[i+7],
-			dev->ctrlmsg[i+8], dev->ctrlmsg[i+9],
-			dev->ctrlmsg[i+10], dev->ctrlmsg[i+11],
-			dev->ctrlmsg[i+12], dev->ctrlmsg[i+13],
-			dev->ctrlmsg[i+14], dev->ctrlmsg[i+15]);
-}
-
 static int send_control_msg(struct au0828_dev *dev, u16 request, u32 value,
 	u16 index)
 {
@@ -118,24 +103,23 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
 	int status = -ENODEV;
 	mutex_lock(&dev->mutex);
 	if (dev->usbdev) {
-
-		memset(dev->ctrlmsg, 0, sizeof(dev->ctrlmsg));
-
-		/* cp must be memory that has been allocated by kmalloc */
 		status = usb_control_msg(dev->usbdev,
 				usb_rcvctrlpipe(dev->usbdev, 0),
 				request,
 				USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
 				value, index,
-				cp, size, 1000);
+				dev->ctrlmsg, size, 1000);
 
 		status = min(status, 0);
 
 		if (status < 0) {
 			printk(KERN_ERR "%s() Failed receiving control message, error %d.\n",
 				__func__, status);
-		} else
-			cmd_msg_dump(dev);
+		}
+
+		/* the host controller requires heap allocated memory, which
+		   is why we didn't just pass "cp" into usb_control_msg */
+		memcpy(cp, dev->ctrlmsg, size);
 	}
 	mutex_unlock(&dev->mutex);
 	return status;
-- 
1.7.1

[PATCH 18/24] xc5000: reset device if encountering PLL lock failure

[Devin Heitmueller]

It's possible for the xc5000 to enter an unknown state such that all
subsequent tuning requests fail.  The only way to recover is to reset the
tuner and reload the firmware.  This problem was detected after several days
straight of issuing tuning requests every five seconds.

Reset the firmware in the event that the PLL is in an unlocked state.  This
solution was provided by the engineer at CrestaTech (the company that acquired
Xceive).

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/common/tuners/xc5000.c |   58 ++++++++++++++++++++++++++++++---
 1 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index 1cfaa7c..7c36465 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -62,6 +62,7 @@ struct xc5000_priv {
 	u8  radio_input;
 
 	int chip_id;
+	u16 pll_register_no;
 };
 
 /* Misc Defines */
@@ -209,16 +210,19 @@ static struct XC_TV_STANDARD XC5000_Standard[MAX_TV_STANDARD] = {
 struct xc5000_fw_cfg {
 	char *name;
 	u16 size;
+	u16 pll_reg;
 };
 
 static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
 	.name = "dvb-fe-xc5000-1.6.114.fw",
 	.size = 12401,
+	.pll_reg = 0x806c,
 };
 
 static const struct xc5000_fw_cfg xc5000c_41_024_5 = {
 	.name = "dvb-fe-xc5000c-41.024.5.fw",
 	.size = 16497,
+	.pll_reg = 0x13,
 };
 
 static inline const struct xc5000_fw_cfg *xc5000_assign_firmware(int chip_id)
@@ -232,7 +236,7 @@ static inline const struct xc5000_fw_cfg *xc5000_assign_firmware(int chip_id)
 	}
 }
 
-static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe);
+static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force);
 static int xc5000_is_firmware_loaded(struct dvb_frontend *fe);
 static int xc5000_readreg(struct xc5000_priv *priv, u16 reg, u16 *val);
 static int xc5000_TunerReset(struct dvb_frontend *fe);
@@ -617,6 +621,7 @@ static int xc5000_fwupload(struct dvb_frontend *fe)
 	int ret;
 	const struct xc5000_fw_cfg *desired_fw =
 		xc5000_assign_firmware(priv->chip_id);
+	priv->pll_register_no = desired_fw->pll_reg;
 
 	/* request the firmware, this will block and timeout */
 	printk(KERN_INFO "xc5000: waiting for firmware upload (%s)...\n",
@@ -666,6 +671,7 @@ static void xc_debug_dump(struct xc5000_priv *priv)
 	u8 hw_majorversion = 0, hw_minorversion = 0;
 	u8 fw_majorversion = 0, fw_minorversion = 0;
 	u16 fw_buildversion = 0;
+	u16 regval;
 
 	/* Wait for stats to stabilize.
 	 * Frame Lines needs two frame times after initial lock
@@ -705,6 +711,11 @@ static void xc_debug_dump(struct xc5000_priv *priv)
 	xc_get_totalgain(priv,  &totalgain);
 	dprintk(1, "*** Total gain = %d.%d dB\n", totalgain / 256,
 		(totalgain % 256) * 100 / 256);
+
+	if (priv->pll_register_no) {
+		xc5000_readreg(priv, priv->pll_register_no, &regval);
+		dprintk(1, "*** PLL lock status = 0x%04x\n", regval);
+	}
 }
 
 static int xc5000_set_params(struct dvb_frontend *fe)
@@ -715,7 +726,7 @@ static int xc5000_set_params(struct dvb_frontend *fe)
 	u32 freq = fe->dtv_property_cache.frequency;
 	u32 delsys  = fe->dtv_property_cache.delivery_system;
 
-	if (xc_load_fw_and_init_tuner(fe) != XC_RESULT_SUCCESS) {
+	if (xc_load_fw_and_init_tuner(fe, 0) != XC_RESULT_SUCCESS) {
 		dprintk(1, "Unable to load firmware and init tuner\n");
 		return -EINVAL;
 	}
@@ -842,6 +853,7 @@ static int xc5000_set_tv_freq(struct dvb_frontend *fe,
 	struct analog_parameters *params)
 {
 	struct xc5000_priv *priv = fe->tuner_priv;
+	u16 pll_lock_status;
 	int ret;
 
 	dprintk(1, "%s() frequency=%d (in units of 62.5khz)\n",
@@ -922,6 +934,21 @@ tune_channel:
 	if (debug)
 		xc_debug_dump(priv);
 
+	if (priv->pll_register_no != 0) {
+		msleep(20);
+		xc5000_readreg(priv, priv->pll_register_no, &pll_lock_status);
+		if (pll_lock_status > 63) {
+			/* PLL is unlocked, force reload of the firmware */
+			dprintk(1, "xc5000: PLL not locked (0x%x).  Reloading...\n",
+				pll_lock_status);
+			if (xc_load_fw_and_init_tuner(fe, 1) != XC_RESULT_SUCCESS) {
+				printk(KERN_ERR "xc5000: Unable to reload fw\n");
+				return -EREMOTEIO;
+			}
+			goto tune_channel;
+		}
+	}
+
 	return 0;
 }
 
@@ -992,7 +1019,7 @@ static int xc5000_set_analog_params(struct dvb_frontend *fe,
 	if (priv->i2c_props.adap == NULL)
 		return -EINVAL;
 
-	if (xc_load_fw_and_init_tuner(fe) != XC_RESULT_SUCCESS) {
+	if (xc_load_fw_and_init_tuner(fe, 0) != XC_RESULT_SUCCESS) {
 		dprintk(1, "Unable to load firmware and init tuner\n");
 		return -EINVAL;
 	}
@@ -1050,19 +1077,28 @@ static int xc5000_get_status(struct dvb_frontend *fe, u32 *status)
 	return 0;
 }
 
-static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
+static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force)
 {
 	struct xc5000_priv *priv = fe->tuner_priv;
 	int ret = XC_RESULT_SUCCESS;
+	u16 pll_lock_status;
+
+	if (force || xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
+
+fw_retry:
 
-	if (xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
 		ret = xc5000_fwupload(fe);
 		if (ret != XC_RESULT_SUCCESS)
 			return ret;
 
+		msleep(20);
+
 		/* Start the tuner self-calibration process */
 		ret |= xc_initialize(priv);
 
+		if (ret != XC_RESULT_SUCCESS)
+			goto fw_retry;
+
 		/* Wait for calibration to complete.
 		 * We could continue but XC5000 will clock stretch subsequent
 		 * I2C transactions until calibration is complete.  This way we
@@ -1070,6 +1106,16 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe)
 		 */
 		xc_wait(100);
 
+		if (priv->pll_register_no) {
+			xc5000_readreg(priv, priv->pll_register_no,
+				       &pll_lock_status);
+			if (pll_lock_status > 63) {
+				/* PLL is unlocked, force reload of the firmware */
+				printk(KERN_ERR "xc5000: PLL not running after fwload.\n");
+				goto fw_retry;
+			}
+		}
+
 		/* Default to "CABLE" mode */
 		ret |= xc_write_reg(priv, XREG_SIGNALSOURCE, XC_RF_MODE_CABLE);
 	}
@@ -1105,7 +1151,7 @@ static int xc5000_init(struct dvb_frontend *fe)
 	struct xc5000_priv *priv = fe->tuner_priv;
 	dprintk(1, "%s()\n", __func__);
 
-	if (xc_load_fw_and_init_tuner(fe) != XC_RESULT_SUCCESS) {
+	if (xc_load_fw_and_init_tuner(fe, 0) != XC_RESULT_SUCCESS) {
 		printk(KERN_ERR "xc5000: Unable to initialise tuner\n");
 		return -EREMOTEIO;
 	}
-- 
1.7.1

[PATCH 19/24] xc5000: add support for firmware load check and init status

[Devin Heitmueller]

The xc5000c and newer versions of the xc5000a firmware need minor revisions
to their initialization process.  Add support for validating the firmware
was properly loaded, as well as checking the init status after initialization.

Based on advice from CrestaTech support as well as xc5000 datasheet v2.3.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/common/tuners/xc5000.c |   39 ++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index 7c36465..3e5f8cd 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -63,6 +63,8 @@ struct xc5000_priv {
 
 	int chip_id;
 	u16 pll_register_no;
+	u8 init_status_supported;
+	u8 fw_checksum_supported;
 };
 
 /* Misc Defines */
@@ -113,6 +115,8 @@ struct xc5000_priv {
 #define XREG_BUSY         0x09
 #define XREG_BUILD        0x0D
 #define XREG_TOTALGAIN    0x0F
+#define XREG_FW_CHECKSUM  0x12
+#define XREG_INIT_STATUS  0x13
 
 /*
    Basic firmware description. This will remain with
@@ -211,6 +215,8 @@ struct xc5000_fw_cfg {
 	char *name;
 	u16 size;
 	u16 pll_reg;
+	u8 init_status_supported;
+	u8 fw_checksum_supported;
 };
 
 static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
@@ -223,6 +229,8 @@ static const struct xc5000_fw_cfg xc5000c_41_024_5 = {
 	.name = "dvb-fe-xc5000c-41.024.5.fw",
 	.size = 16497,
 	.pll_reg = 0x13,
+	.init_status_supported = 1,
+	.fw_checksum_supported = 1,
 };
 
 static inline const struct xc5000_fw_cfg *xc5000_assign_firmware(int chip_id)
@@ -622,6 +630,8 @@ static int xc5000_fwupload(struct dvb_frontend *fe)
 	const struct xc5000_fw_cfg *desired_fw =
 		xc5000_assign_firmware(priv->chip_id);
 	priv->pll_register_no = desired_fw->pll_reg;
+	priv->init_status_supported = desired_fw->init_status_supported;
+	priv->fw_checksum_supported = desired_fw->fw_checksum_supported;
 
 	/* request the firmware, this will block and timeout */
 	printk(KERN_INFO "xc5000: waiting for firmware upload (%s)...\n",
@@ -1082,6 +1092,7 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force)
 	struct xc5000_priv *priv = fe->tuner_priv;
 	int ret = XC_RESULT_SUCCESS;
 	u16 pll_lock_status;
+	u16 fw_ck;
 
 	if (force || xc5000_is_firmware_loaded(fe) != XC_RESULT_SUCCESS) {
 
@@ -1093,6 +1104,21 @@ fw_retry:
 
 		msleep(20);
 
+		if (priv->fw_checksum_supported) {
+			if (xc5000_readreg(priv, XREG_FW_CHECKSUM, &fw_ck)
+			    != XC_RESULT_SUCCESS) {
+				dprintk(1, "%s() FW checksum reading failed.\n",
+					__func__);
+				goto fw_retry;
+			}
+
+			if (fw_ck == 0) {
+				dprintk(1, "%s() FW checksum failed = 0x%04x\n",
+					__func__, fw_ck);
+				goto fw_retry;
+			}
+		}
+
 		/* Start the tuner self-calibration process */
 		ret |= xc_initialize(priv);
 
@@ -1106,6 +1132,19 @@ fw_retry:
 		 */
 		xc_wait(100);
 
+		if (priv->init_status_supported) {
+			if (xc5000_readreg(priv, XREG_INIT_STATUS, &fw_ck) != XC_RESULT_SUCCESS) {
+				dprintk(1, "%s() FW failed reading init status.\n",
+					__func__);
+				goto fw_retry;
+			}
+
+			if (fw_ck == 0) {
+				dprintk(1, "%s() FW init status failed = 0x%04x\n", __func__, fw_ck);
+				goto fw_retry;
+			}
+		}
+
 		if (priv->pll_register_no) {
 			xc5000_readreg(priv, priv->pll_register_no,
 				       &pll_lock_status);
-- 
1.7.1

[PATCH 20/24] au0828: tweak workaround for i2c clock stretching bug

[Devin Heitmueller]

The hack I put in a couple of years ago to avoid clock stretching issues
when talking to the xc5000 worked fine for writes, but intermittently fails
for register reads, because the xc5000 may stretch the clock for longer
between bytes (I was seeing cases of 21 us on the analyzer).

The problem manifested itself as the xc5000 firmware version and PLL lock
register intermittently showing garbage values.

Slow down the i2c bus from 30 KHz to 20 KHz to accommodate.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-cards.c |    4 ++--
 drivers/media/video/au0828/au0828-reg.h   |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-cards.c b/drivers/media/video/au0828/au0828-cards.c
index 1c6015a..4a09b0e 100644
--- a/drivers/media/video/au0828/au0828-cards.c
+++ b/drivers/media/video/au0828/au0828-cards.c
@@ -46,7 +46,7 @@ struct au0828_board au0828_boards[] = {
 		.name	= "Hauppauge HVR850",
 		.tuner_type = TUNER_XC5000,
 		.tuner_addr = 0x61,
-		.i2c_clk_divider = AU0828_I2C_CLK_30KHZ,
+		.i2c_clk_divider = AU0828_I2C_CLK_20KHZ,
 		.input = {
 			{
 				.type = AU0828_VMUX_TELEVISION,
@@ -77,7 +77,7 @@ struct au0828_board au0828_boards[] = {
 		   stretch fits inside of a normal clock cycle, or else the
 		   au0828 fails to set the STOP bit.  A 30 KHz clock puts the
 		   clock pulse width at 18us */
-		.i2c_clk_divider = AU0828_I2C_CLK_30KHZ,
+		.i2c_clk_divider = AU0828_I2C_CLK_20KHZ,
 		.input = {
 			{
 				.type = AU0828_VMUX_TELEVISION,
diff --git a/drivers/media/video/au0828/au0828-reg.h b/drivers/media/video/au0828/au0828-reg.h
index c39f3d2..2140f4c 100644
--- a/drivers/media/video/au0828/au0828-reg.h
+++ b/drivers/media/video/au0828/au0828-reg.h
@@ -63,3 +63,4 @@
 #define AU0828_I2C_CLK_250KHZ 0x07
 #define AU0828_I2C_CLK_100KHZ 0x14
 #define AU0828_I2C_CLK_30KHZ  0x40
+#define AU0828_I2C_CLK_20KHZ  0x60
-- 
1.7.1

[PATCH 21/24] xc5000: show debug version fields in decimal instead of hex

[Devin Heitmueller]

The driver prints out a dotted version number but it's in hex.  As a result,
the version doesn't visibly match the filename for the firmware, and
it caused a bunch of confusion while discussing different versions with the
chip manufacturer.

Change the firmware printout to be in decimal.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/common/tuners/xc5000.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index 3e5f8cd..4bb20fa 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -702,7 +702,7 @@ static void xc_debug_dump(struct xc5000_priv *priv)
 	xc_get_version(priv,  &hw_majorversion, &hw_minorversion,
 		&fw_majorversion, &fw_minorversion);
 	xc_get_buildversion(priv,  &fw_buildversion);
-	dprintk(1, "*** HW: V%02x.%02x, FW: V%02x.%02x.%04x\n",
+	dprintk(1, "*** HW: V%d.%d, FW: V %d.%d.%d\n",
 		hw_majorversion, hw_minorversion,
 		fw_majorversion, fw_minorversion, fw_buildversion);
 
-- 
1.7.1

[PATCH 22/24] au0828: fix a couple of missed edge cases for i2c gate with analog

[Devin Heitmueller]

This patch addresses a couple of cases where I forgot to pop open the gate
when in analog mode (a correlary to fix the change made in patch
1c58d5b4a5fca42dce5428bd79b9405878017735).

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-video.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-video.c b/drivers/media/video/au0828/au0828-video.c
index 4d5b670..fa0fa9a 100644
--- a/drivers/media/video/au0828/au0828-video.c
+++ b/drivers/media/video/au0828/au0828-video.c
@@ -1325,12 +1325,19 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id * norm)
 	struct au0828_fh *fh = priv;
 	struct au0828_dev *dev = fh->dev;
 
+	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
+		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 1);
+
 	/* FIXME: when we support something other than NTSC, we are going to
 	   have to make the au0828 bridge adjust the size of its capture
 	   buffer, which is currently hardcoded at 720x480 */
 
 	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, *norm);
 	dev->std_set_in_tuner_core = 1;
+
+	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
+		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 0);
+
 	return 0;
 }
 
@@ -1510,9 +1517,18 @@ static int vidioc_s_tuner(struct file *file, void *priv,
 		return -EINVAL;
 
 	t->type = V4L2_TUNER_ANALOG_TV;
+
+	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
+		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 1);
+
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
+
+	if (dev->dvb.frontend && dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl)
+		dev->dvb.frontend->ops.analog_ops.i2c_gate_ctrl(dev->dvb.frontend, 0);
+
 	dprintk(1, "VIDIOC_S_TUNER: signal = %x, afc = %x\n", t->signal,
 		t->afc);
+
 	return 0;
 
 }
-- 
1.7.1

[PATCH 23/24] au0828: make xc5000 firmware speedup apply to the xc5000c as well

[Devin Heitmueller]

Make the firmware speedup work for the 5000c as well as the original
xc5000.  This cuts firmware load time in half.

Thanks to John Casey at Hauppauge for loaning me a board for testing.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
---
 drivers/media/video/au0828/au0828-i2c.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/au0828/au0828-i2c.c b/drivers/media/video/au0828/au0828-i2c.c
index 3bc76df..4ded17f 100644
--- a/drivers/media/video/au0828/au0828-i2c.c
+++ b/drivers/media/video/au0828/au0828-i2c.c
@@ -147,7 +147,8 @@ static int i2c_sendbytes(struct i2c_adapter *i2c_adap,
 	au0828_write(dev, AU0828_I2C_MULTIBYTE_MODE_2FF, 0x01);
 
 	/* Set the I2C clock */
-	if ((dev->board.tuner_type == TUNER_XC5000) &&
+	if (((dev->board.tuner_type == TUNER_XC5000) ||
+	     (dev->board.tuner_type == TUNER_XC5000C)) &&
 	    (dev->board.tuner_addr == msg->addr) &&
 	    (msg->len == 64)) {
 		/* Hack to speed up firmware load.  The xc5000 lets us do up
-- 
1.7.1

[PATCH 24/24] xc5000: change filename to production/redistributable xc5000c firmware

[Devin Heitmueller]

The original xc5000c driver support was based on a beta version of the
firmware, and there were no redistribution rights.  Change over to using the
release version, for which freely redistributable firmware can be found here:

http://kernellabs.com/firmware/xc5000/README.xc5000c
http://kernellabs.com/firmware/xc5000/dvb-fe-xc5000c-4.1.30.7.fw

Thanks to Ramon Cazares from Cresta Technology for making the firmware
available as well as working out the licensing issues.

Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
Cc: Michael Krufky <mkrufky@kernellabs.com>
---
 drivers/media/common/tuners/xc5000.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/common/tuners/xc5000.c b/drivers/media/common/tuners/xc5000.c
index 4bb20fa..3f7327c 100644
--- a/drivers/media/common/tuners/xc5000.c
+++ b/drivers/media/common/tuners/xc5000.c
@@ -226,7 +226,7 @@ static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
 };
 
 static const struct xc5000_fw_cfg xc5000c_41_024_5 = {
-	.name = "dvb-fe-xc5000c-41.024.5.fw",
+	.name = "dvb-fe-xc5000c-4.1.30.7.fw",
 	.size = 16497,
 	.pll_reg = 0x13,
 	.init_status_supported = 1,
-- 
1.7.1

Re: [PATCH 00/24] Various HVR-950q and xc5000 fixes

[Hans Verkuil]

Hi Devin!

On Tue August 7 2012 04:46:50 Devin Heitmueller wrote:
> This patch series contains fixes for a variety of problems found in the
> HVR-950q as well as the xc5000 driver.
> 
> Details can be found in the individual patches, but it is worth mentioning
> specifically that this addresses the MythTV problem causing BUG() to occur,
> firmware loading is now significantly improved, and we now have a
> redistributable version for the xc5000c firmware.

Since you're working on the au0828 would it perhaps be possible to have that
driver use unlocked_ioctl instead of ioctl? It would be really nice if we
can get rid of the ioctl v4l2_operation at some point in the future.

Regards,

	Hans

> Devin Heitmueller (24):
>   au8522: fix intermittent lockup of analog video decoder
>   au8522: Fix off-by-one in SNR table for QAM256
>   au8522: properly recover from the au8522 delivering misaligned TS
>     streams
>   au0828: Make the s_reg and g_reg advanced debug calls work against
>     the bridge
>   xc5000: properly show quality register values
>   xc5000: add support for showing the SNR and gain in the debug output
>   xc5000: properly report i2c write failures
>   au0828: fix race condition that causes xc5000 to not bind for digital
>   au0828: make sure video standard is setup in tuner-core
>   au8522: fix regression in logging introduced by separation of modules
>   xc5000: don't invoke auto calibration unless we really did reset
>     tuner
>   au0828: prevent i2c gate from being kept open while in analog mode
>   au0828: fix case where STREAMOFF being called on stopped stream
>     causes BUG()
>   au0828: speed up i2c clock when doing xc5000 firmware load
>   au0828: remove control buffer from send_control_msg
>   au0828: tune retry interval for i2c interaction
>   au0828: fix possible race condition in usage of dev->ctrlmsg
>   xc5000: reset device if encountering PLL lock failure
>   xc5000: add support for firmware load check and init status
>   au0828: tweak workaround for i2c clock stretching bug
>   xc5000: show debug version fields in decimal instead of hex
>   au0828: fix a couple of missed edge cases for i2c gate with analog
>   au0828: make xc5000 firmware speedup apply to the xc5000c as well
>   xc5000: change filename to production/redistributable xc5000c
>     firmware
> 
>  drivers/media/common/tuners/xc5000.c         |  161 +++++++++++++++++++++-----
>  drivers/media/dvb/frontends/au8522_common.c  |   22 +++-
>  drivers/media/dvb/frontends/au8522_decoder.c |   11 +-
>  drivers/media/dvb/frontends/au8522_dig.c     |   98 ++++++++--------
>  drivers/media/dvb/frontends/au8522_priv.h    |   29 ++++-
>  drivers/media/video/au0828/au0828-cards.c    |    4 +-
>  drivers/media/video/au0828/au0828-core.c     |   59 ++++------
>  drivers/media/video/au0828/au0828-dvb.c      |   54 ++++++++-
>  drivers/media/video/au0828/au0828-i2c.c      |   21 +++-
>  drivers/media/video/au0828/au0828-reg.h      |    1 +
>  drivers/media/video/au0828/au0828-video.c    |   76 +++++++++---
>  drivers/media/video/au0828/au0828.h          |    2 +
>  12 files changed, 379 insertions(+), 159 deletions(-)
> 
> --
> 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
> 

Re: [PATCH 00/24] Various HVR-950q and xc5000 fixes

[Devin Heitmueller]

On Tue, Aug 7, 2012 at 2:26 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Since you're working on the au0828 would it perhaps be possible to have that
> driver use unlocked_ioctl instead of ioctl? It would be really nice if we
> can get rid of the ioctl v4l2_operation at some point in the future.

Hi Hans,

I'm pretty sure that actually got done implicitly by patch #8 as a
result of a fix for a race condition at startup.  Please take a look
and let me know if I missed anything:

[PATCH 08/24] au0828: fix race condition that causes xc5000 to not
bind for digital

Thanks,

Devin

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

Re: [PATCH 00/24] Various HVR-950q and xc5000 fixes

[Hans Verkuil]

On Tue 7 August 2012 14:48:41 Devin Heitmueller wrote:
> On Tue, Aug 7, 2012 at 2:26 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > Since you're working on the au0828 would it perhaps be possible to have that
> > driver use unlocked_ioctl instead of ioctl? It would be really nice if we
> > can get rid of the ioctl v4l2_operation at some point in the future.
> 
> Hi Hans,
> 
> I'm pretty sure that actually got done implicitly by patch #8 as a
> result of a fix for a race condition at startup.  Please take a look
> and let me know if I missed anything:
> 
> [PATCH 08/24] au0828: fix race condition that causes xc5000 to not
> bind for digital

Great! That's what I was hoping for. It wasn't clear from the patch subject
that that patch contained these changes, otherwise I wouldn't have bothered
you.

Anyway, another one bites the dust.

Regards,

	Hans

Re: [PATCH 17/24] au0828: fix possible race condition in usage of dev->ctrlmsg

[Mauro Carvalho Chehab]

Em 06-08-2012 23:47, Devin Heitmueller escreveu:
> The register read function is referencing the dev->ctrlmsg structure outside
> of the dev->mutex lock, which can cause corruption of the value if multiple
> callers are invoking au0828_readreg() simultaneously.
> 
> Use a stack variable to hold the result, and copy the buffer returned by
> usb_control_msg() to that variable.

It is NOT OK to use stack to send and/or receive control messages. The USB core
uses DMA transfers for sending/receiving data via USB; the memory used by stack
is not warranted to be at the DMA-able area. This problem is more frequent on
ARM-based machines, but even on Intel, the urb_control_msg() may fail.

> 
> In reality, the whole recv_control_msg() function can probably be collapsed
> into au0288_readreg() since it is the only caller.
> 
> Also get rid of cmd_msg_dump() since the only case in which the function is
> ever called only is ever passed a single byte for the response (and it is
> already logged).
> 
> Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
> ---
>   drivers/media/video/au0828/au0828-core.c |   40 +++++++++---------------------
>   1 files changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/media/video/au0828/au0828-core.c b/drivers/media/video/au0828/au0828-core.c
> index 65914bc..745a80a 100644
> --- a/drivers/media/video/au0828/au0828-core.c
> +++ b/drivers/media/video/au0828/au0828-core.c
> @@ -56,9 +56,12 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
>   
>   u32 au0828_readreg(struct au0828_dev *dev, u16 reg)
>   {
> -	recv_control_msg(dev, CMD_REQUEST_IN, 0, reg, dev->ctrlmsg, 1);
> -	dprintk(8, "%s(0x%04x) = 0x%02x\n", __func__, reg, dev->ctrlmsg[0]);
> -	return dev->ctrlmsg[0];
> +	u8 result = 0;
> +
> +	recv_control_msg(dev, CMD_REQUEST_IN, 0, reg, &result, 1);

As explained above, this won't work, as result is at stack, not warranted to be at the
DMA-able area. So, either you could lock this function, or you'll need to allocate
it with kmalloc() and free it after using the data.

> +	dprintk(8, "%s(0x%04x) = 0x%02x\n", __func__, reg, result);
> +
> +	return result;
>   }
>   
>   u32 au0828_writereg(struct au0828_dev *dev, u16 reg, u32 val)
> @@ -67,24 +70,6 @@ u32 au0828_writereg(struct au0828_dev *dev, u16 reg, u32 val)
>   	return send_control_msg(dev, CMD_REQUEST_OUT, val, reg);
>   }
>   
> -static void cmd_msg_dump(struct au0828_dev *dev)
> -{
> -	int i;
> -
> -	for (i = 0; i < sizeof(dev->ctrlmsg); i += 16)
> -		dprintk(2, "%s() %02x %02x %02x %02x %02x %02x %02x %02x "
> -				"%02x %02x %02x %02x %02x %02x %02x %02x\n",
> -			__func__,
> -			dev->ctrlmsg[i+0], dev->ctrlmsg[i+1],
> -			dev->ctrlmsg[i+2], dev->ctrlmsg[i+3],
> -			dev->ctrlmsg[i+4], dev->ctrlmsg[i+5],
> -			dev->ctrlmsg[i+6], dev->ctrlmsg[i+7],
> -			dev->ctrlmsg[i+8], dev->ctrlmsg[i+9],
> -			dev->ctrlmsg[i+10], dev->ctrlmsg[i+11],
> -			dev->ctrlmsg[i+12], dev->ctrlmsg[i+13],
> -			dev->ctrlmsg[i+14], dev->ctrlmsg[i+15]);
> -}
> -
>   static int send_control_msg(struct au0828_dev *dev, u16 request, u32 value,
>   	u16 index)
>   {
> @@ -118,24 +103,23 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
>   	int status = -ENODEV;
>   	mutex_lock(&dev->mutex);
>   	if (dev->usbdev) {
> -
> -		memset(dev->ctrlmsg, 0, sizeof(dev->ctrlmsg));
> -
> -		/* cp must be memory that has been allocated by kmalloc */
>   		status = usb_control_msg(dev->usbdev,
>   				usb_rcvctrlpipe(dev->usbdev, 0),
>   				request,
>   				USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
>   				value, index,
> -				cp, size, 1000);
> +				dev->ctrlmsg, size, 1000);
>   
>   		status = min(status, 0);
>   
>   		if (status < 0) {
>   			printk(KERN_ERR "%s() Failed receiving control message, error %d.\n",
>   				__func__, status);
> -		} else
> -			cmd_msg_dump(dev);
> +		}
> +
> +		/* the host controller requires heap allocated memory, which
> +		   is why we didn't just pass "cp" into usb_control_msg */
> +		memcpy(cp, dev->ctrlmsg, size);
>   	}
>   	mutex_unlock(&dev->mutex);
>   	return status;
> 

Regards,
Mauro

Re: [PATCH 17/24] au0828: fix possible race condition in usage of dev->ctrlmsg

[Devin Heitmueller]

On Thu, Aug 9, 2012 at 7:48 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 06-08-2012 23:47, Devin Heitmueller escreveu:
>> The register read function is referencing the dev->ctrlmsg structure outside
>> of the dev->mutex lock, which can cause corruption of the value if multiple
>> callers are invoking au0828_readreg() simultaneously.
>>
>> Use a stack variable to hold the result, and copy the buffer returned by
>> usb_control_msg() to that variable.
>
> It is NOT OK to use stack to send and/or receive control messages. The USB core
> uses DMA transfers for sending/receiving data via USB; the memory used by stack
> is not warranted to be at the DMA-able area. This problem is more frequent on
> ARM-based machines, but even on Intel, the urb_control_msg() may fail.
>
>>
>> In reality, the whole recv_control_msg() function can probably be collapsed
>> into au0288_readreg() since it is the only caller.
>>
>> Also get rid of cmd_msg_dump() since the only case in which the function is
>> ever called only is ever passed a single byte for the response (and it is
>> already logged).
>>
>> Signed-off-by: Devin Heitmueller <dheitmueller@kernellabs.com>
>> ---
>>   drivers/media/video/au0828/au0828-core.c |   40 +++++++++---------------------
>>   1 files changed, 12 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/media/video/au0828/au0828-core.c b/drivers/media/video/au0828/au0828-core.c
>> index 65914bc..745a80a 100644
>> --- a/drivers/media/video/au0828/au0828-core.c
>> +++ b/drivers/media/video/au0828/au0828-core.c
>> @@ -56,9 +56,12 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
>>
>>   u32 au0828_readreg(struct au0828_dev *dev, u16 reg)
>>   {
>> -     recv_control_msg(dev, CMD_REQUEST_IN, 0, reg, dev->ctrlmsg, 1);
>> -     dprintk(8, "%s(0x%04x) = 0x%02x\n", __func__, reg, dev->ctrlmsg[0]);
>> -     return dev->ctrlmsg[0];
>> +     u8 result = 0;
>> +
>> +     recv_control_msg(dev, CMD_REQUEST_IN, 0, reg, &result, 1);
>
> As explained above, this won't work, as result is at stack, not warranted to be at the
> DMA-able area. So, either you could lock this function, or you'll need to allocate
> it with kmalloc() and free it after using the data.
>
>> +     dprintk(8, "%s(0x%04x) = 0x%02x\n", __func__, reg, result);
>> +
>> +     return result;
>>   }
>>
>>   u32 au0828_writereg(struct au0828_dev *dev, u16 reg, u32 val)
>> @@ -67,24 +70,6 @@ u32 au0828_writereg(struct au0828_dev *dev, u16 reg, u32 val)
>>       return send_control_msg(dev, CMD_REQUEST_OUT, val, reg);
>>   }
>>
>> -static void cmd_msg_dump(struct au0828_dev *dev)
>> -{
>> -     int i;
>> -
>> -     for (i = 0; i < sizeof(dev->ctrlmsg); i += 16)
>> -             dprintk(2, "%s() %02x %02x %02x %02x %02x %02x %02x %02x "
>> -                             "%02x %02x %02x %02x %02x %02x %02x %02x\n",
>> -                     __func__,
>> -                     dev->ctrlmsg[i+0], dev->ctrlmsg[i+1],
>> -                     dev->ctrlmsg[i+2], dev->ctrlmsg[i+3],
>> -                     dev->ctrlmsg[i+4], dev->ctrlmsg[i+5],
>> -                     dev->ctrlmsg[i+6], dev->ctrlmsg[i+7],
>> -                     dev->ctrlmsg[i+8], dev->ctrlmsg[i+9],
>> -                     dev->ctrlmsg[i+10], dev->ctrlmsg[i+11],
>> -                     dev->ctrlmsg[i+12], dev->ctrlmsg[i+13],
>> -                     dev->ctrlmsg[i+14], dev->ctrlmsg[i+15]);
>> -}
>> -
>>   static int send_control_msg(struct au0828_dev *dev, u16 request, u32 value,
>>       u16 index)
>>   {
>> @@ -118,24 +103,23 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value,
>>       int status = -ENODEV;
>>       mutex_lock(&dev->mutex);
>>       if (dev->usbdev) {
>> -
>> -             memset(dev->ctrlmsg, 0, sizeof(dev->ctrlmsg));
>> -
>> -             /* cp must be memory that has been allocated by kmalloc */
>>               status = usb_control_msg(dev->usbdev,
>>                               usb_rcvctrlpipe(dev->usbdev, 0),
>>                               request,
>>                               USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
>>                               value, index,
>> -                             cp, size, 1000);
>> +                             dev->ctrlmsg, size, 1000);
>>
>>               status = min(status, 0);
>>
>>               if (status < 0) {
>>                       printk(KERN_ERR "%s() Failed receiving control message, error %d.\n",
>>                               __func__, status);
>> -             } else
>> -                     cmd_msg_dump(dev);
>> +             }
>> +
>> +             /* the host controller requires heap allocated memory, which
>> +                is why we didn't just pass "cp" into usb_control_msg */
>> +             memcpy(cp, dev->ctrlmsg, size);
>>       }
>>       mutex_unlock(&dev->mutex);
>>       return status;
>>
>
> Regards,
> Mauro

Hi Mauro,

You seem to have misinterpreted the patch description.  The actual
call to usb_control_msg() does use a heap allocated memory region.
However we copy the result to a stack variable after the call to
usb_control_msg.  This is done so that dev->ctrlmsg[] is used
exclusively inside of the mutex (and not accessed after the mutex is
unlocked).

The change basically goes from:

au0828_readreg() -> recv_control_msg(heap) -> usb_control_msg(heap)

to:

au0828_readreg() -> recv_control_msg(stack) -> usb_control_msg(heap)

In both cases the call into the USB stack provides heap allocated memory.

Please review the implementation of the static recv_control_msg()
function, and if you have any further questions let me know.

Thanks,

Devin

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

Re: [PATCH 07/24] xc5000: properly report i2c write failures

[Devin Heitmueller]

> I can't load firmware like error of below link.
>
> https://bugs.launchpad.net/ubuntu/+source/linux-firmware-nonfree/+bug/1263837
>
> This error is related with this patch. This fix is right but above error is
> created after this fix
> because my device makes WatchDogTimer to 0 when load firmware.
> Maybe it will be related with XREG_BUSY register but i can't check it.
>
> I removed this fix, but i have faced at other error with "xc5000: PLL not
> running after fwload"
> So i have commented like below.
>
> static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
>         .name = XC5000A_FIRMWARE,
>         .size = 12401,
>         //.pll_reg = 0x806c,
> };
>
> Then, xc5000 device works well.
>
> I don't have xc5000 datasheet so i can't debug xc5000 driver anymore.

Hi Joonyoung,

Assuming this is the DViCO FusionHDTV7 device that uses the
au0828/au8522, I suspect that what's happening here is your I2C
controller is not stable.  The I2C clock stretching done by the xc5000
often exposed bugs in various bridge drivers and the au0828 was no
exception.  I had to work around these hardware bugs in the au0828
driver but I made them specific to the HVR-950q since that was the
only device I could test with.

In other words, the xc5000 is most likely doing exactly what it is
supposed to, and the increased robustness of the tuner driver with
those two patches exposed intermittent I2C failures in au0828 that
were previously being silently discarded (resulting in indeterminate
behavior).

I would recommending looking at the changes in au0828-cards.c for the
HVR-950q and add the code necessary to make them also apply for the
DVICO device, and that should resolve your problems (in particular the
i2c_clk_divider field should be set).

Devin

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

Re: [PATCH 07/24] xc5000: properly report i2c write failures

[Joonyoung Shim]

Hi Devin,

2014-02-07 22:46 GMT+09:00 Devin Heitmueller <dheitmueller@kernellabs.com>:
>
> > I can't load firmware like error of below link.
> >
> > https://bugs.launchpad.net/ubuntu/+source/linux-firmware-nonfree/+bug/1263837
> >
> > This error is related with this patch. This fix is right but above error is
> > created after this fix
> > because my device makes WatchDogTimer to 0 when load firmware.
> > Maybe it will be related with XREG_BUSY register but i can't check it.
> >
> > I removed this fix, but i have faced at other error with "xc5000: PLL not
> > running after fwload"
> > So i have commented like below.
> >
> > static const struct xc5000_fw_cfg xc5000a_1_6_114 = {
> >         .name = XC5000A_FIRMWARE,
> >         .size = 12401,
> >         //.pll_reg = 0x806c,
> > };
> >
> > Then, xc5000 device works well.
> >
> > I don't have xc5000 datasheet so i can't debug xc5000 driver anymore.
>
> Hi Joonyoung,
>
> Assuming this is the DViCO FusionHDTV7 device that uses the
> au0828/au8522, I suspect that what's happening here is your I2C
> controller is not stable.  The I2C clock stretching done by the xc5000
> often exposed bugs in various bridge drivers and the au0828 was no
> exception.  I had to work around these hardware bugs in the au0828
> driver but I made them specific to the HVR-950q since that was the
> only device I could test with.
>
> In other words, the xc5000 is most likely doing exactly what it is
> supposed to, and the increased robustness of the tuner driver with
> those two patches exposed intermittent I2C failures in au0828 that
> were previously being silently discarded (resulting in indeterminate
> behavior).
>
> I would recommending looking at the changes in au0828-cards.c for the
> HVR-950q and add the code necessary to make them also apply for the
> DVICO device, and that should resolve your problems (in particular the
> i2c_clk_divider field should be set).
>

As you said, i modified like below patch and it is working well.

Thanks for your advice.

diff --git a/drivers/media/usb/au0828/au0828-cards.c
b/drivers/media/usb/au0828/au0828-cards.c
index 0cb7c28..9936875 100644
--- a/drivers/media/usb/au0828/au0828-cards.c
+++ b/drivers/media/usb/au0828/au0828-cards.c
@@ -108,7 +108,7 @@ struct au0828_board au0828_boards[] = {
         .name    = "DViCO FusionHDTV USB",
         .tuner_type = UNSET,
         .tuner_addr = ADDR_UNSET,
-        .i2c_clk_divider = AU0828_I2C_CLK_250KHZ,
+        .i2c_clk_divider = AU0828_I2C_CLK_20KHZ,
     },
     [AU0828_BOARD_HAUPPAUGE_WOODBURY] = {
         .name = "Hauppauge Woodbury",


- Joonyoung Shim

Re: [PATCH 07/24] xc5000: properly report i2c write failures

[Devin Heitmueller]

On Mon, Feb 10, 2014 at 3:25 AM, Joonyoung Shim <dofmind@gmail.com> wrote:
> As you said, i modified like below patch and it is working well.
>
> Thanks for your advice.
>
> diff --git a/drivers/media/usb/au0828/au0828-cards.c
> b/drivers/media/usb/au0828/au0828-cards.c
> index 0cb7c28..9936875 100644
> --- a/drivers/media/usb/au0828/au0828-cards.c
> +++ b/drivers/media/usb/au0828/au0828-cards.c
> @@ -108,7 +108,7 @@ struct au0828_board au0828_boards[] = {
>          .name    = "DViCO FusionHDTV USB",
>          .tuner_type = UNSET,
>          .tuner_addr = ADDR_UNSET,
> -        .i2c_clk_divider = AU0828_I2C_CLK_250KHZ,
> +        .i2c_clk_divider = AU0828_I2C_CLK_20KHZ,
>      },
>      [AU0828_BOARD_HAUPPAUGE_WOODBURY] = {
>          .name = "Hauppauge Woodbury",

Great.  Feel free to submit a patch to the mailing list with your SOB,
and we'll merge that change upstream.

Devin

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