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

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

[Deepak R Varma]

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

Note: The patches should be applied in the ascending order.

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: use printk with KERN facility level

 .../media/atomisp/i2c/atomisp-gc0310.c        | 12 +--
 .../media/atomisp/i2c/atomisp-gc2235.c        | 18 ++--
 .../atomisp/i2c/atomisp-libmsrlisthelper.c    |  3 +-
 .../media/atomisp/i2c/atomisp-lm3554.c        |  2 +-
 .../media/atomisp/i2c/atomisp-mt9m114.c       | 82 ++++++++++---------
 .../media/atomisp/i2c/atomisp-ov2680.c        | 26 +++---
 .../media/atomisp/i2c/atomisp-ov2722.c        | 10 +--
 7 files changed, 80 insertions(+), 73 deletions(-)

-- 
2.25.1

[PATCH 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>
---
 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 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>
---
 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 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>
---
 .../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 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.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 .../media/atomisp/i2c/atomisp-gc2235.c        |  8 +++----
 .../atomisp/i2c/atomisp-libmsrlisthelper.c    |  3 ++-
 .../media/atomisp/i2c/atomisp-mt9m114.c       | 22 +++++++++++--------
 .../media/atomisp/i2c/atomisp-ov2680.c        |  3 ++-
 4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 548c572d3b57..a585d73665a6 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -747,10 +747,10 @@ static int startup(struct v4l2_subdev *sd)
 
 	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
-		*/
+		 * 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);
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
index b93c80471f22..750b3484eb19 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
@@ -57,7 +57,8 @@ static int set_msr_configuration(struct i2c_client *client, uint8_t *bufptr,
 	 * 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..160bb58ce708 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -478,7 +478,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
 	/* 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);
@@ -1019,8 +1020,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 		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)
@@ -1035,7 +1036,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 
 	/* set coarse integration */
 	/* 3A provide real exposure time.
-		should not translate to any value here. */
+	 * should not translate to any value here.
+	 */
 	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
 				REG_EXPO_COARSE, (u16)(coarse_integration));
 	if (ret) {
@@ -1044,7 +1046,7 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
 	}
 
 	/*
-	// set analog/digital gain
+	 * set analog/digital gain
 	switch(AnalogGain)
 	{
 	case 0:
@@ -1069,8 +1071,9 @@ 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);
@@ -1096,7 +1099,8 @@ static long mt9m114_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
 }
 
 /* This returns the exposure time being used. This should only be used
-   for filling in EXIF data, not for actual image processing. */
+ * 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);
@@ -1297,7 +1301,7 @@ static int mt9m114_g_ev(struct v4l2_subdev *sd, s32 *val)
 
 /* Fake interface
  * mt9m114 now can not support 3a_lock
-*/
+ */
 static int mt9m114_s_3a_lock(struct v4l2_subdev *sd, s32 val)
 {
 	aaalock = val;
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index c17615149f46..91dca275a08e 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -732,7 +732,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
 	 * 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);
-- 
2.25.1

[PATCH 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>
---
 .../media/atomisp/i2c/atomisp-mt9m114.c       | 62 +++++++++----------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
index 160bb58ce708..e63906a69e30 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -999,10 +999,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],
@@ -1010,27 +1010,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;
 	}
 
@@ -1047,38 +1047,38 @@ 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);
+	/* analog_gain_to_write = (u16)((digital_gain << 12) | analog_gain_to_write);
 	 */
-	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 6/6] staging: media: atomisp: use printk with KERN facility level

[Deepak R Varma]

printk() without KERN_<LEVEL> facility is flagged by checkpatch as a
warning. It is better to use pr_info() which comes with an
inbuilt KERN_INFO level.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

