On 08/05/2018 12:09 PM, jacopo mondi wrote: > Hi Hans, > > On Sun, Aug 05, 2018 at 11:59:33AM +0200, Hans Verkuil wrote: >> On 08/05/2018 11:36 AM, jacopo mondi wrote: >>> On Sun, Aug 05, 2018 at 01:14:58AM +0800, kbuild test robot wrote: >>>> tree: git://git.ragnatech.se/linux media-tree >>>> head: 12f336c88090fb8004736fd4329184326a49673b >>>> commit: aab7ed1c392703604fbdc5bd5005dfb61a0b32f9 [273/382] media: i2c: Add driver for Aptina MT9V111 >>>> config: x86_64-randconfig-x010-201831 (attached as .config) >>>> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 >>>> reproduce: >>>> git checkout aab7ed1c392703604fbdc5bd5005dfb61a0b32f9 >>>> # save the attached .config to linux build tree >>>> make ARCH=x86_64 >>>> >>>> All error/warnings (new ones prefixed by >>): >>>> >>>> drivers/media/i2c/mt9v111.c: In function '__mt9v111_get_pad_format': >>>>>> drivers/media/i2c/mt9v111.c:801:10: error: implicit declaration of function 'v4l2_subdev_get_try_format'; did you mean 'v4l2_subdev_notify_event'? [-Werror=implicit-function-declaration] >>>> return v4l2_subdev_get_try_format(&mt9v111->sd, cfg, pad); >>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> I have received this notification a few times now, and it comes from >>> the test build being run a kernel configured without the >>> CONFIG_VIDEO_V4L2_SUBDEV_API symbol. >>> >>> The mt9v111 driver does not list CONFIG_VIDEO_V4L2_SUBDEV_API as a >>> Kconfig dependency and the option does not get selected by the config >>> generated by kbuild, I guess. >>> >>> Should I list CONFIG_VIDEO_V4L2_SUBDEV_API as an mt9v111 dependency >>> with an incremental patch? >> >> Yes please. While you're at it, I'm also getting this warning during the daily build: >> > > On a second thought, the issue here is thatv4l2_subdev_get_try_format() is > protected by: > #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER) > > but that function is only defined if CONFIG_VIDEO_V4L2_SUBDEV_API is > enabled (see > https://elixir.bootlin.com/linux/latest/source/include/media/v4l2-subdev.h#L915) > > As the mt9v111 can work without VIDEO_V4L2_SUBDEV_API selected, I > would change the following bit, instead of listing V4L2_SUBDEV as a > Kconfig dependency: > > @@ -797,7 +797,7 @@ static struct v4l2_mbus_framefmt *__mt9v111_get_pad_format( > { > switch (which) { > case V4L2_SUBDEV_FORMAT_TRY: > -#if IS_ENABLED(CONFIG_MEDIA_CONTROLLER) > +#if IS_ENABLED(CONFIG_VIDEO_V4L2_SUBDEV_API) > return v4l2_subdev_get_try_format(&mt9v111->sd, cfg, pad); > #else > return &cfg->try_fmt; > > With you ack I'll send a patch, sorry but this will probably require another > pull request (or Mauro could collect it directly?) Just let this driver depend on VIDEO_V4L2_SUBDEV_API: most of the i2c drivers depend on it already. I think that the CONFIG_MEDIA_CONTROLLER and CONFIG_VIDEO_V4L2_SUBDEV_API will disappear in the not too distant future and are always enabled. Certainly CONFIG_VIDEO_V4L2_SUBDEV_API doesn't serve much of a purpose anymore IMHO. Regards, Hans > > >> linux-git-x86_64: WARNINGS >> >> /home/hans/work/build/media-git/drivers/media/i2c/mt9v111.c: In function 'mt9v111_set_format': >> /home/hans/work/build/media-git/drivers/media/i2c/mt9v111.c:887:15: warning: 'idx' may be used uninitialized in this function >> [-Wmaybe-uninitialized] >> unsigned int idx; >> ^~~ >> >> There may be a patch for that already (I haven't checked), but if not, can you fix >> this too? > > This has been fixed by a patch from Jasmin and pull request sent by > Sakari. > >> >> I actually wondered if you shouldn't use the v4l2_find_nearest_size() helper for this >> (v4l2-common.h). >> > > Possibly. I won't be able to look into that now and I'll be away > next week, so it might slip to the next cycle though. > > Thanks > j > >> Thanks, >> >> Hans >> >>> >>>> v4l2_subdev_notify_event >>>>>> drivers/media/i2c/mt9v111.c:801:10: warning: return makes pointer from integer without a cast [-Wint-conversion] >>>> return v4l2_subdev_get_try_format(&mt9v111->sd, cfg, pad); >>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> drivers/media/i2c/mt9v111.c: In function 'mt9v111_set_format': >>>> drivers/media/i2c/mt9v111.c:887:15: warning: 'idx' may be used uninitialized in this function [-Wmaybe-uninitialized] >>>> unsigned int idx; >>>> ^~~ >>>> cc1: some warnings being treated as errors >>>> >>>> vim +801 drivers/media/i2c/mt9v111.c >>>> >>>> 791 >>>> 792 static struct v4l2_mbus_framefmt *__mt9v111_get_pad_format( >>>> 793 struct mt9v111_dev *mt9v111, >>>> 794 struct v4l2_subdev_pad_config *cfg, >>>> 795 unsigned int pad, >>>> 796 enum v4l2_subdev_format_whence which) >>>> 797 { >>>> 798 switch (which) { >>>> 799 case V4L2_SUBDEV_FORMAT_TRY: >>>> 800 #if IS_ENABLED(CONFIG_MEDIA_CONTROLLER) >>>> > 801 return v4l2_subdev_get_try_format(&mt9v111->sd, cfg, pad); >>>> 802 #else >>>> 803 return &cfg->try_fmt; >>>> 804 #endif >>>> 805 case V4L2_SUBDEV_FORMAT_ACTIVE: >>>> 806 return &mt9v111->fmt; >>>> 807 default: >>>> 808 return NULL; >>>> 809 } >>>> 810 } >>>> 811 >>>> >>>> --- >>>> 0-DAY kernel test infrastructure Open Source Technology Center >>>> https://lists.01.org/pipermail/kbuild-all Intel Corporation >>> >>> >>
