| Message ID | 20220215230737.1870630-10-djrscally@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support OVTI7251 on Microsoft Surface line | expand |
Hi Daniel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.17-rc4 next-20220215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220216-071110
base: git://linuxtv.org/media_tree.git master
config: riscv-randconfig-r003-20220216 (https://download.01.org/0day-ci/archive/20220216/202202161122.Qh05mcn0-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0e628a783b935c70c80815db6c061ec84f884af5)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/5c7425707438fee74daeb7faf41774d88a04b561
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Scally/Support-OVTI7251-on-Microsoft-Surface-line/20220216-071110
git checkout 5c7425707438fee74daeb7faf41774d88a04b561
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/media/i2c/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/media/i2c/ov7251.c:190:31: warning: unused variable 'ov7251_global_init_setting' [-Wunused-const-variable]
static const struct reg_value ov7251_global_init_setting[] = {
^
1 warning generated.
vim +/ov7251_global_init_setting +190 drivers/media/i2c/ov7251.c
9276bc868a46fc Daniel Scally 2022-02-15 189
d30bb512da3d8e Todor Tomov 2018-04-25 @190 static const struct reg_value ov7251_global_init_setting[] = {
d30bb512da3d8e Todor Tomov 2018-04-25 191 { 0x0103, 0x01 },
d30bb512da3d8e Todor Tomov 2018-04-25 192 { 0x303b, 0x02 },
d30bb512da3d8e Todor Tomov 2018-04-25 193 };
d30bb512da3d8e Todor Tomov 2018-04-25 194
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Daniel, On Tue, Feb 15, 2022 at 11:07:36PM +0000, Daniel Scally wrote: > The .s_power() callback is deprecated, and now that we have pm_runtime > functionality in the driver there's no further use for it. Delete the > function. > > Signed-off-by: Daniel Scally <djrscally@gmail.com> > --- > drivers/media/i2c/ov7251.c | 44 +------------------------------------- > 1 file changed, 1 insertion(+), 43 deletions(-) > > diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c > index d620ed6a4e42..5e7422ca4ab9 100644 > --- a/drivers/media/i2c/ov7251.c > +++ b/drivers/media/i2c/ov7251.c > @@ -903,43 +903,6 @@ static int __maybe_unused ov7251_sensor_resume(struct device *dev) > return ov7251_set_power_on(ov7251); > } > > -static int ov7251_s_power(struct v4l2_subdev *sd, int on) > -{ > - struct ov7251 *ov7251 = to_ov7251(sd); > - int ret = 0; > - > - mutex_lock(&ov7251->lock); > - > - /* If the power state is not modified - no work to do. */ > - if (ov7251->power_on == !!on) > - goto exit; > - > - if (on) { > - ret = ov7251_set_power_on(ov7251); > - if (ret < 0) > - goto exit; > - > - ret = ov7251_set_register_array(ov7251, > - ov7251_global_init_setting, > - ARRAY_SIZE(ov7251_global_init_setting)); Could this be written as part of the power-on sequence after identifying the sensor? Likewise in probe() if it's possible the device won't be powered down before it gets used --- I guess nothing rules that out? > - if (ret < 0) { > - dev_err(ov7251->dev, "could not set init registers\n"); > - ov7251_set_power_off(ov7251); > - goto exit; > - } > - > - ov7251->power_on = true; > - } else { > - ov7251_set_power_off(ov7251); > - ov7251->power_on = false; > - } > - > -exit: > - mutex_unlock(&ov7251->lock); > - > - return ret; > -} > - > static int ov7251_set_hflip(struct ov7251 *ov7251, s32 value) > { > u8 val = ov7251->timing_format2; > @@ -1384,10 +1347,6 @@ static int ov7251_set_frame_interval(struct v4l2_subdev *subdev, > return ret; > } > > -static const struct v4l2_subdev_core_ops ov7251_core_ops = { > - .s_power = ov7251_s_power, > -}; > - > static const struct v4l2_subdev_video_ops ov7251_video_ops = { > .s_stream = ov7251_s_stream, > .g_frame_interval = ov7251_get_frame_interval, > @@ -1405,7 +1364,6 @@ static const struct v4l2_subdev_pad_ops ov7251_subdev_pad_ops = { > }; > > static const struct v4l2_subdev_ops ov7251_subdev_ops = { > - .core = &ov7251_core_ops, > .video = &ov7251_video_ops, > .pad = &ov7251_subdev_pad_ops, > }; > @@ -1690,7 +1648,7 @@ static int ov7251_probe(struct i2c_client *client) > pm_runtime_disable(ov7251->dev); > pm_runtime_put_noidle(ov7251->dev); > power_down: > - ov7251_s_power(&ov7251->sd, false); > + ov7251_set_power_off(ov7251); > free_entity: > media_entity_cleanup(&ov7251->sd.entity); > free_ctrl:
diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c index d620ed6a4e42..5e7422ca4ab9 100644 --- a/drivers/media/i2c/ov7251.c +++ b/drivers/media/i2c/ov7251.c @@ -903,43 +903,6 @@ static int __maybe_unused ov7251_sensor_resume(struct device *dev) return ov7251_set_power_on(ov7251); } -static int ov7251_s_power(struct v4l2_subdev *sd, int on) -{ - struct ov7251 *ov7251 = to_ov7251(sd); - int ret = 0; - - mutex_lock(&ov7251->lock); - - /* If the power state is not modified - no work to do. */ - if (ov7251->power_on == !!on) - goto exit; - - if (on) { - ret = ov7251_set_power_on(ov7251); - if (ret < 0) - goto exit; - - ret = ov7251_set_register_array(ov7251, - ov7251_global_init_setting, - ARRAY_SIZE(ov7251_global_init_setting)); - if (ret < 0) { - dev_err(ov7251->dev, "could not set init registers\n"); - ov7251_set_power_off(ov7251); - goto exit; - } - - ov7251->power_on = true; - } else { - ov7251_set_power_off(ov7251); - ov7251->power_on = false; - } - -exit: - mutex_unlock(&ov7251->lock); - - return ret; -} - static int ov7251_set_hflip(struct ov7251 *ov7251, s32 value) { u8 val = ov7251->timing_format2; @@ -1384,10 +1347,6 @@ static int ov7251_set_frame_interval(struct v4l2_subdev *subdev, return ret; } -static const struct v4l2_subdev_core_ops ov7251_core_ops = { - .s_power = ov7251_s_power, -}; - static const struct v4l2_subdev_video_ops ov7251_video_ops = { .s_stream = ov7251_s_stream, .g_frame_interval = ov7251_get_frame_interval, @@ -1405,7 +1364,6 @@ static const struct v4l2_subdev_pad_ops ov7251_subdev_pad_ops = { }; static const struct v4l2_subdev_ops ov7251_subdev_ops = { - .core = &ov7251_core_ops, .video = &ov7251_video_ops, .pad = &ov7251_subdev_pad_ops, }; @@ -1690,7 +1648,7 @@ static int ov7251_probe(struct i2c_client *client) pm_runtime_disable(ov7251->dev); pm_runtime_put_noidle(ov7251->dev); power_down: - ov7251_s_power(&ov7251->sd, false); + ov7251_set_power_off(ov7251); free_entity: media_entity_cleanup(&ov7251->sd.entity); free_ctrl:
The .s_power() callback is deprecated, and now that we have pm_runtime functionality in the driver there's no further use for it. Delete the function. Signed-off-by: Daniel Scally <djrscally@gmail.com> --- drivers/media/i2c/ov7251.c | 44 +------------------------------------- 1 file changed, 1 insertion(+), 43 deletions(-)