Re: [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level

[Fabio Aiuto]

Hi Deepak,

On Tue, Apr 20, 2021 at 12:46:40AM +0530, Deepak R Varma wrote:
> printk() without KERN_<LEVEL> facility is flagged by checkpatch as a
> warning. It is better to use pr_info() which comes with an
> inbuilt KERN_INFO level.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
>  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> index b572551f1a0d..a0f3c5c32f94 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> @@ -1020,7 +1020,7 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd,
>  		return -EINVAL;
>  	}
>  
> -	printk("%s: before gc0310_write_reg_array %s\n", __func__,
> +	pr_info("%s: before gc0310_write_reg_array %s\n", __func__,
>  	       gc0310_res[dev->fmt_idx].desc);

in a driver is best recommended to replace a raw printk call
with a dev_*() log. Check few lines above here to see
how it is done.


>  	ret = startup(sd);
>  	if (ret) {
> -- 
> 2.25.1
> 
> 


thank you,

fabio

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

[Fabio Aiuto]

Hi,

On Tue, Apr 20, 2021 at 12:45:57AM +0530, Deepak R Varma wrote:
> 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>
> ---
>  .../media/atomisp/i2c/atomisp-mt9m114.c       | 62 +++++++++----------
>  1 file changed, 31 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> index 160bb58ce708..e63906a69e30 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> @@ -999,10 +999,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;
> 

this patch is made of multiple logical changes, i.e. more than one
variable at a time are renamed. Maybe this should be split in
one patch per variable name.

thank you,

fabio

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

[Fabio Aiuto]

On Tue, Apr 20, 2021 at 12:45:04AM +0530, Deepak R Varma wrote:
> Reformat code comment blocks according to the coding style guidelines.
> This resolves different checkpatch script WARNINGs around block comments.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
>  .../media/atomisp/i2c/atomisp-gc2235.c        |  8 +++----
>  .../atomisp/i2c/atomisp-libmsrlisthelper.c    |  3 ++-
>  .../media/atomisp/i2c/atomisp-mt9m114.c       | 22 +++++++++++--------
>  .../media/atomisp/i2c/atomisp-ov2680.c        |  3 ++-
>  4 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> index 548c572d3b57..a585d73665a6 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> @@ -747,10 +747,10 @@ static int startup(struct v4l2_subdev *sd)
>  
>  	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
> -		*/
> +		 * 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);
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> index b93c80471f22..750b3484eb19 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> @@ -57,7 +57,8 @@ static int set_msr_configuration(struct i2c_client *client, uint8_t *bufptr,
>  	 * 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..160bb58ce708 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> @@ -478,7 +478,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
>  	/* 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);
> @@ -1019,8 +1020,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
>  		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)
> @@ -1035,7 +1036,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
>  
>  	/* set coarse integration */
>  	/* 3A provide real exposure time.
> -		should not translate to any value here. */
> +	 * should not translate to any value here.
> +	 */
>  	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
>  				REG_EXPO_COARSE, (u16)(coarse_integration));
>  	if (ret) {
> @@ -1044,7 +1046,7 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
>  	}
>  
>  	/*
> -	// set analog/digital gain
> +	 * set analog/digital gain
>  	switch(AnalogGain)
>  	{
>  	case 0:
> @@ -1069,8 +1071,9 @@ 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);
> +	 */

this is best recommended for one line comment:

/* AnalogGainToWrite = (u16)((DigitalGain << 12) | AnalogGainToWrite); */



>  	AnalogGainToWrite = (u16)((DigitalGain << 12) | (u16)AnalogGain);
>  	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
>  				REG_GAIN, AnalogGainToWrite);
> @@ -1096,7 +1099,8 @@ static long mt9m114_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
>  }
>  
>  /* This returns the exposure time being used. This should only be used
> -   for filling in EXIF data, not for actual image processing. */
> + * 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);
> @@ -1297,7 +1301,7 @@ static int mt9m114_g_ev(struct v4l2_subdev *sd, s32 *val)
>  
>  /* Fake interface
>   * mt9m114 now can not support 3a_lock
> -*/
> + */
>  static int mt9m114_s_3a_lock(struct v4l2_subdev *sd, s32 val)
>  {
>  	aaalock = val;
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> index c17615149f46..91dca275a08e 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> @@ -732,7 +732,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
>  	 * 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);
> -- 
> 2.25.1
> 
> 

multiline comment format should appear this way:

	/*
	 * This is a comment, a
	 * multiline one.
	 */

with the first line of the comment empty.

thank you,

fabio

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

[Hans Verkuil]

On 19/04/2021 21:12, 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>

