[PATCH RESEND v3 0/6] staging: media: atomisp: code cleanup fixes

[PATCH RESEND v3 0/6] staging: media: atomisp: code cleanup fixes

[Deepak R Varma]

This patch set addresses different kinds of checkpatch WARNING and
CHECK complaints.

Note: 
   - Patch set being resent due a email messageID issue in the earlier
     patch set.
   - The patches should be applied in the ascending order.

Changes since v2:
   Generic change:
   1. Correct patch versioning in patch subject

   Patch Specific change:
   1. patch 1/6 : none
   2. patch 2/6 : none
   3. patch 3/6 : none
   4. patch 4/6 :
        a. Tag Fabio Auito for the patch suggestion

   5. patch 5/6 : none
   6. patch 6/6:
        a. Tag Fabio Auito for the patch suggestion

Changes since v1:
   Generic change:
   1. The patch set is being resent from an email account that matches with
      the patch signed-of-by tag. Issue highlighted by Hans Verkuil.

   Patch specific changes:
   1. patch 1/6 : none
   2. patch 2/6 : none
   3. patch 3/6 : none
   4. patch 4/6 : implement following changes suggested by Fabio Aiuto
        a. Corrected commenting style
        b. Similar style implemented for other comment blocks in
           the same files.
   5. patch 5/6 : none
   6. patch 6/6: implement following changes suggested by Fabio Aiuto
        a. use dev_info instead of pr_info
        b. update patch log message accordingly


Deepak R Varma (6):
  staging: media: atomisp: improve function argument alignment
  staging: media: atomisp: balance braces around if...else block
  staging: media: atomisp: use __func__ over function names
  staging: media: atomisp: reformat code comment blocks
  staging: media: atomisp: fix CamelCase variable naming
  staging: media: atomisp: replace raw printk() by dev_info()

 .../media/atomisp/i2c/atomisp-gc0310.c        |  14 +--
 .../media/atomisp/i2c/atomisp-gc2235.c        |  29 ++---
 .../atomisp/i2c/atomisp-libmsrlisthelper.c    |   6 +-
 .../media/atomisp/i2c/atomisp-lm3554.c        |   2 +-
 .../media/atomisp/i2c/atomisp-mt9m114.c       | 106 ++++++++++--------
 .../media/atomisp/i2c/atomisp-ov2680.c        |  43 ++++---
 .../media/atomisp/i2c/atomisp-ov2722.c        |  10 +-
 7 files changed, 116 insertions(+), 94 deletions(-)

-- 
2.25.1

[PATCH RESEND v3 1/6] staging: media: atomisp: improve function argument alignment

[Deepak R Varma]

Improve multi-line function argument alignment according to the code style
guidelines. Resolves checkpatch feedback: "Alignment should match
open parenthesis".

Signed-off-by: Deepak R Varma <drv@mailo.com>
---

Changes since v2:
   - None.
