Skip to contentSkip to conversation

mt9m111: add support for mt9m112 since sensors seem identical

PATCH In review Mike Rapoport 3 participants 10 messages Nov 25, 2008 – Dec 3, 2008 Media (V4L/DVB)

lore
Share & tools
lore

Mike Rapoport

author
#0 lore raw

PATCH 1/1 mt9m111: add support for mt9m112 since sensors seem identical

Signed-off-by Mike Rapoport <[email protected]>
---
 drivers/media/video/mt9m111.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/media/video/mt9m111.c +2 -1
index da0b2d5..49c1167 100644
@@ -841,7 +841,8 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
-	case 0x143a:
+	case 0x143a: /* MT9M111 */
+	case 0x148c: /* MT9M112 */
 		mt9m111->model = V4L2_IDENT_MT9M111;
 		icd->formats = mt9m111_colour_formats;
 		icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
Signature · 6 lines
1.5.6.4

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
#1 lore raw
Quoted 4 lines · Mike Rapoport
Signed-off-by Mike Rapoport <[email protected]>
---
 drivers/media/video/mt9m111.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Yes, only sensor size and version seem different. I presume you have checked that the I2C addresses are the same (0x5d or 0x48), and the datasheets match ?

Then, would be please make me a favor : modify the Kconfig and mt9m111 comments to reflect that mt9m112 support is provided, please ?

Cheers.

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#2 lore raw
In reply to Mike Rapoport
Quoted 11 lines · Mike Rapoport
Signed-off-by Mike Rapoport <[email protected]>
---
 drivers/media/video/mt9m111.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/media/video/mt9m111.c +0 -0
index da0b2d5..49c1167 100644

drivers/media/video/mt9m111.c static int mt9m111_video_probe(struct soc_camera_device *icd) · line 846

Quoted 7 lines
drivers/media/video/mt9m111.c +2 -1
@@ -841,7 +841,8 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
-	case 0x143a:
+	case 0x143a: /* MT9M111 */
+	case 0x148c: /* MT9M112 */
 		mt9m111->model = V4L2_IDENT_MT9M111;

Wouldn't it be better to add a new chip ID? Are there any differences between the two models, that the user might want to know about?

Thanks
Guennadi

Quoted 10 lines
drivers/media/video/mt9m111.c +0 -0
 		icd->formats = mt9m111_colour_formats;
 		icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
Signature · 6 lines
1.5.6.4

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

author
#3 lore raw
Quoted 14 lines · Guennadi Liakhovetski
Quoted 12 lines · Mike Rapoport
Signed-off-by Mike Rapoport <[email protected]>
---
 drivers/media/video/mt9m111.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/media/video/mt9m111.c +0 -0
index da0b2d5..49c1167 100644
@@ -841,7 +841,8 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 	data = reg_read(CHIP_VERSION);
Quoted 8 lines

drivers/media/video/mt9m111.c static int mt9m111_video_probe(struct soc_camera_device *icd) · line 846