WARNING: From:/Signed-off-by: email address mismatch: 'From: Deepak R Varma <mh12gx2825@gmail.com>' != 'Signed-off-by: Deepak R Varma
<drv@mailo.com>'

Which email should I use? Ideally you should post from the same email
as the Signed-off-by.

Regards,

	Hans

> ---
>  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 5/6] staging: media: atomisp: fix CamelCase variable naming

[Hans Verkuil]

On 20/04/2021 10:39, Fabio Aiuto wrote:
> Hi,
> 
> On Tue, Apr 20, 2021 at 12:45:57AM +0530, Deepak R Varma wrote:
>> 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>
>> ---
>>  .../media/atomisp/i2c/atomisp-mt9m114.c       | 62 +++++++++----------
>>  1 file changed, 31 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
>> index 160bb58ce708..e63906a69e30 100644
>> --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
>> @@ -999,10 +999,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;
>>
> 
> this patch is made of multiple logical changes, i.e. more than one
> variable at a time are renamed. Maybe this should be split in
> one patch per variable name.

I'm OK with this, it's pretty readable and obvious what is going on.

Regards,

	Hans

Re: [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level

[Deepak R Varma]

On Tue, Apr 20, 2021 at 10:35:23AM +0200, Fabio Aiuto wrote:
> Hi Deepak,
> 
> On Tue, Apr 20, 2021 at 12:46:40AM +0530, Deepak R Varma wrote:
> > printk() without KERN_<LEVEL> facility is flagged by checkpatch as a
> > warning. It is better to use pr_info() which comes with an
> > inbuilt KERN_INFO level.
> > 
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > index b572551f1a0d..a0f3c5c32f94 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > @@ -1020,7 +1020,7 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd,
> >  		return -EINVAL;
> >  	}
> >  
> > -	printk("%s: before gc0310_write_reg_array %s\n", __func__,
> > +	pr_info("%s: before gc0310_write_reg_array %s\n", __func__,
> >  	       gc0310_res[dev->fmt_idx].desc);
> 
> in a driver is best recommended to replace a raw printk call
> with a dev_*() log. Check few lines above here to see
> how it is done.

Hello Fabio,
thank you for the feedback. Should I resubmit this patch with additional
printk() / pr_info() replacement by dev_*()? OR send in a new patch with
this change? Just want to make sure I am not including more than one
change type in the patch.

deepak.

> 
> 
> >  	ret = startup(sd);
> >  	if (ret) {
> > -- 
> > 2.25.1
> > 
> > 
> 
> 
> thank you,
> 
> fabio

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

[Deepak R Varma]

On Tue, Apr 20, 2021 at 10:44:48AM +0200, Fabio Aiuto wrote:
> On Tue, Apr 20, 2021 at 12:45:04AM +0530, Deepak R Varma wrote:
> > Reformat code comment blocks according to the coding style guidelines.
> > This resolves different checkpatch script WARNINGs around block comments.
> > 
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >  .../media/atomisp/i2c/atomisp-gc2235.c        |  8 +++----
> >  .../atomisp/i2c/atomisp-libmsrlisthelper.c    |  3 ++-
> >  .../media/atomisp/i2c/atomisp-mt9m114.c       | 22 +++++++++++--------
> >  .../media/atomisp/i2c/atomisp-ov2680.c        |  3 ++-
> >  4 files changed, 21 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > index 548c572d3b57..a585d73665a6 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > @@ -747,10 +747,10 @@ static int startup(struct v4l2_subdev *sd)
> >  
> >  	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
> > -		*/
> > +		 * 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);
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> > index b93c80471f22..750b3484eb19 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> > @@ -57,7 +57,8 @@ static int set_msr_configuration(struct i2c_client *client, uint8_t *bufptr,
> >  	 * 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..160bb58ce708 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > @@ -478,7 +478,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
> >  	/* 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);
> > @@ -1019,8 +1020,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> >  		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)
> > @@ -1035,7 +1036,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> >  
> >  	/* set coarse integration */
> >  	/* 3A provide real exposure time.
> > -		should not translate to any value here. */
> > +	 * should not translate to any value here.
> > +	 */
> >  	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
> >  				REG_EXPO_COARSE, (u16)(coarse_integration));
> >  	if (ret) {
> > @@ -1044,7 +1046,7 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> >  	}
> >  
> >  	/*
> > -	// set analog/digital gain
> > +	 * set analog/digital gain
> >  	switch(AnalogGain)
> >  	{
> >  	case 0:
> > @@ -1069,8 +1071,9 @@ 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);
> > +	 */
> 
> this is best recommended for one line comment:
> 
> /* AnalogGainToWrite = (u16)((DigitalGain << 12) | AnalogGainToWrite); */

It then extends beyond 80 character in length. Will that be acceptable?

> 
> 
> 
> >  	AnalogGainToWrite = (u16)((DigitalGain << 12) | (u16)AnalogGain);
> >  	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
> >  				REG_GAIN, AnalogGainToWrite);
> > @@ -1096,7 +1099,8 @@ static long mt9m114_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
> >  }
> >  
> >  /* This returns the exposure time being used. This should only be used
> > -   for filling in EXIF data, not for actual image processing. */
> > + * 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);
> > @@ -1297,7 +1301,7 @@ static int mt9m114_g_ev(struct v4l2_subdev *sd, s32 *val)
> >  
> >  /* Fake interface
> >   * mt9m114 now can not support 3a_lock
> > -*/
> > + */
> >  static int mt9m114_s_3a_lock(struct v4l2_subdev *sd, s32 val)
> >  {
> >  	aaalock = val;
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > index c17615149f46..91dca275a08e 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > @@ -732,7 +732,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
> >  	 * 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);
> > -- 
> > 2.25.1
> > 
> > 
> 
> multiline comment format should appear this way:
> 
> 	/*
> 	 * This is a comment, a
> 	 * multiline one.
> 	 */
> 
> with the first line of the comment empty.

