Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/6] staging: media: atomisp: code cleanup fixes
@ 2021-04-19 19:11 Deepak R Varma
  2021-04-19 19:12 ` [PATCH 1/6] staging: media: atomisp: improve function argument alignment Deepak R Varma
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Deepak R Varma @ 2021-04-19 19:11 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel
  Cc: mh12gx2825

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/6] staging: media: atomisp: improve function argument alignment
  2021-04-19 19:11 [PATCH 0/6] staging: media: atomisp: code cleanup fixes Deepak R Varma
@ 2021-04-19 19:12 ` Deepak R Varma
  2021-04-20 13:24   ` Hans Verkuil
  2021-04-19 19:13 ` [PATCH 2/6] staging: media: atomisp: balance braces around if...else block Deepak R Varma
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Deepak R Varma @ 2021-04-19 19:12 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel
  Cc: mh12gx2825

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/6] staging: media: atomisp: balance braces around if...else block
  2021-04-19 19:11 [PATCH 0/6] staging: media: atomisp: code cleanup fixes Deepak R Varma
  2021-04-19 19:12 ` [PATCH 1/6] staging: media: atomisp: improve function argument alignment Deepak R Varma
@ 2021-04-19 19:13 ` Deepak R Varma
  2021-04-19 19:14 ` [PATCH 3/6] staging: media: atomisp: use __func__ over function names Deepak R Varma
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Deepak R Varma @ 2021-04-19 19:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel
  Cc: mh12gx2825

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/6] staging: media: atomisp: use __func__ over function names
  2021-04-19 19:11 [PATCH 0/6] staging: media: atomisp: code cleanup fixes Deepak R Varma
  2021-04-19 19:12 ` [PATCH 1/6] staging: media: atomisp: improve function argument alignment Deepak R Varma
  2021-04-19 19:13 ` [PATCH 2/6] staging: media: atomisp: balance braces around if...else block Deepak R Varma
@ 2021-04-19 19:14 ` Deepak R Varma
  2021-04-19 19:15 ` [PATCH 4/6] staging: media: atomisp: reformat code comment blocks Deepak R Varma
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Deepak R Varma @ 2021-04-19 19:14 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel
  Cc: mh12gx2825

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/6] staging: media: atomisp: reformat code comment blocks
  2021-04-19 19:11 [PATCH 0/6] staging: media: atomisp: code cleanup fixes Deepak R Varma
                   ` (2 preceding siblings ...)
  2021-04-19 19:14 ` [PATCH 3/6] staging: media: atomisp: use __func__ over function names Deepak R Varma
@ 2021-04-19 19:15 ` Deepak R Varma
  2021-04-20  8:44   ` Fabio Aiuto
  2021-04-19 19:15 ` [PATCH 5/6] staging: media: atomisp: fix CamelCase variable naming Deepak R Varma
  2021-04-19 19:16 ` [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level Deepak R Varma
  5 siblings, 1 reply; 21+ messages in thread
From: Deepak R Varma @ 2021-04-19 19:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel
  Cc: mh12gx2825

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/6] staging: media: atomisp: fix CamelCase variable naming
  2021-04-19 19:11 [PATCH 0/6] staging: media: atomisp: code cleanup fixes Deepak R Varma
                   ` (3 preceding siblings ...)
  2021-04-19 19:15 ` [PATCH 4/6] staging: media: atomisp: reformat code comment blocks Deepak R Varma
@ 2021-04-19 19:15 ` Deepak R Varma
  2021-04-20  8:39   ` Fabio Aiuto
  2021-04-19 19:16 ` [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level Deepak R Varma
  5 siblings, 1 reply; 21+ messages in thread
From: Deepak R Varma @ 2021-04-19 19:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel
  Cc: mh12gx2825

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level
  2021-04-19 19:11 [PATCH 0/6] staging: media: atomisp: code cleanup fixes Deepak R Varma
                   ` (4 preceding siblings ...)
  2021-04-19 19:15 ` [PATCH 5/6] staging: media: atomisp: fix CamelCase variable naming Deepak R Varma
@ 2021-04-19 19:16 ` Deepak R Varma
  2021-04-20  8:35   ` Fabio Aiuto
  5 siblings, 1 reply; 21+ messages in thread
From: Deepak R Varma @ 2021-04-19 19:16 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel
  Cc: mh12gx2825

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level
  2021-04-19 19:16 ` [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level Deepak R Varma
@ 2021-04-20  8:35   ` Fabio Aiuto
  2021-04-20 17:13     ` Deepak R Varma
  0 siblings, 1 reply; 21+ messages in thread
From: Fabio Aiuto @ 2021-04-20  8:35 UTC (permalink / raw)
  To: Deepak R Varma
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/6] staging: media: atomisp: fix CamelCase variable naming
  2021-04-19 19:15 ` [PATCH 5/6] staging: media: atomisp: fix CamelCase variable naming Deepak R Varma
@ 2021-04-20  8:39   ` Fabio Aiuto
  2021-04-20 13:26     ` Hans Verkuil
  0 siblings, 1 reply; 21+ messages in thread
From: Fabio Aiuto @ 2021-04-20  8:39 UTC (permalink / raw)
  To: Deepak R Varma
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] staging: media: atomisp: reformat code comment blocks
  2021-04-19 19:15 ` [PATCH 4/6] staging: media: atomisp: reformat code comment blocks Deepak R Varma
@ 2021-04-20  8:44   ` Fabio Aiuto
  2021-04-20 17:17     ` Deepak R Varma
  2021-04-28  7:53     ` Dan Carpenter
  0 siblings, 2 replies; 21+ messages in thread
