[09/13] imx274: get rid of mode_index

Message ID 1523551878-15754-10-git-send-email-luca@lucaceresoli.net (mailing list archive)
State Superseded, archived
Headers

Commit Message

Luca Ceresoli April 12, 2018, 4:51 p.m. UTC
  After restructuring struct imx274_frmfmt, the mode_index field is
still in use only for two dev_dbg() calls in imx274_s_stream(). Let's
remove it and avoid duplicated information.

Replacing the first usage requires a rather annoying but trivial
computation. The other one can be removed entirely since it prints the
same value anyway.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 drivers/media/i2c/imx274.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)
  

Comments

kernel test robot April 13, 2018, 5:33 p.m. UTC | #1
Hi Luca,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.16 next-20180413]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Luca-Ceresoli/imx274-add-cropping-and-misc-improvements/20180413-234459
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-x017-201814 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:329:0,
                    from include/linux/kernel.h:14,
                    from include/linux/clk.h:16,
                    from drivers/media/i2c/imx274.c:22:
   drivers/media/i2c/imx274.c: In function 'imx274_s_stream':
>> drivers/media/i2c/imx274.c:1027:32: warning: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'int' [-Wformat=]
     dev_dbg(&imx274->client->dev, "%s : %s, mode index %lu\n", __func__,
                                   ^
   drivers/media/i2c/imx274.c:1029:3:
      imx274->mode - &imx274_formats[0]);
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
>> drivers/media/i2c/imx274.c:1027:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(&imx274->client->dev, "%s : %s, mode index %lu\n", __func__,
     ^~~~~~~

vim +1027 drivers/media/i2c/imx274.c

  1011	
  1012	/**
  1013	 * imx274_s_stream - It is used to start/stop the streaming.
  1014	 * @sd: V4L2 Sub device
  1015	 * @on: Flag (True / False)
  1016	 *
  1017	 * This function controls the start or stop of streaming for the
  1018	 * imx274 sensor.
  1019	 *
  1020	 * Return: 0 on success, errors otherwise
  1021	 */
  1022	static int imx274_s_stream(struct v4l2_subdev *sd, int on)
  1023	{
  1024		struct stimx274 *imx274 = to_imx274(sd);
  1025		int ret = 0;
  1026	
> 1027		dev_dbg(&imx274->client->dev, "%s : %s, mode index %lu\n", __func__,
  1028			on ? "Stream Start" : "Stream Stop",
  1029			imx274->mode - &imx274_formats[0]);
  1030	
  1031		mutex_lock(&imx274->lock);
  1032	
  1033		if (on) {
  1034			/* load mode registers */
  1035			ret = imx274_mode_regs(imx274);
  1036			if (ret)
  1037				goto fail;
  1038	
  1039			/*
  1040			 * update frame rate & expsoure. if the last mode is different,
  1041			 * HMAX could be changed. As the result, frame rate & exposure
  1042			 * are changed.
  1043			 * gain is not affected.
  1044			 */
  1045			ret = imx274_set_frame_interval(imx274,
  1046							imx274->frame_interval);
  1047			if (ret)
  1048				goto fail;
  1049	
  1050			/* update exposure time */
  1051			ret = __v4l2_ctrl_s_ctrl(imx274->ctrls.exposure,
  1052						 imx274->ctrls.exposure->val);
  1053			if (ret)
  1054				goto fail;
  1055	
  1056			/* start stream */
  1057			ret = imx274_start_stream(imx274);
  1058			if (ret)
  1059				goto fail;
  1060		} else {
  1061			/* stop stream */
  1062			ret = imx274_write_table(imx274, imx274_stop);
  1063			if (ret)
  1064				goto fail;
  1065		}
  1066	
  1067		mutex_unlock(&imx274->lock);
  1068		dev_dbg(&imx274->client->dev, "%s : Done\n", __func__);
  1069		return 0;
  1070	
  1071	fail:
  1072		mutex_unlock(&imx274->lock);
  1073		dev_err(&imx274->client->dev, "s_stream failed\n");
  1074		return ret;
  1075	}
  1076	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
  
kernel test robot April 18, 2018, 8:38 p.m. UTC | #2
Hi Luca,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.17-rc1 next-20180418]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Luca-Ceresoli/imx274-add-cropping-and-misc-improvements/20180413-234459
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-h1-04190052 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/dynamic_debug.h:6:0,
                    from include/linux/printk.h:329,
                    from include/linux/kernel.h:14,
                    from include/linux/clk.h:16,
                    from drivers/media//i2c/imx274.c:22:
   drivers/media//i2c/imx274.c: In function 'imx274_s_stream':
>> include/linux/jump_label.h:407:59: warning: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'int' [-Wformat=]
     else if (__builtin_types_compatible_p(typeof(*x), struct static_key_false)) \
                                                              ^
   include/linux/dynamic_debug.h:103:2: note: in expansion of macro 'static_branch_unlikely'
     static_branch_unlikely(&descriptor.key.dd_key_false)
     ^
   include/linux/dynamic_debug.h:134:6: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     if (DYNAMIC_DEBUG_BRANCH(descriptor))   \
         ^
   include/linux/device.h:1364:2: note: in expansion of macro 'dynamic_dev_dbg'
     dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
     ^
   drivers/media//i2c/imx274.c:1027:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(&imx274->client->dev, "%s : %s, mode index %lu\n", __func__,
     ^
