All of lore.kernel.org
 help / color / mirror / Atom feed
* [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'?
@ 2018-08-04 17:14 kbuild test robot
  2018-08-05  9:36 ` jacopo mondi
  0 siblings, 1 reply; 7+ messages in thread
From: kbuild test robot @ 2018-08-04 17:14 UTC (permalink / raw)
  To: Jacopo Mondi
  Cc: kbuild-all, Mauro Carvalho Chehab, linux-media, Kieran Bingham,
	Sakari Ailus

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

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);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~
             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: .config.gz --]
[-- Type: application/gzip, Size: 30438 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* 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'?
  2018-08-04 17:14 [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'? kbuild test robot
@ 2018-08-05  9:36 ` jacopo mondi
  2018-08-05  9:59   ` Hans Verkuil
  0 siblings, 1 reply; 7+ messages in thread
From: jacopo mondi @ 2018-08-05  9:36 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Jacopo Mondi, kbuild-all, Mauro Carvalho Chehab, linux-media,
	Kieran Bingham, Sakari Ailus

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

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?

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* 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'?
  2018-08-05  9:36 ` jacopo mondi
@ 2018-08-05  9:59   ` Hans Verkuil
  2018-08-05 10:09     ` jacopo mondi
  0 siblings, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2018-08-05  9:59 UTC (permalink / raw)
  To: jacopo mondi, kbuild test robot
  Cc: Jacopo Mondi, kbuild-all, Mauro Carvalho Chehab, linux-media,
	Kieran Bingham, Sakari Ailus

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:

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?

I actually wondered if you shouldn't use the v4l2_find_nearest_size() helper for this
(v4l2-common.h).

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* 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'?
  2018-08-05  9:59   ` Hans Verkuil
@ 2018-08-05 10:09     ` jacopo mondi
  2018-08-05 10:21       ` Hans Verkuil
  2018-08-05 13:55       ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 7+ messages in thread
From: jacopo mondi @ 2018-08-05 10:09 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: kbuild test robot, Jacopo Mondi, kbuild-all,
	Mauro Carvalho Chehab, linux-media, Kieran Bingham, Sakari Ailus

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* 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'?
  2018-08-05 10:09     ` jacopo mondi
@ 2018-08-05 10:21       ` Hans Verkuil
  2018-08-05 13:55       ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2018-08-05 10:21 UTC (permalink / raw)
  To: jacopo mondi
  Cc: kbuild test robot, Jacopo Mondi, kbuild-all,
	Mauro Carvalho Chehab, linux-media, Kieran Bingham, Sakari Ailus

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* 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'?
  2018-08-05 10:09     ` jacopo mondi
  2018-08-05 10:21       ` Hans Verkuil
@ 2018-08-05 13:55       ` Mauro Carvalho Chehab
  2018-08-05 17:15         ` jacopo mondi
  1 sibling, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2018-08-05 13:55 UTC (permalink / raw)
  To: jacopo mondi
  Cc: Hans Verkuil, kbuild test robot, Jacopo Mondi, kbuild-all,
	linux-media, Kieran Bingham, Sakari Ailus

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!

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.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* 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'?
  2018-08-05 13:55       ` Mauro Carvalho Chehab
@ 2018-08-05 17:15         ` jacopo mondi
  0 siblings, 0 replies; 7+ messages in thread
From: jacopo mondi @ 2018-08-05 17:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Hans Verkuil, kbuild test robot, Jacopo Mondi, linux-media,
	Kieran Bingham, Sakari Ailus

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-08-05 19:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-04 17:14 [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'? kbuild test robot
2018-08-05  9:36 ` jacopo mondi
2018-08-05  9:59   ` Hans Verkuil
2018-08-05 10:09     ` jacopo mondi
2018-08-05 10:21       ` Hans Verkuil
2018-08-05 13:55       ` Mauro Carvalho Chehab
2018-08-05 17:15         ` jacopo mondi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.