From: Fabio Aiuto @ 2021-04-20  8:44 UTC (permalink / raw)
  To: Deepak R Varma
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/6] staging: media: atomisp: improve function argument alignment
  2021-04-19 19:12 ` [PATCH 1/6] staging: media: atomisp: improve function argument alignment Deepak R Varma
@ 2021-04-20 13:24   ` Hans Verkuil
  2021-04-20 17:19     ` Deepak R Varma
  0 siblings, 1 reply; 21+ messages in thread
From: Hans Verkuil @ 2021-04-20 13:24 UTC (permalink / raw)
  To: Deepak R Varma, Mauro Carvalho Chehab, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel

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;
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/6] staging: media: atomisp: fix CamelCase variable naming
  2021-04-20  8:39   ` Fabio Aiuto
@ 2021-04-20 13:26     ` Hans Verkuil
  0 siblings, 0 replies; 21+ messages in thread
From: Hans Verkuil @ 2021-04-20 13:26 UTC (permalink / raw)
  To: Fabio Aiuto, Deepak R Varma
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level
  2021-04-20  8:35   ` Fabio Aiuto
@ 2021-04-20 17:13     ` Deepak R Varma
  2021-04-21  7:22       ` Fabio Aiuto
  0 siblings, 1 reply; 21+ messages in thread
From: Deepak R Varma @ 2021-04-20 17:13 UTC (permalink / raw)
  To: Fabio Aiuto
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] staging: media: atomisp: reformat code comment blocks
  2021-04-20  8:44   ` Fabio Aiuto
@ 2021-04-20 17:17     ` Deepak R Varma
  2021-04-21  7:17       ` Fabio Aiuto
  2021-04-28  7:53     ` Dan Carpenter
  1 sibling, 1 reply; 21+ messages in thread
From: Deepak R Varma @ 2021-04-20 17:17 UTC (permalink / raw)
  To: Fabio Aiuto
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/6] staging: media: atomisp: improve function argument alignment
  2021-04-20 13:24   ` Hans Verkuil
@ 2021-04-20 17:19     ` Deepak R Varma
  2021-04-20 20:59       ` Hans Verkuil
  0 siblings, 1 reply; 21+ messages in thread
From: Deepak R Varma @ 2021-04-20 17:19 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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;
> > 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/6] staging: media: atomisp: improve function argument alignment
  2021-04-20 17:19     ` Deepak R Varma