--
   In file included from include/linux/dynamic_debug.h:6:0,
                    from include/linux/printk.h:329,
                    from include/linux/kernel.h:14,
                    from include/linux/clk.h:16,
                    from drivers/media/i2c/imx274.c:22:
   drivers/media/i2c/imx274.c: In function 'imx274_s_stream':
>> include/linux/jump_label.h:407:59: warning: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'int' [-Wformat=]
     else if (__builtin_types_compatible_p(typeof(*x), struct static_key_false)) \
                                                              ^
   include/linux/dynamic_debug.h:103:2: note: in expansion of macro 'static_branch_unlikely'
     static_branch_unlikely(&descriptor.key.dd_key_false)
     ^
   include/linux/dynamic_debug.h:134:6: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     if (DYNAMIC_DEBUG_BRANCH(descriptor))   \
         ^
   include/linux/device.h:1364:2: note: in expansion of macro 'dynamic_dev_dbg'
     dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
     ^
   drivers/media/i2c/imx274.c:1027:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(&imx274->client->dev, "%s : %s, mode index %lu\n", __func__,
     ^

vim +407 include/linux/jump_label.h

11276d53 Peter Zijlstra 2015-07-24  401  
11276d53 Peter Zijlstra 2015-07-24  402  #define static_branch_unlikely(x)						\
11276d53 Peter Zijlstra 2015-07-24  403  ({										\
11276d53 Peter Zijlstra 2015-07-24  404  	bool branch;								\
11276d53 Peter Zijlstra 2015-07-24  405  	if (__builtin_types_compatible_p(typeof(*x), struct static_key_true))	\
11276d53 Peter Zijlstra 2015-07-24  406  		branch = arch_static_branch_jump(&(x)->key, false);		\
11276d53 Peter Zijlstra 2015-07-24 @407  	else if (__builtin_types_compatible_p(typeof(*x), struct static_key_false)) \
11276d53 Peter Zijlstra 2015-07-24  408  		branch = arch_static_branch(&(x)->key, false);			\
11276d53 Peter Zijlstra 2015-07-24  409  	else									\
11276d53 Peter Zijlstra 2015-07-24  410  		branch = ____wrong_branch_error();				\
81dcf89f Peter Zijlstra 2018-01-18  411  	unlikely(branch);							\
11276d53 Peter Zijlstra 2015-07-24  412  })
11276d53 Peter Zijlstra 2015-07-24  413  

:::::: The code at line 407 was first introduced by commit
:::::: 11276d5306b8e5b438a36bbff855fe792d7eaa61 locking/static_keys: Add a new static_key interface

:::::: TO: Peter Zijlstra <peterz@infradead.org>
:::::: CC: Ingo Molnar <mingo@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
  

Patch

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index 2ec31ae4e60d..25907d0817a4 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -553,8 +553,6 @@  struct imx274_ctrls {
  * @reset_gpio: Pointer to reset gpio
  * @lock: Mutex structure
  * @mode: Parameters for the selected readout mode
- *        (points to imx274_formats[mode_index])
- * @mode_index: Resolution mode index
  */
 struct stimx274 {
 	struct v4l2_subdev sd;
@@ -567,7 +565,6 @@  struct stimx274 {
 	struct gpio_desc *reset_gpio;
 	struct mutex lock; /* mutex lock for operations */
 	const struct imx274_frmfmt *mode;
-	u32 mode_index;
 };
 
 /*
@@ -880,7 +877,6 @@  static int imx274_set_fmt(struct v4l2_subdev *sd,
 		index = 0;
 	}
 
-	imx274->mode_index = index;
 	imx274->mode = &imx274_formats[index];
 
 	if (fmt->width > IMX274_MAX_WIDTH)
@@ -1028,8 +1024,9 @@  static int imx274_s_stream(struct v4l2_subdev *sd, int on)
 	struct stimx274 *imx274 = to_imx274(sd);
 	int ret = 0;
 
-	dev_dbg(&imx274->client->dev, "%s : %s, mode index = %d\n", __func__,
-		on ? "Stream Start" : "Stream Stop", imx274->mode_index);
+	dev_dbg(&imx274->client->dev, "%s : %s, mode index %lu\n", __func__,
+		on ? "Stream Start" : "Stream Stop",
+		imx274->mode - &imx274_formats[0]);
 
 	mutex_lock(&imx274->lock);
 
@@ -1068,8 +1065,7 @@  static int imx274_s_stream(struct v4l2_subdev *sd, int on)
 	}
 
 	mutex_unlock(&imx274->lock);
-	dev_dbg(&imx274->client->dev,
-		"%s : Done: mode = %d\n", __func__, imx274->mode_index);
+	dev_dbg(&imx274->client->dev, "%s : Done\n", __func__);
 	return 0;
 
 fail:
@@ -1625,8 +1621,7 @@  static int imx274_probe(struct i2c_client *client,
 	mutex_init(&imx274->lock);
 
 	/* initialize format */
-	imx274->mode_index = IMX274_MODE_3840X2160;
-	imx274->mode = &imx274_formats[imx274->mode_index];
+	imx274->mode = &imx274_formats[IMX274_MODE_3840X2160];
 	imx274->format.width = imx274->mode->size.width;
 	imx274->format.height = imx274->mode->size.height;
 	imx274->format.field = V4L2_FIELD_NONE;