[PATCH v3 00/24] em28xx: split analog part into a separate module

[PATCH v3 00/24] em28xx: split analog part into a separate module

[Mauro Carvalho Chehab]

This patch series split em28xx into a separate V4L2 driver,
allowing the new dvb-only chips to be supported without requiring
V4L2.

While testing the original patchset, I noticed several issues with
HVR-950. The remaining patches on this series fix most of those
issues.

There's one remaining issue: on my tests, when connecting the device
into an USB 3.0 port, the AC97 EMP202 is not properly detected. 
Also, the audio doesn't work fine. I'm still investigating what
would be the root cause for that.

Mauro Carvalho Chehab (24):
  em28xx: move some video-specific functions to em28xx-video
  em28xx: some cosmetic changes
  em28xx: move analog-specific init to em28xx-video
  em28xx: make em28xx-video to be a separate module
  em28xx: initialize analog I2C devices at the right place
  em28xx-cards: remove a now dead code
  em28xx: fix a cut and paste error
  em28xx: add warn messages for timeout
  em28xx: improve extension information messages
  em28xx: convert i2c wait completion logic to use jiffies
  tvp5150: make read operations atomic
  tuner-xc2028: remove unused code
  em28xx: retry I2C ops if failed by timeout
  em28xx: remove a false positive warning
  em28xx: check if a device has audio earlier
  em28xx: properly implement AC97 wait code
  em28xx: initialize audio latter
  em28xx: improve I2C timeout error message
  em28xx: unify module version
  em28xx: Fix em28xx deplock
  em28xx: USB: adjust for changed 3.8 USB API
  em28xx: use a better value for I2C timeouts
  em28xx: don't return -ENODEV for I2C xfer errors
  em28xx: cleanup I2C debug messages

 drivers/media/i2c/tvp5150.c              |  22 +-
 drivers/media/tuners/tuner-xc2028.c      |   9 -
 drivers/media/usb/em28xx/Kconfig         |   6 +-
 drivers/media/usb/em28xx/Makefile        |   5 +-
 drivers/media/usb/em28xx/em28xx-audio.c  |   9 +-
 drivers/media/usb/em28xx/em28xx-camera.c |   1 +
 drivers/media/usb/em28xx/em28xx-cards.c  | 310 ++--------------
 drivers/media/usb/em28xx/em28xx-core.c   | 295 +--------------
 drivers/media/usb/em28xx/em28xx-dvb.c    |  11 +-
 drivers/media/usb/em28xx/em28xx-i2c.c    | 226 ++++++------
 drivers/media/usb/em28xx/em28xx-input.c  |   7 +-
 drivers/media/usb/em28xx/em28xx-v4l.h    |  24 ++
 drivers/media/usb/em28xx/em28xx-vbi.c    |   1 +
 drivers/media/usb/em28xx/em28xx-video.c  | 607 +++++++++++++++++++++++++++++--
 drivers/media/usb/em28xx/em28xx.h        |  52 +--
 15 files changed, 835 insertions(+), 750 deletions(-)
 create mode 100644 drivers/media/usb/em28xx/em28xx-v4l.h

-- 
1.8.3.1

[PATCH v3 01/24] em28xx: move some video-specific functions to em28xx-video

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Now that we want to split the video handling to a separate
module, move all video-specific functions to em28xx-video.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 107 ---------
 drivers/media/usb/em28xx/em28xx-core.c  | 259 ----------------------
 drivers/media/usb/em28xx/em28xx-video.c | 374 +++++++++++++++++++++++++++++++-
 drivers/media/usb/em28xx/em28xx.h       |   1 +
 4 files changed, 371 insertions(+), 370 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 36853f16bf97..19827e79cf53 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2529,113 +2529,6 @@ static void em28xx_pre_card_setup(struct em28xx *dev)
 	em28xx_set_mode(dev, EM28XX_SUSPEND);
 }
 