Sounds good. The coding style guide also allows the other style for
specific driver types. I will improve this change according to your
feedback and resubmit.

thank you,
deepak.

> 
> thank you,
> 
> fabio

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

[Deepak R Varma]

On Tue, Apr 20, 2021 at 03:24:32PM +0200, Hans Verkuil wrote:
> On 19/04/2021 21:12, 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>
> 
> WARNING: From:/Signed-off-by: email address mismatch: 'From: Deepak R Varma <mh12gx2825@gmail.com>' != 'Signed-off-by: Deepak R Varma
> <drv@mailo.com>'
> 
> Which email should I use? Ideally you should post from the same email
> as the Signed-off-by.

I am really sorry for this. I was trying to set up mutt to handle both
my accounts and guess that led to this issue. I will resubmit the patch
set with the appropriate email match. Will that be okay?

Thank you,
deepak.

> 
> Regards,
> 
> 	Hans
> 
> > ---
> >  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 1/6] staging: media: atomisp: improve function argument alignment

[Hans Verkuil]

On 20/04/2021 19:19, Deepak R Varma wrote:
> On Tue, Apr 20, 2021 at 03:24:32PM +0200, Hans Verkuil wrote:
>> On 19/04/2021 21:12, 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>
>>
>> WARNING: From:/Signed-off-by: email address mismatch: 'From: Deepak R Varma <mh12gx2825@gmail.com>' != 'Signed-off-by: Deepak R Varma
>> <drv@mailo.com>'
>>
>> Which email should I use? Ideally you should post from the same email
>> as the Signed-off-by.
> 
> I am really sorry for this. I was trying to set up mutt to handle both
> my accounts and guess that led to this issue. I will resubmit the patch
> set with the appropriate email match. Will that be okay?

It is sufficient to just let me know which email I should use. I can edit
it so it shows the correct author email.

Regards,

	Hans

> 
> Thank you,
> deepak.
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>> ---
>>>  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 4/6] staging: media: atomisp: reformat code comment blocks

[Fabio Aiuto]

