Re: [ragnatech:media-tree 273/382] drivers/media/i2c/mt9v111.c:801:10: error: implicit declaration of function 'v4l2_subdev_get_try_format'; did you mean 'v4l2_subdev_notify_event'?

[jacopo mondi <jacopo@jmondi.org>]

[-- Attachment #1: Type: text/plain, Size: 4975 bytes --]

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?)


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]