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: 6440 bytes --]

Hi Mauro,

On Sun, Aug 05, 2018 at 10:55:43AM -0300, Mauro Carvalho Chehab wrote:
> Em Sun, 5 Aug 2018 12:09:23 +0200
> jacopo mondi <jacopo@jmondi.org> escreveu:
>
> > 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;
>
> Yeah, this is a way better!
>

I had that patch ready before Hans suggested to go with the other
solution, I'll send it anyhow and let you two decide which one to pick
:)

> Btw, if this is always the case, perhaps we could, instead, add a
> stub for v4l2_subdev_get_try_format() that would return &cfg->try_fmt.
>

Or if you want to wait until next week I can take care of this.

Thanks
   j


> A patch for tvp5150 had the same issue (and it is also used outside
> subdev-based devices).
>
> Perhaps it is time to have stubs for things like that and get
> rid on those ugly ifs in the middle of the drivers.
>
> >
> > 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.
>
> Ok. Anyway, my plan for next week is to try to minimize the number of
> warnings... I'm getting a lot were nowadays with newer gcc versions.
> >
> > >
> > > 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
> > > >
> > > >
> > >
>
>
>
> Thanks,
> Mauro

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