Skip to contentSkip to conversation

em28xx: get rid of the dummy clock source

PATCH New Frank Schäfer 1 participant 5 messages Apr 15, 2017

Subsystems (2)
Media (V4L/DVB) EM28XX VIDEO4LINUX driver
lore
Share & tools
lore

Frank Schäfer

author
#0 lore raw

PATCH 1/5 em28xx: get rid of the dummy clock source

The v4l2 dummy clock has been added with commit fc5d0f8a8878 ("V4L2: em28xx: register a V4L2 clock source") to be able to use the ov2640 soc_camera driver.
Since commit 46796cfcd346 ("ov2640: use standard clk and enable it") it is no longer required.

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-camera.c | 30 ++++++------------------------
 drivers/media/usb/em28xx/em28xx-video.c  |  6 ------
 drivers/media/usb/em28xx/em28xx.h        |  1 -
 3 files changed, 6 insertions(+), 31 deletions(-)
drivers/media/usb/em28xx/em28xx-camera.c +6 -24
index 2f59237ee399..d43f630050bb 100644
@@ -24,7 +24,6 @@
 #include <linux/i2c.h>
 #include <linux/usb.h>
 #include <media/i2c/mt9v011.h>
-#include <media/v4l2-clk.h>
 #include <media/v4l2-common.h>
 
 /* Possible i2c addresses of Micron sensors */

@@ -331,17 +330,9 @@ int em28xx_detect_sensor(struct em28xx *dev)
 
 int em28xx_init_camera(struct em28xx *dev)
 {
-	char clk_name[V4L2_CLK_NAME_SIZE];
 	struct i2c_client *client = &dev->i2c_client[dev->def_i2c_bus];
 	struct i2c_adapter *adap = &dev->i2c_adap[dev->def_i2c_bus];
 	struct em28xx_v4l2 *v4l2 = dev->v4l2;
-	int ret = 0;
-
-	v4l2_clk_name_i2c(clk_name, sizeof(clk_name),
-			  i2c_adapter_id(adap), client->addr);
-	v4l2->clk = v4l2_clk_register_fixed(clk_name, -EINVAL);
-	if (IS_ERR(v4l2->clk))
-		return PTR_ERR(v4l2->clk);
 
 	switch (dev->em28xx_sensor) {
 	case EM28XX_MT9V011:

@@ -371,10 +362,8 @@ int em28xx_init_camera(struct em28xx *dev)
 		pdata.xtal = v4l2->sensor_xtal;
 		if (NULL ==
 		    v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap,
-					      &mt9v011_info, NULL)) {
-			ret = -ENODEV;
-			break;
-		}
+					      &mt9v011_info, NULL))
+			return -ENODEV;
 		/* probably means GRGB 16 bit bayer */
 		v4l2->vinmode = 0x0d;
 		v4l2->vinctl = 0x00;

@@ -430,10 +419,8 @@ int em28xx_init_camera(struct em28xx *dev)
 		subdev =
 		     v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap,
 					       &ov2640_info, NULL);
-		if (NULL == subdev) {
-			ret = -ENODEV;
-			break;
-		}
+		if (subdev == NULL)
+			return -ENODEV;
 
 		format.format.code = MEDIA_BUS_FMT_YUYV8_2X8;
 		format.format.width = 640;

@@ -450,14 +437,9 @@ int em28xx_init_camera(struct em28xx *dev)
 	}
 	case EM28XX_NOSENSOR:
 	default:
-		ret = -EINVAL;
-	}
-
-	if (ret < 0) {
-		v4l2_clk_unregister_fixed(v4l2->clk);
-		v4l2->clk = NULL;
+		return -EINVAL;
 	}
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(em28xx_init_camera);
drivers/media/usb/em28xx/em28xx-video.c +0 -6
index 8d93100334ea..3cbc3d4270a3 100644
@@ -43,7 +43,6 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-event.h>
-#include <media/v4l2-clk.h>
 #include <media/drv-intf/msp3400.h>
 #include <media/tuner.h>
 

@@ -2140,11 +2139,6 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 	v4l2_ctrl_handler_free(&v4l2->ctrl_handler);
 	v4l2_device_unregister(&v4l2->v4l2_dev);
 
-	if (v4l2->clk) {
-		v4l2_clk_unregister_fixed(v4l2->clk);
-		v4l2->clk = NULL;
-	}
-
 	kref_put(&v4l2->ref, em28xx_free_v4l2);
 
 	mutex_unlock(&dev->lock);
