All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacopo Mondi <jacopo@jmondi.org>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org,
	mchehab@kernel.org, hverkuil-cisco@xs4all.nl,
	andrey.konovalov@linaro.org, laurent.pinchart@ideasonboard.com
Subject: Re: [PATCH v5 4/6] media: v4l2-subdev: Assume V4L2_SUBDEV_API is selected
Date: Wed, 29 Apr 2020 10:43:58 +0200	[thread overview]
Message-ID: <20200429084358.ssliiljy4jdzrvl3@uno.localdomain> (raw)
In-Reply-To: <20200429082737.GB9190@paasikivi.fi.intel.com>

Hi Sakari

On Wed, Apr 29, 2020 at 11:27:37AM +0300, Sakari Ailus wrote:
> 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.

that's what I've done now, as soon as I've run a few compile test, I'll send
a new version.

Thanks
   j

>
> --
> Kind regards,
>
> Sakari Ailus

  reply	other threads:[~2020-04-29  8:40 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-28 21:06 [PATCH v5 0/6] media: Register read-only sub-dev devnode Jacopo Mondi
2020-04-28 21:06 ` [PATCH v5 1/6] Documentation: media: Update sub-device API intro Jacopo Mondi
2020-04-28 21:06 ` [PATCH v5 2/6] Documentation: media: Document read-only subdevice Jacopo Mondi
2020-04-28 21:06 ` [PATCH v5 3/6] media: v4l2-dev: Add v4l2_device_register_ro_subdev_node() Jacopo Mondi
2020-04-28 21:06 ` [PATCH v5 4/6] media: v4l2-subdev: Assume V4L2_SUBDEV_API is selected Jacopo Mondi
2020-04-28 21:26   ` Sakari Ailus
2020-04-29  7:02     ` Jacopo Mondi
2020-04-29  8:27       ` Sakari Ailus
2020-04-29  8:43         ` Jacopo Mondi [this message]
2020-04-28 23:44   ` kbuild test robot
2020-04-28 23:44     ` kbuild test robot
2020-04-29  7:04     ` Jacopo Mondi
2020-04-29  8:58   ` [PATCH v5.1] media: v4l2-subdev: Guard whole fops and ioctl hdlr Jacopo Mondi
2020-04-29  9:49     ` Sakari Ailus
2020-04-29 10:16       ` Jacopo Mondi
2020-04-29 11:00         ` Sakari Ailus
2020-04-28 21:06 ` [PATCH v5 5/6] v4l2-subdev: add VIDIOC_SUBDEV_QUERYCAP ioctl Jacopo Mondi
2020-04-28 21:28   ` Sakari Ailus
2020-04-29  8:09     ` Jacopo Mondi
2020-04-29  8:18       ` Sakari Ailus
2020-05-06 13:29         ` Hans Verkuil
2020-05-06 18:34           ` Sakari Ailus
2020-05-07  7:14             ` Hans Verkuil
2020-04-28 21:06 ` [PATCH v5 6/6] v4l: document VIDIOC_SUBDEV_QUERYCAP Jacopo Mondi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200429084358.ssliiljy4jdzrvl3@uno.localdomain \
    --to=jacopo@jmondi.org \
    --cc=andrey.konovalov@linaro.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=libcamera-devel@lists.libcamera.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.