Re: [PATCH] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control

2012-09-26 Thread Prabhakar Lad
Hi Laurent,

Thanks for the review.

On Tue, Sep 25, 2012 at 9:30 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi Prabhakar,

 Thank you for the patch.

 On Tuesday 25 September 2012 18:29:25 Prabhakar wrote:
 From: Lad, Prabhakar prabhakar@ti.com

 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 ---
  drivers/media/i2c/mt9p031.c |   27 
  drivers/media/i2c/mt9t001.c |   33 +++---
  drivers/media/i2c/mt9v032.c |   46 
  3 files changed, 66 insertions(+), 40 deletions(-)

 diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
 index 2c0f407..a45c2ea 100644
 --- a/drivers/media/i2c/mt9p031.c
 +++ b/drivers/media/i2c/mt9p031.c
 @@ -574,11 +574,10 @@ static int mt9p031_set_crop(struct v4l2_subdev
 *subdev, * V4L2 subdev control operations
   */

 -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
 -#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1002)
 -#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1003)
 -#define V4L2_CID_BLC_ANALOG_OFFSET   (V4L2_CID_USER_BASE | 0x1004)
 -#define V4L2_CID_BLC_DIGITAL_OFFSET  (V4L2_CID_USER_BASE | 0x1005)
 +#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1001)
 +#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1002)
 +#define V4L2_CID_BLC_ANALOG_OFFSET   (V4L2_CID_USER_BASE | 0x1003)
 +#define V4L2_CID_BLC_DIGITAL_OFFSET  (V4L2_CID_USER_BASE | 0x1004)

 Let's not change the value of the other device-specific controls.

Ok

  static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
  {
 @@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] =
 { static const struct v4l2_ctrl_config mt9p031_ctrls[] = {
   {
   .ops= mt9p031_ctrl_ops,
 - .id = V4L2_CID_TEST_PATTERN,
 - .type   = V4L2_CTRL_TYPE_MENU,
 - .name   = Test Pattern,
 - .min= 0,
 - .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1,
 - .step   = 0,
 - .def= 0,
 - .flags  = 0,
 - .menu_skip_mask = 0,
 - .qmenu  = mt9p031_test_pattern_menu,
 - }, {
 - .ops= mt9p031_ctrl_ops,
   .id = V4L2_CID_BLC_AUTO,
   .type   = V4L2_CTRL_TYPE_BOOLEAN,
   .name   = BLC, Auto,
 @@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client,
   mt9p031-model = did-driver_data;
   mt9p031-reset = -1;

 - v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5);
 + v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6);

   v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops,
 V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN,
 @@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client,
   v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops,
 V4L2_CID_PIXEL_RATE, pdata-target_freq,
 pdata-target_freq, 1, pdata-target_freq);
 + v4l2_ctrl_new_std_menu_items(mt9p031-ctrls, mt9p031_ctrl_ops,
 +   V4L2_CID_TEST_PATTERN,
 +   ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0,
 +   0, mt9p031_test_pattern_menu);

   for (i = 0; i  ARRAY_SIZE(mt9p031_ctrls); ++i)
   v4l2_ctrl_new_custom(mt9p031-ctrls, mt9p031_ctrls[i], NULL);
 diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
 index 6d343ad..16eac3f 100644
 --- a/drivers/media/i2c/mt9t001.c
 +++ b/drivers/media/i2c/mt9t001.c
 @@ -124,6 +124,7 @@ struct mt9t001 {

   u16 output_control;
   u16 black_level;
 + bool test_pattern;
  };

  static inline struct mt9t001 *to_mt9t001(struct v4l2_subdev *sd)
 @@ -371,10 +372,10 @@ static int mt9t001_set_crop(struct v4l2_subdev
 *subdev, * V4L2 subdev control operations
   */

 -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
 -#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002)
 -#define V4L2_CID_BLACK_LEVEL_OFFSET  (V4L2_CID_USER_BASE | 0x1003)
 -#define V4L2_CID_BLACK_LEVEL_CALIBRATE   (V4L2_CID_USER_BASE | 0x1004)
 +#define V4L2_CID_PARAMETRIC_TEST_PATTERN (V4L2_CID_USER_BASE | 0x1001)

 That's a bit of a long name. What about V4L2_CID_TEST_PATTERN_COLOR ?

Ok.

 +#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002)
 +#define V4L2_CID_BLACK_LEVEL_OFFSET  (V4L2_CID_USER_BASE | 0x1003)
 +#define V4L2_CID_BLACK_LEVEL_CALIBRATE   (V4L2_CID_USER_BASE | 
 0x1004)

  #define V4L2_CID_GAIN_RED(V4L2_CTRL_CLASS_CAMERA | 0x1001)
  #define V4L2_CID_GAIN_GREEN_RED  (V4L2_CTRL_CLASS_CAMERA | 
 0x1002)
 @@ -485,8 +486,15 @@ 