Changes since v1:
   - None.


 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++--
 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 4 ++--
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 2b71de722ec3..6be3ee1d93a5 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -192,8 +192,8 @@ static int __gc0310_buf_reg_array(struct i2c_client *client,
 }
 
 static int __gc0310_write_reg_is_consecutive(struct i2c_client *client,
-	struct gc0310_write_ctrl *ctrl,
-	const struct gc0310_reg *next)
+					     struct gc0310_write_ctrl *ctrl,
+					     const struct gc0310_reg *next)
 {
 	if (ctrl->index == 0)
 		return 1;
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 78147ffb6099..6ba4a8adff7c 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -171,8 +171,8 @@ static int __gc2235_buf_reg_array(struct i2c_client *client,
 }
 
 static int __gc2235_write_reg_is_consecutive(struct i2c_client *client,
-	struct gc2235_write_ctrl *ctrl,
-	const struct gc2235_reg *next)
+					     struct gc2235_write_ctrl *ctrl,
+					     const struct gc2235_reg *next)
 {
 	if (ctrl->index == 0)
 		return 1;
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index eecefcd734d0..aec7392fd1de 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -212,8 +212,8 @@ static int __ov2722_buf_reg_array(struct i2c_client *client,
 }
 
 static int __ov2722_write_reg_is_consecutive(struct i2c_client *client,
-	struct ov2722_write_ctrl *ctrl,
-	const struct ov2722_reg *next)
+					     struct ov2722_write_ctrl *ctrl,
+					     const struct ov2722_reg *next)
 {
 	if (ctrl->index == 0)
 		return 1;
-- 
2.25.1

[PATCH RESEND v3 2/6] staging: media: atomisp: balance braces around if...else block

[Deepak R Varma]

Balance braces around the if else blocks as per the code style guidelines.
Resolves checkpatch script CHECK / WARNING feedback messages.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---

Changes since v2:
   - None.
Changes since v1:
   - None.

 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c  | 4 ++--
 drivers/staging/media/atomisp/i2c/atomisp-gc2235.c  | 4 ++--
 drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 4 ++--
 drivers/staging/media/atomisp/i2c/atomisp-ov2680.c  | 7 ++++---
 drivers/staging/media/atomisp/i2c/atomisp-ov2722.c  | 4 ++--
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 6be3ee1d93a5..d68a2bcc9ae1 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -872,9 +872,9 @@ static int gc0310_s_power(struct v4l2_subdev *sd, int on)
 {
 	int ret;
 
-	if (on == 0)
+	if (on == 0) {
 		return power_down(sd);
-	else {
+	} else {
 		ret = power_up(sd);
 		if (!ret)
 			return gc0310_init(sd);
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 6ba4a8adff7c..e722c639b60d 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -658,9 +658,9 @@ static int gc2235_s_power(struct v4l2_subdev *sd, int on)
 {
 	int ret;
 
-	if (on == 0)
+	if (on == 0) {
 		ret = power_down(sd);
-	else {
+	} else {
 		ret = power_up(sd);
 		if (!ret)
 			ret = __gc2235_init(sd);
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
index f5de81132177..465fc4468442 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -568,9 +568,9 @@ static int power_down(struct v4l2_subdev *sd)
 
 static int mt9m114_s_power(struct v4l2_subdev *sd, int power)
 {
-	if (power == 0)
+	if (power == 0) {
 		return power_down(sd);
-	else {
+	} else {
 		if (power_up(sd))
 			return -EINVAL;
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index c90730513438..92c52431bd8f 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -461,11 +461,12 @@ static int ov2680_v_flip(struct v4l2_subdev *sd, s32 value)
 	ret = ov2680_read_reg(client, 1, OV2680_FLIP_REG, &val);
 	if (ret)
 		return ret;
-	if (value) {
+
+	if (value)
 		val |= OV2680_FLIP_MIRROR_BIT_ENABLE;
-	} else {
+	else
 		val &= ~OV2680_FLIP_MIRROR_BIT_ENABLE;
-	}
+
 	ret = ov2680_write_reg(client, 1,
 			       OV2680_FLIP_REG, val);
 	if (ret)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index aec7392fd1de..d046a9804f63 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -772,9 +772,9 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on)
 {
 	int ret;
 
-	if (on == 0)
+	if (on == 0) {
 		return power_down(sd);
-	else {
+	} else {
 		ret = power_up(sd);
 		if (!ret)
 			return ov2722_init(sd);
-- 
2.25.1

[PATCH RESEND v3 3/6] staging: media: atomisp: use __func__ over function names

[Deepak R Varma]

Replace hard coded function names from the debug print strings by
standard __func__ predefined identifier. This resolves following
checkpatch script WARNING:
Prefer using '"%s...", __func__' to using function's name, in a string.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---

Changes since v2:
   - None.
Changes since v1:
   - None.

 .../staging/media/atomisp/i2c/atomisp-gc0310.c   |  2 +-
 .../staging/media/atomisp/i2c/atomisp-gc2235.c   |  2 +-
 .../staging/media/atomisp/i2c/atomisp-lm3554.c   |  2 +-
 .../staging/media/atomisp/i2c/atomisp-ov2680.c   | 16 ++++++++--------
 .../staging/media/atomisp/i2c/atomisp-ov2722.c   |  2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index d68a2bcc9ae1..b572551f1a0d 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -1292,7 +1292,7 @@ static int gc0310_remove(struct i2c_client *client)
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 	struct gc0310_device *dev = to_gc0310_sensor(sd);
 
-	dev_dbg(&client->dev, "gc0310_remove...\n");
+	dev_dbg(&client->dev, "%s...\n", __func__);
 
 	dev->platform_data->csi_cfg(sd, 0);
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index e722c639b60d..548c572d3b57 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -1034,7 +1034,7 @@ static int gc2235_remove(struct i2c_client *client)
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 	struct gc2235_device *dev = to_gc2235_sensor(sd);
 
-	dev_dbg(&client->dev, "gc2235_remove...\n");
+	dev_dbg(&client->dev, "%s...\n", __func__);
 
 	dev->platform_data->csi_cfg(sd, 0);
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
index 7ca7378b1859..ab10fd98dbc0 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
@@ -680,7 +680,7 @@ static int lm3554_detect(struct v4l2_subdev *sd)
 	int ret;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
-		dev_err(&client->dev, "lm3554_detect i2c error\n");
+		dev_err(&client->dev, "%s i2c error\n", __func__);
 		return -ENODEV;
 	}
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index 92c52431bd8f..c17615149f46 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -146,7 +146,7 @@ static int ov2680_g_bin_factor_x(struct v4l2_subdev *sd, s32 *val)
 	struct ov2680_device *dev = to_ov2680_sensor(sd);
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 
-	dev_dbg(&client->dev,  "++++ov2680_g_bin_factor_x\n");
+	dev_dbg(&client->dev,  "++++%s\n", __func__);
 	*val = ov2680_res[dev->fmt_idx].bin_factor_x;
 
 	return 0;
@@ -158,7 +158,7 @@ static int ov2680_g_bin_factor_y(struct v4l2_subdev *sd, s32 *val)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 
 	*val = ov2680_res[dev->fmt_idx].bin_factor_y;
-	dev_dbg(&client->dev,  "++++ov2680_g_bin_factor_y\n");
+	dev_dbg(&client->dev,  "++++%s\n", __func__);
 	return 0;
 }
 
@@ -173,7 +173,7 @@ static int ov2680_get_intg_factor(struct i2c_client *client,
 	u16 reg_val;
 	int ret;
 
-	dev_dbg(&client->dev,  "++++ov2680_get_intg_factor\n");
+	dev_dbg(&client->dev,  "++++%s\n", __func__);
 	if (!info)
 		return -EINVAL;
 
@@ -251,8 +251,8 @@ static long __ov2680_set_exposure(struct v4l2_subdev *sd, int coarse_itg,
 	int ret, exp_val;
 
 	dev_dbg(&client->dev,
-		"+++++++__ov2680_set_exposure coarse_itg %d, gain %d, digitgain %d++\n",
-		coarse_itg, gain, digitgain);
+		"+++++++%s coarse_itg %d, gain %d, digitgain %d++\n",
+		__func__, coarse_itg, gain, digitgain);
 
 	vts = ov2680_res[dev->fmt_idx].lines_per_frame;
 
@@ -1061,9 +1061,9 @@ static int ov2680_s_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&dev->input_lock);
 	if (enable)
-		dev_dbg(&client->dev, "ov2680_s_stream one\n");
+		dev_dbg(&client->dev, "%s one\n", __func__);
 	else
-		dev_dbg(&client->dev, "ov2680_s_stream off\n");
+		dev_dbg(&client->dev, "%s off\n", __func__);
 
 	ret = ov2680_write_reg(client, 1, OV2680_SW_STREAM,
 			       enable ? OV2680_START_STREAMING :
@@ -1227,7 +1227,7 @@ static int ov2680_remove(struct i2c_client *client)
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 	struct ov2680_device *dev = to_ov2680_sensor(sd);
 
-	dev_dbg(&client->dev, "ov2680_remove...\n");
+	dev_dbg(&client->dev, "%s...\n", __func__);
 
 	dev->platform_data->csi_cfg(sd, 0);
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index d046a9804f63..69409f8447b5 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client)
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 	struct ov2722_device *dev = to_ov2722_sensor(sd);
 
-	dev_dbg(&client->dev, "ov2722_remove...\n");
+	dev_dbg(&client->dev, "%s...\n", __func__);
 
 	dev->platform_data->csi_cfg(sd, 0);
 	v4l2_ctrl_handler_free(&dev->ctrl_handler);
-- 
2.25.1

[PATCH RESEND v3 4/6] staging: media: atomisp: reformat code comment blocks

[Deepak R Varma]

Reformat code comment blocks according to the coding style guidelines.
This resolves different checkpatch script WARNINGs around block comments.

Suggested-by: Fabio Aiuto <fabioaiuto83@gmail.com>
Signed-off-by: Deepak R Varma <drv@mailo.com>
---

Changes since v2:
   - Tag Fabio Auito for the patch suggestion

Changes in v1:
   - implement following changes suggested by Fabio Aiuto
       a. Corrected commenting style
       b. Similar style implemented for other comment blocks in
          the same files.


 .../media/atomisp/i2c/atomisp-gc2235.c        | 19 ++++----
 .../atomisp/i2c/atomisp-libmsrlisthelper.c    |  6 ++-
 .../media/atomisp/i2c/atomisp-mt9m114.c       | 46 +++++++++++--------
 .../media/atomisp/i2c/atomisp-ov2680.c        | 20 +++++---
 4 files changed, 56 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 548c572d3b57..6ee6e8414f0e 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -228,7 +228,7 @@ static int gc2235_g_focal(struct v4l2_subdev *sd, s32 *val)
 
 static int gc2235_g_fnumber(struct v4l2_subdev *sd, s32 *val)
 {
-	/*const f number for imx*/
+	/* const f number for imx */
 	*val = (GC2235_F_NUMBER_DEFAULT_NUM << 16) | GC2235_F_NUMBER_DEM;
 	return 0;
 }
@@ -427,7 +427,8 @@ static long gc2235_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 	return 0;
 }
 
-/* This returns the exposure time being used. This should only be used
+/*
+ * This returns the exposure time being used. This should only be used
  * for filling in EXIF data, not for actual image processing.
  */
 static int gc2235_q_exposure(struct v4l2_subdev *sd, s32 *value)
@@ -746,11 +747,12 @@ static int startup(struct v4l2_subdev *sd)
 	int ret = 0;
 
 	if (is_init == 0) {
-		/* force gc2235 to do a reset in res change, otherwise it
-		* can not output normal after switching res. and it is not
-		* necessary for first time run up after power on, for the sack
-		* of performance
-		*/
+		/*
+		 * force gc2235 to do a reset in res change, otherwise it
+		 * can not output normal after switching res. and it is not
+		 * necessary for first time run up after power on, for the sack
+		 * of performance
+		 */
 		power_down(sd);
 		power_up(sd);
 		gc2235_write_reg_array(client, gc2235_init_settings);
@@ -904,7 +906,8 @@ static int gc2235_s_config(struct v4l2_subdev *sd,
 	    (struct camera_sensor_platform_data *)platform_data;
 
 	mutex_lock(&dev->input_lock);
-	/* power off the module, then power on it in future
+	/*
+	 * power off the module, then power on it in future
 	 * as first power on by board may not fulfill the
 	 * power on sequqence needed by the module
 	 */
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
index b93c80471f22..7a20d918a9d5 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
@@ -50,14 +50,16 @@ struct tbd_data_record_header {
 static int set_msr_configuration(struct i2c_client *client, uint8_t *bufptr,
 				 unsigned int size)
 {
-	/* The configuration data contains any number of sequences where
+	/*
+	 * The configuration data contains any number of sequences where
 	 * the first byte (that is, uint8_t) that marks the number of bytes
 	 * in the sequence to follow, is indeed followed by the indicated
 	 * number of bytes of actual data to be written to sensor.
 	 * By convention, the first two bytes of actual data should be
 	 * understood as an address in the sensor address space (hibyte
 	 * followed by lobyte) where the remaining data in the sequence
-	 * will be written. */
+	 * will be written.
+	 */
 
 	u8 *ptr = bufptr;
 
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
index 465fc4468442..f314e5fed9cb 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -475,10 +475,12 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
 	if (!dev || !dev->platform_data)
 		return -ENODEV;
 
-	/* Note: current modules wire only one GPIO signal (RESET#),
+	/*
+	 * Note: current modules wire only one GPIO signal (RESET#),
 	 * but the schematic wires up two to the connector.  BIOS
 	 * versions have been unfortunately inconsistent with which
-	 * ACPI index RESET# is on, so hit both */
+	 * ACPI index RESET# is on, so hit both
+	 */
 
 	if (flag) {
 		ret = dev->platform_data->gpio0_ctrl(sd, 0);
@@ -560,7 +562,7 @@ static int power_down(struct v4l2_subdev *sd)
 	if (ret)
 		dev_err(&client->dev, "vprog failed.\n");
 
-	/*according to DS, 20ms is needed after power down*/
+	/* according to DS, 20ms is needed after power down */
 	msleep(20);
 
 	return ret;
@@ -947,7 +949,7 @@ static int mt9m114_g_focal(struct v4l2_subdev *sd, s32 *val)
 
 static int mt9m114_g_fnumber(struct v4l2_subdev *sd, s32 *val)
 {
-	/*const f number for mt9m114*/
+	/* const f number for mt9m114 */
 	*val = (MT9M114_F_NUMBER_DEFAULT_NUM << 16) | MT9M114_F_NUMBER_DEM;
 	return 0;
 }
@@ -1014,13 +1016,13 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 	AnalogGain = exposure->gain[0];
 	DigitalGain = exposure->gain[1];
 	if (!dev->streamon) {
-		/*Save the first exposure values while stream is off*/
+		/* Save the first exposure values while stream is off */
 		dev->first_exp = coarse_integration;
 		dev->first_gain = AnalogGain;
 		dev->first_diggain = DigitalGain;
 	}
-	/* DigitalGain = 0x400 * (((u16) DigitalGain) >> 8) +
-	((unsigned int)(0x400 * (((u16) DigitalGain) & 0xFF)) >>8); */
+	/* DigitalGain = 0x400 * (((u16) DigitalGain) >> 8) +		*/
+	/* ((unsigned int)(0x400 * (((u16) DigitalGain) & 0xFF)) >>8);	*/
 
 	/* set frame length */
 	if (FLines < coarse_integration + 6)
@@ -1034,8 +1036,10 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 	}
 
 	/* set coarse integration */
-	/* 3A provide real exposure time.
-		should not translate to any value here. */
+	/*
+	 * 3A provide real exposure time.
+	 * should not translate to any value here.
+	 */
 	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
 				REG_EXPO_COARSE, (u16)(coarse_integration));
 	if (ret) {
@@ -1044,7 +1048,7 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 	}
 
 	/*
-	// set analog/digital gain
+	 * set analog/digital gain
 	switch(AnalogGain)
 	{
 	case 0:
@@ -1069,8 +1073,10 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 	*/
 	if (DigitalGain >= 16 || DigitalGain <= 1)
 		DigitalGain = 1;
-	/* AnalogGainToWrite =
-		(u16)((DigitalGain << 12) | AnalogGainToWrite); */
+
+	/*
+	 * AnalogGainToWrite = (u16)((DigitalGain << 12) | AnalogGainToWrite);
+	 */
 	AnalogGainToWrite = (u16)((DigitalGain << 12) | (u16)AnalogGain);
 	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
 				REG_GAIN, AnalogGainToWrite);
@@ -1095,8 +1101,10 @@ static long mt9m114_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 	return 0;
 }
 
-/* This returns the exposure time being used. This should only be used
-   for filling in EXIF data, not for actual image processing. */
+/*
+ * This returns the exposure time being used. This should only be used
+ * for filling in EXIF data, not for actual image processing.
+ */
 static int mt9m114_g_exposure(struct v4l2_subdev *sd, s32 *value)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -1247,7 +1255,8 @@ static int mt9m114_s_ev(struct v4l2_subdev *sd, s32 val)
 	s32 luma = 0x37;
 	int err;
 
-	/* EV value only support -2 to 2
+	/*
+	 * EV value only support -2 to 2
 	 * 0: 0x37, 1:0x47, 2:0x57, -1:0x27, -2:0x17
 	 */
 	if (val < -2 || val > 2)
@@ -1295,9 +1304,10 @@ static int mt9m114_g_ev(struct v4l2_subdev *sd, s32 *val)
 	return 0;
 }
 
-/* Fake interface
+/*
+ * Fake interface
  * mt9m114 now can not support 3a_lock
-*/
+ */
 static int mt9m114_s_3a_lock(struct v4l2_subdev *sd, s32 val)
 {
 	aaalock = val;
@@ -1843,7 +1853,7 @@ static int mt9m114_probe(struct i2c_client *client)
 		return ret;
 	}
 
-	/*TODO add format code here*/
+	/* TODO add format code here */
 	dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 	dev->pad.flags = MEDIA_PAD_FL_SOURCE;
 	dev->format.code = MEDIA_BUS_FMT_SGRBG10_1X10;
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index c17615149f46..4571f4328b80 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -127,7 +127,7 @@ static int ov2680_g_focal(struct v4l2_subdev *sd, s32 *val)
 
 static int ov2680_g_fnumber(struct v4l2_subdev *sd, s32 *val)
 {
-	/*const f number for ov2680*/
+	/* const f number for ov2680 */
 
 	*val = (OV2680_F_NUMBER_DEFAULT_NUM << 16) | OV2680_F_NUMBER_DEM;
 	return 0;
@@ -399,7 +399,8 @@ static long ov2680_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 	return 0;
 }
 
-/* This returns the exposure time being used. This should only be used
+/*
+ * This returns the exposure time being used. This should only be used
  * for filling in EXIF data, not for actual image processing.
  */
 static int ov2680_q_exposure(struct v4l2_subdev *sd, s32 *value)
@@ -728,11 +729,13 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
 	if (!dev || !dev->platform_data)
 		return -ENODEV;
 
-	/* The OV2680 documents only one GPIO input (#XSHUTDN), but
+	/*
+	 * The OV2680 documents only one GPIO input (#XSHUTDN), but
 	 * existing integrations often wire two (reset/power_down)
 	 * because that is the way other sensors work.  There is no
 	 * way to tell how it is wired internally, so existing
-	 * firmwares expose both and we drive them symmetrically. */
+	 * firmwares expose both and we drive them symmetrically.
+	 */
 	if (flag) {
 		ret = dev->platform_data->gpio0_ctrl(sd, 1);
 		usleep_range(10000, 15000);
@@ -978,7 +981,8 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
 		goto err;
 	}
 
-	/*recall flip functions to avoid flip registers
+	/*
+	 * recall flip functions to avoid flip registers
 	 * were overridden by default setting
 	 */
 	if (h_flag)
@@ -988,7 +992,8 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd,
 
 	v4l2_info(client, "\n%s idx %d\n", __func__, dev->fmt_idx);
 
-	/*ret = startup(sd);
+	/*
+	 * ret = startup(sd);
 	 * if (ret)
 	 * dev_err(&client->dev, "ov2680 startup err\n");
 	 */
@@ -1097,7 +1102,8 @@ static int ov2680_s_config(struct v4l2_subdev *sd,
 	    (struct camera_sensor_platform_data *)platform_data;
 
 	mutex_lock(&dev->input_lock);
-	/* power off the module, then power on it in future
+	/*
+	 * power off the module, then power on it in future
 	 * as first power on by board may not fulfill the
 	 * power on sequqence needed by the module
 	 */
-- 
2.25.1

[PATCH RESEND v3 5/6] staging: media: atomisp: fix CamelCase variable naming

[Deepak R Varma]

Mixed case variable names are discouraged and they result in checkpatch
script "Avoid CamelCase" warnings. Replace such CamelCase variable names
by lower case strings according to the coding style guidelines.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---

Changes since v2:
   - None.

Changes since v1:
   - None.


 .../media/atomisp/i2c/atomisp-mt9m114.c       | 60 +++++++++----------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
index f314e5fed9cb..6de2cee4363e 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -1000,10 +1000,10 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 	struct mt9m114_device *dev = to_mt9m114_sensor(sd);
 	int ret = 0;
 	unsigned int coarse_integration = 0;
-	unsigned int FLines = 0;
-	unsigned int FrameLengthLines = 0; /* ExposureTime.FrameLengthLines; */
-	unsigned int AnalogGain, DigitalGain;
-	u32 AnalogGainToWrite = 0;
+	unsigned int f_lines = 0;
+	unsigned int frame_len_lines = 0; /* ExposureTime.FrameLengthLines; */
+	unsigned int analog_gain, digital_gain;
+	u32 analog_gain_to_write = 0;
 
 	dev_dbg(&client->dev, "%s(0x%X 0x%X 0x%X)\n", __func__,
 		exposure->integration_time[0], exposure->gain[0],
@@ -1011,27 +1011,27 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 
 	coarse_integration = exposure->integration_time[0];
 	/* fine_integration = ExposureTime.FineIntegrationTime; */
-	/* FrameLengthLines = ExposureTime.FrameLengthLines; */
-	FLines = mt9m114_res[dev->res].lines_per_frame;
-	AnalogGain = exposure->gain[0];
-	DigitalGain = exposure->gain[1];
+	/* frame_len_lines = ExposureTime.FrameLengthLines; */
+	f_lines = mt9m114_res[dev->res].lines_per_frame;
+	analog_gain = exposure->gain[0];
+	digital_gain = exposure->gain[1];
 	if (!dev->streamon) {
 		/* Save the first exposure values while stream is off */
 		dev->first_exp = coarse_integration;
-		dev->first_gain = AnalogGain;
-		dev->first_diggain = DigitalGain;
+		dev->first_gain = analog_gain;
+		dev->first_diggain = digital_gain;
 	}
-	/* DigitalGain = 0x400 * (((u16) DigitalGain) >> 8) +		*/
-	/* ((unsigned int)(0x400 * (((u16) DigitalGain) & 0xFF)) >>8);	*/
+	/* digital_gain = 0x400 * (((u16) digital_gain) >> 8) +		*/
+	/* ((unsigned int)(0x400 * (((u16) digital_gain) & 0xFF)) >>8);	*/
 
 	/* set frame length */
-	if (FLines < coarse_integration + 6)
-		FLines = coarse_integration + 6;
-	if (FLines < FrameLengthLines)
-		FLines = FrameLengthLines;
-	ret = mt9m114_write_reg(client, MISENSOR_16BIT, 0x300A, FLines);
+	if (f_lines < coarse_integration + 6)
+		f_lines = coarse_integration + 6;
+	if (f_lines < frame_len_lines)
+		f_lines = frame_len_lines;
+	ret = mt9m114_write_reg(client, MISENSOR_16BIT, 0x300A, f_lines);
 	if (ret) {
-		v4l2_err(client, "%s: fail to set FLines\n", __func__);
+		v4l2_err(client, "%s: fail to set f_lines\n", __func__);
 		return -EINVAL;
 	}
 
@@ -1049,39 +1049,39 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 
 	/*
 	 * set analog/digital gain
-	switch(AnalogGain)
+	switch(analog_gain)
 	{
 	case 0:
-	  AnalogGainToWrite = 0x0;
+	  analog_gain_to_write = 0x0;
 	  break;
 	case 1:
-	  AnalogGainToWrite = 0x20;
+	  analog_gain_to_write = 0x20;
 	  break;
 	case 2:
-	  AnalogGainToWrite = 0x60;
+	  analog_gain_to_write = 0x60;
 	  break;
 	case 4:
-	  AnalogGainToWrite = 0xA0;
+	  analog_gain_to_write = 0xA0;
 	  break;
 	case 8:
-	  AnalogGainToWrite = 0xE0;
+	  analog_gain_to_write = 0xE0;
 	  break;
 	default:
-	  AnalogGainToWrite = 0x20;
+	  analog_gain_to_write = 0x20;
 	  break;
 	}
 	*/
-	if (DigitalGain >= 16 || DigitalGain <= 1)
-		DigitalGain = 1;
+	if (digital_gain >= 16 || digital_gain <= 1)
+		digital_gain = 1;
 
 	/*
 	 * AnalogGainToWrite = (u16)((DigitalGain << 12) | AnalogGainToWrite);
 	 */
-	AnalogGainToWrite = (u16)((DigitalGain << 12) | (u16)AnalogGain);
+	analog_gain_to_write = (u16)((digital_gain << 12) | (u16)analog_gain);
 	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
-				REG_GAIN, AnalogGainToWrite);
+				REG_GAIN, analog_gain_to_write);
 	if (ret) {
-		v4l2_err(client, "%s: fail to set AnalogGainToWrite\n",
+		v4l2_err(client, "%s: fail to set analog_gain_to_write\n",
 			 __func__);
 		return -EINVAL;
 	}
-- 
2.25.1

[PATCH RESEND v3 6/6] staging: media: atomisp: replace raw printk() by dev_info()

[Deepak R Varma]

It is better to use dev_info() instead of raw printk() call. This
addresses the checkpatch complain for not using KERN_<LEVEL> facility in
printk() call.

Suggested-by: Fabio Aiuto <fabioaiuto83@gmail.com>
Signed-off-by: Deepak R Varma <drv@mailo.com>
---

Changes since v2:
   - Tag Fabio Auito for the patch suggestion

Changes in v1:
   - implement following changes suggested by Fabio Aiuto
       a. use dev_info instead of pr_info
       b. update patch log message accordingly


 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index b572551f1a0d..7e4e123fdb52 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -1020,8 +1020,8 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd,
 		return -EINVAL;
 	}
 
-	printk("%s: before gc0310_write_reg_array %s\n", __func__,
-	       gc0310_res[dev->fmt_idx].desc);
+	dev_info(&client->dev, "%s: before gc0310_write_reg_array %s\n",
+		 __func__, gc0310_res[dev->fmt_idx].desc);
 	ret = startup(sd);
 	if (ret) {
 		dev_err(&client->dev, "gc0310 startup err\n");
-- 
2.25.1

Re: [PATCH RESEND v3 2/6] staging: media: atomisp: balance braces around if...else block

[Fabio Aiuto]

On Sun, Apr 25, 2021 at 02:12:20PM +0530, Deepak R Varma wrote:
> Balance braces around the if else blocks as per the code style guidelines.
> Resolves checkpatch script CHECK / WARNING feedback messages.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> 
> Changes since v2:
>    - None.
> Changes since v1:
>    - None.
> 
>  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c  | 4 ++--
>  drivers/staging/media/atomisp/i2c/atomisp-gc2235.c  | 4 ++--
>  drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 4 ++--
>  drivers/staging/media/atomisp/i2c/atomisp-ov2680.c  | 7 ++++---
>  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c  | 4 ++--
>  5 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> index 6be3ee1d93a5..d68a2bcc9ae1 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> @@ -872,9 +872,9 @@ static int gc0310_s_power(struct v4l2_subdev *sd, int on)
>  {
>  	int ret;
>  
> -	if (on == 0)
> +	if (on == 0) {
>  		return power_down(sd);
> -	else {
> +	} else {
>  		ret = power_up(sd);
>  		if (!ret)
>  			return gc0310_init(sd);
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> index 6ba4a8adff7c..e722c639b60d 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> @@ -658,9 +658,9 @@ static int gc2235_s_power(struct v4l2_subdev *sd, int on)
>  {
>  	int ret;
>  
> -	if (on == 0)
> +	if (on == 0) {
>  		ret = power_down(sd);
> -	else {
> +	} else {
>  		ret = power_up(sd);
>  		if (!ret)
>  			ret = __gc2235_init(sd);
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> index f5de81132177..465fc4468442 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> @@ -568,9 +568,9 @@ static int power_down(struct v4l2_subdev *sd)
>  
>  static int mt9m114_s_power(struct v4l2_subdev *sd, int power)
>  {
> -	if (power == 0)
> +	if (power == 0) {
>  		return power_down(sd);
> -	else {
> +	} else {
>  		if (power_up(sd))
>  			return -EINVAL;
>  
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> index c90730513438..92c52431bd8f 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> @@ -461,11 +461,12 @@ static int ov2680_v_flip(struct v4l2_subdev *sd, s32 value)
>  	ret = ov2680_read_reg(client, 1, OV2680_FLIP_REG, &val);
>  	if (ret)
>  		return ret;
> -	if (value) {
> +
> +	if (value)
>  		val |= OV2680_FLIP_MIRROR_BIT_ENABLE;
> -	} else {
> +	else
>  		val &= ~OV2680_FLIP_MIRROR_BIT_ENABLE;
> -	}
> +

Hi Deepak,

what you did above is not what is written in the commit message
description about. Here unneeded bracks are removed in both
branches, is not a matter of braces balancing.

thank you,

fabio 

>  	ret = ov2680_write_reg(client, 1,
>  			       OV2680_FLIP_REG, val);
>  	if (ret)
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> index aec7392fd1de..d046a9804f63 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> @@ -772,9 +772,9 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on)
>  {
>  	int ret;
>  
> -	if (on == 0)
> +	if (on == 0) {
>  		return power_down(sd);
> -	else {
> +	} else {
>  		ret = power_up(sd);
>  		if (!ret)
>  			return ov2722_init(sd);
> -- 
> 2.25.1
> 
> 
> 
> 

Re: [PATCH RESEND v3 2/6] staging: media: atomisp: balance braces around if...else block

[Deepak R Varma]

On Mon, Apr 26, 2021 at 11:56:11AM +0200, Fabio Aiuto wrote:
> On Sun, Apr 25, 2021 at 02:12:20PM +0530, Deepak R Varma wrote:
> > Balance braces around the if else blocks as per the code style guidelines.
> > Resolves checkpatch script CHECK / WARNING feedback messages.
> > 
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> > 
> > Changes since v2:
> >    - None.
> > Changes since v1:
> >    - None.
> > 
> >  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c  | 4 ++--
> >  drivers/staging/media/atomisp/i2c/atomisp-gc2235.c  | 4 ++--
> >  drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 4 ++--
> >  drivers/staging/media/atomisp/i2c/atomisp-ov2680.c  | 7 ++++---
> >  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c  | 4 ++--
> >  5 files changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > index 6be3ee1d93a5..d68a2bcc9ae1 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > @@ -872,9 +872,9 @@ static int gc0310_s_power(struct v4l2_subdev *sd, int on)
> >  {
> >  	int ret;
> >  
> > -	if (on == 0)
> > +	if (on == 0) {
> >  		return power_down(sd);
> > -	else {
> > +	} else {
> >  		ret = power_up(sd);
> >  		if (!ret)
> >  			return gc0310_init(sd);
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > index 6ba4a8adff7c..e722c639b60d 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > @@ -658,9 +658,9 @@ static int gc2235_s_power(struct v4l2_subdev *sd, int on)
> >  {
> >  	int ret;
> >  
> > -	if (on == 0)
> > +	if (on == 0) {
> >  		ret = power_down(sd);
> > -	else {
> > +	} else {
> >  		ret = power_up(sd);
> >  		if (!ret)
> >  			ret = __gc2235_init(sd);
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > index f5de81132177..465fc4468442 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > @@ -568,9 +568,9 @@ static int power_down(struct v4l2_subdev *sd)
> >  
> >  static int mt9m114_s_power(struct v4l2_subdev *sd, int power)
> >  {
> > -	if (power == 0)
> > +	if (power == 0) {
> >  		return power_down(sd);
> > -	else {
> > +	} else {
> >  		if (power_up(sd))
> >  			return -EINVAL;
> >  
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > index c90730513438..92c52431bd8f 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > @@ -461,11 +461,12 @@ static int ov2680_v_flip(struct v4l2_subdev *sd, s32 value)
> >  	ret = ov2680_read_reg(client, 1, OV2680_FLIP_REG, &val);
> >  	if (ret)
> >  		return ret;
> > -	if (value) {
> > +
> > +	if (value)
> >  		val |= OV2680_FLIP_MIRROR_BIT_ENABLE;
> > -	} else {
> > +	else
> >  		val &= ~OV2680_FLIP_MIRROR_BIT_ENABLE;
> > -	}
> > +
> 
> Hi Deepak,
> 
> what you did above is not what is written in the commit message
> description about. Here unneeded bracks are removed in both
> branches, is not a matter of braces balancing.

Okay. I was thinking adding where necessary and removing where not
would lead to expected balancing.
I will send this as a separate patch in this patch set. Is it okay to
add a new patch to the set now?

Thank you,
deepak.

> 
> thank you,
> 
> fabio 
> 
> >  	ret = ov2680_write_reg(client, 1,
> >  			       OV2680_FLIP_REG, val);
> >  	if (ret)
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> > index aec7392fd1de..d046a9804f63 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> > @@ -772,9 +772,9 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on)
> >  {
> >  	int ret;
> >  
> > -	if (on == 0)
> > +	if (on == 0) {
> >  		return power_down(sd);
> > -	else {
> > +	} else {
> >  		ret = power_up(sd);
> >  		if (!ret)
> >  			return ov2722_init(sd);
> > -- 
> > 2.25.1
> > 
> > 
> > 
> > 

Re: [PATCH RESEND v3 2/6] staging: media: atomisp: balance braces around if...else block

[Fabio Aiuto]

On Mon, Apr 26, 2021 at 08:00:22PM +0530, Deepak R Varma wrote:
> On Mon, Apr 26, 2021 at 11:56:11AM +0200, Fabio Aiuto wrote:
> > On Sun, Apr 25, 2021 at 02:12:20PM +0530, Deepak R Varma wrote:
> > > Balance braces around the if else blocks as per the code style guidelines.
> > > Resolves checkpatch script CHECK / WARNING feedback messages.
> > > 
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > > 
> > > Changes since v2:
> > >    - None.
> > > Changes since v1:
> > >    - None.
> > > 
> > >  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c  | 4 ++--
> > >  drivers/staging/media/atomisp/i2c/atomisp-gc2235.c  | 4 ++--
> > >  drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 4 ++--
> > >  drivers/staging/media/atomisp/i2c/atomisp-ov2680.c  | 7 ++++---
> > >  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c  | 4 ++--
> > >  5 files changed, 12 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > > index 6be3ee1d93a5..d68a2bcc9ae1 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > > @@ -872,9 +872,9 @@ static int gc0310_s_power(struct v4l2_subdev *sd, int on)
> > >  {
> > >  	int ret;
> > >  
> > > -	if (on == 0)
> > > +	if (on == 0) {
> > >  		return power_down(sd);
> > > -	else {
> > > +	} else {
> > >  		ret = power_up(sd);
> > >  		if (!ret)
> > >  			return gc0310_init(sd);
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > > index 6ba4a8adff7c..e722c639b60d 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > > @@ -658,9 +658,9 @@ static int gc2235_s_power(struct v4l2_subdev *sd, int on)
> > >  {
> > >  	int ret;
> > >  
> > > -	if (on == 0)
> > > +	if (on == 0) {
> > >  		ret = power_down(sd);
> > > -	else {
> > > +	} else {
> > >  		ret = power_up(sd);
> > >  		if (!ret)
> > >  			ret = __gc2235_init(sd);
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > > index f5de81132177..465fc4468442 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > > @@ -568,9 +568,9 @@ static int power_down(struct v4l2_subdev *sd)
> > >  
> > >  static int mt9m114_s_power(struct v4l2_subdev *sd, int power)
> > >  {
> > > -	if (power == 0)
> > > +	if (power == 0) {
> > >  		return power_down(sd);
> > > -	else {
> > > +	} else {
> > >  		if (power_up(sd))
> > >  			return -EINVAL;
> > >  
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > > index c90730513438..92c52431bd8f 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > > @@ -461,11 +461,12 @@ static int ov2680_v_flip(struct v4l2_subdev *sd, s32 value)
> > >  	ret = ov2680_read_reg(client, 1, OV2680_FLIP_REG, &val);
> > >  	if (ret)
> > >  		return ret;
> > > -	if (value) {
> > > +
> > > +	if (value)
> > >  		val |= OV2680_FLIP_MIRROR_BIT_ENABLE;
> > > -	} else {
> > > +	else
> > >  		val &= ~OV2680_FLIP_MIRROR_BIT_ENABLE;
> > > -	}
> > > +
> > 
> > Hi Deepak,
> > 
> > what you did above is not what is written in the commit message
> > description about. Here unneeded bracks are removed in both
> > branches, is not a matter of braces balancing.
> 
> Okay. I was thinking adding where necessary and removing where not
> would lead to expected balancing.
> I will send this as a separate patch in this patch set. Is it okay to
> add a new patch to the set now?

yes, a patchset from vN to v(N + 1) is allowed to increase in patch number,
at least in my experience.

> 
> Thank you,
> deepak.
> 
> > 
> > thank you,
> > 
> > fabio 
> > 
> > >  	ret = ov2680_write_reg(client, 1,
> > >  			       OV2680_FLIP_REG, val);
> > >  	if (ret)
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> > > index aec7392fd1de..d046a9804f63 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> > > @@ -772,9 +772,9 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on)
> > >  {
> > >  	int ret;
> > >  
> > > -	if (on == 0)
> > > +	if (on == 0) {
> > >  		return power_down(sd);
> > > -	else {
> > > +	} else {
> > >  		ret = power_up(sd);
> > >  		if (!ret)
> > >  			return ov2722_init(sd);
> > > -- 
> > > 2.25.1
> > > 
> > > 
> > > 
> > > 
> 
> 

thank you,

fabio

Re: [PATCH RESEND v3 3/6] staging: media: atomisp: use __func__ over function names

[Dan Carpenter]

On Sun, Apr 25, 2021 at 02:13:15PM +0530, Deepak R Varma wrote:
> Replace hard coded function names from the debug print strings by
> standard __func__ predefined identifier. This resolves following
> checkpatch script WARNING:
> Prefer using '"%s...", __func__' to using function's name, in a string.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> 
> Changes since v2:
>    - None.
> Changes since v1:
>    - None.
> 
>  .../staging/media/atomisp/i2c/atomisp-gc0310.c   |  2 +-
>  .../staging/media/atomisp/i2c/atomisp-gc2235.c   |  2 +-
>  .../staging/media/atomisp/i2c/atomisp-lm3554.c   |  2 +-
>  .../staging/media/atomisp/i2c/atomisp-ov2680.c   | 16 ++++++++--------
>  .../staging/media/atomisp/i2c/atomisp-ov2722.c   |  2 +-
>  5 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> index d68a2bcc9ae1..b572551f1a0d 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> @@ -1292,7 +1292,7 @@ static int gc0310_remove(struct i2c_client *client)
>  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
>  	struct gc0310_device *dev = to_gc0310_sensor(sd);
>  
> -	dev_dbg(&client->dev, "gc0310_remove...\n");
> +	dev_dbg(&client->dev, "%s...\n", __func__);
>  
>  	dev->platform_data->csi_cfg(sd, 0);
>  
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> index e722c639b60d..548c572d3b57 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> @@ -1034,7 +1034,7 @@ static int gc2235_remove(struct i2c_client *client)
>  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
>  	struct gc2235_device *dev = to_gc2235_sensor(sd);
>  
> -	dev_dbg(&client->dev, "gc2235_remove...\n");
> +	dev_dbg(&client->dev, "%s...\n", __func__);

Just delete printks that only print the function name.  We have ftrace
for that.  There are several others below.

regards,
dan carpenter

Re: [PATCH RESEND v3 1/6] staging: media: atomisp: improve function argument alignment

[Hans Verkuil]

Hi Deepak,

I'm dropping this patch since others have fixed this already.

Regards,

	Hans

On 25/04/2021 10:41, Deepak R Varma wrote:
> Improve multi-line function argument alignment according to the code style
> guidelines. Resolves checkpatch feedback: "Alignment should match
> open parenthesis".
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> 
> Changes since v2:
>    - None.
> Changes since v1:
>    - None.
> 
> 
>  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++--
>  drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 4 ++--
>  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> index 2b71de722ec3..6be3ee1d93a5 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> @@ -192,8 +192,8 @@ static int __gc0310_buf_reg_array(struct i2c_client *client,
>  }
>  
>  static int __gc0310_write_reg_is_consecutive(struct i2c_client *client,
> -	struct gc0310_write_ctrl *ctrl,
> -	const struct gc0310_reg *next)
> +					     struct gc0310_write_ctrl *ctrl,
> +					     const struct gc0310_reg *next)
>  {
>  	if (ctrl->index == 0)
>  		return 1;
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> index 78147ffb6099..6ba4a8adff7c 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> @@ -171,8 +171,8 @@ static int __gc2235_buf_reg_array(struct i2c_client *client,
>  }
>  
>  static int __gc2235_write_reg_is_consecutive(struct i2c_client *client,
> -	struct gc2235_write_ctrl *ctrl,
> -	const struct gc2235_reg *next)
> +					     struct gc2235_write_ctrl *ctrl,
> +					     const struct gc2235_reg *next)
>  {
>  	if (ctrl->index == 0)
>  		return 1;
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> index eecefcd734d0..aec7392fd1de 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> @@ -212,8 +212,8 @@ static int __ov2722_buf_reg_array(struct i2c_client *client,
>  }
>  
>  static int __ov2722_write_reg_is_consecutive(struct i2c_client *client,
> -	struct ov2722_write_ctrl *ctrl,
> -	const struct ov2722_reg *next)
> +					     struct ov2722_write_ctrl *ctrl,
> +					     const struct ov2722_reg *next)
>  {
>  	if (ctrl->index == 0)
>  		return 1;
> 

Re: [PATCH RESEND v3 3/6] staging: media: atomisp: use __func__ over function names

[Deepak R Varma]

On Wed, Apr 28, 2021 at 01:27:17PM +0300, Dan Carpenter wrote:
> On Sun, Apr 25, 2021 at 02:13:15PM +0530, Deepak R Varma wrote:
> > Replace hard coded function names from the debug print strings by
> > standard __func__ predefined identifier. This resolves following
> > checkpatch script WARNING:
> > Prefer using '"%s...", __func__' to using function's name, in a string.
> > 
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> > 
> > Changes since v2:
> >    - None.
> > Changes since v1:
> >    - None.
> > 
> >  .../staging/media/atomisp/i2c/atomisp-gc0310.c   |  2 +-
> >  .../staging/media/atomisp/i2c/atomisp-gc2235.c   |  2 +-
> >  .../staging/media/atomisp/i2c/atomisp-lm3554.c   |  2 +-
> >  .../staging/media/atomisp/i2c/atomisp-ov2680.c   | 16 ++++++++--------
> >  .../staging/media/atomisp/i2c/atomisp-ov2722.c   |  2 +-
> >  5 files changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > index d68a2bcc9ae1..b572551f1a0d 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > @@ -1292,7 +1292,7 @@ static int gc0310_remove(struct i2c_client *client)
> >  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> >  	struct gc0310_device *dev = to_gc0310_sensor(sd);
> >  
> > -	dev_dbg(&client->dev, "gc0310_remove...\n");
> > +	dev_dbg(&client->dev, "%s...\n", __func__);
> >  
> >  	dev->platform_data->csi_cfg(sd, 0);
> >  
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > index e722c639b60d..548c572d3b57 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > @@ -1034,7 +1034,7 @@ static int gc2235_remove(struct i2c_client *client)
> >  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> >  	struct gc2235_device *dev = to_gc2235_sensor(sd);
> >  
> > -	dev_dbg(&client->dev, "gc2235_remove...\n");
> > +	dev_dbg(&client->dev, "%s...\n", __func__);
> 
> Just delete printks that only print the function name.  We have ftrace
> for that.  There are several others below.

Thank you Dan, Hans, Fabio, Sakari and everyone for your review and comments.
I have received feedback and suggestions on the patch set.
I am going to rebuild the patch set according to the feedback and send as v4.

Appreciate your comments and time,
deepak.

> 
> regards,
> dan carpenter
>