On Tue, Apr 20, 2021 at 10:47:29PM +0530, Deepak R Varma wrote:
> On Tue, Apr 20, 2021 at 10:44:48AM +0200, Fabio Aiuto wrote:
> > On Tue, Apr 20, 2021 at 12:45:04AM +0530, Deepak R Varma wrote:
> > > Reformat code comment blocks according to the coding style guidelines.
> > > This resolves different checkpatch script WARNINGs around block comments.
> > > 
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > >  .../media/atomisp/i2c/atomisp-gc2235.c        |  8 +++----
> > >  .../atomisp/i2c/atomisp-libmsrlisthelper.c    |  3 ++-
> > >  .../media/atomisp/i2c/atomisp-mt9m114.c       | 22 +++++++++++--------
> > >  .../media/atomisp/i2c/atomisp-ov2680.c        |  3 ++-
> > >  4 files changed, 21 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > > index 548c572d3b57..a585d73665a6 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > > @@ -747,10 +747,10 @@ static int startup(struct v4l2_subdev *sd)
> > >  
> > >  	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
> > > -		*/
> > > +		 * 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);
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> > > index b93c80471f22..750b3484eb19 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> > > @@ -57,7 +57,8 @@ static int set_msr_configuration(struct i2c_client *client, uint8_t *bufptr,
> > >  	 * 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..160bb58ce708 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > > @@ -478,7 +478,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
> > >  	/* 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);
> > > @@ -1019,8 +1020,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> > >  		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)
> > > @@ -1035,7 +1036,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> > >  
> > >  	/* set coarse integration */
> > >  	/* 3A provide real exposure time.
> > > -		should not translate to any value here. */
> > > +	 * should not translate to any value here.
> > > +	 */
> > >  	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
> > >  				REG_EXPO_COARSE, (u16)(coarse_integration));
> > >  	if (ret) {
> > > @@ -1044,7 +1046,7 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> > >  	}
> > >  
> > >  	/*
> > > -	// set analog/digital gain
> > > +	 * set analog/digital gain
> > >  	switch(AnalogGain)
> > >  	{
> > >  	case 0:
> > > @@ -1069,8 +1071,9 @@ 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);
> > > +	 */
> > 
> > this is best recommended for one line comment:
> > 
> > /* AnalogGainToWrite = (u16)((DigitalGain << 12) | AnalogGainToWrite); */
> 
> It then extends beyond 80 character in length. Will that be acceptable?
> 

if the intention is to stay within 80 characters in length then it's better

	/*
	 * AnalogGainToWrite = (u16)(DigitalGain << 12) | AnalogGainToWrite);
	 */

thank you,

fabio