[PATCH] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control

2012-09-25 Thread Prabhakar
From: Lad, Prabhakar prabhakar@ti.com

Signed-off-by: Lad, Prabhakar prabhakar@ti.com
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
---
 drivers/media/i2c/mt9p031.c |   27 
 drivers/media/i2c/mt9t001.c |   33 +++---
 drivers/media/i2c/mt9v032.c |   46 +-
 3 files changed, 66 insertions(+), 40 deletions(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 2c0f407..a45c2ea 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -574,11 +574,10 @@ static int mt9p031_set_crop(struct v4l2_subdev *subdev,
  * V4L2 subdev control operations
  */
 
-#define V4L2_CID_TEST_PATTERN  (V4L2_CID_USER_BASE | 0x1001)
-#define V4L2_CID_BLC_AUTO  (V4L2_CID_USER_BASE | 0x1002)
-#define V4L2_CID_BLC_TARGET_LEVEL  (V4L2_CID_USER_BASE | 0x1003)
-#define V4L2_CID_BLC_ANALOG_OFFSET (V4L2_CID_USER_BASE | 0x1004)
-#define V4L2_CID_BLC_DIGITAL_OFFSET(V4L2_CID_USER_BASE | 0x1005)
+#define V4L2_CID_BLC_AUTO  (V4L2_CID_USER_BASE | 0x1001)
+#define V4L2_CID_BLC_TARGET_LEVEL  (V4L2_CID_USER_BASE | 0x1002)
+#define V4L2_CID_BLC_ANALOG_OFFSET (V4L2_CID_USER_BASE | 0x1003)
+#define V4L2_CID_BLC_DIGITAL_OFFSET(V4L2_CID_USER_BASE | 0x1004)
 
 static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
 {
@@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] = {
 static const struct v4l2_ctrl_config mt9p031_ctrls[] = {
{
.ops= mt9p031_ctrl_ops,
-   .id = V4L2_CID_TEST_PATTERN,
-   .type   = V4L2_CTRL_TYPE_MENU,
-   .name   = Test Pattern,
-   .min= 0,
-   .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1,
-   .step   = 0,
-   .def= 0,
-   .flags  = 0,
-   .menu_skip_mask = 0,
-   .qmenu  = mt9p031_test_pattern_menu,
-   }, {
-   .ops= mt9p031_ctrl_ops,
.id = V4L2_CID_BLC_AUTO,
.type   = V4L2_CTRL_TYPE_BOOLEAN,
.name   = BLC, Auto,
@@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client,
mt9p031-model = did-driver_data;
mt9p031-reset = -1;
 
-   v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5);
+   v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6);
 
v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops,
  V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN,
@@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client,
v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops,
  V4L2_CID_PIXEL_RATE, pdata-target_freq,
  pdata-target_freq, 1, pdata-target_freq);
+   v4l2_ctrl_new_std_menu_items(mt9p031-ctrls, mt9p031_ctrl_ops,
+ V4L2_CID_TEST_PATTERN,
+ ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0,
+ 0, mt9p031_test_pattern_menu);
 
for (i = 0; i  ARRAY_SIZE(mt9p031_ctrls); ++i)
v4l2_ctrl_new_custom(mt9p031-ctrls, mt9p031_ctrls[i], NULL);
diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
index 6d343ad..16eac3f 100644
--- a/drivers/media/i2c/mt9t001.c
+++ b/drivers/media/i2c/mt9t001.c
@@ -124,6 +124,7 @@ struct mt9t001 {
 
u16 output_control;
u16 black_level;
+   bool test_pattern;
 };
 
 static inline struct mt9t001 *to_mt9t001(struct v4l2_subdev *sd)
@@ -371,10 +372,10 @@ static int mt9t001_set_crop(struct v4l2_subdev *subdev,
  * V4L2 subdev control operations
  */
 
-#define V4L2_CID_TEST_PATTERN  (V4L2_CID_USER_BASE | 0x1001)
-#define V4L2_CID_BLACK_LEVEL_AUTO  (V4L2_CID_USER_BASE | 0x1002)
-#define V4L2_CID_BLACK_LEVEL_OFFSET(V4L2_CID_USER_BASE | 0x1003)
-#define V4L2_CID_BLACK_LEVEL_CALIBRATE (V4L2_CID_USER_BASE | 0x1004)
+#define V4L2_CID_PARAMETRIC_TEST_PATTERN   (V4L2_CID_USER_BASE | 0x1001)
+#define V4L2_CID_BLACK_LEVEL_AUTO  (V4L2_CID_USER_BASE | 0x1002)
+#define V4L2_CID_BLACK_LEVEL_OFFSET(V4L2_CID_USER_BASE | 0x1003)
+#define V4L2_CID_BLACK_LEVEL_CALIBRATE (V4L2_CID_USER_BASE | 0x1004)
 
 #define V4L2_CID_GAIN_RED  (V4L2_CTRL_CLASS_CAMERA | 0x1001)
 #define V4L2_CID_GAIN_GREEN_RED(V4L2_CTRL_CLASS_CAMERA | 
0x1002)
@@ -485,8 +486,15 @@ static int mt9t001_s_ctrl(struct v4l2_ctrl *ctrl)
 
return mt9t001_write(client, MT9T001_SHUTTER_WIDTH_HIGH,
 ctrl-val  16);
-
case V4L2_CID_TEST_PATTERN:
+   mt9t001-test_pattern = ctrl-val;
+   break;
+
+   case V4L2_CID_PARAMETRIC_TEST_PATTERN:
+   if 

Re: [PATCH] media: mt9p031/mt9t001/mt9v032: use V4L2_CID_TEST_PATTERN for test pattern control

2012-09-25 Thread Laurent Pinchart
Hi Prabhakar,

Thank you for the patch.

On Tuesday 25 September 2012 18:29:25 Prabhakar wrote:
 From: Lad, Prabhakar prabhakar@ti.com
 
 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 ---
  drivers/media/i2c/mt9p031.c |   27 
  drivers/media/i2c/mt9t001.c |   33 +++---
  drivers/media/i2c/mt9v032.c |   46 
  3 files changed, 66 insertions(+), 40 deletions(-)
 
 diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
 index 2c0f407..a45c2ea 100644
 --- a/drivers/media/i2c/mt9p031.c
 +++ b/drivers/media/i2c/mt9p031.c
 @@ -574,11 +574,10 @@ static int mt9p031_set_crop(struct v4l2_subdev
 *subdev, * V4L2 subdev control operations
   */
 
 -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
 -#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1002)
 -#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1003)
 -#define V4L2_CID_BLC_ANALOG_OFFSET   (V4L2_CID_USER_BASE | 0x1004)
 -#define V4L2_CID_BLC_DIGITAL_OFFSET  (V4L2_CID_USER_BASE | 0x1005)
 +#define V4L2_CID_BLC_AUTO(V4L2_CID_USER_BASE | 0x1001)
 +#define V4L2_CID_BLC_TARGET_LEVEL(V4L2_CID_USER_BASE | 0x1002)
 +#define V4L2_CID_BLC_ANALOG_OFFSET   (V4L2_CID_USER_BASE | 0x1003)
 +#define V4L2_CID_BLC_DIGITAL_OFFSET  (V4L2_CID_USER_BASE | 0x1004)

Let's not change the value of the other device-specific controls.

  static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
  {
 @@ -740,18 +739,6 @@ static const char * const mt9p031_test_pattern_menu[] =
 { static const struct v4l2_ctrl_config mt9p031_ctrls[] = {
   {
   .ops= mt9p031_ctrl_ops,
 - .id = V4L2_CID_TEST_PATTERN,
 - .type   = V4L2_CTRL_TYPE_MENU,
 - .name   = Test Pattern,
 - .min= 0,
 - .max= ARRAY_SIZE(mt9p031_test_pattern_menu) - 1,
 - .step   = 0,
 - .def= 0,
 - .flags  = 0,
 - .menu_skip_mask = 0,
 - .qmenu  = mt9p031_test_pattern_menu,
 - }, {
 - .ops= mt9p031_ctrl_ops,
   .id = V4L2_CID_BLC_AUTO,
   .type   = V4L2_CTRL_TYPE_BOOLEAN,
   .name   = BLC, Auto,
 @@ -950,7 +937,7 @@ static int mt9p031_probe(struct i2c_client *client,
   mt9p031-model = did-driver_data;
   mt9p031-reset = -1;
 
 - v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 5);
 + v4l2_ctrl_handler_init(mt9p031-ctrls, ARRAY_SIZE(mt9p031_ctrls) + 6);
 
   v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops,
 V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN,
 @@ -966,6 +953,10 @@ static int mt9p031_probe(struct i2c_client *client,
   v4l2_ctrl_new_std(mt9p031-ctrls, mt9p031_ctrl_ops,
 V4L2_CID_PIXEL_RATE, pdata-target_freq,
 pdata-target_freq, 1, pdata-target_freq);
 + v4l2_ctrl_new_std_menu_items(mt9p031-ctrls, mt9p031_ctrl_ops,
 +   V4L2_CID_TEST_PATTERN,
 +   ARRAY_SIZE(mt9p031_test_pattern_menu) - 1, 0,
 +   0, mt9p031_test_pattern_menu);
 
   for (i = 0; i  ARRAY_SIZE(mt9p031_ctrls); ++i)
   v4l2_ctrl_new_custom(mt9p031-ctrls, mt9p031_ctrls[i], NULL);
 diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
 index 6d343ad..16eac3f 100644
 --- a/drivers/media/i2c/mt9t001.c
 +++ b/drivers/media/i2c/mt9t001.c
 @@ -124,6 +124,7 @@ struct mt9t001 {
 
   u16 output_control;
   u16 black_level;
 + bool test_pattern;
  };
 
  static inline struct mt9t001 *to_mt9t001(struct v4l2_subdev *sd)
 @@ -371,10 +372,10 @@ static int mt9t001_set_crop(struct v4l2_subdev
 *subdev, * V4L2 subdev control operations
   */
 
 -#define V4L2_CID_TEST_PATTERN(V4L2_CID_USER_BASE | 0x1001)
 -#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002)
 -#define V4L2_CID_BLACK_LEVEL_OFFSET  (V4L2_CID_USER_BASE | 0x1003)
 -#define V4L2_CID_BLACK_LEVEL_CALIBRATE   (V4L2_CID_USER_BASE | 0x1004)
 +#define V4L2_CID_PARAMETRIC_TEST_PATTERN (V4L2_CID_USER_BASE | 0x1001)

That's a bit of a long name. What about V4L2_CID_TEST_PATTERN_COLOR ?

 +#define V4L2_CID_BLACK_LEVEL_AUTO(V4L2_CID_USER_BASE | 0x1002)
 +#define V4L2_CID_BLACK_LEVEL_OFFSET  (V4L2_CID_USER_BASE | 0x1003)
 +#define V4L2_CID_BLACK_LEVEL_CALIBRATE   (V4L2_CID_USER_BASE | 
 0x1004)
 
  #define V4L2_CID_GAIN_RED(V4L2_CTRL_CLASS_CAMERA | 0x1001)
  #define V4L2_CID_GAIN_GREEN_RED  (V4L2_CTRL_CLASS_CAMERA | 
 0x1002)
 @@ -485,8 +486,15 @@ static int mt9t001_s_ctrl(struct v4l2_ctrl *ctrl)
 
   return mt9t001_write(client, MT9T001_SHUTTER_WIDTH_HIGH,