[libcamera-devel] [PATCH v5 4/6] media: v4l2-subdev: Assume V4L2_SUBDEV_API is selected

[Sakari Ailus]

Hi Jacopo,

On Wed, Apr 29, 2020 at 09:02:15AM +0200, Jacopo Mondi wrote:
> Hi Sakari,
> 
> On Wed, Apr 29, 2020 at 12:26:43AM +0300, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Tue, Apr 28, 2020 at 11:06:07PM +0200, Jacopo Mondi wrote:
> > > A sub-device device node can be registered in user space only if the
> > > CONFIG_V4L2_SUBDEV_API Kconfig option is selected.
> > >
> > > Remove checks from the v4l2-subdev file handle open/close functions and
> > > ioctl handler as they are only accessible if a device node was registered
> > > to user space in first place.
> >
> > Is there other motivation with this than clean up things a little?
> >
> 
> I had to add yet-another #if defined and I got fed up. If you don't
> have a device node registered you won't be able to access any of the
> functions where the existing #if defined() where placed.
> 
> > The change increases the binary size marginally if the Kconfig option is
> > disabled.
> >
> 
> I see. Should we instead guard the whole file handle operations and
> ioctl handler instead of having #if defined() spread inside them ? I
> assume they where there as leftover, as I'm still missing the point,
> give that, as said, without V4L2_SUBDEV_API, you can't register any
> device node to userspace..

I think that's why those #ifdefs have been originally put there --- it's
just dead code without the subdev nodes, and the compiler won't be able to
figure this out.

But it seems, later on, when people have added code that supports
sub-device nodes, no #ifdefs have been added.

I think I'd make sense to remove the current #ifdefs and add dummy ops for
everything where needed that truly depends on that Kconfig option (i.e.
sub-device nodes). Or just to remove these, as your patch does. It's not a
lot of code.

-- 
Kind regards,

Sakari Ailus