-static void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
-{
-	memset(ctl, 0, sizeof(*ctl));
-
-	ctl->fname   = XC2028_DEFAULT_FIRMWARE;
-	ctl->max_len = 64;
-	ctl->mts = em28xx_boards[dev->model].mts_firmware;
-
-	switch (dev->model) {
-	case EM2880_BOARD_EMPIRE_DUAL_TV:
-	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
-	case EM2882_BOARD_TERRATEC_HYBRID_XS:
-		ctl->demod = XC3028_FE_ZARLINK456;
-		break;
-	case EM2880_BOARD_TERRATEC_HYBRID_XS:
-	case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
-	case EM2881_BOARD_PINNACLE_HYBRID_PRO:
-		ctl->demod = XC3028_FE_ZARLINK456;
-		break;
-	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900_R2:
-	case EM2882_BOARD_PINNACLE_HYBRID_PRO_330E:
-		ctl->demod = XC3028_FE_DEFAULT;
-		break;
-	case EM2880_BOARD_AMD_ATI_TV_WONDER_HD_600:
-		ctl->demod = XC3028_FE_DEFAULT;
-		ctl->fname = XC3028L_DEFAULT_FIRMWARE;
-		break;
-	case EM2883_BOARD_HAUPPAUGE_WINTV_HVR_850:
-	case EM2883_BOARD_HAUPPAUGE_WINTV_HVR_950:
-	case EM2880_BOARD_PINNACLE_PCTV_HD_PRO:
-		/* FIXME: Better to specify the needed IF */
-		ctl->demod = XC3028_FE_DEFAULT;
-		break;
-	case EM2883_BOARD_KWORLD_HYBRID_330U:
-	case EM2882_BOARD_DIKOM_DK300:
-	case EM2882_BOARD_KWORLD_VS_DVBT:
-		ctl->demod = XC3028_FE_CHINA;
-		ctl->fname = XC2028_DEFAULT_FIRMWARE;
-		break;
-	case EM2882_BOARD_EVGA_INDTUBE:
-		ctl->demod = XC3028_FE_CHINA;
-		ctl->fname = XC3028L_DEFAULT_FIRMWARE;
-		break;
-	default:
-		ctl->demod = XC3028_FE_OREN538;
-	}
-}
-
-static void em28xx_tuner_setup(struct em28xx *dev)
-{
-	struct tuner_setup           tun_setup;
-	struct v4l2_frequency        f;
-
-	if (dev->tuner_type == TUNER_ABSENT)
-		return;
-
-	memset(&tun_setup, 0, sizeof(tun_setup));
-
-	tun_setup.mode_mask = T_ANALOG_TV | T_RADIO;
-	tun_setup.tuner_callback = em28xx_tuner_callback;
-
-	if (dev->board.radio.type) {
-		tun_setup.type = dev->board.radio.type;
-		tun_setup.addr = dev->board.radio_addr;
-
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_type_addr, &tun_setup);
-	}
-
-	if ((dev->tuner_type != TUNER_ABSENT) && (dev->tuner_type)) {
-		tun_setup.type   = dev->tuner_type;
-		tun_setup.addr   = dev->tuner_addr;
-
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_type_addr, &tun_setup);
-	}
-
-	if (dev->tda9887_conf) {
-		struct v4l2_priv_tun_config tda9887_cfg;
-
-		tda9887_cfg.tuner = TUNER_TDA9887;
-		tda9887_cfg.priv = &dev->tda9887_conf;
-
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_config, &tda9887_cfg);
-	}
-
-	if (dev->tuner_type == TUNER_XC2028) {
-		struct v4l2_priv_tun_config  xc2028_cfg;
-		struct xc2028_ctrl           ctl;
-
-		memset(&xc2028_cfg, 0, sizeof(xc2028_cfg));
-		memset(&ctl, 0, sizeof(ctl));
-
-		em28xx_setup_xc3028(dev, &ctl);
-
-		xc2028_cfg.tuner = TUNER_XC2028;
-		xc2028_cfg.priv  = &ctl;
-
-		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_config, &xc2028_cfg);
-	}
-
-	/* configure tuner */
-	f.tuner = 0;
-	f.type = V4L2_TUNER_ANALOG_TV;
-	f.frequency = 9076;     /* just a magic number */
-	dev->ctl_freq = f.frequency;
-	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
-}
-
 static int em28xx_hint_board(struct em28xx *dev)
 {
 	int i;
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index f6076a512e8f..3012912d2997 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -53,14 +53,6 @@ MODULE_PARM_DESC(reg_debug, "enable debug messages [URB reg]");
 		printk(KERN_INFO "%s %s :"fmt, \
 			 dev->name, __func__ , ##arg); } while (0)
 
-static int alt;
-module_param(alt, int, 0644);
-MODULE_PARM_DESC(alt, "alternate setting to use for video endpoint");
-
-static unsigned int disable_vbi;
-module_param(disable_vbi, int, 0644);
-MODULE_PARM_DESC(disable_vbi, "disable vbi support");
-
 /* FIXME */
 #define em28xx_isocdbg(fmt, arg...) do {\
 	if (core_debug) \
@@ -603,24 +595,6 @@ init_audio:
 }
 EXPORT_SYMBOL_GPL(em28xx_audio_setup);
 
-int em28xx_colorlevels_set_default(struct em28xx *dev)
-{
-	em28xx_write_reg(dev, EM28XX_R20_YGAIN, CONTRAST_DEFAULT);
-	em28xx_write_reg(dev, EM28XX_R21_YOFFSET, BRIGHTNESS_DEFAULT);
-	em28xx_write_reg(dev, EM28XX_R22_UVGAIN, SATURATION_DEFAULT);
-	em28xx_write_reg(dev, EM28XX_R23_UOFFSET, BLUE_BALANCE_DEFAULT);
-	em28xx_write_reg(dev, EM28XX_R24_VOFFSET, RED_BALANCE_DEFAULT);
-	em28xx_write_reg(dev, EM28XX_R25_SHARPNESS, SHARPNESS_DEFAULT);
-
-	em28xx_write_reg(dev, EM28XX_R14_GAMMA, 0x20);
-	em28xx_write_reg(dev, EM28XX_R15_RGAIN, 0x20);
-	em28xx_write_reg(dev, EM28XX_R16_GGAIN, 0x20);
-	em28xx_write_reg(dev, EM28XX_R17_BGAIN, 0x20);
-	em28xx_write_reg(dev, EM28XX_R18_ROFFSET, 0x00);
-	em28xx_write_reg(dev, EM28XX_R19_GOFFSET, 0x00);
-	return em28xx_write_reg(dev, EM28XX_R1A_BOFFSET, 0x00);
-}
-
 const struct em28xx_led *em28xx_find_led(struct em28xx *dev,
 					 enum em28xx_led_role role)
 {
@@ -696,227 +670,6 @@ int em28xx_capture_start(struct em28xx *dev, int start)
 	return rc;
 }
 
-int em28xx_vbi_supported(struct em28xx *dev)
-{
-	/* Modprobe option to manually disable */
-	if (disable_vbi == 1)
-		return 0;
-
-	if (dev->board.is_webcam)
-		return 0;
-
-	/* FIXME: check subdevices for VBI support */
-
-	if (dev->chip_id == CHIP_ID_EM2860 ||
-	    dev->chip_id == CHIP_ID_EM2883)
-		return 1;
-
-	/* Version of em28xx that does not support VBI */
-	return 0;
-}
-
-int em28xx_set_outfmt(struct em28xx *dev)
-{
-	int ret;
-	u8 fmt, vinctrl;
-
-	fmt = dev->format->reg;
-	if (!dev->is_em25xx)
-		fmt |= 0x20;
-	/*
-	 * NOTE: it's not clear if this is really needed !
-	 * The datasheets say bit 5 is a reserved bit and devices seem to work
-	 * fine without it. But the Windows driver sets it for em2710/50+em28xx
-	 * devices and we've always been setting it, too.
-	 *
-	 * em2765 (em25xx, em276x/7x/8x) devices do NOT work with this bit set,
-	 * it's likely used for an additional (compressed ?) format there.
-	 */
-	ret = em28xx_write_reg(dev, EM28XX_R27_OUTFMT, fmt);
-	if (ret < 0)
-		return ret;
-
-	ret = em28xx_write_reg(dev, EM28XX_R10_VINMODE, dev->vinmode);
-	if (ret < 0)
-		return ret;
-
-	vinctrl = dev->vinctl;
-	if (em28xx_vbi_supported(dev) == 1) {
-		vinctrl |= EM28XX_VINCTRL_VBI_RAW;
-		em28xx_write_reg(dev, EM28XX_R34_VBI_START_H, 0x00);
-		em28xx_write_reg(dev, EM28XX_R36_VBI_WIDTH, dev->vbi_width/4);
-		em28xx_write_reg(dev, EM28XX_R37_VBI_HEIGHT, dev->vbi_height);
-		if (dev->norm & V4L2_STD_525_60) {
-			/* NTSC */
-			em28xx_write_reg(dev, EM28XX_R35_VBI_START_V, 0x09);
-		} else if (dev->norm & V4L2_STD_625_50) {
-			/* PAL */
-			em28xx_write_reg(dev, EM28XX_R35_VBI_START_V, 0x07);
-		}
-	}
-
-	return em28xx_write_reg(dev, EM28XX_R11_VINCTRL, vinctrl);
-}
-
-static int em28xx_accumulator_set(struct em28xx *dev, u8 xmin, u8 xmax,
-				  u8 ymin, u8 ymax)
-{
-	em28xx_coredbg("em28xx Scale: (%d,%d)-(%d,%d)\n",
-			xmin, ymin, xmax, ymax);
-
-	em28xx_write_regs(dev, EM28XX_R28_XMIN, &xmin, 1);
-	em28xx_write_regs(dev, EM28XX_R29_XMAX, &xmax, 1);
-	em28xx_write_regs(dev, EM28XX_R2A_YMIN, &ymin, 1);
-	return em28xx_write_regs(dev, EM28XX_R2B_YMAX, &ymax, 1);
-}
-
-static void em28xx_capture_area_set(struct em28xx *dev, u8 hstart, u8 vstart,
-				   u16 width, u16 height)
-{
-	u8 cwidth = width >> 2;
-	u8 cheight = height >> 2;
-	u8 overflow = (height >> 9 & 0x02) | (width >> 10 & 0x01);
-	/* NOTE: size limit: 2047x1023 = 2MPix */
-
-	em28xx_coredbg("capture area set to (%d,%d): %dx%d\n",
-		       hstart, vstart,
-		       ((overflow & 2) << 9 | cwidth << 2),
-		       ((overflow & 1) << 10 | cheight << 2));
-
-	em28xx_write_regs(dev, EM28XX_R1C_HSTART, &hstart, 1);
-	em28xx_write_regs(dev, EM28XX_R1D_VSTART, &vstart, 1);
-	em28xx_write_regs(dev, EM28XX_R1E_CWIDTH, &cwidth, 1);
-	em28xx_write_regs(dev, EM28XX_R1F_CHEIGHT, &cheight, 1);
-	em28xx_write_regs(dev, EM28XX_R1B_OFLOW, &overflow, 1);
-
-	/* FIXME: function/meaning of these registers ? */
-	/* FIXME: align width+height to multiples of 4 ?! */
-	if (dev->is_em25xx) {
-		em28xx_write_reg(dev, 0x34, width >> 4);
-		em28xx_write_reg(dev, 0x35, height >> 4);
-	}
-}
-
-static int em28xx_scaler_set(struct em28xx *dev, u16 h, u16 v)
-{
-	u8 mode;
-	/* the em2800 scaler only supports scaling down to 50% */
-
-	if (dev->board.is_em2800) {
-		mode = (v ? 0x20 : 0x00) | (h ? 0x10 : 0x00);
-	} else {
-		u8 buf[2];
-
-		buf[0] = h;
-		buf[1] = h >> 8;
-		em28xx_write_regs(dev, EM28XX_R30_HSCALELOW, (char *)buf, 2);
-
-		buf[0] = v;
-		buf[1] = v >> 8;
-		em28xx_write_regs(dev, EM28XX_R32_VSCALELOW, (char *)buf, 2);
-		/* it seems that both H and V scalers must be active
-		   to work correctly */
-		mode = (h || v) ? 0x30 : 0x00;
-	}
-	return em28xx_write_reg_bits(dev, EM28XX_R26_COMPR, mode, 0x30);
-}
-
-/* FIXME: this only function read values from dev */
-int em28xx_resolution_set(struct em28xx *dev)
-{
-	int width, height;
-	width = norm_maxw(dev);
-	height = norm_maxh(dev);
-
-	/* Properly setup VBI */
-	dev->vbi_width = 720;
-	if (dev->norm & V4L2_STD_525_60)
-		dev->vbi_height = 12;
-	else
-		dev->vbi_height = 18;
-
-	em28xx_set_outfmt(dev);
-
-	em28xx_accumulator_set(dev, 1, (width - 4) >> 2, 1, (height - 4) >> 2);
-
-	/* If we don't set the start position to 2 in VBI mode, we end up
-	   with line 20/21 being YUYV encoded instead of being in 8-bit
-	   greyscale.  The core of the issue is that line 21 (and line 23 for
-	   PAL WSS) are inside of active video region, and as a result they
-	   get the pixelformatting associated with that area.  So by cropping
-	   it out, we end up with the same format as the rest of the VBI
-	   region */
-	if (em28xx_vbi_supported(dev) == 1)
-		em28xx_capture_area_set(dev, 0, 2, width, height);
-	else
-		em28xx_capture_area_set(dev, 0, 0, width, height);
-
-	return em28xx_scaler_set(dev, dev->hscale, dev->vscale);
-}
-
-/* Set USB alternate setting for analog video */
-int em28xx_set_alternate(struct em28xx *dev)
-{
-	int errCode;
-	int i;
-	unsigned int min_pkt_size = dev->width * 2 + 4;
-
-	/* NOTE: for isoc transfers, only alt settings > 0 are allowed
-		 bulk transfers seem to work only with alt=0 ! */
-	dev->alt = 0;
-	if ((alt > 0) && (alt < dev->num_alt)) {
-		em28xx_coredbg("alternate forced to %d\n", dev->alt);
-		dev->alt = alt;
-		goto set_alt;
-	}
-	if (dev->analog_xfer_bulk)
-		goto set_alt;
-
-	/* When image size is bigger than a certain value,
-	   the frame size should be increased, otherwise, only
-	   green screen will be received.
-	 */
-	if (dev->width * 2 * dev->height > 720 * 240 * 2)
-		min_pkt_size *= 2;
-
-	for (i = 0; i < dev->num_alt; i++) {
-		/* stop when the selected alt setting offers enough bandwidth */
-		if (dev->alt_max_pkt_size_isoc[i] >= min_pkt_size) {
-			dev->alt = i;
-			break;
-		/* otherwise make sure that we end up with the maximum bandwidth
-		   because the min_pkt_size equation might be wrong...
-		*/
-		} else if (dev->alt_max_pkt_size_isoc[i] >
-			   dev->alt_max_pkt_size_isoc[dev->alt])
-			dev->alt = i;
-	}
-
-set_alt:
-	/* NOTE: for bulk transfers, we need to call usb_set_interface()
-	 * even if the previous settings were the same. Otherwise streaming
-	 * fails with all urbs having status = -EOVERFLOW ! */
-	if (dev->analog_xfer_bulk) {
-		dev->max_pkt_size = 512; /* USB 2.0 spec */
-		dev->packet_multiplier = EM28XX_BULK_PACKET_MULTIPLIER;
-	} else { /* isoc */
-		em28xx_coredbg("minimum isoc packet size: %u (alt=%d)\n",
-			       min_pkt_size, dev->alt);
-		dev->max_pkt_size =
-				  dev->alt_max_pkt_size_isoc[dev->alt];
-		dev->packet_multiplier = EM28XX_NUM_ISOC_PACKETS;
-	}
-	em28xx_coredbg("setting alternate %d with wMaxPacketSize=%u\n",
-		       dev->alt, dev->max_pkt_size);
-	errCode = usb_set_interface(dev->udev, 0, dev->alt);
-	if (errCode < 0) {
-		em28xx_errdev("cannot change alternate number to %d (error=%i)\n",
-			      dev->alt, errCode);
-		return errCode;
-	}
-	return 0;
-}
-
 int em28xx_gpio_set(struct em28xx *dev, struct em28xx_reg_seq *gpio)
 {
 	int rc = 0;
@@ -1282,18 +1035,6 @@ int em28xx_init_usb_xfer(struct em28xx *dev, enum em28xx_mode mode,
 EXPORT_SYMBOL_GPL(em28xx_init_usb_xfer);
 
 /*
- * em28xx_wake_i2c()
- * configure i2c attached devices
- */
-void em28xx_wake_i2c(struct em28xx *dev)
-{
-	v4l2_device_call_all(&dev->v4l2_dev, 0, core,  reset, 0);
-	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
-			INPUT(dev->ctl_input)->vmux, 0, 0);
-	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
-}
-
-/*
  * Device control list
  */
 
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index dd19c9ff76e0..70ffe259df5b 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -53,15 +53,23 @@
 
 #define EM28XX_VERSION "0.2.0"
 
+static unsigned int isoc_debug;
+module_param(isoc_debug, int, 0644);
+MODULE_PARM_DESC(isoc_debug, "enable debug messages [isoc transfers]");
+
+static unsigned int disable_vbi;
+module_param(disable_vbi, int, 0644);
+MODULE_PARM_DESC(disable_vbi, "disable vbi support");
+
+static int alt;
+module_param(alt, int, 0644);
+MODULE_PARM_DESC(alt, "alternate setting to use for video endpoint");
+
 #define em28xx_videodbg(fmt, arg...) do {\
 	if (video_debug) \
 		printk(KERN_INFO "%s %s :"fmt, \
 			 dev->name, __func__ , ##arg); } while (0)
 
-static unsigned int isoc_debug;
-module_param(isoc_debug, int, 0644);
-MODULE_PARM_DESC(isoc_debug, "enable debug messages [isoc transfers]");
-
 #define em28xx_isocdbg(fmt, arg...) \
 do {\
 	if (isoc_debug) { \
@@ -135,6 +143,257 @@ static struct em28xx_fmt format[] = {
 	},
 };
 
+/*
+ * em28xx_wake_i2c()
+ * configure i2c attached devices
+ */
+void em28xx_wake_i2c(struct em28xx *dev)
+{
+	v4l2_device_call_all(&dev->v4l2_dev, 0, core,  reset, 0);
+	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
+			INPUT(dev->ctl_input)->vmux, 0, 0);
+	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
+}
+
+int em28xx_colorlevels_set_default(struct em28xx *dev)
+{
+	em28xx_write_reg(dev, EM28XX_R20_YGAIN, CONTRAST_DEFAULT);
+	em28xx_write_reg(dev, EM28XX_R21_YOFFSET, BRIGHTNESS_DEFAULT);
+	em28xx_write_reg(dev, EM28XX_R22_UVGAIN, SATURATION_DEFAULT);
+	em28xx_write_reg(dev, EM28XX_R23_UOFFSET, BLUE_BALANCE_DEFAULT);
+	em28xx_write_reg(dev, EM28XX_R24_VOFFSET, RED_BALANCE_DEFAULT);
+	em28xx_write_reg(dev, EM28XX_R25_SHARPNESS, SHARPNESS_DEFAULT);
+
+	em28xx_write_reg(dev, EM28XX_R14_GAMMA, 0x20);
+	em28xx_write_reg(dev, EM28XX_R15_RGAIN, 0x20);
+	em28xx_write_reg(dev, EM28XX_R16_GGAIN, 0x20);
+	em28xx_write_reg(dev, EM28XX_R17_BGAIN, 0x20);
+	em28xx_write_reg(dev, EM28XX_R18_ROFFSET, 0x00);
+	em28xx_write_reg(dev, EM28XX_R19_GOFFSET, 0x00);
+	return em28xx_write_reg(dev, EM28XX_R1A_BOFFSET, 0x00);
+}
+
+int em28xx_set_outfmt(struct em28xx *dev)
+{
+	int ret;
+	u8 fmt, vinctrl;
+
+	fmt = dev->format->reg;
+	if (!dev->is_em25xx)
+		fmt |= 0x20;
+	/*
+	 * NOTE: it's not clear if this is really needed !
+	 * The datasheets say bit 5 is a reserved bit and devices seem to work
+	 * fine without it. But the Windows driver sets it for em2710/50+em28xx
+	 * devices and we've always been setting it, too.
+	 *
+	 * em2765 (em25xx, em276x/7x/8x) devices do NOT work with this bit set,
+	 * it's likely used for an additional (compressed ?) format there.
+	 */
+	ret = em28xx_write_reg(dev, EM28XX_R27_OUTFMT, fmt);
+	if (ret < 0)
+		return ret;
+
+	ret = em28xx_write_reg(dev, EM28XX_R10_VINMODE, dev->vinmode);
+	if (ret < 0)
+		return ret;
+
+	vinctrl = dev->vinctl;
+	if (em28xx_vbi_supported(dev) == 1) {
+		vinctrl |= EM28XX_VINCTRL_VBI_RAW;
+		em28xx_write_reg(dev, EM28XX_R34_VBI_START_H, 0x00);
+		em28xx_write_reg(dev, EM28XX_R36_VBI_WIDTH, dev->vbi_width/4);
+		em28xx_write_reg(dev, EM28XX_R37_VBI_HEIGHT, dev->vbi_height);
+		if (dev->norm & V4L2_STD_525_60) {
+			/* NTSC */
+			em28xx_write_reg(dev, EM28XX_R35_VBI_START_V, 0x09);
+		} else if (dev->norm & V4L2_STD_625_50) {
+			/* PAL */
+			em28xx_write_reg(dev, EM28XX_R35_VBI_START_V, 0x07);
+		}
+	}
+
+	return em28xx_write_reg(dev, EM28XX_R11_VINCTRL, vinctrl);
+}
+
+static int em28xx_accumulator_set(struct em28xx *dev, u8 xmin, u8 xmax,
+				  u8 ymin, u8 ymax)
+{
+	em28xx_videodbg("em28xx Scale: (%d,%d)-(%d,%d)\n",
+			xmin, ymin, xmax, ymax);
+
+	em28xx_write_regs(dev, EM28XX_R28_XMIN, &xmin, 1);
+	em28xx_write_regs(dev, EM28XX_R29_XMAX, &xmax, 1);
+	em28xx_write_regs(dev, EM28XX_R2A_YMIN, &ymin, 1);
+	return em28xx_write_regs(dev, EM28XX_R2B_YMAX, &ymax, 1);
+}
+
+static void em28xx_capture_area_set(struct em28xx *dev, u8 hstart, u8 vstart,
+				   u16 width, u16 height)
+{
+	u8 cwidth = width >> 2;
+	u8 cheight = height >> 2;
+	u8 overflow = (height >> 9 & 0x02) | (width >> 10 & 0x01);
+	/* NOTE: size limit: 2047x1023 = 2MPix */
+
+	em28xx_videodbg("capture area set to (%d,%d): %dx%d\n",
+		       hstart, vstart,
+		       ((overflow & 2) << 9 | cwidth << 2),
+		       ((overflow & 1) << 10 | cheight << 2));
+
+	em28xx_write_regs(dev, EM28XX_R1C_HSTART, &hstart, 1);
+	em28xx_write_regs(dev, EM28XX_R1D_VSTART, &vstart, 1);
+	em28xx_write_regs(dev, EM28XX_R1E_CWIDTH, &cwidth, 1);
+	em28xx_write_regs(dev, EM28XX_R1F_CHEIGHT, &cheight, 1);
+	em28xx_write_regs(dev, EM28XX_R1B_OFLOW, &overflow, 1);
+
+	/* FIXME: function/meaning of these registers ? */
+	/* FIXME: align width+height to multiples of 4 ?! */
+	if (dev->is_em25xx) {
+		em28xx_write_reg(dev, 0x34, width >> 4);
+		em28xx_write_reg(dev, 0x35, height >> 4);
+	}
+}
+
+static int em28xx_scaler_set(struct em28xx *dev, u16 h, u16 v)
+{
+	u8 mode;
+	/* the em2800 scaler only supports scaling down to 50% */
+
+	if (dev->board.is_em2800) {
+		mode = (v ? 0x20 : 0x00) | (h ? 0x10 : 0x00);
+	} else {
+		u8 buf[2];
+
+		buf[0] = h;
+		buf[1] = h >> 8;
+		em28xx_write_regs(dev, EM28XX_R30_HSCALELOW, (char *)buf, 2);
+
+		buf[0] = v;
+		buf[1] = v >> 8;
+		em28xx_write_regs(dev, EM28XX_R32_VSCALELOW, (char *)buf, 2);
+		/* it seems that both H and V scalers must be active
+		   to work correctly */
+		mode = (h || v) ? 0x30 : 0x00;
+	}
+	return em28xx_write_reg_bits(dev, EM28XX_R26_COMPR, mode, 0x30);
+}
+
+/* FIXME: this only function read values from dev */
+int em28xx_resolution_set(struct em28xx *dev)
+{
+	int width, height;
+	width = norm_maxw(dev);
+	height = norm_maxh(dev);
+
+	/* Properly setup VBI */
+	dev->vbi_width = 720;
+	if (dev->norm & V4L2_STD_525_60)
+		dev->vbi_height = 12;
+	else
+		dev->vbi_height = 18;
+
+	em28xx_set_outfmt(dev);
+
+	em28xx_accumulator_set(dev, 1, (width - 4) >> 2, 1, (height - 4) >> 2);
+
+	/* If we don't set the start position to 2 in VBI mode, we end up
+	   with line 20/21 being YUYV encoded instead of being in 8-bit
+	   greyscale.  The core of the issue is that line 21 (and line 23 for
+	   PAL WSS) are inside of active video region, and as a result they
+	   get the pixelformatting associated with that area.  So by cropping
+	   it out, we end up with the same format as the rest of the VBI
+	   region */
+	if (em28xx_vbi_supported(dev) == 1)
+		em28xx_capture_area_set(dev, 0, 2, width, height);
+	else
+		em28xx_capture_area_set(dev, 0, 0, width, height);
+
+	return em28xx_scaler_set(dev, dev->hscale, dev->vscale);
+}
+
+int em28xx_vbi_supported(struct em28xx *dev)
+{
+	/* Modprobe option to manually disable */
+	if (disable_vbi == 1)
+		return 0;
+
+	if (dev->board.is_webcam)
+		return 0;
+
+	/* FIXME: check subdevices for VBI support */
+
+	if (dev->chip_id == CHIP_ID_EM2860 ||
+	    dev->chip_id == CHIP_ID_EM2883)
+		return 1;
+
+	/* Version of em28xx that does not support VBI */
+	return 0;
+}
+
+/* Set USB alternate setting for analog video */
+int em28xx_set_alternate(struct em28xx *dev)
+{
+	int errCode;
+	int i;
+	unsigned int min_pkt_size = dev->width * 2 + 4;
+
+	/* NOTE: for isoc transfers, only alt settings > 0 are allowed
+		 bulk transfers seem to work only with alt=0 ! */
+	dev->alt = 0;
+	if ((alt > 0) && (alt < dev->num_alt)) {
+		em28xx_videodbg("alternate forced to %d\n", dev->alt);
+		dev->alt = alt;
+		goto set_alt;
+	}
+	if (dev->analog_xfer_bulk)
+		goto set_alt;
+
+	/* When image size is bigger than a certain value,
+	   the frame size should be increased, otherwise, only
+	   green screen will be received.
+	 */
+	if (dev->width * 2 * dev->height > 720 * 240 * 2)
+		min_pkt_size *= 2;
+
+	for (i = 0; i < dev->num_alt; i++) {
+		/* stop when the selected alt setting offers enough bandwidth */
+		if (dev->alt_max_pkt_size_isoc[i] >= min_pkt_size) {
+			dev->alt = i;
+			break;
+		/* otherwise make sure that we end up with the maximum bandwidth
+		   because the min_pkt_size equation might be wrong...
+		*/
+		} else if (dev->alt_max_pkt_size_isoc[i] >
+			   dev->alt_max_pkt_size_isoc[dev->alt])
+			dev->alt = i;
+	}
+
+set_alt:
+	/* NOTE: for bulk transfers, we need to call usb_set_interface()
+	 * even if the previous settings were the same. Otherwise streaming
+	 * fails with all urbs having status = -EOVERFLOW ! */
+	if (dev->analog_xfer_bulk) {
+		dev->max_pkt_size = 512; /* USB 2.0 spec */
+		dev->packet_multiplier = EM28XX_BULK_PACKET_MULTIPLIER;
+	} else { /* isoc */
+		em28xx_videodbg("minimum isoc packet size: %u (alt=%d)\n",
+			       min_pkt_size, dev->alt);
+		dev->max_pkt_size =
+				  dev->alt_max_pkt_size_isoc[dev->alt];
+		dev->packet_multiplier = EM28XX_NUM_ISOC_PACKETS;
+	}
+	em28xx_videodbg("setting alternate %d with wMaxPacketSize=%u\n",
+		       dev->alt, dev->max_pkt_size);
+	errCode = usb_set_interface(dev->udev, 0, dev->alt);
+	if (errCode < 0) {
+		em28xx_errdev("cannot change alternate number to %d (error=%i)\n",
+			      dev->alt, errCode);
+		return errCode;
+	}
+	return 0;
+}
+
 /* ------------------------------------------------------------------
 	DMA and thread functions
    ------------------------------------------------------------------*/
@@ -1817,6 +2076,113 @@ static struct video_device *em28xx_vdev_init(struct em28xx *dev,
 	return vfd;
 }
 
+static void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
+{
+	memset(ctl, 0, sizeof(*ctl));
+
+	ctl->fname   = XC2028_DEFAULT_FIRMWARE;
+	ctl->max_len = 64;
+	ctl->mts = em28xx_boards[dev->model].mts_firmware;
+
+	switch (dev->model) {
+	case EM2880_BOARD_EMPIRE_DUAL_TV:
+	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
+	case EM2882_BOARD_TERRATEC_HYBRID_XS:
+		ctl->demod = XC3028_FE_ZARLINK456;
+		break;
+	case EM2880_BOARD_TERRATEC_HYBRID_XS:
+	case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
+	case EM2881_BOARD_PINNACLE_HYBRID_PRO:
+		ctl->demod = XC3028_FE_ZARLINK456;
+		break;
+	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900_R2:
+	case EM2882_BOARD_PINNACLE_HYBRID_PRO_330E:
+		ctl->demod = XC3028_FE_DEFAULT;
+		break;
+	case EM2880_BOARD_AMD_ATI_TV_WONDER_HD_600:
+		ctl->demod = XC3028_FE_DEFAULT;
+		ctl->fname = XC3028L_DEFAULT_FIRMWARE;
+		break;
+	case EM2883_BOARD_HAUPPAUGE_WINTV_HVR_850:
+	case EM2883_BOARD_HAUPPAUGE_WINTV_HVR_950:
+	case EM2880_BOARD_PINNACLE_PCTV_HD_PRO:
+		/* FIXME: Better to specify the needed IF */
+		ctl->demod = XC3028_FE_DEFAULT;
+		break;
+	case EM2883_BOARD_KWORLD_HYBRID_330U:
+	case EM2882_BOARD_DIKOM_DK300:
+	case EM2882_BOARD_KWORLD_VS_DVBT:
+		ctl->demod = XC3028_FE_CHINA;
+		ctl->fname = XC2028_DEFAULT_FIRMWARE;
+		break;
+	case EM2882_BOARD_EVGA_INDTUBE:
+		ctl->demod = XC3028_FE_CHINA;
+		ctl->fname = XC3028L_DEFAULT_FIRMWARE;
+		break;
+	default:
+		ctl->demod = XC3028_FE_OREN538;
+	}
+}
+
+void em28xx_tuner_setup(struct em28xx *dev)
+{
+	struct tuner_setup           tun_setup;
+	struct v4l2_frequency        f;
+
+	if (dev->tuner_type == TUNER_ABSENT)
+		return;
+
+	memset(&tun_setup, 0, sizeof(tun_setup));
+
+	tun_setup.mode_mask = T_ANALOG_TV | T_RADIO;
+	tun_setup.tuner_callback = em28xx_tuner_callback;
+
+	if (dev->board.radio.type) {
+		tun_setup.type = dev->board.radio.type;
+		tun_setup.addr = dev->board.radio_addr;
+
+		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_type_addr, &tun_setup);
+	}
+
+	if ((dev->tuner_type != TUNER_ABSENT) && (dev->tuner_type)) {
+		tun_setup.type   = dev->tuner_type;
+		tun_setup.addr   = dev->tuner_addr;
+
+		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_type_addr, &tun_setup);
+	}
+
+	if (dev->tda9887_conf) {
+		struct v4l2_priv_tun_config tda9887_cfg;
+
+		tda9887_cfg.tuner = TUNER_TDA9887;
+		tda9887_cfg.priv = &dev->tda9887_conf;
+
+		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_config, &tda9887_cfg);
+	}
+
+	if (dev->tuner_type == TUNER_XC2028) {
+		struct v4l2_priv_tun_config  xc2028_cfg;
+		struct xc2028_ctrl           ctl;
+
+		memset(&xc2028_cfg, 0, sizeof(xc2028_cfg));
+		memset(&ctl, 0, sizeof(ctl));
+
+		em28xx_setup_xc3028(dev, &ctl);
+
+		xc2028_cfg.tuner = TUNER_XC2028;
+		xc2028_cfg.priv  = &ctl;
+
+		v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_config, &xc2028_cfg);
+	}
+
+	/* configure tuner */
+	f.tuner = 0;
+	f.type = V4L2_TUNER_ANALOG_TV;
+	f.frequency = 9076;     /* just a magic number */
+	dev->ctl_freq = f.frequency;
+	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
+}
+
 int em28xx_register_analog_devices(struct em28xx *dev)
 {
 	u8 val;
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 191ef3593891..0259270dda46 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -748,6 +748,7 @@ void em28xx_init_extension(struct em28xx *dev);
 void em28xx_close_extension(struct em28xx *dev);
 
 /* Provided by em28xx-video.c */
+void em28xx_tuner_setup(struct em28xx *dev);
 int em28xx_vb2_setup(struct em28xx *dev);
 int em28xx_register_analog_devices(struct em28xx *dev);
 void em28xx_release_analog_resources(struct em28xx *dev);
-- 
1.8.3.1

[PATCH v3 02/24] em28xx: some cosmetic changes

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

In order to make easier for the next patches, do some
cosmetic changes.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c |  2 +-
 drivers/media/usb/em28xx/em28xx-video.c |  2 --
 drivers/media/usb/em28xx/em28xx.h       | 11 ++++++-----
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 19827e79cf53..551cbc294190 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2106,7 +2106,7 @@ struct em28xx_board em28xx_boards[] = {
 	},
 	/* 1b80:e1cc Delock 61959
 	 * Empia EM2874B + Micronas DRX 3913KA2 + NXP TDA18271HDC2
-         * mostly the same as MaxMedia UB-425-TC but different remote */
+	 * mostly the same as MaxMedia UB-425-TC but different remote */
 	[EM2874_BOARD_DELOCK_61959] = {
 		.name          = "Delock 61959",
 		.tuner_type    = TUNER_ABSENT,
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 70ffe259df5b..8b8a4eb96875 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -51,8 +51,6 @@
 
 #define DRIVER_DESC         "Empia em28xx based USB video device driver"
 
-#define EM28XX_VERSION "0.2.0"
-
 static unsigned int isoc_debug;
 module_param(isoc_debug, int, 0644);
 MODULE_PARM_DESC(isoc_debug, "enable debug messages [isoc transfers]");
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 0259270dda46..7ae05ebc13c1 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -26,6 +26,8 @@
 #ifndef _EM28XX_H
 #define _EM28XX_H
 
+#define EM28XX_VERSION "0.2.0"
+
 #include <linux/workqueue.h>
 #include <linux/i2c.h>
 #include <linux/mutex.h>
@@ -522,9 +524,12 @@ struct em28xx {
 	int model;		/* index in the device_data struct */
 	int devno;		/* marks the number of this device */
 	enum em28xx_chip_id chip_id;
-	unsigned int is_em25xx:1;	/* em25xx/em276x/7x/8x family bridge */
 
+	unsigned int is_em25xx:1;	/* em25xx/em276x/7x/8x family bridge */
 	unsigned char disconnected:1;	/* device has been diconnected */
+	unsigned int has_audio_class:1;
+	unsigned int has_alsa_audio:1;
+	unsigned int is_audio_only:1;
 
 	int audio_ifnum;
 
@@ -544,10 +549,6 @@ struct em28xx {
 	/* Vinmode/Vinctl used at the driver */
 	int vinmode, vinctl;
 
-	unsigned int has_audio_class:1;
-	unsigned int has_alsa_audio:1;
-	unsigned int is_audio_only:1;
-
 	/* Controls audio streaming */
 	struct work_struct wq_trigger;	/* Trigger to start/stop audio for alsa module */
 	atomic_t       stream_started;	/* stream should be running if true */
-- 
1.8.3.1

[PATCH v3 03/24] em28xx: move analog-specific init to em28xx-video

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

There are several init code inside em28xx-cards that are actually
part of analog initialization. Move the code to em28x-video, in
order to remove part of the mess.

In thesis, no functional changes so far.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 77 ---------------------------
 drivers/media/usb/em28xx/em28xx-video.c | 94 ++++++++++++++++++++++++++++++---
 2 files changed, 87 insertions(+), 84 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 551cbc294190..4a439e5031e6 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2907,7 +2907,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 			   struct usb_interface *interface,
 			   int minor)
 {
-	struct v4l2_ctrl_handler *hdl = &dev->ctrl_handler;
 	int retval;
 	static const char *default_chip_name = "em28xx";
 	const char *chip_name = default_chip_name;
@@ -3034,15 +3033,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 		}
 	}
 
-	retval = v4l2_device_register(&interface->dev, &dev->v4l2_dev);
-	if (retval < 0) {
-		em28xx_errdev("Call to v4l2_device_register() failed!\n");
-		return retval;
-	}
-
-	v4l2_ctrl_handler_init(hdl, 8);
-	dev->v4l2_dev.ctrl_handler = hdl;
-
 	rt_mutex_init(&dev->i2c_bus_lock);
 
 	/* register i2c bus 0 */
@@ -3071,75 +3061,11 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 		}
 	}
 