@ 2021-04-20 20:59       ` Hans Verkuil
  2021-04-21 10:19         ` Deepak R Varma
  0 siblings, 1 reply; 21+ messages in thread
From: Hans Verkuil @ 2021-04-20 20:59 UTC (permalink / raw)
  To: Deepak R Varma
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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;
>>>
>>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] staging: media: atomisp: reformat code comment blocks
  2021-04-20 17:17     ` Deepak R Varma
@ 2021-04-21  7:17       ` Fabio Aiuto
  0 siblings, 0 replies; 21+ messages in thread
From: Fabio Aiuto @ 2021-04-21  7:17 UTC (permalink / raw)
  To: Deepak R Varma
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level
  2021-04-20 17:13     ` Deepak R Varma
@ 2021-04-21  7:22       ` Fabio Aiuto
  2021-04-21 10:16         ` Deepak R Varma
  0 siblings, 1 reply; 21+ messages in thread
From: Fabio Aiuto @ 2021-04-21  7:22 UTC (permalink / raw)
  To: Deepak R Varma
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level
  2021-04-21  7:22       ` Fabio Aiuto
@ 2021-04-21 10:16         ` Deepak R Varma
  0 siblings, 0 replies; 21+ messages in thread
From: Deepak R Varma @ 2021-04-21 10:16 UTC (permalink / raw)
  To: Fabio Aiuto
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/6] staging: media: atomisp: improve function argument alignment
  2021-04-20 20:59       ` Hans Verkuil
@ 2021-04-21 10:19         ` Deepak R Varma
  0 siblings, 0 replies; 21+ messages in thread
From: Deepak R Varma @ 2021-04-21 10:19 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman,
	linux-media, linux-staging, linux-kernel

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;
> >>>
> >>
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/6] staging: media: atomisp: reformat code comment blocks
  2021-04-20  8:44   ` Fabio Aiuto
  2021-04-20 17:17     ` Deepak R Varma
@ 2021-04-28  7:53     ` Dan Carpenter
  1 sibling, 0 replies; 21+ messages in thread
From: Dan Carpenter @ 2021-04-28  7:53 UTC (permalink / raw)
  To: Fabio Aiuto
  Cc: Deepak R Varma, Mauro Carvalho Chehab, Sakari Ailus,
	Greg Kroah-Hartman, linux-media, linux-staging, linux-kernel

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, back to index

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19 19:11 [PATCH 0/6] staging: media: atomisp: code cleanup fixes Deepak R Varma
2021-04-19 19:12 ` [PATCH 1/6] staging: media: atomisp: improve function argument alignment Deepak R Varma
2021-04-20 13:24   ` Hans Verkuil
2021-04-20 17:19     ` Deepak R Varma
2021-04-20 20:59       ` Hans Verkuil
2021-04-21 10:19         ` Deepak R Varma
2021-04-19 19:13 ` [PATCH 2/6] staging: media: atomisp: balance braces around if...else block Deepak R Varma
2021-04-19 19:14 ` [PATCH 3/6] staging: media: atomisp: use __func__ over function names Deepak R Varma
2021-04-19 19:15 ` [PATCH 4/6] staging: media: atomisp: reformat code comment blocks Deepak R Varma
2021-04-20  8:44   ` Fabio Aiuto
2021-04-20 17:17     ` Deepak R Varma
2021-04-21  7:17       ` Fabio Aiuto
2021-04-28  7:53     ` Dan Carpenter
2021-04-19 19:15 ` [PATCH 5/6] staging: media: atomisp: fix CamelCase variable naming Deepak R Varma
2021-04-20  8:39   ` Fabio Aiuto
2021-04-20 13:26     ` Hans Verkuil
2021-04-19 19:16 ` [PATCH 6/6] staging: media: atomisp: use printk with KERN facility level Deepak R Varma
2021-04-20  8:35   ` Fabio Aiuto
2021-04-20 17:13     ` Deepak R Varma
2021-04-21  7:22       ` Fabio Aiuto
2021-04-21 10:16         ` Deepak R Varma

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git