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