OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[Hans de Goede]

Hi All,

The ov5693 driver uses v4l2_subdev_get_try_crop() /
v4l2_subdev_get_try_format() both of which are
only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .

Yet it does not do select VIDEO_V4L2_SUBDEV_API
in its Kconfig bits ?

Note I've not seen any build errors because of this,
I guess we somehow end up getting away with this...

But still I think the select should be added ?

Regards,

Hans

Re: OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[Hans de Goede]

Hi,

On 6/14/23 17:47, Hans de Goede wrote:
> Hi All,
> 
> The ov5693 driver uses v4l2_subdev_get_try_crop() /
> v4l2_subdev_get_try_format() both of which are
> only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .
> 
> Yet it does not do select VIDEO_V4L2_SUBDEV_API
> in its Kconfig bits ?
> 
> Note I've not seen any build errors because of this,
> I guess we somehow end up getting away with this...
> 
> But still I think the select should be added ?

Hmm, so while looking at the VIDEO_IMX290 entry
in drivers/media/i2c/Kconfig I notice that
the VIDEO_IMX274 entry directly above it seems
to be missing "select V4L2_FWNODE".

I am starting to think that we need a
V4L2_SENSOR_DRIVER_DEPS which can be selected
which then in turn selects all the bits modern
sensor drivers need ?

Regards,

Hans

Re: OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[sakari.ailus]

Hi Hans,

On Wed, Jun 14, 2023 at 05:47:01PM +0200, Hans de Goede wrote:
> Hi All,
> 
> The ov5693 driver uses v4l2_subdev_get_try_crop() /
> v4l2_subdev_get_try_format() both of which are
> only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .
> 
> Yet it does not do select VIDEO_V4L2_SUBDEV_API
> in its Kconfig bits ?
> 
> Note I've not seen any build errors because of this,
> I guess we somehow end up getting away with this...
> 
> But still I think the select should be added ?

I agree.

The reason there haven't been compile failures is that there's a vast
number of sensor drivers that all select this so for a failure you'd need
to select this one but none of the others.

I can send a fix.

-- 
Regards,

Sakari Ailus

Re: OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[Hans de Goede]

Hi Sakari,

On 6/14/23 18:05, sakari.ailus@linux.intel.com wrote:
> Hi Hans,
> 
> On Wed, Jun 14, 2023 at 05:47:01PM +0200, Hans de Goede wrote:
>> Hi All,
>>
>> The ov5693 driver uses v4l2_subdev_get_try_crop() /
>> v4l2_subdev_get_try_format() both of which are
>> only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .
>>
>> Yet it does not do select VIDEO_V4L2_SUBDEV_API
>> in its Kconfig bits ?
>>
>> Note I've not seen any build errors because of this,
>> I guess we somehow end up getting away with this...
>>
>> But still I think the select should be added ?
> 
> I agree.
> 
> The reason there haven't been compile failures is that there's a vast
> number of sensor drivers that all select this so for a failure you'd need
> to select this one but none of the others.
> 
> I can send a fix.

Also see my follow-up email. If we're going to fix this
we really should fix it properly. As mentioned in
my folow-up email an intermediate Kconfig option
might be best.

E.g. doing:

grep -l v4l2_async_register_subdev drivers/media/i2c/*.c

And comparing that to Kconfig finds the following Kconfig
entries lacking a select V4L2_FWNODE / select V4l2_ASYNC

VIDEO_IMX208
VIDEO_IMX258
VIDEO_IMX274
VIDEO_IMX319
VIDEO_IMX355
VIDEO_OV6650
VIDEO_OV7740
VIDEO_OV9640
VIDEO_OV9650

and I stopped checking after the ov* drivers since I think
the above list makes my point.

Regards,

Hans

Re: OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[sakari.ailus]

Hi Hans,

On Wed, Jun 14, 2023 at 06:50:14PM +0200, Hans de Goede wrote:
> Hi Sakari,
> 
> On 6/14/23 18:05, sakari.ailus@linux.intel.com wrote:
> > Hi Hans,
> > 
> > On Wed, Jun 14, 2023 at 05:47:01PM +0200, Hans de Goede wrote:
> >> Hi All,
> >>
> >> The ov5693 driver uses v4l2_subdev_get_try_crop() /
> >> v4l2_subdev_get_try_format() both of which are
> >> only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .
> >>
> >> Yet it does not do select VIDEO_V4L2_SUBDEV_API
> >> in its Kconfig bits ?
> >>
> >> Note I've not seen any build errors because of this,
> >> I guess we somehow end up getting away with this...
> >>
> >> But still I think the select should be added ?
> > 
> > I agree.
> > 
> > The reason there haven't been compile failures is that there's a vast
> > number of sensor drivers that all select this so for a failure you'd need
> > to select this one but none of the others.
> > 
> > I can send a fix.
> 
> Also see my follow-up email. If we're going to fix this
> we really should fix it properly. As mentioned in
> my folow-up email an intermediate Kconfig option
> might be best.
> 
> E.g. doing:
> 
> grep -l v4l2_async_register_subdev drivers/media/i2c/*.c
> 
> And comparing that to Kconfig finds the following Kconfig
> entries lacking a select V4L2_FWNODE / select V4l2_ASYNC
> 
> VIDEO_IMX208
> VIDEO_IMX258
> VIDEO_IMX274
> VIDEO_IMX319
> VIDEO_IMX355
> VIDEO_OV6650
> VIDEO_OV7740
> VIDEO_OV9640
> VIDEO_OV9650
> 
> and I stopped checking after the ov* drivers since I think
> the above list makes my point.

Yeah, sometimes difficult to find errors get repeated. Luckily it's "just"
a compilation problem.

Using V4L2 fwnode and V4L2 sub-device APIs are still unrelated as such
although in practice they do often happen together. There are still quite a
few sensor drivers that don't need both of them. Some can be compiled with
VIDEO_V4L2_SUBDEV_API disabled, too, but I'm not sure how useful that
really is. The rest are probably not usable outside a very specific scope,
such as I²C async matching used by a handful of receiver drivers (none use
MC, thus no sub-device API either).

Perhaps we could group these in two classes where the common class has
V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API selected? I'm not sure having an
intermediate, somewhat obscure, option would be helpful.

Also cc Hans and Laurent.

-- 
Regards,

Sakari Ailus

Re: OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[Laurent Pinchart]

On Wed, Jun 14, 2023 at 08:30:16PM +0000, sakari.ailus@linux.intel.com wrote:
> On Wed, Jun 14, 2023 at 06:50:14PM +0200, Hans de Goede wrote:
> > On 6/14/23 18:05, sakari.ailus@linux.intel.com wrote:
> > > On Wed, Jun 14, 2023 at 05:47:01PM +0200, Hans de Goede wrote:
> > >> Hi All,
> > >>
> > >> The ov5693 driver uses v4l2_subdev_get_try_crop() /
> > >> v4l2_subdev_get_try_format() both of which are
> > >> only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .
> > >>
> > >> Yet it does not do select VIDEO_V4L2_SUBDEV_API
> > >> in its Kconfig bits ?
> > >>
> > >> Note I've not seen any build errors because of this,
> > >> I guess we somehow end up getting away with this...
> > >>
> > >> But still I think the select should be added ?
> > > 
> > > I agree.
> > > 
> > > The reason there haven't been compile failures is that there's a vast
> > > number of sensor drivers that all select this so for a failure you'd need
> > > to select this one but none of the others.
> > > 
> > > I can send a fix.
> > 
> > Also see my follow-up email. If we're going to fix this
> > we really should fix it properly. As mentioned in
> > my folow-up email an intermediate Kconfig option
> > might be best.
> > 
> > E.g. doing:
> > 
> > grep -l v4l2_async_register_subdev drivers/media/i2c/*.c
> > 
> > And comparing that to Kconfig finds the following Kconfig
> > entries lacking a select V4L2_FWNODE / select V4l2_ASYNC
> > 
> > VIDEO_IMX208
> > VIDEO_IMX258
> > VIDEO_IMX274
> > VIDEO_IMX319
> > VIDEO_IMX355
> > VIDEO_OV6650
> > VIDEO_OV7740
> > VIDEO_OV9640
> > VIDEO_OV9650
> > 
> > and I stopped checking after the ov* drivers since I think
> > the above list makes my point.
> 
> Yeah, sometimes difficult to find errors get repeated. Luckily it's "just"
> a compilation problem.
> 
> Using V4L2 fwnode and V4L2 sub-device APIs are still unrelated as such
> although in practice they do often happen together. There are still quite a
> few sensor drivers that don't need both of them. Some can be compiled with
> VIDEO_V4L2_SUBDEV_API disabled, too, but I'm not sure how useful that
> really is. The rest are probably not usable outside a very specific scope,
> such as I²C async matching used by a handful of receiver drivers (none use
> MC, thus no sub-device API either).
> 
> Perhaps we could group these in two classes where the common class has
> V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API selected? I'm not sure having an
> intermediate, somewhat obscure, option would be helpful.
> 
> Also cc Hans and Laurent.

I'm all for simplifying the current state and removing the need to get
every Kconfig entry right by moving the dependencies to a common
location.

-- 
Regards,

Laurent Pinchart

Re: OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[Laurent Pinchart]

On Thu, Jun 15, 2023 at 12:22:58AM +0300, Laurent Pinchart wrote:
> On Wed, Jun 14, 2023 at 08:30:16PM +0000, sakari.ailus@linux.intel.com wrote:
> > On Wed, Jun 14, 2023 at 06:50:14PM +0200, Hans de Goede wrote:
> > > On 6/14/23 18:05, sakari.ailus@linux.intel.com wrote:
> > > > On Wed, Jun 14, 2023 at 05:47:01PM +0200, Hans de Goede wrote:
> > > >> Hi All,
> > > >>
> > > >> The ov5693 driver uses v4l2_subdev_get_try_crop() /
> > > >> v4l2_subdev_get_try_format() both of which are
> > > >> only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .
> > > >>
> > > >> Yet it does not do select VIDEO_V4L2_SUBDEV_API
> > > >> in its Kconfig bits ?
> > > >>
> > > >> Note I've not seen any build errors because of this,
> > > >> I guess we somehow end up getting away with this...
> > > >>
> > > >> But still I think the select should be added ?
> > > > 
> > > > I agree.
> > > > 
> > > > The reason there haven't been compile failures is that there's a vast
> > > > number of sensor drivers that all select this so for a failure you'd need
> > > > to select this one but none of the others.
> > > > 
> > > > I can send a fix.
> > > 
> > > Also see my follow-up email. If we're going to fix this
> > > we really should fix it properly. As mentioned in
> > > my folow-up email an intermediate Kconfig option
> > > might be best.
> > > 
> > > E.g. doing:
> > > 
> > > grep -l v4l2_async_register_subdev drivers/media/i2c/*.c
> > > 
> > > And comparing that to Kconfig finds the following Kconfig
> > > entries lacking a select V4L2_FWNODE / select V4l2_ASYNC
> > > 
> > > VIDEO_IMX208
> > > VIDEO_IMX258
> > > VIDEO_IMX274
> > > VIDEO_IMX319
> > > VIDEO_IMX355
> > > VIDEO_OV6650
> > > VIDEO_OV7740
> > > VIDEO_OV9640
> > > VIDEO_OV9650
> > > 
> > > and I stopped checking after the ov* drivers since I think
> > > the above list makes my point.
> > 
> > Yeah, sometimes difficult to find errors get repeated. Luckily it's "just"
> > a compilation problem.
> > 
> > Using V4L2 fwnode and V4L2 sub-device APIs are still unrelated as such
> > although in practice they do often happen together. There are still quite a
> > few sensor drivers that don't need both of them. Some can be compiled with
> > VIDEO_V4L2_SUBDEV_API disabled, too, but I'm not sure how useful that
> > really is. The rest are probably not usable outside a very specific scope,
> > such as I²C async matching used by a handful of receiver drivers (none use
> > MC, thus no sub-device API either).
> > 
> > Perhaps we could group these in two classes where the common class has
> > V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API selected? I'm not sure having an
> > intermediate, somewhat obscure, option would be helpful.
> > 
> > Also cc Hans and Laurent.
> 
> I'm all for simplifying the current state and removing the need to get
> every Kconfig entry right by moving the dependencies to a common
> location.

Also, if sensor drivers are encouraged to use new APIs, but not all of
them have been converted, I'd be fine selecting the new APIs
unconditionally even if no selected sensor driver uses them.

-- 
Regards,

Laurent Pinchart

Re: OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[sakari.ailus]

Hi Laurent,

On Thu, Jun 15, 2023 at 12:23:45AM +0300, Laurent Pinchart wrote:
> On Thu, Jun 15, 2023 at 12:22:58AM +0300, Laurent Pinchart wrote:
> > On Wed, Jun 14, 2023 at 08:30:16PM +0000, sakari.ailus@linux.intel.com wrote:
> > > On Wed, Jun 14, 2023 at 06:50:14PM +0200, Hans de Goede wrote:
> > > > On 6/14/23 18:05, sakari.ailus@linux.intel.com wrote:
> > > > > On Wed, Jun 14, 2023 at 05:47:01PM +0200, Hans de Goede wrote:
> > > > >> Hi All,
> > > > >>
> > > > >> The ov5693 driver uses v4l2_subdev_get_try_crop() /
> > > > >> v4l2_subdev_get_try_format() both of which are
> > > > >> only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .
> > > > >>
> > > > >> Yet it does not do select VIDEO_V4L2_SUBDEV_API
> > > > >> in its Kconfig bits ?
> > > > >>
> > > > >> Note I've not seen any build errors because of this,
> > > > >> I guess we somehow end up getting away with this...
> > > > >>
> > > > >> But still I think the select should be added ?
> > > > > 
> > > > > I agree.
> > > > > 
> > > > > The reason there haven't been compile failures is that there's a vast
> > > > > number of sensor drivers that all select this so for a failure you'd need
> > > > > to select this one but none of the others.
> > > > > 
> > > > > I can send a fix.
> > > > 
> > > > Also see my follow-up email. If we're going to fix this
> > > > we really should fix it properly. As mentioned in
> > > > my folow-up email an intermediate Kconfig option
> > > > might be best.
> > > > 
> > > > E.g. doing:
> > > > 
> > > > grep -l v4l2_async_register_subdev drivers/media/i2c/*.c
> > > > 
> > > > And comparing that to Kconfig finds the following Kconfig
> > > > entries lacking a select V4L2_FWNODE / select V4l2_ASYNC
> > > > 
> > > > VIDEO_IMX208
> > > > VIDEO_IMX258
> > > > VIDEO_IMX274
> > > > VIDEO_IMX319
> > > > VIDEO_IMX355
> > > > VIDEO_OV6650
> > > > VIDEO_OV7740
> > > > VIDEO_OV9640
> > > > VIDEO_OV9650
> > > > 
> > > > and I stopped checking after the ov* drivers since I think
> > > > the above list makes my point.
> > > 
> > > Yeah, sometimes difficult to find errors get repeated. Luckily it's "just"
> > > a compilation problem.
> > > 
> > > Using V4L2 fwnode and V4L2 sub-device APIs are still unrelated as such
> > > although in practice they do often happen together. There are still quite a
> > > few sensor drivers that don't need both of them. Some can be compiled with
> > > VIDEO_V4L2_SUBDEV_API disabled, too, but I'm not sure how useful that
> > > really is. The rest are probably not usable outside a very specific scope,
> > > such as I²C async matching used by a handful of receiver drivers (none use
> > > MC, thus no sub-device API either).
> > > 
> > > Perhaps we could group these in two classes where the common class has
> > > V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API selected? I'm not sure having an
> > > intermediate, somewhat obscure, option would be helpful.
> > > 
> > > Also cc Hans and Laurent.
> > 
> > I'm all for simplifying the current state and removing the need to get
> > every Kconfig entry right by moving the dependencies to a common
> > location.
> 
> Also, if sensor drivers are encouraged to use new APIs, but not all of
> them have been converted, I'd be fine selecting the new APIs
> unconditionally even if no selected sensor driver uses them.

There are old drivers that do not need these features and there has been
discussion there may be a desire to disable features to make the kernel
smaller (for e.g. OpenWRT), however I suspect in this case it's more likely
drivers that are little used.

Similarly to sensor drivers needing V4L2_FWNODE, lens drivers will need
V4L2_ASYNC.

I can post patches, not quite sure yet whether this approach is also better
on stable kernels. Quite possibly so.

-- 
Kind regards,

Sakari Ailus

Re: OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[Laurent Pinchart]

Hi Sakari,

On Thu, Jun 15, 2023 at 08:22:51AM +0000, sakari.ailus@linux.intel.com wrote:
> On Thu, Jun 15, 2023 at 12:23:45AM +0300, Laurent Pinchart wrote:
> > On Thu, Jun 15, 2023 at 12:22:58AM +0300, Laurent Pinchart wrote:
> > > On Wed, Jun 14, 2023 at 08:30:16PM +0000, sakari.ailus@linux.intel.com wrote:
> > > > On Wed, Jun 14, 2023 at 06:50:14PM +0200, Hans de Goede wrote:
> > > > > On 6/14/23 18:05, sakari.ailus@linux.intel.com wrote:
> > > > > > On Wed, Jun 14, 2023 at 05:47:01PM +0200, Hans de Goede wrote:
> > > > > >> Hi All,
> > > > > >>
> > > > > >> The ov5693 driver uses v4l2_subdev_get_try_crop() /
> > > > > >> v4l2_subdev_get_try_format() both of which are
> > > > > >> only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .
> > > > > >>
> > > > > >> Yet it does not do select VIDEO_V4L2_SUBDEV_API
> > > > > >> in its Kconfig bits ?
> > > > > >>
> > > > > >> Note I've not seen any build errors because of this,
> > > > > >> I guess we somehow end up getting away with this...
> > > > > >>
> > > > > >> But still I think the select should be added ?
> > > > > > 
> > > > > > I agree.
> > > > > > 
> > > > > > The reason there haven't been compile failures is that there's a vast
> > > > > > number of sensor drivers that all select this so for a failure you'd need
> > > > > > to select this one but none of the others.
> > > > > > 
> > > > > > I can send a fix.
> > > > > 
> > > > > Also see my follow-up email. If we're going to fix this
> > > > > we really should fix it properly. As mentioned in
> > > > > my folow-up email an intermediate Kconfig option
> > > > > might be best.
> > > > > 
> > > > > E.g. doing:
> > > > > 
> > > > > grep -l v4l2_async_register_subdev drivers/media/i2c/*.c
> > > > > 
> > > > > And comparing that to Kconfig finds the following Kconfig
> > > > > entries lacking a select V4L2_FWNODE / select V4l2_ASYNC
> > > > > 
> > > > > VIDEO_IMX208
> > > > > VIDEO_IMX258
> > > > > VIDEO_IMX274
> > > > > VIDEO_IMX319
> > > > > VIDEO_IMX355
> > > > > VIDEO_OV6650
> > > > > VIDEO_OV7740
> > > > > VIDEO_OV9640
> > > > > VIDEO_OV9650
> > > > > 
> > > > > and I stopped checking after the ov* drivers since I think
> > > > > the above list makes my point.
> > > > 
> > > > Yeah, sometimes difficult to find errors get repeated. Luckily it's "just"
> > > > a compilation problem.
> > > > 
> > > > Using V4L2 fwnode and V4L2 sub-device APIs are still unrelated as such
> > > > although in practice they do often happen together. There are still quite a
> > > > few sensor drivers that don't need both of them. Some can be compiled with
> > > > VIDEO_V4L2_SUBDEV_API disabled, too, but I'm not sure how useful that
> > > > really is. The rest are probably not usable outside a very specific scope,
> > > > such as I²C async matching used by a handful of receiver drivers (none use
> > > > MC, thus no sub-device API either).
> > > > 
> > > > Perhaps we could group these in two classes where the common class has
> > > > V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API selected? I'm not sure having an
> > > > intermediate, somewhat obscure, option would be helpful.
> > > > 
> > > > Also cc Hans and Laurent.
> > > 
> > > I'm all for simplifying the current state and removing the need to get
> > > every Kconfig entry right by moving the dependencies to a common
> > > location.
> > 
> > Also, if sensor drivers are encouraged to use new APIs, but not all of
> > them have been converted, I'd be fine selecting the new APIs
> > unconditionally even if no selected sensor driver uses them.
> 
> There are old drivers that do not need these features and there has been
> discussion there may be a desire to disable features to make the kernel
> smaller (for e.g. OpenWRT), however I suspect in this case it's more likely
> drivers that are little used.

Just to be clear, I didn't mean selecting V4L2_FWNODE, V4L2_ASYNC and
VIDEO_V4L2_SUBDEV_API unconditionally, but when any sensor driver is
selected. If a device has a camera, memory requirements increase
significantly and I don't think enabling the above options will make a
real difference.

> Similarly to sensor drivers needing V4L2_FWNODE, lens drivers will need
> V4L2_ASYNC.
> 
> I can post patches, not quite sure yet whether this approach is also better
> on stable kernels. Quite possibly so.

-- 
Regards,

Laurent Pinchart

Re: OV5693 Kconfig missing a select VIDEO_V4L2_SUBDEV_API ?

[sakari.ailus]

Hi Laurent,

On Thu, Jun 15, 2023 at 12:13:53PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Thu, Jun 15, 2023 at 08:22:51AM +0000, sakari.ailus@linux.intel.com wrote:
> > On Thu, Jun 15, 2023 at 12:23:45AM +0300, Laurent Pinchart wrote:
> > > On Thu, Jun 15, 2023 at 12:22:58AM +0300, Laurent Pinchart wrote:
> > > > On Wed, Jun 14, 2023 at 08:30:16PM +0000, sakari.ailus@linux.intel.com wrote:
> > > > > On Wed, Jun 14, 2023 at 06:50:14PM +0200, Hans de Goede wrote:
> > > > > > On 6/14/23 18:05, sakari.ailus@linux.intel.com wrote:
> > > > > > > On Wed, Jun 14, 2023 at 05:47:01PM +0200, Hans de Goede wrote:
> > > > > > >> Hi All,
> > > > > > >>
> > > > > > >> The ov5693 driver uses v4l2_subdev_get_try_crop() /
> > > > > > >> v4l2_subdev_get_try_format() both of which are
> > > > > > >> only defined if CONFIG_VIDEO_V4L2_SUBDEV_API=y .
> > > > > > >>
> > > > > > >> Yet it does not do select VIDEO_V4L2_SUBDEV_API
> > > > > > >> in its Kconfig bits ?
> > > > > > >>
> > > > > > >> Note I've not seen any build errors because of this,
> > > > > > >> I guess we somehow end up getting away with this...
> > > > > > >>
> > > > > > >> But still I think the select should be added ?
> > > > > > > 
> > > > > > > I agree.
> > > > > > > 
> > > > > > > The reason there haven't been compile failures is that there's a vast
> > > > > > > number of sensor drivers that all select this so for a failure you'd need
> > > > > > > to select this one but none of the others.
> > > > > > > 
> > > > > > > I can send a fix.
> > > > > > 
> > > > > > Also see my follow-up email. If we're going to fix this
> > > > > > we really should fix it properly. As mentioned in
> > > > > > my folow-up email an intermediate Kconfig option
> > > > > > might be best.
> > > > > > 
> > > > > > E.g. doing:
> > > > > > 
> > > > > > grep -l v4l2_async_register_subdev drivers/media/i2c/*.c
> > > > > > 
> > > > > > And comparing that to Kconfig finds the following Kconfig
> > > > > > entries lacking a select V4L2_FWNODE / select V4l2_ASYNC
> > > > > > 
> > > > > > VIDEO_IMX208
> > > > > > VIDEO_IMX258
> > > > > > VIDEO_IMX274
> > > > > > VIDEO_IMX319
> > > > > > VIDEO_IMX355
> > > > > > VIDEO_OV6650
> > > > > > VIDEO_OV7740
> > > > > > VIDEO_OV9640
> > > > > > VIDEO_OV9650
> > > > > > 
> > > > > > and I stopped checking after the ov* drivers since I think
> > > > > > the above list makes my point.
> > > > > 
> > > > > Yeah, sometimes difficult to find errors get repeated. Luckily it's "just"
> > > > > a compilation problem.
> > > > > 
> > > > > Using V4L2 fwnode and V4L2 sub-device APIs are still unrelated as such
> > > > > although in practice they do often happen together. There are still quite a
> > > > > few sensor drivers that don't need both of them. Some can be compiled with
> > > > > VIDEO_V4L2_SUBDEV_API disabled, too, but I'm not sure how useful that
> > > > > really is. The rest are probably not usable outside a very specific scope,
> > > > > such as I²C async matching used by a handful of receiver drivers (none use
> > > > > MC, thus no sub-device API either).
> > > > > 
> > > > > Perhaps we could group these in two classes where the common class has
> > > > > V4L2_FWNODE and VIDEO_V4L2_SUBDEV_API selected? I'm not sure having an
> > > > > intermediate, somewhat obscure, option would be helpful.
> > > > > 
> > > > > Also cc Hans and Laurent.
> > > > 
> > > > I'm all for simplifying the current state and removing the need to get
> > > > every Kconfig entry right by moving the dependencies to a common
> > > > location.
> > > 
> > > Also, if sensor drivers are encouraged to use new APIs, but not all of
> > > them have been converted, I'd be fine selecting the new APIs
> > > unconditionally even if no selected sensor driver uses them.
> > 
> > There are old drivers that do not need these features and there has been
> > discussion there may be a desire to disable features to make the kernel
> > smaller (for e.g. OpenWRT), however I suspect in this case it's more likely
> > drivers that are little used.
> 
> Just to be clear, I didn't mean selecting V4L2_FWNODE, V4L2_ASYNC and
> VIDEO_V4L2_SUBDEV_API unconditionally, but when any sensor driver is
> selected. If a device has a camera, memory requirements increase
> significantly and I don't think enabling the above options will make a
> real difference.

How about something like this:

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 256d55bb2b1da..8a6961aafef98 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -25,8 +25,14 @@ config VIDEO_IR_I2C
 # V4L2 I2C drivers that are related with Camera support
 #
 
-menu "Camera sensor devices"
-	visible if MEDIA_CAMERA_SUPPORT
+menuconfig VIDEO_CAMERA_SENSOR
+	bool "Camera sensor devices"
+	depends on MEDIA_CAMERA_SUPPORT
+	select V4L2_FWNODE
+	select VIDEO_V4L2_SUBDEV_API
+	default y
+
+if VIDEO_CAMERA_SENSOR
 
 config VIDEO_APTINA_PLL
 	tristate
@@ -797,7 +803,7 @@ config VIDEO_ST_VGXY61
 source "drivers/media/i2c/ccs/Kconfig"
 source "drivers/media/i2c/et8ek8/Kconfig"
 
-endmenu
+endif # VIDEO_CAMERA_SENSOR
 
 menu "Lens drivers"
 	visible if MEDIA_CAMERA_SUPPORT

-- 
Regards,

Sakari Ailus