-	/*
-	 * Default format, used for tvp5150 or saa711x output formats
-	 */
-	dev->vinmode = 0x10;
-	dev->vinctl  = EM28XX_VINCTRL_INTERLACED |
-		       EM28XX_VINCTRL_CCIR656_ENABLE;
-
 	/* Do board specific init and eeprom reading */
 	em28xx_card_setup(dev);
 
-	/* Configure audio */
-	retval = em28xx_audio_setup(dev);
-	if (retval < 0) {
-		em28xx_errdev("%s: Error while setting audio - error [%d]!\n",
-			__func__, retval);
-		goto fail;
-	}
-	if (dev->audio_mode.ac97 != EM28XX_NO_AC97) {
-		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
-			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
-		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
-			V4L2_CID_AUDIO_VOLUME, 0, 0x1f, 1, 0x1f);
-	} else {
-		/* install the em28xx notify callback */
-		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_MUTE),
-				em28xx_ctrl_notify, dev);
-		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_VOLUME),
-				em28xx_ctrl_notify, dev);
-	}
-
-	/* wake i2c devices */
-	em28xx_wake_i2c(dev);
-
-	/* init video dma queues */
-	INIT_LIST_HEAD(&dev->vidq.active);
-	INIT_LIST_HEAD(&dev->vbiq.active);
-
-	if (dev->board.has_msp34xx) {
-		/* Send a reset to other chips via gpio */
-		retval = em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xf7);
-		if (retval < 0) {
-			em28xx_errdev("%s: em28xx_write_reg - "
-				      "msp34xx(1) failed! error [%d]\n",
-				      __func__, retval);
-			goto fail;
-		}
-		msleep(3);
-
-		retval = em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xff);
-		if (retval < 0) {
-			em28xx_errdev("%s: em28xx_write_reg - "
-				      "msp34xx(2) failed! error [%d]\n",
-				      __func__, retval);
-			goto fail;
-		}
-		msleep(3);
-	}
-
-	retval = em28xx_register_analog_devices(dev);
-	if (retval < 0) {
-		goto fail;
-	}
-
-	/* Save some power by putting tuner to sleep */
-	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
-
 	return 0;
 
-fail:
 	if (dev->def_i2c_bus)
 		em28xx_i2c_unregister(dev, 1);
 	em28xx_i2c_unregister(dev, 0);
@@ -3388,9 +3314,6 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 	/* save our data pointer in this interface device */
 	usb_set_intfdata(interface, dev);
 
-	/* initialize videobuf2 stuff */
-	em28xx_vb2_setup(dev);
-
 	/* allocate device struct */
 	mutex_init(&dev->lock);
 	mutex_lock(&dev->lock);
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 8b8a4eb96875..63f09b62d546 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2186,10 +2186,80 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 	u8 val;
 	int ret;
 	unsigned int maxw;
+	struct v4l2_ctrl_handler *hdl = &dev->ctrl_handler;
+
+	if (!dev->has_video) {
+		/* This device does not support the v4l2 extension */
+		return 0;
+	}
 
 	printk(KERN_INFO "%s: v4l2 driver version %s\n",
 		dev->name, EM28XX_VERSION);
 
+	mutex_lock(&dev->lock);
+
+	ret = v4l2_device_register(&dev->udev->dev, &dev->v4l2_dev);
+	if (ret < 0) {
+		em28xx_errdev("Call to v4l2_device_register() failed!\n");
+		goto err;
+	}
+
+	v4l2_ctrl_handler_init(hdl, 8);
+	dev->v4l2_dev.ctrl_handler = hdl;
+
+	/*
+	 * Default format, used for tvp5150 or saa711x output formats
+	 */
+	dev->vinmode = 0x10;
+	dev->vinctl  = EM28XX_VINCTRL_INTERLACED |
+		       EM28XX_VINCTRL_CCIR656_ENABLE;
+
+	/* Configure audio */
+	ret = em28xx_audio_setup(dev);
+	if (ret < 0) {
+		em28xx_errdev("%s: Error while setting audio - error [%d]!\n",
+			__func__, ret);
+		goto err;
+	}
+	if (dev->audio_mode.ac97 != EM28XX_NO_AC97) {
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
+			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
+			V4L2_CID_AUDIO_VOLUME, 0, 0x1f, 1, 0x1f);
+	} else {
+		/* install the em28xx notify callback */
+		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_MUTE),
+				em28xx_ctrl_notify, dev);
+		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_VOLUME),
+				em28xx_ctrl_notify, dev);
+	}
+
+	/* wake i2c devices */
+	em28xx_wake_i2c(dev);
+
+	/* init video dma queues */
+	INIT_LIST_HEAD(&dev->vidq.active);
+	INIT_LIST_HEAD(&dev->vbiq.active);
+
+	if (dev->board.has_msp34xx) {
+		/* Send a reset to other chips via gpio */
+		ret = em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xf7);
+		if (ret < 0) {
+			em28xx_errdev("%s: em28xx_write_reg - msp34xx(1) failed! error [%d]\n",
+				      __func__, ret);
+			goto err;
+		}
+		msleep(3);
+
+		ret = em28xx_write_reg(dev, EM2820_R08_GPIO_CTRL, 0xff);
+		if (ret < 0) {
+			em28xx_errdev("%s: em28xx_write_reg - msp34xx(2) failed! error [%d]\n",
+				      __func__, ret);
+			goto err;
+		}
+		msleep(3);
+	}
+
 	/* set default norm */
 	dev->norm = V4L2_STD_PAL;
 	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_std, dev->norm);
@@ -2252,14 +2322,16 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 	/* Reset image controls */
 	em28xx_colorlevels_set_default(dev);
 	v4l2_ctrl_handler_setup(&dev->ctrl_handler);
-	if (dev->ctrl_handler.error)
-		return dev->ctrl_handler.error;
+	ret = dev->ctrl_handler.error;
+	if (ret)
+		goto err;
 
 	/* allocate and fill video video_device struct */
 	dev->vdev = em28xx_vdev_init(dev, &em28xx_video_template, "video");
 	if (!dev->vdev) {
 		em28xx_errdev("cannot allocate video_device.\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto err;
 	}
 	dev->vdev->queue = &dev->vb_vidq;
 	dev->vdev->queue->lock = &dev->vb_queue_lock;
@@ -2289,7 +2361,7 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 	if (ret) {
 		em28xx_errdev("unable to register video device (error=%i).\n",
 			      ret);
-		return ret;
+		goto err;
 	}
 
 	/* Allocate and fill vbi video_device struct */
@@ -2318,7 +2390,7 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 					    vbi_nr[dev->devno]);
 		if (ret < 0) {
 			em28xx_errdev("unable to register vbi device\n");
-			return ret;
+			goto err;
 		}
 	}
 
@@ -2327,13 +2399,14 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 						  "radio");
 		if (!dev->radio_dev) {
 			em28xx_errdev("cannot allocate video_device.\n");
-			return -ENODEV;
+			ret = -ENODEV;
+			goto err;
 		}
 		ret = video_register_device(dev->radio_dev, VFL_TYPE_RADIO,
 					    radio_nr[dev->devno]);
 		if (ret < 0) {
 			em28xx_errdev("can't register radio device\n");
-			return ret;
+			goto err;
 		}
 		em28xx_info("Registered radio device as %s\n",
 			    video_device_node_name(dev->radio_dev));
@@ -2346,5 +2419,12 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 		em28xx_info("V4L2 VBI device registered as %s\n",
 			    video_device_node_name(dev->vbi_dev));
 
+	/* Save some power by putting tuner to sleep */
+	v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
+
+	/* initialize videobuf2 stuff */
+	em28xx_vb2_setup(dev);
 	return 0;
+
+err:
 }
-- 
1.8.3.1

[PATCH v3 04/24] em28xx: make em28xx-video to be a separate module

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Now that all analog-specific code are at em28xx-video, convert
it into an em28xx extension and load it as a separate module.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/Kconfig         |  6 ++-
 drivers/media/usb/em28xx/Makefile        |  5 ++-
 drivers/media/usb/em28xx/em28xx-camera.c |  1 +
 drivers/media/usb/em28xx/em28xx-cards.c  | 25 +++--------
 drivers/media/usb/em28xx/em28xx-core.c   | 12 +++++
 drivers/media/usb/em28xx/em28xx-v4l.h    | 24 ++++++++++
 drivers/media/usb/em28xx/em28xx-vbi.c    |  1 +
 drivers/media/usb/em28xx/em28xx-video.c  | 77 ++++++++++++++++++++++----------
 drivers/media/usb/em28xx/em28xx.h        | 23 ++--------
 9 files changed, 110 insertions(+), 64 deletions(-)
 create mode 100644 drivers/media/usb/em28xx/em28xx-v4l.h

diff --git a/drivers/media/usb/em28xx/Kconfig b/drivers/media/usb/em28xx/Kconfig
index d6ba514d31eb..838fc9dbb747 100644
--- a/drivers/media/usb/em28xx/Kconfig
+++ b/drivers/media/usb/em28xx/Kconfig
@@ -1,8 +1,12 @@
 config VIDEO_EM28XX
-	tristate "Empia EM28xx USB video capture support"
+	tristate "Empia EM28xx USB devices support"
 	depends on VIDEO_DEV && I2C
 	select VIDEO_TUNER
 	select VIDEO_TVEEPROM
+
+config VIDEO_EM28XX_V4L2
+	tristate "Empia EM28xx analog TV, video capture and/or webcam support"
+	depends on VIDEO_EM28XX && I2C
 	select VIDEOBUF2_VMALLOC
 	select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEO_TVP5150 if MEDIA_SUBDRV_AUTOSELECT
diff --git a/drivers/media/usb/em28xx/Makefile b/drivers/media/usb/em28xx/Makefile
index ad6d48557940..3f850d5063d0 100644
--- a/drivers/media/usb/em28xx/Makefile
+++ b/drivers/media/usb/em28xx/Makefile
@@ -1,10 +1,11 @@
-em28xx-y +=	em28xx-video.o em28xx-i2c.o em28xx-cards.o
-em28xx-y +=	em28xx-core.o  em28xx-vbi.o em28xx-camera.o
+em28xx-y +=	em28xx-core.o em28xx-i2c.o em28xx-cards.o em28xx-camera.o
 
+em28xx-v4l-objs := em28xx-video.o em28xx-vbi.o
 em28xx-alsa-objs := em28xx-audio.o
 em28xx-rc-objs := em28xx-input.o
 
 obj-$(CONFIG_VIDEO_EM28XX) += em28xx.o
+obj-$(CONFIG_VIDEO_EM28XX_V4L2) += em28xx-v4l.o
 obj-$(CONFIG_VIDEO_EM28XX_ALSA) += em28xx-alsa.o
 obj-$(CONFIG_VIDEO_EM28XX_DVB) += em28xx-dvb.o
 obj-$(CONFIG_VIDEO_EM28XX_RC) += em28xx-rc.o
diff --git a/drivers/media/usb/em28xx/em28xx-camera.c b/drivers/media/usb/em28xx/em28xx-camera.c
index d666741797d4..c29f5c4e7b40 100644
--- a/drivers/media/usb/em28xx/em28xx-camera.c
+++ b/drivers/media/usb/em28xx/em28xx-camera.c
@@ -454,3 +454,4 @@ int em28xx_init_camera(struct em28xx *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(em28xx_init_camera);
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 4a439e5031e6..53dc82409bc2 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2159,6 +2159,8 @@ struct em28xx_board em28xx_boards[] = {
 		.ir_codes      = RC_MAP_PINNACLE_PCTV_HD,
 	},
 };
+EXPORT_SYMBOL_GPL(em28xx_boards);
+
 const unsigned int em28xx_bcount = ARRAY_SIZE(em28xx_boards);
 
 /* table of devices that work with this driver */
@@ -2827,10 +2829,6 @@ static void em28xx_card_setup(struct em28xx *dev)
 				"tuner", dev->tuner_addr, NULL);
 		}
 	}
-
-	em28xx_tuner_setup(dev);
-
-	em28xx_init_camera(dev);
 }
 
 
@@ -2848,11 +2846,12 @@ static void request_module_async(struct work_struct *work)
 	em28xx_init_extension(dev);
 
 #if defined(CONFIG_MODULES) && defined(MODULE)
+	if (dev->has_video)
+		request_module("em28xx-v4l");
 	if (dev->has_audio_class)
 		request_module("snd-usb-audio");
 	else if (dev->has_alsa_audio)
 		request_module("em28xx-alsa");
-
 	if (dev->board.has_dvb)
 		request_module("em28xx-dvb");
 	if (dev->board.buttons ||
@@ -2881,18 +2880,12 @@ void em28xx_release_resources(struct em28xx *dev)
 {
 	/*FIXME: I2C IR should be disconnected */
 
-	em28xx_release_analog_resources(dev);
-
 	if (dev->def_i2c_bus)
 		em28xx_i2c_unregister(dev, 1);
 	em28xx_i2c_unregister(dev, 0);
 	if (dev->clk)
 		v4l2_clk_unregister_fixed(dev->clk);
 
-	v4l2_ctrl_handler_free(&dev->ctrl_handler);
-
-	v4l2_device_unregister(&dev->v4l2_dev);
-
 	usb_put_dev(dev->udev);
 
 	/* Mark device as unused */
@@ -3277,6 +3270,7 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 	dev->alt   = -1;
 	dev->is_audio_only = has_audio && !(has_video || has_dvb);
 	dev->has_alsa_audio = has_audio;
+	dev->has_video = has_video;
 	dev->audio_ifnum = ifnum;
 
 	/* Checks if audio is provided by some interface */
@@ -3316,10 +3310,9 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 
 	/* allocate device struct */
 	mutex_init(&dev->lock);
-	mutex_lock(&dev->lock);
 	retval = em28xx_init_dev(dev, udev, interface, nr);
 	if (retval) {
-		goto unlock_and_free;
+		goto err_free;
 	}
 
 	if (usb_xfer_mode < 0) {
@@ -3362,7 +3355,7 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 		if (retval) {
 			printk(DRIVER_NAME
 			       ": Failed to pre-allocate USB transfer buffers for DVB.\n");
-			goto unlock_and_free;
+			goto err_free;
 		}
 	}
 
@@ -3371,13 +3364,9 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 	/* Should be the last thing to do, to avoid newer udev's to
 	   open the device before fully initializing it
 	 */
-	mutex_unlock(&dev->lock);
 
 	return 0;
 
-unlock_and_free:
-	mutex_unlock(&dev->lock);
-
 err_free:
 	kfree(dev->alt_max_pkt_size_isoc);
 	kfree(dev);
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index 3012912d2997..1113d4e107d8 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -33,6 +33,18 @@
 
 #include "em28xx.h"
 
+#define DRIVER_AUTHOR "Ludovico Cavedon <cavedon@sssup.it>, " \
+		      "Markus Rechberger <mrechberger@gmail.com>, " \
+		      "Mauro Carvalho Chehab <mchehab@infradead.org>, " \
+		      "Sascha Sommer <saschasommer@freenet.de>"
+
+#define DRIVER_DESC         "Empia em28xx based USB core driver"
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
+MODULE_VERSION(EM28XX_VERSION);
+
 /* #define ENABLE_DEBUG_ISOC_FRAMES */
 
 static unsigned int core_debug;
diff --git a/drivers/media/usb/em28xx/em28xx-v4l.h b/drivers/media/usb/em28xx/em28xx-v4l.h
new file mode 100644
index 000000000000..2cf75a547dbe
--- /dev/null
+++ b/drivers/media/usb/em28xx/em28xx-v4l.h
@@ -0,0 +1,24 @@
+/*
+   em28xx-video.c - driver for Empia EM2800/EM2820/2840 USB
+                    video capture devices
+
+   Copyright (C) 2013 Mauro Carvalho Chehab <m.chehab@samsung.com>
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+
+int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count);
+int em28xx_stop_vbi_streaming(struct vb2_queue *vq);
+extern struct vb2_ops em28xx_vbi_qops;
diff --git a/drivers/media/usb/em28xx/em28xx-vbi.c b/drivers/media/usb/em28xx/em28xx-vbi.c
index 39f39c527c13..db3d655600df 100644
--- a/drivers/media/usb/em28xx/em28xx-vbi.c
+++ b/drivers/media/usb/em28xx/em28xx-vbi.c
@@ -27,6 +27,7 @@
 #include <linux/init.h>
 
 #include "em28xx.h"
+#include "em28xx-v4l.h"
 
 static unsigned int vbibufs = 5;
 module_param(vbibufs, int, 0644);
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 63f09b62d546..b0b1a3d4534b 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -38,6 +38,7 @@
 #include <linux/slab.h>
 
 #include "em28xx.h"
+#include "em28xx-v4l.h"
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-event.h>
@@ -141,11 +142,13 @@ static struct em28xx_fmt format[] = {
 	},
 };
 
+static int em28xx_vbi_supported(struct em28xx *dev);
+
 /*
  * em28xx_wake_i2c()
  * configure i2c attached devices
  */
-void em28xx_wake_i2c(struct em28xx *dev)
+static void em28xx_wake_i2c(struct em28xx *dev)
 {
 	v4l2_device_call_all(&dev->v4l2_dev, 0, core,  reset, 0);
 	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_routing,
@@ -153,7 +156,7 @@ void em28xx_wake_i2c(struct em28xx *dev)
 	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
 }
 
-int em28xx_colorlevels_set_default(struct em28xx *dev)
+static int em28xx_colorlevels_set_default(struct em28xx *dev)
 {
 	em28xx_write_reg(dev, EM28XX_R20_YGAIN, CONTRAST_DEFAULT);
 	em28xx_write_reg(dev, EM28XX_R21_YOFFSET, BRIGHTNESS_DEFAULT);
@@ -171,7 +174,7 @@ int em28xx_colorlevels_set_default(struct em28xx *dev)
 	return em28xx_write_reg(dev, EM28XX_R1A_BOFFSET, 0x00);
 }
 
-int em28xx_set_outfmt(struct em28xx *dev)
+static int em28xx_set_outfmt(struct em28xx *dev)
 {
 	int ret;
 	u8 fmt, vinctrl;
@@ -278,7 +281,7 @@ static int em28xx_scaler_set(struct em28xx *dev, u16 h, u16 v)
 }
 
 /* FIXME: this only function read values from dev */
-int em28xx_resolution_set(struct em28xx *dev)
+static int em28xx_resolution_set(struct em28xx *dev)
 {
 	int width, height;
 	width = norm_maxw(dev);
@@ -310,7 +313,7 @@ int em28xx_resolution_set(struct em28xx *dev)
 	return em28xx_scaler_set(dev, dev->hscale, dev->vscale);
 }
 
-int em28xx_vbi_supported(struct em28xx *dev)
+static int em28xx_vbi_supported(struct em28xx *dev)
 {
 	/* Modprobe option to manually disable */
 	if (disable_vbi == 1)
@@ -330,7 +333,7 @@ int em28xx_vbi_supported(struct em28xx *dev)
 }
 
 /* Set USB alternate setting for analog video */
-int em28xx_set_alternate(struct em28xx *dev)
+static int em28xx_set_alternate(struct em28xx *dev)
 {
 	int errCode;
 	int i;
@@ -1020,7 +1023,7 @@ static struct vb2_ops em28xx_video_qops = {
 	.wait_finish    = vb2_ops_wait_finish,
 };
 
-int em28xx_vb2_setup(struct em28xx *dev)
+static int em28xx_vb2_setup(struct em28xx *dev)
 {
 	int rc;
 	struct vb2_queue *q;
@@ -1088,7 +1091,7 @@ static void video_mux(struct em28xx *dev, int index)
 	em28xx_audio_analog_set(dev);
 }
 
-void em28xx_ctrl_notify(struct v4l2_ctrl *ctrl, void *priv)
+static void em28xx_ctrl_notify(struct v4l2_ctrl *ctrl, void *priv)
 {
 	struct em28xx *dev = priv;
 
@@ -1625,7 +1628,7 @@ static int vidioc_g_register(struct file *file, void *priv,
 		reg->val = ret;
 	} else {
 		__le16 val = 0;
-		ret = em28xx_read_reg_req_len(dev, USB_REQ_GET_STATUS,
+		ret = dev->em28xx_read_reg_req_len(dev, USB_REQ_GET_STATUS,
 						   reg->reg, (char *)&val, 2);
 		if (ret < 0)
 			return ret;
@@ -1872,11 +1875,11 @@ static int em28xx_v4l2_open(struct file *filp)
 }
 
 /*
- * em28xx_realease_resources()
+ * em28xx_v4l2_fini()
  * unregisters the v4l2,i2c and usb devices
  * called when the device gets disconected or at module unload
 */
-void em28xx_release_analog_resources(struct em28xx *dev)
+static int em28xx_v4l2_fini(struct em28xx *dev)
 {
 
 	/*FIXME: I2C IR should be disconnected */
@@ -1906,6 +1909,8 @@ void em28xx_release_analog_resources(struct em28xx *dev)
 			video_device_release(dev->vdev);
 		dev->vdev = NULL;
 	}
+
+	return 0;
 }
 
 /*
@@ -1924,16 +1929,18 @@ static int em28xx_v4l2_close(struct file *filp)
 	vb2_fop_release(filp);
 	mutex_lock(&dev->lock);
 
+	if (dev->disconnected) {
+		v4l2_ctrl_handler_free(&dev->ctrl_handler);
+		v4l2_device_unregister(&dev->v4l2_dev);
+
+		kfree(dev->alt_max_pkt_size_isoc);
+		dev->users = 0;
+		goto exit;
+	}
+
 	if (dev->users == 1) {
 		/* the device is already disconnect,
 		   free the remaining resources */
-		if (dev->disconnected) {
-			em28xx_release_resources(dev);
-			kfree(dev->alt_max_pkt_size_isoc);
-			mutex_unlock(&dev->lock);
-			kfree(dev);
-			return 0;
-		}
 
 		/* Save some power by putting tuner to sleep */
 		v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
@@ -1952,6 +1959,7 @@ static int em28xx_v4l2_close(struct file *filp)
 	}
 
 	dev->users--;
+exit:
 	mutex_unlock(&dev->lock);
 	return 0;
 }
@@ -2047,8 +2055,6 @@ static struct video_device em28xx_radio_template = {
 
 /******************************** usb interface ******************************/
 
-
-
 static struct video_device *em28xx_vdev_init(struct em28xx *dev,
 					const struct video_device *template,
 					const char *type_name)
@@ -2122,7 +2128,7 @@ static void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
 	}
 }
 
-void em28xx_tuner_setup(struct em28xx *dev)
+static void em28xx_tuner_setup(struct em28xx *dev)
 {
 	struct tuner_setup           tun_setup;
 	struct v4l2_frequency        f;
@@ -2181,7 +2187,7 @@ void em28xx_tuner_setup(struct em28xx *dev)
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_frequency, &f);
 }
 