drivers/media/usb/em28xx/em28xx.h +0 -1
index e9f379959fa5..e8d97d5ec161 100644
@@ -510,7 +510,6 @@ struct em28xx_v4l2 {
 
 	struct v4l2_device v4l2_dev;
 	struct v4l2_ctrl_handler ctrl_handler;
-	struct v4l2_clk *clk;
 
 	struct video_device vdev;
 	struct video_device vbi_dev;
Signature · 1 line
2.12.2
author
#1 lore raw

PATCH 2/5 em28xx: add missing auto-selections for build

With MEDIA_SUBDRV_AUTOSELECT enabled in the kernel config, the em28xx driver currently does't select some used subdrivers.
Fix this by adding the missing auto-selections to the Kconfig file.

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/Kconfig | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/media/usb/em28xx/Kconfig +6 -1
index aa131cf9989b..4cc029f18aa8 100644
@@ -12,7 +12,7 @@ config VIDEO_EM28XX_V4L2
 	select VIDEO_TVP5150 if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEO_MSP3400 if MEDIA_SUBDRV_AUTOSELECT
 	select VIDEO_MT9V011 if MEDIA_SUBDRV_AUTOSELECT && MEDIA_CAMERA_SUPPORT
-
+	select VIDEO_OV2640 if MEDIA_SUBDRV_AUTOSELECT && MEDIA_CAMERA_SUPPORT
 	---help---
 	  This is a video4linux driver for Empia 28xx based TV cards.
 

@@ -39,6 +39,7 @@ config VIDEO_EM28XX_DVB
 	depends on VIDEO_EM28XX && DVB_CORE
 	select DVB_LGDT330X if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_LGDT3305 if MEDIA_SUBDRV_AUTOSELECT
+	select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_ZL10353 if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_TDA10023 if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_S921 if MEDIA_SUBDRV_AUTOSELECT

@@ -61,6 +62,10 @@ config VIDEO_EM28XX_DVB
 	select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_TC90522 if MEDIA_SUBDRV_AUTOSELECT
 	select MEDIA_TUNER_QM1D1C0042 if MEDIA_SUBDRV_AUTOSELECT
+	select MEDIA_TUNER_SIMPLE if MEDIA_SUBDRV_AUTOSELECT
+	select MEDIA_TUNER_XC2028 if MEDIA_SUBDRV_AUTOSELECT
+	select MEDIA_TUNER_XC5000 if MEDIA_SUBDRV_AUTOSELECT
+	select MEDIA_TUNER_MT2060 if MEDIA_SUBDRV_AUTOSELECT
 	---help---
 	  This adds support for DVB cards based on the
 	  Empiatech em28xx chips.
Signature · 1 line
2.12.2
author
#2 lore raw

PATCH 3/5 em28xx: don't treat device as webcam if an unknown sensor is detected

In reply to Frank Schäfer

With an unknown sensor, norm_maxw() and norm_maxh() return 0 as max. height and width values, which causes a devide by zero in size_to_scale(). Of course we could use speculative default values for unknown sensors, but the chance that the device works at this resolution without any driver/setup is very low and therefore not worth the efforts. Instead, just don't treat the device as camera.
A message will then be printed to the log that the device isn't supported.

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/media/usb/em28xx/em28xx-cards.c +7 -3
index 5f80a1b2fb8c..48c7fec47509 100644
@@ -2917,7 +2917,9 @@ static void em28xx_card_setup(struct em28xx *dev)
 	 * If sensor is not found, then it isn't a webcam.
 	 */
 	if (dev->board.is_webcam) {
-		if (em28xx_detect_sensor(dev) < 0)
+		em28xx_detect_sensor(dev);
+		if (dev->em28xx_sensor == EM28XX_NOSENSOR)
+			/* NOTE: error/unknown sensor/no sensor */
 			dev->board.is_webcam = 0;
 	}
 

@@ -3665,9 +3667,11 @@ static int em28xx_usb_probe(struct usb_interface *interface,
 		try_bulk = usb_xfer_mode > 0;
 	}
 
-	/* Disable V4L2 if the device doesn't have a decoder */
+	/* Disable V4L2 if the device doesn't have a decoder or image sensor */
 	if (has_video &&
-	    dev->board.decoder == EM28XX_NODECODER && !dev->board.is_webcam) {
+	    dev->board.decoder == EM28XX_NODECODER &&
+	    dev->em28xx_sensor == EM28XX_NOSENSOR) {
+
 		dev_err(&interface->dev,
 			"Currently, V4L2 is not supported on this model\n");
 		has_video = false;
Signature · 1 line
2.12.2
author
#3 lore raw

PATCH 4/5 em28xx: shed some light on video input formats

In reply to Frank Schäfer

CbYCrY has been identified by looking into the tvp5150 driver and the saa7115 datasheet.
YUV formats have been verified with em2765 + ov2640 (VAD Laplace webcam). RGB8 formats have been verified with em2710/em2820 + mt9v011 (Silvercrest webcam 1.3mpix).
I also did some cross-checking with these two camera devices and 0x08-0x0b are at least 16 bits per pixel formats on em2710/em2820, too, and 0x0c-0x0f are at least 8 bits per pixel formats on em2765, too.

Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-camera.c | 10 ++++------
 drivers/media/usb/em28xx/em28xx-reg.h    | 18 ++++++++++++++++++
 drivers/media/usb/em28xx/em28xx-video.c  |  2 +-
 3 files changed, 23 insertions(+), 7 deletions(-)
drivers/media/usb/em28xx/em28xx-camera.c +4 -6
index d43f630050bb..95eaa55356a9 100644
@@ -364,8 +364,7 @@ int em28xx_init_camera(struct em28xx *dev)
 		    v4l2_i2c_new_subdev_board(&v4l2->v4l2_dev, adap,
 					      &mt9v011_info, NULL))
 			return -ENODEV;
-		/* probably means GRGB 16 bit bayer */
-		v4l2->vinmode = 0x0d;
+		v4l2->vinmode = EM28XX_VINMODE_RGB8_GRBG;
 		v4l2->vinctl = 0x00;
 
 		break;

@@ -376,8 +375,7 @@ int em28xx_init_camera(struct em28xx *dev)
 
 		em28xx_initialize_mt9m001(dev);
 
-		/* probably means BGGR 16 bit bayer */
-		v4l2->vinmode = 0x0c;
+		v4l2->vinmode = EM28XX_VINMODE_RGB8_BGGR;
 		v4l2->vinctl = 0x00;
 
 		break;

@@ -389,7 +387,7 @@ int em28xx_init_camera(struct em28xx *dev)
 		em28xx_write_reg(dev, EM28XX_R0F_XCLK, dev->board.xclk);
 		em28xx_initialize_mt9m111(dev);
 
-		v4l2->vinmode = 0x0a;
+		v4l2->vinmode = EM28XX_VINMODE_YUV422_UYVY;
 		v4l2->vinctl = 0x00;
 
 		break;

@@ -430,7 +428,7 @@ int em28xx_init_camera(struct em28xx *dev)
 		/* NOTE: for UXGA=1600x1200 switch to 12MHz */
 		dev->board.xclk = EM28XX_XCLK_FREQUENCY_24MHZ;
 		em28xx_write_reg(dev, EM28XX_R0F_XCLK, dev->board.xclk);
-		v4l2->vinmode = 0x08;
+		v4l2->vinmode = EM28XX_VINMODE_YUV422_YUYV;
 		v4l2->vinctl = 0x00;
 
 		break;
drivers/media/usb/em28xx/em28xx-reg.h +18 -0
index afe7a66d7dc8..747525ca7ed5 100644
@@ -93,6 +93,24 @@
 #define EM28XX_XCLK_FREQUENCY_24MHZ	0x0b
 
 #define EM28XX_R10_VINMODE	0x10
+	  /* used by all non-camera devices: */
+#define   EM28XX_VINMODE_YUV422_CbYCrY  0x10
+	  /* used by camera devices: */
+#define   EM28XX_VINMODE_YUV422_YUYV    0x08
+#define   EM28XX_VINMODE_YUV422_YVYU    0x09
+#define   EM28XX_VINMODE_YUV422_UYVY    0x0a
+#define   EM28XX_VINMODE_YUV422_VYUY    0x0b
+#define   EM28XX_VINMODE_RGB8_BGGR      0x0c
+#define   EM28XX_VINMODE_RGB8_GRBG      0x0d
+#define   EM28XX_VINMODE_RGB8_GBRG      0x0e
+#define   EM28XX_VINMODE_RGB8_RGGB      0x0f
+	  /*
+	   * apparently:
+	   *   bit 0: swap component 1+2 with 3+4
+	   *                 => e.g.: YUYV => YVYU, BGGR => GRBG
+	   *   bit 1: swap component 1 with 2 and 3 with 4
+	   *                 => e.g.: YUYV => UYVY, BGGR => GBRG
+	   */
 
 #define EM28XX_R11_VINCTRL	0x11
drivers/media/usb/em28xx/em28xx-video.c +1 -1
index 3cbc3d4270a3..aaa83f9e5c1a 100644
@@ -2459,7 +2459,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 	/*
 	 * Default format, used for tvp5150 or saa711x output formats
 	 */
-	v4l2->vinmode = 0x10;
+	v4l2->vinmode = EM28XX_VINMODE_YUV422_CbYCrY;
 	v4l2->vinctl  = EM28XX_VINCTRL_INTERLACED |
 			EM28XX_VINCTRL_CCIR656_ENABLE;
Signature · 1 line
2.12.2
author
#4 lore raw

PATCH 5/5 em28xx: add support for V4L2_PIX_FMT_SRGGB8

In reply to Frank Schäfer
Signed-off-by Frank Schäfer <[email protected]>
---
 drivers/media/usb/em28xx/em28xx-video.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/media/usb/em28xx/em28xx-video.c +5 -0
index aaa83f9e5c1a..8d253a5df0a9 100644
@@ -116,6 +116,11 @@ static struct em28xx_fmt format[] = {
 		.depth    = 16,
 		.reg      = EM28XX_OUTFMT_RGB_16_656,
 	}, {
+		.name     = "8 bpp Bayer RGRG..GBGB",
+		.fourcc   = V4L2_PIX_FMT_SRGGB8,
+		.depth    = 8,
+		.reg      = EM28XX_OUTFMT_RGB_8_RGRG,
+	}, {
 		.name     = "8 bpp Bayer BGBG..GRGR",
 		.fourcc   = V4L2_PIX_FMT_SBGGR8,
 		.depth    = 8,
Signature · 1 line
2.12.2