Quoted 5 lines
drivers/media/video/mt9m111.c +2 -1
 	switch (data) {
-	case 0x143a:
+	case 0x143a: /* MT9M111 */
+	case 0x148c: /* MT9M112 */
 		mt9m111->model = V4L2_IDENT_MT9M111;

Wouldn't it be better to add a new chip ID? Are there any differences between the two models, that the user might want to know about?

I don't have mt9m111 datasheet, I can only judge by "feature comparison" table in the mt9m112 datasheet. It seems that sensors differ in there advanced image processing and low power mode capabilities.
If you think it's worse adding new chip ID, I'll prepare the patches.

Quoted 18 lines

Thanks
Guennadi

Quoted 10 lines
drivers/media/video/mt9m111.c +0 -0
 		icd->formats = mt9m111_colour_formats;
 		icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
Signature · 6 lines
1.5.6.4

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Signature · 8 lines
Sincerely yours,
Mike.


--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
author
#4 lore raw

Guennadi, Robert,

Quoted 24 lines · Mike Rapoport
Quoted 14 lines · Guennadi Liakhovetski
Quoted 12 lines · Mike Rapoport
Signed-off-by Mike Rapoport <[email protected]>
---
 drivers/media/video/mt9m111.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/media/video/mt9m111.c +0 -0
index da0b2d5..49c1167 100644
@@ -841,7 +841,8 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 	data = reg_read(CHIP_VERSION);
Quoted 7 lines

drivers/media/video/mt9m111.c static int mt9m111_video_probe(struct soc_camera_device *icd) · line 846

Quoted 5 lines
drivers/media/video/mt9m111.c +2 -1
 	switch (data) {
-	case 0x143a:
+	case 0x143a: /* MT9M111 */
+	case 0x148c: /* MT9M112 */
 		mt9m111->model = V4L2_IDENT_MT9M111;

Wouldn't it be better to add a new chip ID? Are there any differences between the two models, that the user might want to know about?

Quoted 4 lines

I don't have mt9m111 datasheet, I can only judge by "feature comparison" table in the mt9m112 datasheet. It seems that sensors differ in there advanced image processing and low power mode capabilities.
If you think it's worse adding new chip ID, I'll prepare the patches.

Any comments? Should I add a new chip ID, or modifying Kconfig and comments would be enough?

Quoted 18 lines
Quoted 17 lines

Thanks
Guennadi

Quoted 10 lines
drivers/media/video/mt9m111.c +0 -0
 		icd->formats = mt9m111_colour_formats;
 		icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
Signature · 6 lines
1.5.6.4

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Signature · 7 lines
Sincerely yours,
Mike.

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
#5 lore raw
Quoted 27 lines · Mike Rapoport

Guennadi, Robert,

Quoted 24 lines · Mike Rapoport
Quoted 21 lines · Guennadi Liakhovetski
Quoted 10 lines · Mike Rapoport
Signed-off-by Mike Rapoport <[email protected]>
---
 drivers/media/video/mt9m111.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/media/video/mt9m111.c +0 -0
index da0b2d5..49c1167 100644

drivers/media/video/mt9m111.c static int mt9m111_video_probe(struct soc_camera_device *icd) · line 846

Quoted 7 lines
drivers/media/video/mt9m111.c +2 -1
@@ -841,7 +841,8 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
-	case 0x143a:
+	case 0x143a: /* MT9M111 */
+	case 0x148c: /* MT9M112 */
 		mt9m111->model = V4L2_IDENT_MT9M111;

Wouldn't it be better to add a new chip ID? Are there any differences between the two models, that the user might want to know about?

Quoted 7 lines
Quoted 4 lines

I don't have mt9m111 datasheet, I can only judge by "feature comparison" table in the mt9m112 datasheet. It seems that sensors differ in there advanced image processing and low power mode capabilities.
If you think it's worse adding new chip ID, I'll prepare the patches.

Any comments? Should I add a new chip ID, or modifying Kconfig and comments would be enough?

Personally, I think, it is nicer for a user to get precise information about their hardware. So, yes, I would add another ID, even if we don't support any extra features for now.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

author
#6 lore raw

PATCH 1/2 mt9m111: add support for mt9m112 since sensors seem identical

Add MT9M112 chip id

Signed-off-by Mike Rapoport <[email protected]>
 include/media/v4l2-chip-ident.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
include/media/v4l2-chip-ident.h +1 -0
index d73a8e9..50df634 100644
@@ -166,6 +166,7 @@ enum {
 	V4L2_IDENT_MT9M001C12ST		= 45000,
 	V4L2_IDENT_MT9M001C12STM	= 45005,
 	V4L2_IDENT_MT9M111		= 45007,
+	V4L2_IDENT_MT9M112		= 45008,
 	V4L2_IDENT_MT9V022IX7ATC	= 45010, /* No way to detect "normal" I77ATx */
 	V4L2_IDENT_MT9V022IX7ATM	= 45015, /* and "lead free" IA7ATx chips */
 };

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

author
#7 lore raw

PATCH 2/2 mt9m111: add support for mt9m112 since sensors seem identical

In reply to Guennadi Liakhovetski

Update mt9m11 driver and Kconfig

Signed-off-by Mike Rapoport <[email protected]>
 drivers/media/video/Kconfig   |    4 ++--
 drivers/media/video/mt9m111.c |   16 ++++++++++------
 2 files changed, 12 insertions(+), 8 deletions(-)
drivers/media/video/Kconfig +2 -2
index 47102c2..0848032 100644
@@ -725,10 +725,10 @@ config MT9M001_PCA9536_SWITCH
 	  extender to switch between 8 and 10 bit datawidth modes
 
 config SOC_CAMERA_MT9M111
-	tristate "mt9m111 support"
+	tristate "mt9m111 and mt9m112 support"
 	depends on SOC_CAMERA && I2C
 	help
-	  This driver supports MT9M111 cameras from Micron
+	  This driver supports MT9M111 and MT9M112 cameras from Micron
 
 config SOC_CAMERA_MT9V022
 	tristate "mt9v022 support"
drivers/media/video/mt9m111.c +10 -6
index da0b2d5..9a428b7 100644
@@ -1,5 +1,5 @@
 /*
- * Driver for MT9M111 CMOS Image Sensor from Micron
+ * Driver for MT9M111/MT9M112 CMOS Image Sensor from Micron
  *
  * Copyright (C) 2008, Robert Jarzmik <[email protected]>
  *

@@ -19,7 +19,7 @@
 #include <media/soc_camera.h>
 
 /*
- * mt9m111 i2c address is 0x5d or 0x48 (depending on SAddr pin)
+ * mt9m111 and mt9m112 i2c address is 0x5d or 0x48 (depending on SAddr pin)
  * The platform has to define i2c_board_info and call i2c_register_board_info()
  */
 

@@ -841,10 +841,11 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
-	case 0x143a:
+	case 0x143a: /* MT9M111 */
 		mt9m111->model = V4L2_IDENT_MT9M111;
-		icd->formats = mt9m111_colour_formats;
-		icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
+		break;
+	case 0x148c: /* MT9M112 */
+		mt9m111->model = V4L2_IDENT_MT9M112;
 		break;
 	default:
 		ret = -ENODEV;

@@ -853,6 +854,9 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 		goto ei2c;
 	}
 
+	icd->formats = mt9m111_colour_formats;
+	icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
+
 	dev_info(&icd->dev, "Detected a MT9M111 chip ID 0x143a\n");
 
 	ret = soc_camera_video_start(icd);

@@ -968,6 +972,6 @@ static void __exit mt9m111_mod_exit(void)
 module_init(mt9m111_mod_init);
 module_exit(mt9m111_mod_exit);
 
-MODULE_DESCRIPTION("Micron MT9M111 Camera driver");
+MODULE_DESCRIPTION("Micron MT9M111/MT9M112 Camera driver");
 MODULE_AUTHOR("Robert Jarzmik");
 MODULE_LICENSE("GPL");

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#8 lore raw
Quoted 61 lines · Mike Rapoport

Update mt9m11 driver and Kconfig

Signed-off-by Mike Rapoport <[email protected]>
 drivers/media/video/Kconfig   |    4 ++--
 drivers/media/video/mt9m111.c |   16 ++++++++++------
 2 files changed, 12 insertions(+), 8 deletions(-)
drivers/media/video/Kconfig +2 -2
index 47102c2..0848032 100644
@@ -725,10 +725,10 @@ config MT9M001_PCA9536_SWITCH
 	  extender to switch between 8 and 10 bit datawidth modes
 
 config SOC_CAMERA_MT9M111
-	tristate "mt9m111 support"
+	tristate "mt9m111 and mt9m112 support"
 	depends on SOC_CAMERA && I2C
 	help
-	  This driver supports MT9M111 cameras from Micron
+	  This driver supports MT9M111 and MT9M112 cameras from Micron
 
 config SOC_CAMERA_MT9V022
 	tristate "mt9v022 support"
drivers/media/video/mt9m111.c +6 -5
index da0b2d5..9a428b7 100644
@@ -1,5 +1,5 @@
 /*
- * Driver for MT9M111 CMOS Image Sensor from Micron
+ * Driver for MT9M111/MT9M112 CMOS Image Sensor from Micron
  *
  * Copyright (C) 2008, Robert Jarzmik <[email protected]>
  *

@@ -19,7 +19,7 @@
 #include <media/soc_camera.h>
 
 /*
- * mt9m111 i2c address is 0x5d or 0x48 (depending on SAddr pin)
+ * mt9m111 and mt9m112 i2c address is 0x5d or 0x48 (depending on SAddr pin)
  * The platform has to define i2c_board_info and call i2c_register_board_info()
  */
 

@@ -841,10 +841,11 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
-	case 0x143a:
+	case 0x143a: /* MT9M111 */
 		mt9m111->model = V4L2_IDENT_MT9M111;
-		icd->formats = mt9m111_colour_formats;
-		icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
+		break;
+	case 0x148c: /* MT9M112 */
+		mt9m111->model = V4L2_IDENT_MT9M112;
 		break;
 	default:
 		ret = -ENODEV;

drivers/media/video/mt9m111.c static int mt9m111_video_probe(struct soc_camera_device *icd) · line 860

Quoted 7 lines
drivers/media/video/mt9m111.c +3 -0
@@ -853,6 +854,9 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 		goto ei2c;
 	}
 