-int em28xx_register_analog_devices(struct em28xx *dev)
+static int em28xx_v4l2_init(struct em28xx *dev)
 {
 	u8 val;
 	int ret;
@@ -2214,6 +2220,10 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 	dev->vinctl  = EM28XX_VINCTRL_INTERLACED |
 		       EM28XX_VINCTRL_CCIR656_ENABLE;
 
+        /* Initialize tuner and camera */
+	em28xx_tuner_setup(dev);
+	em28xx_init_camera(dev);
+
 	/* Configure audio */
 	ret = em28xx_audio_setup(dev);
 	if (ret < 0) {
@@ -2424,7 +2434,28 @@ int em28xx_register_analog_devices(struct em28xx *dev)
 
 	/* initialize videobuf2 stuff */
 	em28xx_vb2_setup(dev);
-	return 0;
 
 err:
+	mutex_unlock(&dev->lock);
+	return ret;
+}
+
+static struct em28xx_ops v4l2_ops = {
+	.id   = EM28XX_V4L2,
+	.name = "Em28xx v4l2 Extension",
+	.init = em28xx_v4l2_init,
+	.fini = em28xx_v4l2_fini,
+};
+
+static int __init em28xx_dvb_register(void)
+{
+	return em28xx_register_extension(&v4l2_ops);
 }
+
+static void __exit em28xx_dvb_unregister(void)
+{
+	em28xx_unregister_extension(&v4l2_ops);
+}
+
+module_init(em28xx_dvb_register);
+module_exit(em28xx_dvb_unregister);
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 7ae05ebc13c1..9d6f43e4681f 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -26,7 +26,7 @@
 #ifndef _EM28XX_H
 #define _EM28XX_H
 
-#define EM28XX_VERSION "0.2.0"
+#define EM28XX_VERSION "0.2.1"
 
 #include <linux/workqueue.h>
 #include <linux/i2c.h>
@@ -474,6 +474,7 @@ struct em28xx_eeprom {
 #define EM28XX_AUDIO   0x10
 #define EM28XX_DVB     0x20
 #define EM28XX_RC      0x30
+#define EM28XX_V4L2    0x40
 
 /* em28xx resource types (used for res_get/res_lock etc */
 #define EM28XX_RESOURCE_VIDEO 0x01
@@ -527,6 +528,7 @@ struct em28xx {
 
 	unsigned int is_em25xx:1;	/* em25xx/em276x/7x/8x family bridge */
 	unsigned char disconnected:1;	/* device has been diconnected */
+	unsigned int has_video:1;
 	unsigned int has_audio_class:1;
 	unsigned int has_alsa_audio:1;
 	unsigned int is_audio_only:1;
@@ -723,14 +725,9 @@ int em28xx_write_ac97(struct em28xx *dev, u8 reg, u16 val);
 int em28xx_audio_analog_set(struct em28xx *dev);
 int em28xx_audio_setup(struct em28xx *dev);
 
-int em28xx_colorlevels_set_default(struct em28xx *dev);
 const struct em28xx_led *em28xx_find_led(struct em28xx *dev,
 					 enum em28xx_led_role role);
 int em28xx_capture_start(struct em28xx *dev, int start);
-int em28xx_vbi_supported(struct em28xx *dev);
-int em28xx_set_outfmt(struct em28xx *dev);
-int em28xx_resolution_set(struct em28xx *dev);
-int em28xx_set_alternate(struct em28xx *dev);
 int em28xx_alloc_urbs(struct em28xx *dev, enum em28xx_mode mode, int xfer_bulk,
 		      int num_bufs, int max_pkt_size, int packet_multiplier);
 int em28xx_init_usb_xfer(struct em28xx *dev, enum em28xx_mode mode,
@@ -742,31 +739,17 @@ void em28xx_uninit_usb_xfer(struct em28xx *dev, enum em28xx_mode mode);
 void em28xx_stop_urbs(struct em28xx *dev);
 int em28xx_set_mode(struct em28xx *dev, enum em28xx_mode set_mode);
 int em28xx_gpio_set(struct em28xx *dev, struct em28xx_reg_seq *gpio);
-void em28xx_wake_i2c(struct em28xx *dev);
 int em28xx_register_extension(struct em28xx_ops *dev);
 void em28xx_unregister_extension(struct em28xx_ops *dev);
 void em28xx_init_extension(struct em28xx *dev);
 void em28xx_close_extension(struct em28xx *dev);
 
-/* Provided by em28xx-video.c */
-void em28xx_tuner_setup(struct em28xx *dev);
-int em28xx_vb2_setup(struct em28xx *dev);
-int em28xx_register_analog_devices(struct em28xx *dev);
-void em28xx_release_analog_resources(struct em28xx *dev);
-void em28xx_ctrl_notify(struct v4l2_ctrl *ctrl, void *priv);
-int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count);
-int em28xx_stop_vbi_streaming(struct vb2_queue *vq);
-extern const struct v4l2_ctrl_ops em28xx_ctrl_ops;
-
 /* Provided by em28xx-cards.c */
 extern struct em28xx_board em28xx_boards[];
 extern struct usb_device_id em28xx_id_table[];
 int em28xx_tuner_callback(void *ptr, int component, int command, int arg);
 void em28xx_release_resources(struct em28xx *dev);
 
-/* Provided by em28xx-vbi.c */
-extern struct vb2_ops em28xx_vbi_qops;
-
 /* Provided by em28xx-camera.c */
 int em28xx_detect_sensor(struct em28xx *dev);
 int em28xx_init_camera(struct em28xx *dev);
-- 
1.8.3.1

[PATCH v3 05/24] em28xx: initialize analog I2C devices at the right place

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

In order to initialize the analog tuner, v4l2 should be registere
first, or otherwise we get an oops:

[   51.783537] BUG: unable to handle kernel NULL pointer dereference at        )
[   51.784479] IP: [<ffffffff81319fbb>] __list_add+0x1b/0xc0
[   51.784479] PGD 0
[   51.784479] Oops: 0000 [#1] SMP
[   51.784479] Modules linked in: tvp5150 em28xx(+) tveeprom v4l2_common videode
[   51.784479] CPU: 0 PID: 946 Comm: systemd-udevd Not tainted 3.13.0-rc1+ #38
[   51.784479] Hardware name: PCCHIPS P17G/P17G, BIOS 080012  05/14/2008
[   51.784479] task: ffff880027482080 ti: ffff88003c9b6000 task.ti: ffff88003c90
[   51.784479] RIP: 0010:[<ffffffff81319fbb>]  [<ffffffff81319fbb>] __list_add+0
[   51.784479] RSP: 0018:ffff88003c9b7a10  EFLAGS: 00010246
[   51.784479] RAX: 0000000000000000 RBX: ffff880036d12428 RCX: 0000000000000000
[   51.784479] RDX: ffff880036ce6040 RSI: 0000000000000000 RDI: ffff880036d12428
[   51.784479] RBP: ffff88003c9b7a28 R08: 0000000000000000 R09: 0000000000000001
[   51.784479] R10: 0000000000000001 R11: 0000000000000000 R12: ffff880036ce6040
[   51.784479] R13: 0000000000000000 R14: ffff880036ce62c0 R15: ffffffffa045c176
[   51.784479] FS:  00007fba89124880(0000) GS:ffff88003f400000(0000) knlGS:00000
[   51.784479] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[   51.784479] CR2: 0000000000000000 CR3: 000000003bccf000 CR4: 00000000000007f0
[   51.784479] Stack:
[   51.784479]  ffff880036d12428 ffff880036ce6038 0000000000000000 ffff88003c9b0
[   51.784479]  ffffffffa0425bbc ffff880028246800 ffff880036d12428 ffff880036ce8
[   51.784479]  ffff88003c9b7a80 ffffffffa044d733 ffff88003c9b7a90 ffff880036ce8
[   51.784479] Call Trace:
[   51.784479]  [<ffffffffa0425bbc>] v4l2_device_register_subdev+0xdc/0x120 [vi]
[   51.784479]  [<ffffffffa044d733>] v4l2_i2c_new_subdev_board+0xa3/0x100 [v4l2]
[   51.784479]  [<ffffffffa044d7fa>] v4l2_i2c_new_subdev+0x6a/0x90 [v4l2_common]
[   51.784479]  [<ffffffffa0455dcb>] em28xx_usb_probe+0xd3b/0x10a0 [em28xx]
[   51.784479]  [<ffffffff81478f74>] usb_probe_interface+0x1c4/0x2f0
[   51.784479]  [<ffffffff81400127>] driver_probe_device+0x87/0x390
[   51.784479]  [<ffffffff81400503>] __driver_attach+0x93/0xa0
[   51.784479]  [<ffffffff81400470>] ? __device_attach+0x40/0x40
[   51.784479]  [<ffffffff813fe153>] bus_for_each_dev+0x63/0xa0
[   51.784479]  [<ffffffff813ffb7e>] driver_attach+0x1e/0x20
[   51.784479]  [<ffffffff813ff760>] bus_add_driver+0x180/0x250
[   51.784479]  [<ffffffff81400b34>] driver_register+0x64/0xf0
[   51.784479]  [<ffffffff81477751>] usb_register_driver+0x81/0x160
[   51.784479]  [<ffffffffa0467000>] ? 0xffffffffa0466fff
[   51.784479]  [<ffffffffa046701e>] em28xx_usb_driver_init+0x1e/0x1000 [em28xx]
[   51.784479]  [<ffffffff8100214a>] do_one_initcall+0xfa/0x1b0
[   51.784479]  [<ffffffff81053793>] ? set_memory_nx+0x43/0x50
[   51.784479]  [<ffffffff810d9926>] load_module+0x1bc6/0x24b0
[   51.784479]  [<ffffffff810d5940>] ? store_uevent+0x40/0x40
[   51.784479]  [<ffffffff810da386>] SyS_finit_module+0x86/0xb0
[   51.784479]  [<ffffffff81666529>] system_call_fastpath+0x16/0x1b
[   51.784479] Code: ff ff 5b 41 5c 41 5d 41 5e 41 5f 5d c3 0f 1f 00 55 48 89 e
[   51.784479] RIP  [<ffffffff81319fbb>] __list_add+0x1b/0xc0
[   51.784479]  RSP <ffff88003c9b7a10>
[   51.784479] CR2: 0000000000000000
[   52.218397] ---[ end trace 0bd601544e51b8a3 ]---

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 64 ------------------------------
 drivers/media/usb/em28xx/em28xx-video.c | 70 ++++++++++++++++++++++++++++++++-
 2 files changed, 69 insertions(+), 65 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 53dc82409bc2..4fccbed539f9 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2362,24 +2362,6 @@ static struct em28xx_hash_table em28xx_i2c_hash[] = {
 };
 /* NOTE: introduce a separate hash table for devices with 16 bit eeproms */
 
-/* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */
-static unsigned short saa711x_addrs[] = {
-	0x4a >> 1, 0x48 >> 1,   /* SAA7111, SAA7111A and SAA7113 */
-	0x42 >> 1, 0x40 >> 1,   /* SAA7114, SAA7115 and SAA7118 */
-	I2C_CLIENT_END };
-
-static unsigned short tvp5150_addrs[] = {
-	0xb8 >> 1,
-	0xba >> 1,
-	I2C_CLIENT_END
-};
-
-static unsigned short msp3400_addrs[] = {
-	0x80 >> 1,
-	0x88 >> 1,
-	I2C_CLIENT_END
-};
-
 int em28xx_tuner_callback(void *ptr, int component, int command, int arg)
 {
 	struct em28xx_i2c_bus *i2c_bus = ptr;
@@ -2784,54 +2766,8 @@ static void em28xx_card_setup(struct em28xx *dev)
 	/* Allow override tuner type by a module parameter */
 	if (tuner >= 0)
 		dev->tuner_type = tuner;
-
-	/* request some modules */
-	if (dev->board.has_msp34xx)
-		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-			"msp3400", 0, msp3400_addrs);
-
-	if (dev->board.decoder == EM28XX_SAA711X)
-		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-			"saa7115_auto", 0, saa711x_addrs);
-
-	if (dev->board.decoder == EM28XX_TVP5150)
-		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-			"tvp5150", 0, tvp5150_addrs);
-
-	if (dev->board.adecoder == EM28XX_TVAUDIO)
-		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-			"tvaudio", dev->board.tvaudio_addr, NULL);
-
-	if (dev->board.tuner_type != TUNER_ABSENT) {
-		int has_demod = (dev->tda9887_conf & TDA9887_PRESENT);
-
-		if (dev->board.radio.type)
-			v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-				"tuner", dev->board.radio_addr, NULL);
-
-		if (has_demod)
-			v4l2_i2c_new_subdev(&dev->v4l2_dev,
-				&dev->i2c_adap[dev->def_i2c_bus], "tuner",
-				0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
-		if (dev->tuner_addr == 0) {
-			enum v4l2_i2c_tuner_type type =
-				has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV;
-			struct v4l2_subdev *sd;
-
-			sd = v4l2_i2c_new_subdev(&dev->v4l2_dev,
-				&dev->i2c_adap[dev->def_i2c_bus], "tuner",
-				0, v4l2_i2c_tuner_addrs(type));
-
-			if (sd)
-				dev->tuner_addr = v4l2_i2c_subdev_addr(sd);
-		} else {
-			v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
-				"tuner", dev->tuner_addr, NULL);
-		}
-	}
 }
 
-
 static void request_module_async(struct work_struct *work)
 {
 	struct em28xx *dev = container_of(work,
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index b0b1a3d4534b..3baf22464c0d 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2053,6 +2053,25 @@ static struct video_device em28xx_radio_template = {
 	.ioctl_ops 	      = &radio_ioctl_ops,
 };
 
+
+/* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */
+static unsigned short saa711x_addrs[] = {
+	0x4a >> 1, 0x48 >> 1,   /* SAA7111, SAA7111A and SAA7113 */
+	0x42 >> 1, 0x40 >> 1,   /* SAA7114, SAA7115 and SAA7118 */
+	I2C_CLIENT_END };
+
+static unsigned short tvp5150_addrs[] = {
+	0xb8 >> 1,
+	0xba >> 1,
+	I2C_CLIENT_END
+};
+
+static unsigned short msp3400_addrs[] = {
+	0x80 >> 1,
+	0x88 >> 1,
+	I2C_CLIENT_END
+};
+
 /******************************** usb interface ******************************/
 
 static struct video_device *em28xx_vdev_init(struct em28xx *dev,
@@ -2220,7 +2239,56 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	dev->vinctl  = EM28XX_VINCTRL_INTERLACED |
 		       EM28XX_VINCTRL_CCIR656_ENABLE;
 
-        /* Initialize tuner and camera */
+
+
+	/* request some modules */
+
+	if (dev->board.has_msp34xx)
+		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
+			"msp3400", 0, msp3400_addrs);
+
+	if (dev->board.decoder == EM28XX_SAA711X)
+		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
+			"saa7115_auto", 0, saa711x_addrs);
+
+	if (dev->board.decoder == EM28XX_TVP5150)
+		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
+			"tvp5150", 0, tvp5150_addrs);
+
+	if (dev->board.adecoder == EM28XX_TVAUDIO)
+		v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
+			"tvaudio", dev->board.tvaudio_addr, NULL);
+
+	/* Initialize tuner and camera */
+
+	if (dev->board.tuner_type != TUNER_ABSENT) {
+		int has_demod = (dev->tda9887_conf & TDA9887_PRESENT);
+
+		if (dev->board.radio.type)
+			v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
+				"tuner", dev->board.radio_addr, NULL);
+
+		if (has_demod)
+			v4l2_i2c_new_subdev(&dev->v4l2_dev,
+				&dev->i2c_adap[dev->def_i2c_bus], "tuner",
+				0, v4l2_i2c_tuner_addrs(ADDRS_DEMOD));
+		if (dev->tuner_addr == 0) {
+			enum v4l2_i2c_tuner_type type =
+				has_demod ? ADDRS_TV_WITH_DEMOD : ADDRS_TV;
+			struct v4l2_subdev *sd;
+
+			sd = v4l2_i2c_new_subdev(&dev->v4l2_dev,
+				&dev->i2c_adap[dev->def_i2c_bus], "tuner",
+				0, v4l2_i2c_tuner_addrs(type));
+
+			if (sd)
+				dev->tuner_addr = v4l2_i2c_subdev_addr(sd);
+		} else {
+			v4l2_i2c_new_subdev(&dev->v4l2_dev, &dev->i2c_adap[dev->def_i2c_bus],
+				"tuner", dev->tuner_addr, NULL);
+		}
+	}
+
 	em28xx_tuner_setup(dev);
 	em28xx_init_camera(dev);
 
