[libcamera-devel,2/4] libcamera: device_enumerator_udev: Delay device node lookup

Message ID 20190912200330.19004-3-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Fix udev device enumerator with V4L2 M2M devices
Related show

Commit Message

Laurent Pinchart Sept. 12, 2019, 8:03 p.m. UTC
When populating media devices, we look up device nodes for every entity
in the media device, regardless of if the entity is present in the
orphans list. This causes unnecessary lookups (that may also fail as the
device node may not be ready yet at that time). Move the lookup at a
later time, when the device node is actually needed.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/device_enumerator_udev.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kieran Bingham Sept. 13, 2019, 8:43 a.m. UTC | #1
Hi Laurent

On 12/09/2019 21:03, Laurent Pinchart wrote:
> When populating media devices, we look up device nodes for every entity
> in the media device, regardless of if the entity is present in the
> orphans list. This causes unnecessary lookups (that may also fail as the
> device node may not be ready yet at that time). Move the lookup at a
> later time, when the device node is actually needed.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/device_enumerator_udev.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> index e0c646c997de..210f5c1f2870 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -180,10 +180,10 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
>  
>  		dev_t devnum = makedev(entity->deviceMajor(),
>  				       entity->deviceMinor());
> -		std::string deviceNode = lookupDeviceNode(devnum);
>  

I guess as this line was only just added it could have been put in the
right place in the previous patch - but I like the fact that this is
split out to explain /why/ it is better located inside the conditional.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


>  		/* Take device from orphan list first, if it is in the list. */
>  		if (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) {
> +			std::string deviceNode = lookupDeviceNode(devnum);
>  			if (deviceNode.empty())
>  				return -EINVAL;
>  
>

Patch

diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
index e0c646c997de..210f5c1f2870 100644
--- a/src/libcamera/device_enumerator_udev.cpp
+++ b/src/libcamera/device_enumerator_udev.cpp
@@ -180,10 +180,10 @@  int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
 
 		dev_t devnum = makedev(entity->deviceMajor(),
 				       entity->deviceMinor());
-		std::string deviceNode = lookupDeviceNode(devnum);
 
 		/* Take device from orphan list first, if it is in the list. */
 		if (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) {
+			std::string deviceNode = lookupDeviceNode(devnum);
 			if (deviceNode.empty())
 				return -EINVAL;