+	icd->formats = mt9m111_colour_formats;
+	icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
+
 	dev_info(&icd->dev, "Detected a MT9M111 chip ID 0x143a\n");

Hm, looks like you missed at least one: ^^^^^^^^^^^^^^^^^^^^^^. Please update.

Also, if you search for "dev_" you'll see a couple more strings like

		dev_err(&icd->dev,
			"No MT9M111 chip detected, register read %x\n", data);

I think, you can just update them to mt9m11x.

Thanks
Guennadi

drivers/media/video/mt9m111.c static void __exit mt9m111_mod_exit(void) · line 977

Quoted 11 lines
drivers/media/video/mt9m111.c +1 -1
 	ret = soc_camera_video_start(icd);

@@ -968,6 +972,6 @@ static void __exit mt9m111_mod_exit(void)
 module_init(mt9m111_mod_init);
 module_exit(mt9m111_mod_exit);
 
-MODULE_DESCRIPTION("Micron MT9M111 Camera driver");
+MODULE_DESCRIPTION("Micron MT9M111/MT9M112 Camera driver");
 MODULE_AUTHOR("Robert Jarzmik");
 MODULE_LICENSE("GPL");

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

author
#9 lore raw
Quoted 11 lines · Guennadi Liakhovetski

Review

Quoted 5 lines · Mike Rapoport
(quoted diff) +3 -0
+	icd->formats = mt9m111_colour_formats;
+	icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
+
 	dev_info(&icd->dev, "Detected a MT9M111 chip ID 0x143a\n");