-- 
1.8.3.1

[PATCH v3 06/24] em28xx-cards: remove a now dead code

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

As V4L2 registration now occurs asynchronously, the code doesn't
fail there anymore.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 4fccbed539f9..d1c75e66554c 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2972,7 +2972,7 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 	if (retval < 0) {
 		em28xx_errdev("%s: em28xx_i2c_register bus 0 - error [%d]!\n",
 			__func__, retval);
-		goto unregister_dev;
+		return retval;
 	}
 
 	/* register i2c bus 1 */
@@ -2986,7 +2986,7 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 		if (retval < 0) {
 			em28xx_errdev("%s: em28xx_i2c_register bus 1 - error [%d]!\n",
 				__func__, retval);
-			goto unregister_dev;
+			return retval;
 		}
 	}
 
@@ -2994,16 +2994,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 	em28xx_card_setup(dev);
 
 	return 0;
-
-	if (dev->def_i2c_bus)
-		em28xx_i2c_unregister(dev, 1);
-	em28xx_i2c_unregister(dev, 0);
-	v4l2_ctrl_handler_free(&dev->ctrl_handler);
-
-unregister_dev:
-	v4l2_device_unregister(&dev->v4l2_dev);
-
-	return retval;
 }
 
 /* high bandwidth multiplier, as encoded in highspeed endpoint descriptors */
-- 
1.8.3.1

[PATCH v3 07/24] em28xx: fix a cut and paste error

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Don't use "dvb" at em28xx v4l module. This was due to a cut
and paste from em28xx-dvb extension.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 3baf22464c0d..664546959ea0 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2515,15 +2515,15 @@ static struct em28xx_ops v4l2_ops = {
 	.fini = em28xx_v4l2_fini,
 };
 
-static int __init em28xx_dvb_register(void)
+static int __init em28xx_v4l_register(void)
 {
 	return em28xx_register_extension(&v4l2_ops);
 }
 
-static void __exit em28xx_dvb_unregister(void)
+static void __exit em28xx_v4l_unregister(void)
 {
 	em28xx_unregister_extension(&v4l2_ops);
 }
 
-module_init(em28xx_dvb_register);
-module_exit(em28xx_dvb_unregister);
+module_init(em28xx_v4l_register);
+module_exit(em28xx_v4l_unregister);
-- 
1.8.3.1

[PATCH v3 08/24] em28xx: add warn messages for timeout

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

changeset 45f04e82d035 added a logic to check if em28xx got
a timeout on an I2C transfer.

That patch started to produce a series of errors that is present
with HVR-950, like:

[ 4032.218656] xc2028 19-0061: Error on line 1299: -19

However, as there are several places where -ENODEV is produced,
there's no way to know what's happening.

So, let's add a printk to report what error condition was reached:

[ 4032.218652] em2882/3 #0: I2C transfer timeout on writing to addr 0xc2
[ 4032.218656] xc2028 19-0061: Error on line 1299: -19

Interesting enough, when connected to an USB3 port, the number of
errors increase:

[ 4249.941375] em2882/3 #0: I2C transfer timeout on writing to addr 0xb8
[ 4249.941378] tvp5150 19-005c: i2c i/o error: rc == -19 (should be 2)
[ 4250.023854] em2882/3 #0: I2C transfer timeout on writing to addr 0xc2
[ 4250.023857] xc2028 19-0061: Error on line 1299: -19

Due to that, I suspect that the logic in the driver is wrong: instead
of just returning an error if 0x10 is returned, it should be waiting for
a while and read the I2C status register again.

However, more tests are needed.

For now, instead of just returning -ENODEV, output an error message
to help debug what's happening.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index c4ff9739a7ae..9e6a11d01858 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -80,6 +80,7 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 		if (ret == 0x80 + len - 1) {
 			return len;
 		} else if (ret == 0x94 + len - 1) {
+			em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
 			return -ENODEV;
 		} else if (ret < 0) {
 			em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
@@ -123,6 +124,7 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 		if (ret == 0x84 + len - 1) {
 			break;
 		} else if (ret == 0x94 + len - 1) {
+			em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
 			return -ENODEV;
 		} else if (ret < 0) {
 			em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
@@ -198,6 +200,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 		if (ret == 0) { /* success */
 			return len;
 		} else if (ret == 0x10) {
+			em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", addr);
 			return -ENODEV;
 		} else if (ret < 0) {
 			em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n",
@@ -255,6 +258,7 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len)
 	}
 	if (ret > 0) {
 		if (ret == 0x10) {
+			em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr);
 			return -ENODEV;
 		} else {
 			em28xx_warn("unknown i2c error (status=%i)\n", ret);
@@ -316,8 +320,10 @@ static int em25xx_bus_B_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 	 */
 	if (!ret)
 		return len;
-	else if (ret > 0)
+	else if (ret > 0) {
+		em28xx_warn("Bus B R08 returned 0x%02x: I2C timeout", ret);
 		return -ENODEV;
+	}
 
 	return ret;
 	/*
@@ -367,8 +373,10 @@ static int em25xx_bus_B_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 	 */
 	if (!ret)
 		return len;
-	else if (ret > 0)
+	else if (ret > 0) {
+		em28xx_warn("Bus B R08 returned 0x%02x: I2C timeout", ret);
 		return -ENODEV;
+	}
 
 	return ret;
 	/*
-- 
1.8.3.1

[PATCH v3 09/24] em28xx: improve extension information messages

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Add a message with consistent prints before and after each
extension initialization, and provide a better text for module
load.

While here, add a missing sanity check for extension finish
code at em28xx-v4l extension.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-audio.c |  4 +++-
 drivers/media/usb/em28xx/em28xx-core.c  |  2 +-
 drivers/media/usb/em28xx/em28xx-dvb.c   |  7 ++++---
 drivers/media/usb/em28xx/em28xx-input.c |  4 ++++
 drivers/media/usb/em28xx/em28xx-video.c | 10 ++++++++--
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c
index 2fdb66ee44ab..263886adcf26 100644
--- a/drivers/media/usb/em28xx/em28xx-audio.c
+++ b/drivers/media/usb/em28xx/em28xx-audio.c
@@ -649,7 +649,8 @@ static int em28xx_audio_init(struct em28xx *dev)
 		return 0;
 	}
 
-	printk(KERN_INFO "em28xx-audio.c: probing for em28xx Audio Vendor Class\n");
+	em28xx_info("Binding audio extension\n");
+
 	printk(KERN_INFO "em28xx-audio.c: Copyright (C) 2006 Markus "
 			 "Rechberger\n");
 	printk(KERN_INFO "em28xx-audio.c: Copyright (C) 2007-2011 Mauro Carvalho Chehab\n");
@@ -702,6 +703,7 @@ static int em28xx_audio_init(struct em28xx *dev)
 	adev->sndcard = card;
 	adev->udev = dev->udev;
 
+	em28xx_info("Audio extension successfully initialized\n");
 	return 0;
 }
 
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index 1113d4e107d8..33cf26e106b5 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -1069,7 +1069,7 @@ int em28xx_register_extension(struct em28xx_ops *ops)
 		ops->init(dev);
 	}
 	mutex_unlock(&em28xx_devlist_mutex);
-	printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name);
+	printk(KERN_INFO "em28xx: Registered (%s) extension\n", ops->name);
 	return 0;
 }
 EXPORT_SYMBOL(em28xx_register_extension);
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index ddc0e609065d..f72663a9b5c5 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -274,7 +274,7 @@ static int em28xx_stop_feed(struct dvb_demux_feed *feed)
 static int em28xx_dvb_bus_ctrl(struct dvb_frontend *fe, int acquire)
 {
 	struct em28xx_i2c_bus *i2c_bus = fe->dvb->priv;
-        struct em28xx *dev = i2c_bus->dev;
+	struct em28xx *dev = i2c_bus->dev;
 
 	if (acquire)
 		return em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
@@ -992,10 +992,11 @@ static int em28xx_dvb_init(struct em28xx *dev)
 
 	if (!dev->board.has_dvb) {
 		/* This device does not support the extension */
-		printk(KERN_INFO "em28xx_dvb: This device does not support the extension\n");
 		return 0;
 	}
 
+	em28xx_info("Binding DVB extension\n");
+
 	dvb = kzalloc(sizeof(struct em28xx_dvb), GFP_KERNEL);
 
 	if (dvb == NULL) {
@@ -1407,7 +1408,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
 	/* MFE lock */
 	dvb->adapter.mfe_shared = mfe_shared;
 
-	em28xx_info("Successfully loaded em28xx-dvb\n");
+	em28xx_info("DVB extension successfully initialized\n");
 ret:
 	em28xx_set_mode(dev, EM28XX_SUSPEND);
 	mutex_unlock(&dev->lock);
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index 93a7d02b9cb4..eed7dd79f734 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -692,6 +692,8 @@ static int em28xx_ir_init(struct em28xx *dev)
 		return 0;
 	}
 
+	em28xx_info("Registering input extension\n");
+
 	ir = kzalloc(sizeof(*ir), GFP_KERNEL);
 	rc = rc_allocate_device();
 	if (!ir || !rc)
@@ -785,6 +787,8 @@ static int em28xx_ir_init(struct em28xx *dev)
 	if (err)
 		goto error;
 
+	em28xx_info("Input extension successfully initalized\n");
+
 	return 0;
 
 error:
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 664546959ea0..4d6c9f8e1497 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1884,6 +1884,11 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 
 	/*FIXME: I2C IR should be disconnected */
 
+	if (!dev->has_video) {
+		/* This device does not support the v4l2 extension */
+		return 0;
+	}
+
 	if (dev->radio_dev) {
 		if (video_is_registered(dev->radio_dev))
 			video_unregister_device(dev->radio_dev);
@@ -2218,8 +2223,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 		return 0;
 	}
 
-	printk(KERN_INFO "%s: v4l2 driver version %s\n",
-		dev->name, EM28XX_VERSION);
+	em28xx_info("Registering V4L2 extension\n");
 
 	mutex_lock(&dev->lock);
 
@@ -2503,6 +2507,8 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	/* initialize videobuf2 stuff */
 	em28xx_vb2_setup(dev);
 
+	em28xx_info("V4L2 extension successfully initialized\n");
+
 err:
 	mutex_unlock(&dev->lock);
 	return ret;
-- 
1.8.3.1

[PATCH v3 10/24] em28xx: convert i2c wait completion logic to use jiffies

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

The I2C wait completion/timeout logic currently assumes that
msleep(5) will wait exaclty 5 ms. This is not true at all,
as it depends on CONFIG_HZ.

Convert it to use jiffies, in order to not wait for more time
than needed.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 65 ++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 9e6a11d01858..9fa7ed51e5b1 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -26,6 +26,7 @@
 #include <linux/kernel.h>
 #include <linux/usb.h>
 #include <linux/i2c.h>
+#include <linux/jiffies.h>
 
 #include "em28xx.h"
 #include "tuner-xc2028.h"
@@ -48,8 +49,8 @@ MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]");
  */
 static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 {
+	unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
 	int ret;
-	int write_timeout;
 	u8 b2[6];
 
 	if (len < 1 || len > 4)
@@ -74,15 +75,15 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 		return (ret < 0) ? ret : -EIO;
 	}
 	/* wait for completion */