Re: [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level

[Fabio Aiuto]

On Tue, Apr 20, 2021 at 10:43:53PM +0530, Deepak R Varma wrote:
> On Tue, Apr 20, 2021 at 10:35:23AM +0200, Fabio Aiuto wrote:
> > Hi Deepak,
> > 
> > On Tue, Apr 20, 2021 at 12:46:40AM +0530, Deepak R Varma wrote:
> > > printk() without KERN_<LEVEL> facility is flagged by checkpatch as a
> > > warning. It is better to use pr_info() which comes with an
> > > inbuilt KERN_INFO level.
> > > 
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > >  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > > index b572551f1a0d..a0f3c5c32f94 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > > @@ -1020,7 +1020,7 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd,
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > -	printk("%s: before gc0310_write_reg_array %s\n", __func__,
> > > +	pr_info("%s: before gc0310_write_reg_array %s\n", __func__,
> > >  	       gc0310_res[dev->fmt_idx].desc);
> > 
> > in a driver is best recommended to replace a raw printk call
> > with a dev_*() log. Check few lines above here to see
> > how it is done.
> 
> Hello Fabio,
> thank you for the feedback. Should I resubmit this patch with additional
> printk() / pr_info() replacement by dev_*()? OR send in a new patch with
> this change? Just want to make sure I am not including more than one
> change type in the patch.
> 
> deepak.
> 

Hi Deepak,

what I would do is to just resend this patch with replacement by dev_*(),
which is just one change. Then if you want to replace all other
raw printk/pr_*() occurrences is matter of another separate
patch/series.

> > 
> > 
> > >  	ret = startup(sd);
> > >  	if (ret) {
> > > -- 
> > > 2.25.1
> > > 
> > > 
> > 
> > 
> > thank you,
> > 
> > fabio

thank you,

fabio

Re: [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level

[Deepak R Varma]

On Wed, Apr 21, 2021 at 09:22:17AM +0200, Fabio Aiuto wrote:
> On Tue, Apr 20, 2021 at 10:43:53PM +0530, Deepak R Varma wrote:
> > On Tue, Apr 20, 2021 at 10:35:23AM +0200, Fabio Aiuto wrote:
> > > Hi Deepak,
> > > 
> > > On Tue, Apr 20, 2021 at 12:46:40AM +0530, Deepak R Varma wrote:
> > > > printk() without KERN_<LEVEL> facility is flagged by checkpatch as a
> > > > warning. It is better to use pr_info() which comes with an
> > > > inbuilt KERN_INFO level.
> > > > 
> > > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > > ---
> > > >  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > > > index b572551f1a0d..a0f3c5c32f94 100644
> > > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> > > > @@ -1020,7 +1020,7 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd,
> > > >  		return -EINVAL;
> > > >  	}
> > > >  
> > > > -	printk("%s: before gc0310_write_reg_array %s\n", __func__,
> > > > +	pr_info("%s: before gc0310_write_reg_array %s\n", __func__,
> > > >  	       gc0310_res[dev->fmt_idx].desc);
> > > 
> > > in a driver is best recommended to replace a raw printk call
> > > with a dev_*() log. Check few lines above here to see
> > > how it is done.
> > 
> > Hello Fabio,
> > thank you for the feedback. Should I resubmit this patch with additional
> > printk() / pr_info() replacement by dev_*()? OR send in a new patch with
> > this change? Just want to make sure I am not including more than one
> > change type in the patch.
> > 
> > deepak.
> > 
> 
> Hi Deepak,
> 
> what I would do is to just resend this patch with replacement by dev_*(),
> which is just one change. Then if you want to replace all other
> raw printk/pr_*() occurrences is matter of another separate
> patch/series.
> 
Hi Fabio,
That sounds good. Thank you for your comments. I will resubmit the patch
set according to the feedbacks received.

Have a good day.
deepak.

> > > 
> > > 
> > > >  	ret = startup(sd);
> > > >  	if (ret) {
> > > > -- 
> > > > 2.25.1
> > > > 
> > > > 
> > > 
> > > 
> > > thank you,
> > > 
> > > fabio
> 
> thank you,
> 
> fabio

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

[Deepak R Varma]

On Tue, Apr 20, 2021 at 10:59:16PM +0200, Hans Verkuil wrote:
> On 20/04/2021 19:19, Deepak R Varma wrote:
> > On Tue, Apr 20, 2021 at 03:24:32PM +0200, Hans Verkuil wrote:
> >> On 19/04/2021 21:12, 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>
> >>
> >> WARNING: From:/Signed-off-by: email address mismatch: 'From: Deepak R Varma <mh12gx2825@gmail.com>' != 'Signed-off-by: Deepak R Varma
> >> <drv@mailo.com>'
> >>
> >> Which email should I use? Ideally you should post from the same email
> >> as the Signed-off-by.
> > 
> > I am really sorry for this. I was trying to set up mutt to handle both
> > my accounts and guess that led to this issue. I will resubmit the patch
> > set with the appropriate email match. Will that be okay?
> 
> It is sufficient to just let me know which email I should use. I can edit
> it so it shows the correct author email.

Hello Hans,
You can use drv@mailo.com as the signed off email id.

I have received feedback on other patches of this set, so I am going to
resubmit the patch set v1 from the correct email ID.

Thank you,
deepak.


> 
> Regards,
> 
> 	Hans
> 
> > 
> > Thank you,
> > deepak.
> > 
> >>
> >> Regards,
> >>
> >> 	Hans
> >>
> >>> ---
> >>>  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 4/6] staging: media: atomisp: reformat code comment blocks

[Dan Carpenter]

On Tue, Apr 20, 2021 at 10:44:48AM +0200, Fabio Aiuto wrote:
> On Tue, Apr 20, 2021 at 12:45:04AM +0530, Deepak R Varma wrote:
> >  	switch(AnalogGain)
> >  	{
> >  	case 0:
> > @@ -1069,8 +1071,9 @@ 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);
> > +	 */
> 
> this is best recommended for one line comment:
> 
> /* AnalogGainToWrite = (u16)((DigitalGain << 12) | AnalogGainToWrite); */
> 

I'm going through old emails...

Just delete all commented out code.

regards,
dan carpenter