Hm, looks like you missed at least one: ^^^^^^^^^^^^^^^^^^^^^^. Please update.

Quoted 6 lines

Also, if you search for "dev_" you'll see a couple more strings like

		dev_err(&icd->dev,
			"No MT9M111 chip detected, register read %x\n", data);

I think, you can just update them to mt9m11x.

Done.

Signed-off-by Mike Rapoport <[email protected]>
 drivers/media/video/Kconfig   |    4 ++--
 drivers/media/video/mt9m111.c |   28 ++++++++++++++++------------
 2 files changed, 18 insertions(+), 14 deletions(-)
drivers/media/video/Kconfig +2 -2
index 47102c2..0848032 100644
@@ -725,10 +725,10 @@ config MT9M001_PCA9536_SWITCH
 	  extender to switch between 8 and 10 bit datawidth modes
 
 config SOC_CAMERA_MT9M111
-	tristate "mt9m111 support"
+	tristate "mt9m111 and mt9m112 support"
 	depends on SOC_CAMERA && I2C
 	help
-	  This driver supports MT9M111 cameras from Micron
+	  This driver supports MT9M111 and MT9M112 cameras from Micron
 
 config SOC_CAMERA_MT9V022
 	tristate "mt9v022 support"
drivers/media/video/mt9m111.c +16 -12
index da0b2d5..ac48001 100644
@@ -1,5 +1,5 @@
 /*
- * Driver for MT9M111 CMOS Image Sensor from Micron
+ * Driver for MT9M111/MT9M112 CMOS Image Sensor from Micron
  *
  * Copyright (C) 2008, Robert Jarzmik <[email protected]>
  *

@@ -19,7 +19,7 @@
 #include <media/soc_camera.h>
 
 /*
- * mt9m111 i2c address is 0x5d or 0x48 (depending on SAddr pin)
+ * mt9m111 and mt9m112 i2c address is 0x5d or 0x48 (depending on SAddr pin)
  * The platform has to define i2c_board_info and call i2c_register_board_info()
  */
 