-	for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0;
-	     write_timeout -= 5) {
+	while (time_is_after_jiffies(timeout)) {
 		ret = dev->em28xx_read_reg(dev, 0x05);
-		if (ret == 0x80 + len - 1) {
+		if (ret == 0x80 + len - 1)
 			return len;
-		} else if (ret == 0x94 + len - 1) {
+		if (ret == 0x94 + len - 1) {
 			em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
 			return -ENODEV;
-		} else if (ret < 0) {
+		}
+		if (ret < 0) {
 			em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
 				    ret);
 			return ret;
@@ -99,9 +100,9 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
  */
 static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 {
+	unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
 	u8 buf2[4];
 	int ret;
-	int read_timeout;
 	int i;
 
 	if (len < 1 || len > 4)
@@ -118,15 +119,15 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 	}
 
 	/* wait for completion */
-	for (read_timeout = EM2800_I2C_XFER_TIMEOUT; read_timeout > 0;
-	     read_timeout -= 5) {
+	while (time_is_after_jiffies(timeout)) {
 		ret = dev->em28xx_read_reg(dev, 0x05);
-		if (ret == 0x84 + len - 1) {
+		if (ret == 0x84 + len - 1)
 			break;
-		} else if (ret == 0x94 + len - 1) {
+		if (ret == 0x94 + len - 1) {
 			em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
 			return -ENODEV;
-		} else if (ret < 0) {
+		}
+		if (ret < 0) {
 			em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
 				    ret);
 			return ret;
@@ -170,7 +171,8 @@ static int em2800_i2c_check_for_device(struct em28xx *dev, u8 addr)
 static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 				 u16 len, int stop)
 {
-	int write_timeout, ret;
+	unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
+	int ret;
 
 	if (len < 1 || len > 64)
 		return -EOPNOTSUPP;
@@ -193,17 +195,18 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 		}
 	}
 
-	/* Check success of the i2c operation */
-	for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0;
-	     write_timeout -= 5) {
+	/* wait for completion */
+	while (time_is_after_jiffies(timeout)) {
 		ret = dev->em28xx_read_reg(dev, 0x05);
-		if (ret == 0) { /* success */
+		if (ret == 0) /* success */
 			return len;
-		} else if (ret == 0x10) {
-			em28xx_warn("I2C transfer timeout on writing to addr 0x%02x", addr);
+		if (ret == 0x10) {
+			em28xx_warn("I2C transfer timeout on writing to addr 0x%02x",
+				    addr);
 			return -ENODEV;
-		} else if (ret < 0) {
-			em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n",
+		}
+		if (ret < 0) {
+			em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
 				    ret);
 			return ret;
 		}
@@ -214,6 +217,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 		 * (even with high payload) ...
 		 */
 	}
+
 	em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
 	return -EIO;
 }
@@ -251,21 +255,20 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len)
 
 	/* Check success of the i2c operation */
 	ret = dev->em28xx_read_reg(dev, 0x05);
+	if (ret == 0) /* success */
+		return len;
 	if (ret < 0) {
-		em28xx_warn("failed to read i2c transfer status from bridge (error=%i)\n",
+		em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
 			    ret);
 		return ret;
 	}
-	if (ret > 0) {
-		if (ret == 0x10) {
-			em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr);
-			return -ENODEV;
-		} else {
-			em28xx_warn("unknown i2c error (status=%i)\n", ret);
-			return -EIO;
-		}
+	if (ret == 0x10) {
+		em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr);
+		return -ENODEV;
 	}
-	return len;
+
+	em28xx_warn("unknown i2c error (status=%i)\n", ret);
+	return -EIO;
 }
 
 /*
-- 
1.8.3.1

[PATCH v3 11/24] tvp5150: make read operations atomic

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Instead of using two I2C operations between write and read,
use just one i2c_transfer. That allows I2C mutexes to not
let any other I2C transfer between the two.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/i2c/tvp5150.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
index 89c0b13463b7..d6ba457fcf67 100644
--- a/drivers/media/i2c/tvp5150.c
+++ b/drivers/media/i2c/tvp5150.c
@@ -58,21 +58,19 @@ static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
 	struct i2c_client *c = v4l2_get_subdevdata(sd);
 	unsigned char buffer[1];
 	int rc;
+	struct i2c_msg msg[] = {
+		{ .addr = c->addr, .flags = 0,
+		  .buf = &addr, .len = 1 },
+		{ .addr = c->addr, .flags = I2C_M_RD,
+		  .buf = buffer, .len = 1 }
+	};
 
 	buffer[0] = addr;
 
-	rc = i2c_master_send(c, buffer, 1);
-	if (rc < 0) {
-		v4l2_err(sd, "i2c i/o error: rc == %d (should be 1)\n", rc);
-		return rc;
-	}
-
-	msleep(10);
-
-	rc = i2c_master_recv(c, buffer, 1);
-	if (rc < 0) {
-		v4l2_err(sd, "i2c i/o error: rc == %d (should be 1)\n", rc);
-		return rc;
+	rc = i2c_transfer(c->adapter, msg, 2);
+	if (rc < 0 || rc != 2) {
+		v4l2_err(sd, "i2c i/o error: rc == %d (should be 2)\n", rc);
+		return rc < 0 ? rc : -EIO;
 	}
 
 	v4l2_dbg(2, debug, sd, "tvp5150: read 0x%02x = 0x%02x\n", addr, buffer[0]);
-- 
1.8.3.1

Re: [PATCH v3 11/24] tvp5150: make read operations atomic

[Frank Schäfer]

Am 28.12.2013 13:16, schrieb Mauro Carvalho Chehab:
> From: Mauro Carvalho Chehab <m.chehab@samsung.com>
>
> Instead of using two I2C operations between write and read,
> use just one i2c_transfer. That allows I2C mutexes to not
> let any other I2C transfer between the two.
>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
>  drivers/media/i2c/tvp5150.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 89c0b13463b7..d6ba457fcf67 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -58,21 +58,19 @@ static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
>  	struct i2c_client *c = v4l2_get_subdevdata(sd);
>  	unsigned char buffer[1];
>  	int rc;
> +	struct i2c_msg msg[] = {
> +		{ .addr = c->addr, .flags = 0,
> +		  .buf = &addr, .len = 1 },
I would use        .buf = buffer        here, too.

> +		{ .addr = c->addr, .flags = I2C_M_RD,
> +		  .buf = buffer, .len = 1 }
> +	};
>  
>  	buffer[0] = addr;
>  
> -	rc = i2c_master_send(c, buffer, 1);
> -	if (rc < 0) {
> -		v4l2_err(sd, "i2c i/o error: rc == %d (should be 1)\n", rc);
> -		return rc;
> -	}
> -
> -	msleep(10);
That's the critical change.

> -
> -	rc = i2c_master_recv(c, buffer, 1);
> -	if (rc < 0) {
> -		v4l2_err(sd, "i2c i/o error: rc == %d (should be 1)\n", rc);
> -		return rc;
> +	rc = i2c_transfer(c->adapter, msg, 2);
> +	if (rc < 0 || rc != 2) {
> +		v4l2_err(sd, "i2c i/o error: rc == %d (should be 2)\n", rc);
> +		return rc < 0 ? rc : -EIO;
>  	}
>  
>  	v4l2_dbg(2, debug, sd, "tvp5150: read 0x%02x = 0x%02x\n", addr, buffer[0]);
Looks good and works without problems with my HVR-900 and WinTV 2
devices (both em28xx).

Re: [PATCH v3 11/24] tvp5150: make read operations atomic

[Mauro Carvalho Chehab]

Em Wed, 01 Jan 2014 19:52:08 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:

> Am 28.12.2013 13:16, schrieb Mauro Carvalho Chehab:
> > From: Mauro Carvalho Chehab <m.chehab@samsung.com>
> >
> > Instead of using two I2C operations between write and read,
> > use just one i2c_transfer. That allows I2C mutexes to not
> > let any other I2C transfer between the two.
> >
> > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> > ---
> >  drivers/media/i2c/tvp5150.c | 22 ++++++++++------------
> >  1 file changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > index 89c0b13463b7..d6ba457fcf67 100644
> > --- a/drivers/media/i2c/tvp5150.c
> > +++ b/drivers/media/i2c/tvp5150.c
> > @@ -58,21 +58,19 @@ static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
> >  	struct i2c_client *c = v4l2_get_subdevdata(sd);
> >  	unsigned char buffer[1];
> >  	int rc;
> > +	struct i2c_msg msg[] = {
> > +		{ .addr = c->addr, .flags = 0,
> > +		  .buf = &addr, .len = 1 },
> I would use        .buf = buffer        here, too.

Why? The address needed is already at addr, and it is also an unsigned char.

Using buffer would require an extra data copy.

> 
> > +		{ .addr = c->addr, .flags = I2C_M_RD,
> > +		  .buf = buffer, .len = 1 }
> > +	};
> >  
> >  	buffer[0] = addr;
> >  
> > -	rc = i2c_master_send(c, buffer, 1);
> > -	if (rc < 0) {
> > -		v4l2_err(sd, "i2c i/o error: rc == %d (should be 1)\n", rc);
> > -		return rc;
> > -	}
> > -
> > -	msleep(10);
> That's the critical change.

I don't think so. I'm not sure why I added this at the first place on the
original patch with where I added this driver, but it is very doubtful
that a msleep() is needed here.

This code is really old (from the time I added support for WinTV USB 2).

I suspect I added the sleep there just because the I2C logs, during the
driver development phase, to be an exact mimic on what it was got via
USB dumps.

> 
> > -
> > -	rc = i2c_master_recv(c, buffer, 1);
> > -	if (rc < 0) {
> > -		v4l2_err(sd, "i2c i/o error: rc == %d (should be 1)\n", rc);
> > -		return rc;
> > +	rc = i2c_transfer(c->adapter, msg, 2);
> > +	if (rc < 0 || rc != 2) {
> > +		v4l2_err(sd, "i2c i/o error: rc == %d (should be 2)\n", rc);
> > +		return rc < 0 ? rc : -EIO;
> >  	}
> >  
> >  	v4l2_dbg(2, debug, sd, "tvp5150: read 0x%02x = 0x%02x\n", addr, buffer[0]);
> Looks good and works without problems with my HVR-900 and WinTV 2
> devices (both em28xx).
> 


-- 

Cheers,
Mauro

Re: [PATCH v3 11/24] tvp5150: make read operations atomic

[Frank Schäfer]

On 02.01.2014 20:20, Mauro Carvalho Chehab wrote:
> Em Wed, 01 Jan 2014 19:52:08 +0100
> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
>> Am 28.12.2013 13:16, schrieb Mauro Carvalho Chehab:
>>> From: Mauro Carvalho Chehab <m.chehab@samsung.com>
>>>
>>> Instead of using two I2C operations between write and read,
>>> use just one i2c_transfer. That allows I2C mutexes to not
>>> let any other I2C transfer between the two.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
>>> ---
>>>   drivers/media/i2c/tvp5150.c | 22 ++++++++++------------
>>>   1 file changed, 10 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
>>> index 89c0b13463b7..d6ba457fcf67 100644
>>> --- a/drivers/media/i2c/tvp5150.c
>>> +++ b/drivers/media/i2c/tvp5150.c
>>> @@ -58,21 +58,19 @@ static int tvp5150_read(struct v4l2_subdev *sd, unsigned char addr)
>>>   	struct i2c_client *c = v4l2_get_subdevdata(sd);
>>>   	unsigned char buffer[1];
>>>   	int rc;
>>> +	struct i2c_msg msg[] = {
>>> +		{ .addr = c->addr, .flags = 0,
>>> +		  .buf = &addr, .len = 1 },
>> I would use        .buf = buffer        here, too.
> Why? The address needed is already at addr, and it is also an unsigned char.
>
> Using buffer would require an extra data copy.
You are still doing this...

>
>>
>>> +		{ .addr = c->addr, .flags = I2C_M_RD,
>>> +		  .buf = buffer, .len = 1 }
>>> +	};
>>>   
>>>   	buffer[0] = addr;
... here. ;)

>>>   
>>> -	rc = i2c_master_send(c, buffer, 1);
>>> -	if (rc < 0) {
>>> -		v4l2_err(sd, "i2c i/o error: rc == %d (should be 1)\n", rc);
>>> -		return rc;
>>> -	}
>>> -
>>> -	msleep(10);
>> That's the critical change.
> I don't think so. I'm not sure why I added this at the first place on the
> original patch with where I added this driver, but it is very doubtful
> that a msleep() is needed here.
>
> This code is really old (from the time I added support for WinTV USB 2).
>
> I suspect I added the sleep there just because the I2C logs, during the
> driver development phase, to be an exact mimic on what it was got via
> USB dumps.
>
>>> -
>>> -	rc = i2c_master_recv(c, buffer, 1);
>>> -	if (rc < 0) {
>>> -		v4l2_err(sd, "i2c i/o error: rc == %d (should be 1)\n", rc);
>>> -		return rc;
>>> +	rc = i2c_transfer(c->adapter, msg, 2);
>>> +	if (rc < 0 || rc != 2) {
>>> +		v4l2_err(sd, "i2c i/o error: rc == %d (should be 2)\n", rc);
>>> +		return rc < 0 ? rc : -EIO;
>>>   	}
>>>   
>>>   	v4l2_dbg(2, debug, sd, "tvp5150: read 0x%02x = 0x%02x\n", addr, buffer[0]);
>> Looks good and works without problems with my HVR-900 and WinTV 2
>> devices (both em28xx).
>>
>

[PATCH v3 12/24] tuner-xc2028: remove unused code

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

This macro is not used. remove it.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/tuners/tuner-xc2028.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/tuners/tuner-xc2028.c b/drivers/media/tuners/tuner-xc2028.c
index 4be5cf808a40..1057da54c6e0 100644
--- a/drivers/media/tuners/tuner-xc2028.c
+++ b/drivers/media/tuners/tuner-xc2028.c
@@ -134,15 +134,6 @@ struct xc2028_data {
 	_rc;								\
 })
 
-#define i2c_rcv(priv, buf, size) ({					\
-	int _rc;							\
-	_rc = tuner_i2c_xfer_recv(&priv->i2c_props, buf, size);		\
-	if (size != _rc)						\
-		tuner_err("i2c input error: rc = %d (should be %d)\n",	\
-			   _rc, (int)size); 				\
-	_rc;								\
-})
-
 #define i2c_send_recv(priv, obuf, osize, ibuf, isize) ({		\
 	int _rc;							\
 	_rc = tuner_i2c_xfer_send_recv(&priv->i2c_props, obuf, osize,	\
-- 
1.8.3.1

Re: [PATCH v3 12/24] tuner-xc2028: remove unused code

[Frank Schäfer]

Am 28.12.2013 13:16, schrieb Mauro Carvalho Chehab:
> From: Mauro Carvalho Chehab <m.chehab@samsung.com>
>
> This macro is not used. remove it.
>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
>  drivers/media/tuners/tuner-xc2028.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/media/tuners/tuner-xc2028.c b/drivers/media/tuners/tuner-xc2028.c
> index 4be5cf808a40..1057da54c6e0 100644
> --- a/drivers/media/tuners/tuner-xc2028.c
> +++ b/drivers/media/tuners/tuner-xc2028.c
> @@ -134,15 +134,6 @@ struct xc2028_data {
>  	_rc;								\
>  })
>  
> -#define i2c_rcv(priv, buf, size) ({					\
> -	int _rc;							\
> -	_rc = tuner_i2c_xfer_recv(&priv->i2c_props, buf, size);		\
> -	if (size != _rc)						\
> -		tuner_err("i2c input error: rc = %d (should be %d)\n",	\
> -			   _rc, (int)size); 				\
> -	_rc;								\
> -})
> -
>  #define i2c_send_recv(priv, obuf, osize, ibuf, isize) ({		\
>  	int _rc;							\
>  	_rc = tuner_i2c_xfer_send_recv(&priv->i2c_props, obuf, osize,	\
Looks good and works, so this patch can is ready for the media tree.

[PATCH v3 13/24] em28xx: retry I2C ops if failed by timeout

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

At least on HVR-950, sometimes an I2C operation fails.

This seems to be more frequent when the device is connected
into an USB 3.0 port.

Instead of report an error, try to repeat it, for up to
20 ms. That makes the code more reliable.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 67 ++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 9fa7ed51e5b1..26f7b0a2e83a 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -181,6 +181,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 	 * Zero length reads always succeed, even if no device is connected
 	 */
 
+retry:
 	/* Write to i2c device */
 	ret = dev->em28xx_write_regs_req(dev, stop ? 2 : 3, addr, buf, len);
 	if (ret != len) {
@@ -200,11 +201,8 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 		ret = dev->em28xx_read_reg(dev, 0x05);
 		if (ret == 0) /* success */
 			return len;
-		if (ret == 0x10) {
-			em28xx_warn("I2C transfer timeout on writing to addr 0x%02x",
-				    addr);
-			return -ENODEV;
-		}
+		if (ret == 0x10)
+			goto retry;
 		if (ret < 0) {
 			em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
 				    ret);
@@ -218,6 +216,11 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 		 */
 	}
 
+	if (ret == 0x10) {
+		em28xx_warn("I2C transfer timeout on writing to addr 0x%02x",
+			    addr);
+		return -ENODEV;
+	}
 	em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
 	return -EIO;
 }
@@ -228,6 +231,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
  */
 static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len)
 {
+	unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
 	int ret;
 
 	if (len < 1 || len > 64)
@@ -237,30 +241,35 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len)
 	 * Zero length reads always succeed, even if no device is connected
 	 */
 
-	/* Read data from i2c device */
-	ret = dev->em28xx_read_reg_req_len(dev, 2, addr, buf, len);
-	if (ret < 0) {
-		em28xx_warn("reading from i2c device at 0x%x failed (error=%i)\n",
-			    addr, ret);
-		return ret;
-	}
-	/*
-	 * NOTE: some devices with two i2c busses have the bad habit to return 0
-	 * bytes if we are on bus B AND there was no write attempt to the
-	 * specified slave address before AND no device is present at the
-	 * requested slave address.
-	 * Anyway, the next check will fail with -ENODEV in this case, so avoid
-	 * spamming the system log on device probing and do nothing here.
-	 */
-
-	/* Check success of the i2c operation */
-	ret = dev->em28xx_read_reg(dev, 0x05);
-	if (ret == 0) /* success */
-		return len;
-	if (ret < 0) {
-		em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
-			    ret);
-		return ret;
+	while (time_is_after_jiffies(timeout)) {
+		/* Read data from i2c device */
+		ret = dev->em28xx_read_reg_req_len(dev, 2, addr, buf, len);
+		if (ret < 0) {
+			em28xx_warn("reading from i2c device at 0x%x failed (error=%i)\n",
+				    addr, ret);
+			return ret;
+		}
+		/*
+		 * NOTE: some devices with two i2c busses have the bad habit to return 0
+		* bytes if we are on bus B AND there was no write attempt to the
+		* specified slave address before AND no device is present at the
+		* requested slave address.
+		* Anyway, the next check will fail with -ENODEV in this case, so avoid
+		* spamming the system log on device probing and do nothing here.
+		*/
+
+		/* Check success of the i2c operation */
+		ret = dev->em28xx_read_reg(dev, 0x05);
+		if (ret == 0) /* success */
+			return len;
+		if (ret < 0) {
+			em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
+				    ret);
+			return ret;
+		}
+		if (ret != 0x10)
+			break;
+		msleep(5);
 	}
 	if (ret == 0x10) {
 		em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr);
-- 
1.8.3.1

[PATCH v3 14/24] em28xx: remove a false positive warning

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

gcc knows nothing about jiffies. So, it produces this error:

	drivers/media/usb/em28xx/em28xx-i2c.c: In function ‘em28xx_i2c_recv_bytes’:
	drivers/media/usb/em28xx/em28xx-i2c.c:274:5: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized]

It is a false positive, however, removing it is as easy as replacing
a while by a do/while construction.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 26f7b0a2e83a..d972e2f67214 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -241,7 +241,7 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len)
 	 * Zero length reads always succeed, even if no device is connected
 	 */
 
-	while (time_is_after_jiffies(timeout)) {
+	do {
 		/* Read data from i2c device */
 		ret = dev->em28xx_read_reg_req_len(dev, 2, addr, buf, len);
 		if (ret < 0) {
@@ -270,7 +270,8 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len)
 		if (ret != 0x10)
 			break;
 		msleep(5);
-	}
+	} while (time_is_after_jiffies(timeout));
+
 	if (ret == 0x10) {
 		em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr);
 		return -ENODEV;
-- 
1.8.3.1

[PATCH v3 15/24] em28xx: check if a device has audio earlier

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Better to split chipset detection from the audio setup.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 11 +++++++++++
 drivers/media/usb/em28xx/em28xx-core.c  | 12 +-----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index d1c75e66554c..4fe742429f2c 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2930,6 +2930,16 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
 		}
 	}
 
+	if (dev->chip_id == CHIP_ID_EM2870 ||
+	    dev->chip_id == CHIP_ID_EM2874 ||
+	    dev->chip_id == CHIP_ID_EM28174 ||
+	    dev->chip_id == CHIP_ID_EM28178) {
+		/* Digital only device - don't load any alsa module */
+		dev->audio_mode.has_audio = false;
+		dev->has_audio_class = false;
+		dev->has_alsa_audio = false;
+	}
+
 	if (chip_name != default_chip_name)
 		printk(KERN_INFO DRIVER_NAME
 		       ": chip ID is %s\n", chip_name);
@@ -3196,6 +3206,7 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 	dev->alt   = -1;
 	dev->is_audio_only = has_audio && !(has_video || has_dvb);
 	dev->has_alsa_audio = has_audio;
+	dev->audio_mode.has_audio = has_audio;
 	dev->has_video = has_video;
 	dev->audio_ifnum = ifnum;
 
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index 33cf26e106b5..818248d3fd28 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -505,18 +505,8 @@ int em28xx_audio_setup(struct em28xx *dev)
 	int vid1, vid2, feat, cfg;
 	u32 vid;
 
-	if (dev->chip_id == CHIP_ID_EM2870 ||
-	    dev->chip_id == CHIP_ID_EM2874 ||
-	    dev->chip_id == CHIP_ID_EM28174 ||
-	    dev->chip_id == CHIP_ID_EM28178) {
-		/* Digital only device - don't load any alsa module */
-		dev->audio_mode.has_audio = false;
-		dev->has_audio_class = false;
-		dev->has_alsa_audio = false;
+	if (!dev->audio_mode.has_audio)
 		return 0;
-	}
-
-	dev->audio_mode.has_audio = true;
 
 	/* See how this device is configured */
 	cfg = em28xx_read_reg(dev, EM28XX_R00_CHIPCFG);
-- 
1.8.3.1

[PATCH v3 16/24] em28xx: properly implement AC97 wait code

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Instead of assuming that msleep() is precise, use a jiffies
based code to wait for AC97 to be available.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-core.c | 7 +++++--
 drivers/media/usb/em28xx/em28xx.h      | 5 ++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index 818248d3fd28..36b2f1ab4474 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -23,6 +23,7 @@
  */
 
 #include <linux/init.h>
+#include <linux/jiffies.h>
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -254,16 +255,18 @@ EXPORT_SYMBOL_GPL(em28xx_toggle_reg_bits);
  */
 static int em28xx_is_ac97_ready(struct em28xx *dev)
 {
-	int ret, i;
+	unsigned long timeout = jiffies + msecs_to_jiffies(EM2800_AC97_XFER_TIMEOUT);
+	int ret;
 
 	/* Wait up to 50 ms for AC97 command to complete */
-	for (i = 0; i < 10; i++, msleep(5)) {
+	while (time_is_after_jiffies(timeout)) {
 		ret = em28xx_read_reg(dev, EM28XX_R43_AC97BUSY);
 		if (ret < 0)
 			return ret;
 
 		if (!(ret & 0x01))
 			return 0;
+		msleep (5);
 	}
 
 	em28xx_warn("AC97 command still being executed: not handled properly!\n");
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 9d6f43e4681f..ac79501f5d9f 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -182,9 +182,12 @@
 
 #define EM28XX_INTERLACED_DEFAULT 1
 
-/* time in msecs to wait for i2c writes to finish */
+/* time in msecs to wait for i2c xfers to finish */
 #define EM2800_I2C_XFER_TIMEOUT		20
 
+/* time in msecs to wait for AC97 xfers to finish */
+#define EM2800_AC97_XFER_TIMEOUT	100
+
 /* max. number of button state polling addresses */
 #define EM28XX_NUM_BUTTON_ADDRESSES_MAX		5
 
-- 
1.8.3.1

[PATCH v3 17/24] em28xx: initialize audio latter

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Better to first write the GPIOs of the input mux, before initializing
the audio.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 42 ++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 4d6c9f8e1497..ea3653248d25 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2243,8 +2243,6 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	dev->vinctl  = EM28XX_VINCTRL_INTERLACED |
 		       EM28XX_VINCTRL_CCIR656_ENABLE;
 
-
-
 	/* request some modules */
 
 	if (dev->board.has_msp34xx)
@@ -2296,26 +2294,6 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	em28xx_tuner_setup(dev);
 	em28xx_init_camera(dev);
 
-	/* Configure audio */
-	ret = em28xx_audio_setup(dev);
-	if (ret < 0) {
-		em28xx_errdev("%s: Error while setting audio - error [%d]!\n",
-			__func__, ret);
-		goto err;
-	}
-	if (dev->audio_mode.ac97 != EM28XX_NO_AC97) {
-		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
-			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
-		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
-			V4L2_CID_AUDIO_VOLUME, 0, 0x1f, 1, 0x1f);
-	} else {
-		/* install the em28xx notify callback */
-		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_MUTE),
-				em28xx_ctrl_notify, dev);
-		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_VOLUME),
-				em28xx_ctrl_notify, dev);
-	}
-
 	/* wake i2c devices */
 	em28xx_wake_i2c(dev);
 
@@ -2361,6 +2339,26 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 
 	video_mux(dev, 0);
 
+	/* Configure audio */
+	ret = em28xx_audio_setup(dev);
+	if (ret < 0) {
+		em28xx_errdev("%s: Error while setting audio - error [%d]!\n",
+			__func__, ret);
+		goto err;
+	}
+	if (dev->audio_mode.ac97 != EM28XX_NO_AC97) {
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
+			V4L2_CID_AUDIO_MUTE, 0, 1, 1, 1);
+		v4l2_ctrl_new_std(hdl, &em28xx_ctrl_ops,
+			V4L2_CID_AUDIO_VOLUME, 0, 0x1f, 1, 0x1f);
+	} else {
+		/* install the em28xx notify callback */
+		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_MUTE),
+				em28xx_ctrl_notify, dev);
+		v4l2_ctrl_notify(v4l2_ctrl_find(hdl, V4L2_CID_AUDIO_VOLUME),
+				em28xx_ctrl_notify, dev);
+	}
+
 	/* Audio defaults */
 	dev->mute = 1;
 	dev->volume = 0x1f;
-- 
1.8.3.1

[PATCH v3 18/24] em28xx: improve I2C timeout error message

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Since sometimes em28xx is returning 0x02 or 0x04 at the I2C status
register, output what's the returned status:

[ 1090.939820] em2882/3 #0: write to i2c device at 0xc2 timed out (ret=0x04)
[ 1090.939826] xc2028 19-0061: Error on line 1290: -5
[ 1091.140136] em2882/3 #0: write to i2c device at 0xc2 timed out (ret=0x04)
[ 1091.140155] xc2028 19-0061: Error on line 1290: -5
[ 1091.828622] em2882/3 #0: write to i2c device at 0xc2 timed out (ret=0x02)
[ 1091.828625] xc2028 19-0061: i2c input error: rc = -5 (should be 2)
[ 1091.928731] em2882/3 #0: write to i2c device at 0xc2 timed out (ret=0x02)
[ 1091.928734] xc2028 19-0061: i2c input error: rc = -5 (should be 2)

As that may help to latter improve the code.

Also, as those errors are now present, remove that bogus comment that
only 0x00 and 0x10 values are present.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index d972e2f67214..420fddf7da3a 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -209,11 +209,6 @@ retry:
 			return ret;
 		}
 		msleep(5);
-		/*
-		 * NOTE: do we really have to wait for success ?
-		 * Never seen anything else than 0x00 or 0x10
-		 * (even with high payload) ...
-		 */
 	}
 
 	if (ret == 0x10) {
@@ -221,7 +216,8 @@ retry:
 			    addr);
 		return -ENODEV;
 	}
-	em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
+	em28xx_warn("write to i2c device at 0x%x timed out (ret=0x%02x)\n",
+		    addr, ret);
 	return -EIO;
 }
 
-- 
1.8.3.1

[PATCH v3 19/24] em28xx: unify module version

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

Use the same module version on all em28xx sub-modules, and use
the same naming convention to describe the driver.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-audio.c | 3 ++-
 drivers/media/usb/em28xx/em28xx-core.c  | 2 --
 drivers/media/usb/em28xx/em28xx-dvb.c   | 4 +++-
 drivers/media/usb/em28xx/em28xx-input.c | 3 ++-
 drivers/media/usb/em28xx/em28xx-video.c | 4 +---
 drivers/media/usb/em28xx/em28xx.h       | 1 +
 6 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c
index 263886adcf26..a6eef06ffdcd 100644
--- a/drivers/media/usb/em28xx/em28xx-audio.c
+++ b/drivers/media/usb/em28xx/em28xx-audio.c
@@ -747,7 +747,8 @@ static void __exit em28xx_alsa_unregister(void)
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Markus Rechberger <mrechberger@gmail.com>");
 MODULE_AUTHOR("Mauro Carvalho Chehab <mchehab@redhat.com>");
-MODULE_DESCRIPTION("Em28xx Audio driver");
+MODULE_DESCRIPTION(DRIVER_DESC " - audio interface");
+MODULE_VERSION(EM28XX_VERSION);
 
 module_init(em28xx_alsa_register);
 module_exit(em28xx_alsa_unregister);
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index 36b2f1ab4474..2ad84ff1fc4f 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -39,8 +39,6 @@
 		      "Mauro Carvalho Chehab <mchehab@infradead.org>, " \
 		      "Sascha Sommer <saschasommer@freenet.de>"
 
-#define DRIVER_DESC         "Empia em28xx based USB core driver"
-
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index f72663a9b5c5..7fa1c804c34c 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -54,9 +54,11 @@
 #include "m88ds3103.h"
 #include "m88ts2022.h"
 
-MODULE_DESCRIPTION("driver for em28xx based DVB cards");
 MODULE_AUTHOR("Mauro Carvalho Chehab <mchehab@infradead.org>");
 MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION(DRIVER_DESC " - digital TV interface");
+MODULE_VERSION(EM28XX_VERSION);
+
 
 static unsigned int debug;
 module_param(debug, int, 0644);
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index eed7dd79f734..f3b629dd57ae 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -836,7 +836,8 @@ static void __exit em28xx_rc_unregister(void)
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Mauro Carvalho Chehab <mchehab@redhat.com>");
-MODULE_DESCRIPTION("Em28xx Input driver");
+MODULE_DESCRIPTION(DRIVER_DESC " - input interface");
+MODULE_VERSION(EM28XX_VERSION);
 
 module_init(em28xx_rc_register);
 module_exit(em28xx_rc_unregister);
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index ea3653248d25..c0f74e58037c 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -50,8 +50,6 @@
 		      "Mauro Carvalho Chehab <mchehab@infradead.org>, " \
 		      "Sascha Sommer <saschasommer@freenet.de>"
 
-#define DRIVER_DESC         "Empia em28xx based USB video device driver"
-
 static unsigned int isoc_debug;
 module_param(isoc_debug, int, 0644);
 MODULE_PARM_DESC(isoc_debug, "enable debug messages [isoc transfers]");
@@ -78,7 +76,7 @@ do {\
   } while (0)
 
 MODULE_AUTHOR(DRIVER_AUTHOR);
-MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_DESCRIPTION(DRIVER_DESC " - v4l2 interface");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(EM28XX_VERSION);
 
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index ac79501f5d9f..db47c2236ca4 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -27,6 +27,7 @@
 #define _EM28XX_H
 
 #define EM28XX_VERSION "0.2.1"
+#define DRIVER_DESC    "Empia em28xx device driver"
 
 #include <linux/workqueue.h>
 #include <linux/i2c.h>
-- 
1.8.3.1

[PATCH v3 20/24] em28xx: Fix em28xx deplock

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

When em28xx extensions are loaded/removed, there are two locks:

a single static em28xx_devlist_mutex that registers each extension
and the struct em28xx dev->lock.

When extensions are registered, em28xx_devlist_mutex is taken first,
and then dev->lock.

Be sure that, when extensions are being removed, the same order
will be used.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 10 ++++++----
 drivers/media/usb/em28xx/em28xx-core.c  |  2 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 4fe742429f2c..16383f46dae9 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3334,9 +3334,7 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
 	dev->disconnected = 1;
 
 	if (dev->is_audio_only) {
-		mutex_lock(&dev->lock);
 		em28xx_close_extension(dev);
-		mutex_unlock(&dev->lock);
 		return;
 	}
 
@@ -3355,19 +3353,23 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
 		em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
 		em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE);
 	}
+	mutex_unlock(&dev->lock);
 
 	em28xx_close_extension(dev);
+
 	/* NOTE: must be called BEFORE the resources are released */
 
+	mutex_lock(&dev->lock);
 	if (!dev->users)
 		em28xx_release_resources(dev);
 
-	mutex_unlock(&dev->lock);
-
 	if (!dev->users) {
+		mutex_unlock(&dev->lock);
 		kfree(dev->alt_max_pkt_size_isoc);
 		kfree(dev);
+		return;
 	}
+	mutex_unlock(&dev->lock);
 }
 
 static struct usb_driver em28xx_usb_driver = {
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index 2ad84ff1fc4f..d6928d83fb2a 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -1098,8 +1098,10 @@ void em28xx_close_extension(struct em28xx *dev)
 
 	mutex_lock(&em28xx_devlist_mutex);
 	list_for_each_entry(ops, &em28xx_extension_devlist, next) {
+		mutex_lock(&dev->lock);
 		if (ops->fini)
 			ops->fini(dev);
+		mutex_unlock(&dev->lock);
 	}
 	list_del(&dev->devlist);
 	mutex_unlock(&em28xx_devlist_mutex);
-- 
1.8.3.1

Re: [PATCH v3 20/24] em28xx: Fix em28xx deplock

[Frank Schäfer]

Am 28.12.2013 13:16, schrieb Mauro Carvalho Chehab:
> From: Mauro Carvalho Chehab <m.chehab@samsung.com>
>
> When em28xx extensions are loaded/removed, there are two locks:
>
> a single static em28xx_devlist_mutex that registers each extension
> and the struct em28xx dev->lock.
>
> When extensions are registered, em28xx_devlist_mutex is taken first,
> and then dev->lock.
>
> Be sure that, when extensions are being removed, the same order
> will be used.
>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
>  drivers/media/usb/em28xx/em28xx-cards.c | 10 ++++++----
>  drivers/media/usb/em28xx/em28xx-core.c  |  2 ++
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index 4fe742429f2c..16383f46dae9 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -3334,9 +3334,7 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
>  	dev->disconnected = 1;
>  
>  	if (dev->is_audio_only) {
> -		mutex_lock(&dev->lock);
>  		em28xx_close_extension(dev);
> -		mutex_unlock(&dev->lock);
>  		return;
>  	}
>  
> @@ -3355,19 +3353,23 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
>  		em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);
>  		em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE);
>  	}
> +	mutex_unlock(&dev->lock);
>  
>  	em28xx_close_extension(dev);
> +
>  	/* NOTE: must be called BEFORE the resources are released */
>  
> +	mutex_lock(&dev->lock);
>  	if (!dev->users)
>  		em28xx_release_resources(dev);
>  

> -	mutex_unlock(&dev->lock);
> -
>  	if (!dev->users) {
> +		mutex_unlock(&dev->lock);
>  		kfree(dev->alt_max_pkt_size_isoc);
>  		kfree(dev);
> +		return;
>  	}
> +	mutex_unlock(&dev->lock);
No functional change here, it just needlessly complicates the code.
I assume it's a leftover from experiments. ;)

