Skip to contentSkip to conversation

v4l2: device_register_subdev_nodes: allow calling multiple times

RFC In review Pavel Machek 3 participants 5 messages Feb 14, 2017 Media (V4L/DVB)

lore
Share & tools
lore

Pavel Machek

author
#0 lore raw

RFC 7/13 v4l2: device_register_subdev_nodes: allow calling multiple times

From: Sebastian Reichel <[email protected]>

Without this, exposure / gain controls do not work in the camera application.

Signed-off-by Sebastian Reichel <[email protected]>
Signed-off-by Ivaylo Dimitrov <[email protected]>
Signed-off-by Pavel Machek <[email protected]>
---
 drivers/media/v4l2-core/v4l2-device.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/media/v4l2-core/v4l2-device.c +3 -0
index f364cc1..b3afbe8 100644
@@ -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;
Signature · 6 lines
2.1.4


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#1 lore raw

Hi Pavel and Sebastian,

Quoted 3 lines · Pavel Machek

From: Sebastian Reichel <[email protected]>

Without this, exposure / gain controls do not work in the camera application.

:-)

Quoted 13 lines
Signed-off-by Sebastian Reichel <[email protected]>
Signed-off-by Ivaylo Dimitrov <[email protected]>
Signed-off-by Pavel Machek <[email protected]>
---
 drivers/media/v4l2-core/v4l2-device.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/media/v4l2-core/v4l2-device.c +0 -0
index f364cc1..b3afbe8 100644

drivers/media/v4l2-core/v4l2-device.c int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev) · line 239

Quoted 5 lines
drivers/media/v4l2-core/v4l2-device.c +2 -0
@@ -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.

Quoted 4 lines
drivers/media/v4l2-core/v4l2-device.c +1 -0
+
 		vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
 		if (!vdev) {
 			err = -ENOMEM;
Signature · 4 lines
Kind regards,

Sakari Ailus
e-mail: [email protected]	XMPP: [email protected]
#2 lore raw

Hi Sakari,

Quoted 30 lines · On Wednesday 15 Feb 2017 00:02:57 Sakari Ailus wrote:
Quoted 4 lines · Pavel Machek

From: Sebastian Reichel <[email protected]>

Without this, exposure / gain controls do not work in the camera application.:

:-)
:

Quoted 13 lines
Signed-off-by Sebastian Reichel <[email protected]>
Signed-off-by Ivaylo Dimitrov <[email protected]>
Signed-off-by Pavel Machek <[email protected]>

---

 drivers/media/v4l2-core/v4l2-device.c | 3 +++
 1 file changed, 3 insertions(+)
(quoted diff) +0 -0

b/drivers/media/v4l2-core/v4l2-device.c index f364cc1..b3afbe8 100644

drivers/media/v4l2-core/v4l2-device.c int v4l2_device_register_subdev_nodes(struct · line 240

Quoted 6 lines
drivers/media/v4l2-core/v4l2-device.c +2 -0
@@ -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. :-)

Quoted 3 lines

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 ?

Quoted 4 lines
Quoted 4 lines
drivers/media/v4l2-core/v4l2-device.c +1 -0
+
 		vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
 		if (!vdev) {

err = -ENOMEM;

Signature · 3 lines
Regards,

Laurent Pinchart
#3 lore raw

Hi Laurent,

Quoted 33 lines · Laurent Pinchart

Hi Sakari,

Quoted 30 lines · On Wednesday 15 Feb 2017 00:02:57 Sakari Ailus wrote:
Quoted 4 lines · Pavel Machek

From: Sebastian Reichel <[email protected]>

Without this, exposure / gain controls do not work in the camera application.:

:-)
:

Quoted 13 lines
Signed-off-by Sebastian Reichel <[email protected]>
Signed-off-by Ivaylo Dimitrov <[email protected]>
Signed-off-by Pavel Machek <[email protected]>

---

 drivers/media/v4l2-core/v4l2-device.c | 3 +++
 1 file changed, 3 insertions(+)
(quoted diff) +0 -0

b/drivers/media/v4l2-core/v4l2-device.c index f364cc1..b3afbe8 100644

drivers/media/v4l2-core/v4l2-device.c int v4l2_device_register_subdev_nodes(struct · line 240

Quoted 6 lines
drivers/media/v4l2-core/v4l2-device.c +2 -0
@@ -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. :-)

Quoted 6 lines
Quoted 3 lines

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

Signature · 4 lines
Kind regards,

Sakari Ailus
e-mail: [email protected]	XMPP: [email protected]
author
#4 lore raw
In reply to Sakari Ailus
Quoted 25 lines · Sakari Ailus

Hi Pavel and Sebastian,

Quoted 3 lines · Pavel Machek

From: Sebastian Reichel <[email protected]>

Without this, exposure / gain controls do not work in the camera application.

:-)

Quoted 16 lines
Signed-off-by Sebastian Reichel <[email protected]>
Signed-off-by Ivaylo Dimitrov <[email protected]>
Signed-off-by Pavel Machek <[email protected]>
---
 drivers/media/v4l2-core/v4l2-device.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/media/v4l2-core/v4l2-device.c +0 -0
index f364cc1..b3afbe8 100644
@@ -235,6 +235,9 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev)
 		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
 			continue;
Quoted 5 lines

drivers/media/v4l2-core/v4l2-device.c int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev) · line 239

Quoted 2 lines
drivers/media/v4l2-core/v4l2-device.c +2 -0
+		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 :-).

Quoted 3 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html