@@ -145,7 +145,7 @@ enum mt9m111_context {
 struct mt9m111 {
 	struct i2c_client *client;
 	struct soc_camera_device icd;
-	int model;	/* V4L2_IDENT_MT9M111* codes from v4l2-chip-ident.h */
+	int model;	/* V4L2_IDENT_MT9M11x* codes from v4l2-chip-ident.h */
 	enum mt9m111_context context;
 	unsigned int left, top, width, height;
 	u32 pixfmt;

@@ -798,7 +798,7 @@ static int mt9m111_init(struct soc_camera_device *icd)
 	if (!ret)
 		ret = mt9m111_set_autoexposure(icd, mt9m111->autoexposure);
 	if (ret)
-		dev_err(&icd->dev, "mt9m111 init failed: %d\n", ret);
+		dev_err(&icd->dev, "mt9m11x init failed: %d\n", ret);
 	return ret;
 }
 

@@ -808,7 +808,7 @@ static int mt9m111_release(struct soc_camera_device *icd)
 
 	ret = mt9m111_disable(icd);
 	if (ret < 0)
-		dev_err(&icd->dev, "mt9m111 release failed: %d\n", ret);
+		dev_err(&icd->dev, "mt9m11x release failed: %d\n", ret);
 
 	return ret;
 }

@@ -841,19 +841,23 @@ static int mt9m111_video_probe(struct soc_camera_device *icd)
 	data = reg_read(CHIP_VERSION);
 
 	switch (data) {
-	case 0x143a:
+	case 0x143a: /* MT9M111 */
 		mt9m111->model = V4L2_IDENT_MT9M111;
-		icd->formats = mt9m111_colour_formats;
-		icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
+		break;
+	case 0x148c: /* MT9M112 */
+		mt9m111->model = V4L2_IDENT_MT9M112;
 		break;
 	default:
 		ret = -ENODEV;
 		dev_err(&icd->dev,
-			"No MT9M111 chip detected, register read %x\n", data);
+			"No MT9M11x chip detected, register read %x\n", data);
 		goto ei2c;
 	}
 
-	dev_info(&icd->dev, "Detected a MT9M111 chip ID 0x143a\n");
+	icd->formats = mt9m111_colour_formats;
+	icd->num_formats = ARRAY_SIZE(mt9m111_colour_formats);
+
+	dev_info(&icd->dev, "Detected a MT9M11x chip ID %x\n", data);
 
 	ret = soc_camera_video_start(icd);
 	if (ret)

@@ -889,7 +893,7 @@ static int mt9m111_probe(struct i2c_client *client,
 	int ret;
 
 	if (!icl) {
-		dev_err(&client->dev, "MT9M111 driver needs platform data\n");
+		dev_err(&client->dev, "MT9M11x driver needs platform data\n");
 		return -EINVAL;
 	}
 

@@ -968,6 +972,6 @@ static void __exit mt9m111_mod_exit(void)
 module_init(mt9m111_mod_init);
 module_exit(mt9m111_mod_exit);
 
-MODULE_DESCRIPTION("Micron MT9M111 Camera driver");
+MODULE_DESCRIPTION("Micron MT9M111/MT9M112 Camera driver");
 MODULE_AUTHOR("Robert Jarzmik");
 MODULE_LICENSE("GPL");
Signature · 7 lines
Sincerely yours,
Mike.

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list