>  }
>  
>  static struct usb_driver em28xx_usb_driver = {
> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
> index 2ad84ff1fc4f..d6928d83fb2a 100644
> --- a/drivers/media/usb/em28xx/em28xx-core.c
> +++ b/drivers/media/usb/em28xx/em28xx-core.c
> @@ -1098,8 +1098,10 @@ void em28xx_close_extension(struct em28xx *dev)
>  
>  	mutex_lock(&em28xx_devlist_mutex);
>  	list_for_each_entry(ops, &em28xx_extension_devlist, next) {
> +		mutex_lock(&dev->lock);
>  		if (ops->fini)
>  			ops->fini(dev);
> +		mutex_unlock(&dev->lock);
>  	}

Why not move the locking/unlocking of dev->lock outside
list_for_each_entry() ?
No need to do this one time for each extension.

>  	list_del(&dev->devlist);
>  	mutex_unlock(&em28xx_devlist_mutex);
Apart from these 2 minor issues, the patch looks good and should fix the
warning.

[PATCH v3 21/24] em28xx: USB: adjust for changed 3.8 USB API

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

The recent changes in the USB API ("implement new semantics for
URB_ISO_ASAP") made the former meaning of the URB_ISO_ASAP flag the
default, and changed this flag to mean that URBs can be delayed.
This is not the behaviour wanted by any of the audio drivers because
it leads to discontinuous playback with very small period sizes.
Therefore, our URBs need to be submitted without this flag.

This patch implements the same fix as found at snd-usb-audio driver
(commit c75c5ab575af7db707689cdbb5a5c458e9a034bb)

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-audio.c | 2 +-
 drivers/media/usb/em28xx/em28xx-core.c  | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c
index a6eef06ffdcd..54f4eb6d513c 100644
--- a/drivers/media/usb/em28xx/em28xx-audio.c
+++ b/drivers/media/usb/em28xx/em28xx-audio.c
@@ -195,7 +195,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev)
 		urb->dev = dev->udev;
 		urb->context = dev;
 		urb->pipe = usb_rcvisocpipe(dev->udev, EM28XX_EP_AUDIO);
-		urb->transfer_flags = URB_ISO_ASAP;
+		urb->transfer_flags = 0;
 		urb->transfer_buffer = dev->adev.transfer_buffer[i];
 		urb->interval = 1;
 		urb->complete = em28xx_audio_isocirq;
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index d6928d83fb2a..8376b9e6397f 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -953,8 +953,7 @@ int em28xx_alloc_urbs(struct em28xx *dev, enum em28xx_mode mode, int xfer_bulk,
 			usb_fill_int_urb(urb, dev->udev, pipe,
 					 usb_bufs->transfer_buffer[i], sb_size,
 					 em28xx_irq_callback, dev, 1);
-			urb->transfer_flags = URB_ISO_ASAP |
-					      URB_NO_TRANSFER_DMA_MAP;
+			urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
 			k = 0;
 			for (j = 0; j < usb_bufs->num_packets; j++) {
 				urb->iso_frame_desc[j].offset = k;
-- 
1.8.3.1

Re: [PATCH v3 21/24] em28xx: USB: adjust for changed 3.8 USB API

[Hans Verkuil]

On 12/28/2013 01:16 PM, Mauro Carvalho Chehab wrote:
> From: Mauro Carvalho Chehab <m.chehab@samsung.com>
> 
> The recent changes in the USB API ("implement new semantics for
> URB_ISO_ASAP") made the former meaning of the URB_ISO_ASAP flag the
> default, and changed this flag to mean that URBs can be delayed.
> This is not the behaviour wanted by any of the audio drivers because
> it leads to discontinuous playback with very small period sizes.
> Therefore, our URBs need to be submitted without this flag.

Does this affect other drivers as well? E.g. cx231xx-audio.c uses this
as well.

Regards,

	Hans

> 
> This patch implements the same fix as found at snd-usb-audio driver
> (commit c75c5ab575af7db707689cdbb5a5c458e9a034bb)
> 
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
>  drivers/media/usb/em28xx/em28xx-audio.c | 2 +-
>  drivers/media/usb/em28xx/em28xx-core.c  | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c
> index a6eef06ffdcd..54f4eb6d513c 100644
> --- a/drivers/media/usb/em28xx/em28xx-audio.c
> +++ b/drivers/media/usb/em28xx/em28xx-audio.c
> @@ -195,7 +195,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev)
>  		urb->dev = dev->udev;
>  		urb->context = dev;
>  		urb->pipe = usb_rcvisocpipe(dev->udev, EM28XX_EP_AUDIO);
> -		urb->transfer_flags = URB_ISO_ASAP;
> +		urb->transfer_flags = 0;
>  		urb->transfer_buffer = dev->adev.transfer_buffer[i];
>  		urb->interval = 1;
>  		urb->complete = em28xx_audio_isocirq;
> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
> index d6928d83fb2a..8376b9e6397f 100644
> --- a/drivers/media/usb/em28xx/em28xx-core.c
> +++ b/drivers/media/usb/em28xx/em28xx-core.c
> @@ -953,8 +953,7 @@ int em28xx_alloc_urbs(struct em28xx *dev, enum em28xx_mode mode, int xfer_bulk,
>  			usb_fill_int_urb(urb, dev->udev, pipe,
>  					 usb_bufs->transfer_buffer[i], sb_size,
>  					 em28xx_irq_callback, dev, 1);
> -			urb->transfer_flags = URB_ISO_ASAP |
> -					      URB_NO_TRANSFER_DMA_MAP;
> +			urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
>  			k = 0;
>  			for (j = 0; j < usb_bufs->num_packets; j++) {
>  				urb->iso_frame_desc[j].offset = k;
> 

Re: [PATCH v3 21/24] em28xx: USB: adjust for changed 3.8 USB API

[Mauro Carvalho Chehab]

Em Sat, 28 Dec 2013 13:26:00 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 12/28/2013 01:16 PM, Mauro Carvalho Chehab wrote:
> > From: Mauro Carvalho Chehab <m.chehab@samsung.com>
> > 
> > The recent changes in the USB API ("implement new semantics for
> > URB_ISO_ASAP") made the former meaning of the URB_ISO_ASAP flag the
> > default, and changed this flag to mean that URBs can be delayed.
> > This is not the behaviour wanted by any of the audio drivers because
> > it leads to discontinuous playback with very small period sizes.
> > Therefore, our URBs need to be submitted without this flag.
> 
> Does this affect other drivers as well? E.g. cx231xx-audio.c uses this
> as well.

Likely yes. 

I should have tagged this specific patch as RFC, more tests are needed
for this change.

In a matter of fact, at least on the test machine I'm using here, I didn't
notice any difference with or without this patch. 

However, on another test machine, audio was not behaving well with some
USB devices, and I'm hoping that this change will fix it.
 
Unfortunately, I'm currently without access to any other test hardware
until the end of the next week.

> Regards,
> 
> 	Hans
> 
> > 
> > This patch implements the same fix as found at snd-usb-audio driver
> > (commit c75c5ab575af7db707689cdbb5a5c458e9a034bb)
> > 
> > Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> > ---
> >  drivers/media/usb/em28xx/em28xx-audio.c | 2 +-
> >  drivers/media/usb/em28xx/em28xx-core.c  | 3 +--
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/media/usb/em28xx/em28xx-audio.c b/drivers/media/usb/em28xx/em28xx-audio.c
> > index a6eef06ffdcd..54f4eb6d513c 100644
> > --- a/drivers/media/usb/em28xx/em28xx-audio.c
> > +++ b/drivers/media/usb/em28xx/em28xx-audio.c
> > @@ -195,7 +195,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev)
> >  		urb->dev = dev->udev;
> >  		urb->context = dev;
> >  		urb->pipe = usb_rcvisocpipe(dev->udev, EM28XX_EP_AUDIO);
> > -		urb->transfer_flags = URB_ISO_ASAP;
> > +		urb->transfer_flags = 0;
> >  		urb->transfer_buffer = dev->adev.transfer_buffer[i];
> >  		urb->interval = 1;
> >  		urb->complete = em28xx_audio_isocirq;
> > diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
> > index d6928d83fb2a..8376b9e6397f 100644
> > --- a/drivers/media/usb/em28xx/em28xx-core.c
> > +++ b/drivers/media/usb/em28xx/em28xx-core.c
> > @@ -953,8 +953,7 @@ int em28xx_alloc_urbs(struct em28xx *dev, enum em28xx_mode mode, int xfer_bulk,
> >  			usb_fill_int_urb(urb, dev->udev, pipe,
> >  					 usb_bufs->transfer_buffer[i], sb_size,
> >  					 em28xx_irq_callback, dev, 1);
> > -			urb->transfer_flags = URB_ISO_ASAP |
> > -					      URB_NO_TRANSFER_DMA_MAP;
> > +			urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
> >  			k = 0;
> >  			for (j = 0; j < usb_bufs->num_packets; j++) {
> >  				urb->iso_frame_desc[j].offset = k;
> > 
> 


-- 

Cheers,
Mauro

[PATCH v3 22/24] em28xx: use a better value for I2C timeouts

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

In the lack of a better spec, let's assume the timeout
values compatible with SMBus spec:
	http://smbus.org/specs/smbus110.pdf

at chapter 8 - Electrical Characteristics of SMBus devices

Ok, SMBus is a subset of I2C, and not all devices will be
following it, but the timeout value before this patch was not
even following the spec.

So, while we don't have a better guess for it, use 35 + 1
ms as the timeout.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx.h | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index db47c2236ca4..9af19332b0f1 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -183,8 +183,21 @@
 
 #define EM28XX_INTERLACED_DEFAULT 1
 
-/* time in msecs to wait for i2c xfers to finish */
-#define EM2800_I2C_XFER_TIMEOUT		20
+/*
+ * Time in msecs to wait for i2c xfers to finish.
+ * 35ms is the maximum time a SMBUS device could wait when
+ * clock stretching is used. As the transfer itself will take
+ * some time to happen, set it to 35 ms.
+ *
+ * Ok, I2C doesn't specify any limit. So, eventually, we may need
+ * to increase this timeout.
+ *
+ * FIXME: this assumes that an I2C message is not longer than 1ms.
+ * This is actually dependent on the I2C bus speed, although most
+ * devices use a 100kHz clock. So, this assumtion is true most of
+ * the time.
+ */
+#define EM2800_I2C_XFER_TIMEOUT		36
 
 /* time in msecs to wait for AC97 xfers to finish */
 #define EM2800_AC97_XFER_TIMEOUT	100
-- 
1.8.3.1

[PATCH v3 23/24] em28xx: don't return -ENODEV for I2C xfer errors

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

-ENODEV reports a permanent condition where a device is not found.
Except during device detection, this is not the case of I2C
transfer timeout errors.

So, change them to return -EIO instead.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 420fddf7da3a..16862c4cc745 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -81,7 +81,7 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 			return len;
 		if (ret == 0x94 + len - 1) {
 			em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
-			return -ENODEV;
+			return -EIO;
 		}
 		if (ret < 0) {
 			em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
@@ -125,7 +125,7 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 			break;
 		if (ret == 0x94 + len - 1) {
 			em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
-			return -ENODEV;
+			return -EIO;
 		}
 		if (ret < 0) {
 			em28xx_warn("failed to get i2c transfer status from bridge register (error=%i)\n",
@@ -214,7 +214,7 @@ retry:
 	if (ret == 0x10) {
 		em28xx_warn("I2C transfer timeout on writing to addr 0x%02x",
 			    addr);
-		return -ENODEV;
+		return -EIO;
 	}
 	em28xx_warn("write to i2c device at 0x%x timed out (ret=0x%02x)\n",
 		    addr, ret);
@@ -250,7 +250,7 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len)
 		* bytes if we are on bus B AND there was no write attempt to the
 		* specified slave address before AND no device is present at the
 		* requested slave address.
-		* Anyway, the next check will fail with -ENODEV in this case, so avoid
+		* Anyway, the next check will fail with -EIO in this case, so avoid
 		* spamming the system log on device probing and do nothing here.
 		*/
 
@@ -270,7 +270,7 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len)
 
 	if (ret == 0x10) {
 		em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr);
-		return -ENODEV;
+		return -EIO;
 	}
 
 	em28xx_warn("unknown i2c error (status=%i)\n", ret);
@@ -331,7 +331,7 @@ static int em25xx_bus_B_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 		return len;
 	else if (ret > 0) {
 		em28xx_warn("Bus B R08 returned 0x%02x: I2C timeout", ret);
-		return -ENODEV;
+		return -EIO;
 	}
 
 	return ret;
@@ -370,8 +370,6 @@ static int em25xx_bus_B_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 	 * bytes if we are on bus B AND there was no write attempt to the
 	 * specified slave address before AND no device is present at the
 	 * requested slave address.
-	 * Anyway, the next check will fail with -ENODEV in this case, so avoid
-	 * spamming the system log on device probing and do nothing here.
 	 */
 
 	/* Check success */
@@ -384,7 +382,7 @@ static int em25xx_bus_B_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 		return len;
 	else if (ret > 0) {
 		em28xx_warn("Bus B R08 returned 0x%02x: I2C timeout", ret);
-		return -ENODEV;
+		return -EIO;
 	}
 
 	return ret;
@@ -426,7 +424,7 @@ static inline int i2c_check_for_device(struct em28xx_i2c_bus *i2c_bus, u16 addr)
 		rc = em2800_i2c_check_for_device(dev, addr);
 	else if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM25XX_BUS_B)
 		rc = em25xx_bus_B_check_for_device(dev, addr);
-	if (rc == -ENODEV) {
+	if (rc < 0) {
 		if (i2c_debug)
 			printk(" no device\n");
 	}
@@ -516,7 +514,7 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
 			       addr, msgs[i].len);
 		if (!msgs[i].len) { /* no len: check only for device presence */
 			rc = i2c_check_for_device(i2c_bus, addr);
-			if (rc == -ENODEV) {
+			if (rc < 0) {
 				rt_mutex_unlock(&dev->i2c_bus_lock);
 				return rc;
 			}
-- 
1.8.3.1

[PATCH v3 24/24] em28xx: cleanup I2C debug messages

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <m.chehab@samsung.com>

The I2C output messages is too polluted. Clean it a little
bit, by:
	- use the proper core support for memory dumps;
	- hide most stuff under the i2c_debug umbrella;
	- add the missing KERN_CONT where needed;
	- use 2 levels or verbosity. Only the second one
	  will show the I2C transfer data.

Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 91 ++++++++++++++++++-----------------
 1 file changed, 47 insertions(+), 44 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index 16862c4cc745..ff288023e330 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -41,7 +41,7 @@ MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
 
 static unsigned int i2c_debug;
 module_param(i2c_debug, int, 0644);
-MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]");
+MODULE_PARM_DESC(i2c_debug, "i2c debug message level (1: normal debug, 2: show I2C transfers)");
 
 /*
  * em2800_i2c_send_bytes()
@@ -80,7 +80,9 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 		if (ret == 0x80 + len - 1)
 			return len;
 		if (ret == 0x94 + len - 1) {
-			em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
+			if (i2c_debug)
+				em28xx_warn("R05 returned 0x%02x: I2C timeout",
+					    ret);
 			return -EIO;
 		}
 		if (ret < 0) {
@@ -90,7 +92,8 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 		}
 		msleep(5);
 	}
-	em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
+	if (i2c_debug)
+		em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
 	return -EIO;
 }
 
@@ -124,7 +127,9 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 		if (ret == 0x84 + len - 1)
 			break;
 		if (ret == 0x94 + len - 1) {
-			em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
+			if (i2c_debug)
+				em28xx_warn("R05 returned 0x%02x: I2C timeout",
+					    ret);
 			return -EIO;
 		}
 		if (ret < 0) {
@@ -134,8 +139,11 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 		}
 		msleep(5);
 	}
-	if (ret != 0x84 + len - 1)
-		em28xx_warn("read from i2c device at 0x%x timed out\n", addr);
+	if (ret != 0x84 + len - 1) {
+		if (i2c_debug)
+			em28xx_warn("read from i2c device at 0x%x timed out\n",
+				    addr);
+	}
 
 	/* get the received message */
 	ret = dev->em28xx_read_reg_req_len(dev, 0x00, 4-len, buf2, len);
@@ -212,8 +220,9 @@ retry:
 	}
 
 	if (ret == 0x10) {
-		em28xx_warn("I2C transfer timeout on writing to addr 0x%02x",
-			    addr);
+		if (i2c_debug)
+			em28xx_warn("I2C transfer timeout on writing to addr 0x%02x",
+				    addr);
 		return -EIO;
 	}
 	em28xx_warn("write to i2c device at 0x%x timed out (ret=0x%02x)\n",
@@ -269,7 +278,9 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16 len)
 	} while (time_is_after_jiffies(timeout));
 
 	if (ret == 0x10) {
-		em28xx_warn("I2C transfer timeout on read from addr 0x%02x", addr);
+		if (i2c_debug)
+			em28xx_warn("I2C transfer timeout on read from addr 0x%02x",
+				    addr);
 		return -EIO;
 	}
 
@@ -381,7 +392,8 @@ static int em25xx_bus_B_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 	if (!ret)
 		return len;
 	else if (ret > 0) {
-		em28xx_warn("Bus B R08 returned 0x%02x: I2C timeout", ret);
+		if (i2c_debug)
+			em28xx_warn("Bus B R08 returned 0x%02x: I2C timeout", ret);
 		return -EIO;
 	}
 
@@ -424,10 +436,6 @@ static inline int i2c_check_for_device(struct em28xx_i2c_bus *i2c_bus, u16 addr)
 		rc = em2800_i2c_check_for_device(dev, addr);
 	else if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM25XX_BUS_B)
 		rc = em25xx_bus_B_check_for_device(dev, addr);
-	if (rc < 0) {
-		if (i2c_debug)
-			printk(" no device\n");
-	}
 	return rc;
 }
 
@@ -436,7 +444,7 @@ static inline int i2c_recv_bytes(struct em28xx_i2c_bus *i2c_bus,
 {
 	struct em28xx *dev = i2c_bus->dev;
 	u16 addr = msg.addr << 1;
-	int byte, rc = -EOPNOTSUPP;
+	int rc = -EOPNOTSUPP;
 
 	if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM28XX)
 		rc = em28xx_i2c_recv_bytes(dev, addr, msg.buf, msg.len);
@@ -444,10 +452,6 @@ static inline int i2c_recv_bytes(struct em28xx_i2c_bus *i2c_bus,
 		rc = em2800_i2c_recv_bytes(dev, addr, msg.buf, msg.len);
 	else if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM25XX_BUS_B)
 		rc = em25xx_bus_B_recv_bytes(dev, addr, msg.buf, msg.len);
-	if (i2c_debug) {
-		for (byte = 0; byte < msg.len; byte++)
-			printk(" %02x", msg.buf[byte]);
-	}
 	return rc;
 }
 
@@ -456,12 +460,8 @@ static inline int i2c_send_bytes(struct em28xx_i2c_bus *i2c_bus,
 {
 	struct em28xx *dev = i2c_bus->dev;
 	u16 addr = msg.addr << 1;
-	int byte, rc = -EOPNOTSUPP;
+	int rc = -EOPNOTSUPP;
 
-	if (i2c_debug) {
-		for (byte = 0; byte < msg.len; byte++)
-			printk(" %02x", msg.buf[byte]);
-	}
 	if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM28XX)
 		rc = em28xx_i2c_send_bytes(dev, addr, msg.buf, msg.len, stop);
 	else if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM2800)
@@ -506,7 +506,7 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
 	}
 	for (i = 0; i < num; i++) {
 		addr = msgs[i].addr << 1;
-		if (i2c_debug)
+		if (i2c_debug > 1)
 			printk(KERN_DEBUG "%s at %s: %s %s addr=%02x len=%d:",
 			       dev->name, __func__ ,
 			       (msgs[i].flags & I2C_M_RD) ? "read" : "write",
@@ -515,24 +515,33 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
 		if (!msgs[i].len) { /* no len: check only for device presence */
 			rc = i2c_check_for_device(i2c_bus, addr);
 			if (rc < 0) {
+				if (i2c_debug > 1)
+					printk(KERN_CONT " no device\n");
 				rt_mutex_unlock(&dev->i2c_bus_lock);
 				return rc;
 			}
 		} else if (msgs[i].flags & I2C_M_RD) {
 			/* read bytes */
 			rc = i2c_recv_bytes(i2c_bus, msgs[i]);
+
+			if (i2c_debug > 1 && rc >= 0)
+				printk(KERN_CONT " %*ph",
+				       msgs[i].len, msgs[i].buf);
 		} else {
+			if (i2c_debug > 1)
+				printk(KERN_CONT " %*ph",
+				       msgs[i].len, msgs[i].buf);
+
 			/* write bytes */
 			rc = i2c_send_bytes(i2c_bus, msgs[i], i == num - 1);
 		}
 		if (rc < 0) {
-			if (i2c_debug)
-				printk(" ERROR: %i\n", rc);
+			if (i2c_debug > 1)
+				printk(KERN_CONT " ERROR: %i\n", rc);
 			rt_mutex_unlock(&dev->i2c_bus_lock);
 			return rc;
-		}
-		if (i2c_debug)
-			printk("\n");
+		} else if (i2c_debug > 1)
+			printk(KERN_CONT "\n");
 	}
 
 	rt_mutex_unlock(&dev->i2c_bus_lock);
@@ -615,7 +624,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned bus,
 	 * calculation and returned device dataset. Simplifies the code a lot,
 	 * but we might have to deal with multiple sizes in the future !
 	 */
-	int i, err;
+	int err;
 	struct em28xx_eeprom *dev_config;
 	u8 buf, *data;
 
@@ -646,20 +655,14 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned bus,
 		goto error;
 	}
 
-	/* Display eeprom content */
-	for (i = 0; i < len; i++) {
-		if (0 == (i % 16)) {
-			if (dev->eeprom_addrwidth_16bit)
-				em28xx_info("i2c eeprom %04x:", i);
-			else
-				em28xx_info("i2c eeprom %02x:", i);
-		}
-		printk(" %02x", data[i]);
-		if (15 == (i % 16))
-			printk("\n");
+	if (i2c_debug) {
+		/* Display eeprom content */
+		print_hex_dump(KERN_INFO, "eeprom ", DUMP_PREFIX_OFFSET,
+			       16, 1, data, len, true);
+
+		if (dev->eeprom_addrwidth_16bit)
+			em28xx_info("eeprom %06x: ... (skipped)\n", 256);
 	}
-	if (dev->eeprom_addrwidth_16bit)
-		em28xx_info("i2c eeprom %04x: ... (skipped)\n", i);
 
 	if (dev->eeprom_addrwidth_16bit &&
 	    data[0] == 0x26 && data[3] == 0x00) {
-- 
1.8.3.1

Re: [PATCH v3 00/24] em28xx: split analog part into a separate module

[Frank Schäfer]

Am 28.12.2013 13:15, schrieb Mauro Carvalho Chehab:
> This patch series split em28xx into a separate V4L2 driver,
> allowing the new dvb-only chips to be supported without requiring
> V4L2.
>
> While testing the original patchset, I noticed several issues with
> HVR-950. The remaining patches on this series fix most of those
> issues.
>
> There's one remaining issue: on my tests, when connecting the device
> into an USB 3.0 port, the AC97 EMP202 is not properly detected. 
> Also, the audio doesn't work fine. I'm still investigating what
> would be the root cause for that.
>
> Mauro Carvalho Chehab (24):
>   em28xx: move some video-specific functions to em28xx-video
>   em28xx: some cosmetic changes
>   em28xx: move analog-specific init to em28xx-video
>   em28xx: make em28xx-video to be a separate module
>   em28xx: initialize analog I2C devices at the right place
>   em28xx-cards: remove a now dead code
>   em28xx: fix a cut and paste error

I tried to review the core/V4L2 split patches [1-7] in detail, but it's
nearly impossible.
With one patch, you introduce issues which you then fix with other
patches later.
Patch 3 for example breaks compilation 2 times and introduces a deadlock
and an oops...

Can you please rework these patches and resend them when you think they
are ready for reviewing ?

I've made some basic tests with the whole series applied and didn't
observe any problems so far.
Unfortunately I don't have a DVB-only device, too.

Regards,
Frank

>   em28xx: add warn messages for timeout
>   em28xx: improve extension information messages
>   em28xx: convert i2c wait completion logic to use jiffies
>   tvp5150: make read operations atomic
>   tuner-xc2028: remove unused code
>   em28xx: retry I2C ops if failed by timeout
>   em28xx: remove a false positive warning
>   em28xx: check if a device has audio earlier
>   em28xx: properly implement AC97 wait code
>   em28xx: initialize audio latter
>   em28xx: improve I2C timeout error message
>   em28xx: unify module version
>   em28xx: Fix em28xx deplock
>   em28xx: USB: adjust for changed 3.8 USB API
>   em28xx: use a better value for I2C timeouts
>   em28xx: don't return -ENODEV for I2C xfer errors
>   em28xx: cleanup I2C debug messages
>
>  drivers/media/i2c/tvp5150.c              |  22 +-
>  drivers/media/tuners/tuner-xc2028.c      |   9 -
>  drivers/media/usb/em28xx/Kconfig         |   6 +-
>  drivers/media/usb/em28xx/Makefile        |   5 +-
>  drivers/media/usb/em28xx/em28xx-audio.c  |   9 +-
>  drivers/media/usb/em28xx/em28xx-camera.c |   1 +
>  drivers/media/usb/em28xx/em28xx-cards.c  | 310 ++--------------
>  drivers/media/usb/em28xx/em28xx-core.c   | 295 +--------------
>  drivers/media/usb/em28xx/em28xx-dvb.c    |  11 +-
>  drivers/media/usb/em28xx/em28xx-i2c.c    | 226 ++++++------
>  drivers/media/usb/em28xx/em28xx-input.c  |   7 +-
>  drivers/media/usb/em28xx/em28xx-v4l.h    |  24 ++
>  drivers/media/usb/em28xx/em28xx-vbi.c    |   1 +
>  drivers/media/usb/em28xx/em28xx-video.c  | 607 +++++++++++++++++++++++++++++--
>  drivers/media/usb/em28xx/em28xx.h        |  52 +--
>  15 files changed, 835 insertions(+), 750 deletions(-)
>  create mode 100644 drivers/media/usb/em28xx/em28xx-v4l.h
>

Re: [PATCH v3 00/24] em28xx: split analog part into a separate module

[Mauro Carvalho Chehab]

Em Fri, 03 Jan 2014 21:29:20 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:

> Am 28.12.2013 13:15, schrieb Mauro Carvalho Chehab:
> > This patch series split em28xx into a separate V4L2 driver,
> > allowing the new dvb-only chips to be supported without requiring
> > V4L2.
> >
> > While testing the original patchset, I noticed several issues with
> > HVR-950. The remaining patches on this series fix most of those
> > issues.
> >
> > There's one remaining issue: on my tests, when connecting the device
> > into an USB 3.0 port, the AC97 EMP202 is not properly detected. 
> > Also, the audio doesn't work fine. I'm still investigating what
> > would be the root cause for that.
> >
> > Mauro Carvalho Chehab (24):
> >   em28xx: move some video-specific functions to em28xx-video
> >   em28xx: some cosmetic changes
> >   em28xx: move analog-specific init to em28xx-video
> >   em28xx: make em28xx-video to be a separate module
> >   em28xx: initialize analog I2C devices at the right place
> >   em28xx-cards: remove a now dead code
> >   em28xx: fix a cut and paste error
> 
> I tried to review the core/V4L2 split patches [1-7] in detail, but it's
> nearly impossible.
> With one patch, you introduce issues which you then fix with other
> patches later.
> Patch 3 for example breaks compilation 2 times and introduces a deadlock
> and an oops...

That's actually the only patch that broke compilation. This patch was
actually generated as an intermediate step to patch 4, using git citool,
in order to make easier to review the code changes, splitting the code
move from the actual device split.

Anyway, it shouldn't break compilation.

The OOPS was fixed on a separate patch (patch 5). I opted to keep it in
separate, as it seemed interesting to preserve in git history why that
change happened. Eventually, I can reorder the patch to avoid the OOPS
to actually happen on bisect.

> Can you please rework these patches and resend them when you think they
> are ready for reviewing ?

Done. See Patch series v4.

> I've made some basic tests with the whole series applied and didn't
> observe any problems so far.

Ok. Then the better seems to apply them upstream, in order to allow
them to have a broader testing.

> Unfortunately I don't have a DVB-only device, too.

I'm acquiring two new em28xx devices. Let's hope that at least one of them
is DVB-only.

> 
> Regards,
> Frank
> 
> >   em28xx: add warn messages for timeout
> >   em28xx: improve extension information messages
> >   em28xx: convert i2c wait completion logic to use jiffies
> >   tvp5150: make read operations atomic
> >   tuner-xc2028: remove unused code
> >   em28xx: retry I2C ops if failed by timeout
> >   em28xx: remove a false positive warning
> >   em28xx: check if a device has audio earlier
> >   em28xx: properly implement AC97 wait code
> >   em28xx: initialize audio latter
> >   em28xx: improve I2C timeout error message
> >   em28xx: unify module version
> >   em28xx: Fix em28xx deplock
> >   em28xx: USB: adjust for changed 3.8 USB API
> >   em28xx: use a better value for I2C timeouts
> >   em28xx: don't return -ENODEV for I2C xfer errors
> >   em28xx: cleanup I2C debug messages
> >
> >  drivers/media/i2c/tvp5150.c              |  22 +-
> >  drivers/media/tuners/tuner-xc2028.c      |   9 -
> >  drivers/media/usb/em28xx/Kconfig         |   6 +-
> >  drivers/media/usb/em28xx/Makefile        |   5 +-
> >  drivers/media/usb/em28xx/em28xx-audio.c  |   9 +-
> >  drivers/media/usb/em28xx/em28xx-camera.c |   1 +
> >  drivers/media/usb/em28xx/em28xx-cards.c  | 310 ++--------------
> >  drivers/media/usb/em28xx/em28xx-core.c   | 295 +--------------
> >  drivers/media/usb/em28xx/em28xx-dvb.c    |  11 +-
> >  drivers/media/usb/em28xx/em28xx-i2c.c    | 226 ++++++------
> >  drivers/media/usb/em28xx/em28xx-input.c  |   7 +-
> >  drivers/media/usb/em28xx/em28xx-v4l.h    |  24 ++
> >  drivers/media/usb/em28xx/em28xx-vbi.c    |   1 +
> >  drivers/media/usb/em28xx/em28xx-video.c  | 607 +++++++++++++++++++++++++++++--
> >  drivers/media/usb/em28xx/em28xx.h        |  52 +--
> >  15 files changed, 835 insertions(+), 750 deletions(-)
> >  create mode 100644 drivers/media/usb/em28xx/em28xx-v4l.h
> >
> 
> --
> 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


-- 

Cheers,
Mauro