| Message ID | 20170214134000.GA8550@amd (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Sakari Ailus |
| Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1cddLC-0001TA-9i; Tue, 14 Feb 2017 13:40:22 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.84_2/mailfrontend-6) with esmtp id 1cddLA-000704-3c; Tue, 14 Feb 2017 14:40:21 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753995AbdBNNkR (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Tue, 14 Feb 2017 08:40:17 -0500 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:59097 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753972AbdBNNkD (ORCPT <rfc822;linux-media@vger.kernel.org>); Tue, 14 Feb 2017 08:40:03 -0500 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 512) id 2793E817C9; Tue, 14 Feb 2017 14:40:01 +0100 (CET) Date: Tue, 14 Feb 2017 14:40:00 +0100 From: Pavel Machek <pavel@ucw.cz> To: sakari.ailus@iki.fi Cc: sre@kernel.org, pali.rohar@gmail.com, pavel@ucw.cz, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, laurent.pinchart@ideasonboard.com, mchehab@kernel.org, ivo.g.dimitrov.75@gmail.com Subject: [RFC 07/13] v4l2: device_register_subdev_nodes: allow calling multiple times Message-ID: <20170214134000.GA8550@amd> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="MGYHOYXEY6WxJCY8" Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2017.2.14.133016 X-PMX-Spam: Gauge=IIIIIIIII, Probability=9%, Report=' BODY_PARA_IS_SENTENCE_URL 0.1, MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, KNOWN_FREEWEB_URI 0.05, MSGID_ADDED_BY_MTA 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1400_1499 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, INVALID_MSGID_NO_FQDN 0, LEGITIMATE_SIGNS 0, MULTIPLE_REAL_RCPTS 0, NO_URI_HTTPS 0, URI_ENDS_IN_HTML 0, __ANY_URI 0, __ATTACHMENT_SIZE_0_10K 0, __CD 0, __CP_URI_IN_BODY 0, __CT 0, __CTYPE_HAS_BOUNDARY 0, __CTYPE_MULTIPART 0, __FRAUD_BODY_WEBMAIL 0, __FRAUD_WEBMAIL 0, __FROM_DOMAIN_IN_ANY_CC2 0, __FROM_DOMAIN_IN_RCPT 0, __HAS_ATTACHMENT 0, __HAS_ATTACHMENT1 0, __HAS_ATTACHMENT2 0, __HAS_CC_HDR 0, __HAS_FROM 0, __HAS_LIST_ID 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __KNOWN_FREEWEB_URI2 0, __MIME_TEXT_P 0, __MIME_TEXT_P1 0, __MIME_TEXT_P2 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __MULTIPLE_URI_TEXT 0, __NO_HTML_TAG_RAW 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_IN_BODY 0, __URI_NS , __URI_WITH_PATH 0, __USER_AGENT 0' |
Commit Message
Pavel Machek
Feb. 14, 2017, 1:40 p.m. UTC
From: Sebastian Reichel <sre@kernel.org> Without this, exposure / gain controls do not work in the camera application. Signed-off-by: Sebastian Reichel <sre@kernel.org> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> Signed-off-by: Pavel Machek <pavel@ucw.cz> --- drivers/media/v4l2-core/v4l2-device.c | 3 +++ 1 file changed, 3 insertions(+)
Comments
Hi Pavel and Sebastian, On Tue, Feb 14, 2017 at 02:40:00PM +0100, Pavel Machek wrote: > From: Sebastian Reichel <sre@kernel.org> > > Without this, exposure / gain controls do not work in the camera application. :-) > > Signed-off-by: Sebastian Reichel <sre@kernel.org> > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > Signed-off-by: Pavel Machek <pavel@ucw.cz> > --- > drivers/media/v4l2-core/v4l2-device.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c > index f364cc1..b3afbe8 100644 > --- a/drivers/media/v4l2-core/v4l2-device.c > +++ b/drivers/media/v4l2-core/v4l2-device.c > @@ -235,6 +235,9 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev) > if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE)) > continue; > > + if(sd->devnode) > + continue; This has been recognised as a problem but you're the first to submit a patch I believe. Please add an appropriate description. :-) s/if\(/if (/ I think this one should go in before the rest. > + > vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); > if (!vdev) { > err = -ENOMEM;
Hi Sakari, On Wednesday 15 Feb 2017 00:02:57 Sakari Ailus wrote: > On Tue, Feb 14, 2017 at 02:40:00PM +0100, Pavel Machek wrote: > > From: Sebastian Reichel <sre@kernel.org> > > > > Without this, exposure / gain controls do not work in the camera > > application.: > :-) > : > > Signed-off-by: Sebastian Reichel <sre@kernel.org> > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > --- > > > > drivers/media/v4l2-core/v4l2-device.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/media/v4l2-core/v4l2-device.c > > b/drivers/media/v4l2-core/v4l2-device.c index f364cc1..b3afbe8 100644 > > --- a/drivers/media/v4l2-core/v4l2-device.c > > +++ b/drivers/media/v4l2-core/v4l2-device.c > > @@ -235,6 +235,9 @@ int v4l2_device_register_subdev_nodes(struct > > v4l2_device *v4l2_dev) > > if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE)) > > continue; > > > > + if(sd->devnode) > > + continue; > > This has been recognised as a problem but you're the first to submit a patch > I believe. Please add an appropriate description. :-) > > s/if\(/if (/ > > I think this one should go in before the rest. But how can this happen ? Is v4l2_device_register_subdev_nodes() called multiple times ? Do we want to allow that ? > > + > > vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); > > if (!vdev) { > > err = -ENOMEM;
Hi Laurent, On Wed, Feb 15, 2017 at 12:06:17AM +0200, Laurent Pinchart wrote: > Hi Sakari, > > On Wednesday 15 Feb 2017 00:02:57 Sakari Ailus wrote: > > On Tue, Feb 14, 2017 at 02:40:00PM +0100, Pavel Machek wrote: > > > From: Sebastian Reichel <sre@kernel.org> > > > > > > Without this, exposure / gain controls do not work in the camera > > > application.: > > :-) > > : > > > Signed-off-by: Sebastian Reichel <sre@kernel.org> > > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > > --- > > > > > > drivers/media/v4l2-core/v4l2-device.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/media/v4l2-core/v4l2-device.c > > > b/drivers/media/v4l2-core/v4l2-device.c index f364cc1..b3afbe8 100644 > > > --- a/drivers/media/v4l2-core/v4l2-device.c > > > +++ b/drivers/media/v4l2-core/v4l2-device.c > > > @@ -235,6 +235,9 @@ int v4l2_device_register_subdev_nodes(struct > > > v4l2_device *v4l2_dev) > > > if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE)) > > > continue; > > > > > > + if(sd->devnode) > > > + continue; > > > > This has been recognised as a problem but you're the first to submit a patch > > I believe. Please add an appropriate description. :-) > > > > s/if\(/if (/ > > > > I think this one should go in before the rest. > > But how can this happen ? Is v4l2_device_register_subdev_nodes() called > multiple times ? Do we want to allow that ? A driver could do this even accidentally. It's much better to do the right thing than to start corrupting system memory sooner or later. In the future we'll need to be able to dynamically register device nodes after the registration of the original device nodes in a media device has completed anyway. I don't think this would be the means for that though. What happens here though is that both the video bus switch and isp notifiers completing will call the function, thus ending up trying to register many of the nodes twice. Philipp had a different approach to the problem --- postponing the complete until add the sub-devices have been bound. The patch is called "v4l2-async: allow subdevices to add further subdevices to the notifier waiting list".
On Wed 2017-02-15 00:02:57, Sakari Ailus wrote: > Hi Pavel and Sebastian, > > On Tue, Feb 14, 2017 at 02:40:00PM +0100, Pavel Machek wrote: > > From: Sebastian Reichel <sre@kernel.org> > > > > Without this, exposure / gain controls do not work in the camera application. > > :-) > > > > > Signed-off-by: Sebastian Reichel <sre@kernel.org> > > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > > --- > > drivers/media/v4l2-core/v4l2-device.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c > > index f364cc1..b3afbe8 100644 > > --- a/drivers/media/v4l2-core/v4l2-device.c > > +++ b/drivers/media/v4l2-core/v4l2-device.c > > @@ -235,6 +235,9 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev) > > if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE)) > > continue; > > > > + if(sd->devnode) > > + continue; > > This has been recognised as a problem but you're the first to submit a patch > I believe. Please add an appropriate description. :-) Ugh. Will try :-). > s/if\(/if (/ > > I think this one should go in before the rest. Easy enough, and I'll move it to the first in the series. Pavel
diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c index f364cc1..b3afbe8 100644 --- a/drivers/media/v4l2-core/v4l2-device.c +++ b/drivers/media/v4l2-core/v4l2-device.c @@ -235,6 +235,9 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev) if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE)) continue; + if(sd->devnode) + continue; + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL); if (!vdev) { err = -ENOMEM;