[libcamera-devel,1/2] libcamera: device_enumerator_udev: Update pending list in addUdevDevice
diff mbox series

Message ID 20200321180951.32685-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit e75ef59e028e5b686d686789439ede443c9c4cde
Headers show
Series
  • [libcamera-devel,1/2] libcamera: device_enumerator_udev: Update pending list in addUdevDevice
Related show

Commit Message

Laurent Pinchart March 21, 2020, 6:09 p.m. UTC 
Media devices that have unmet dependencies are added to the pending list
in populateMediaDevice(). Move the code to the caller, addUdevDevice(),
as it logically belongs there.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/device_enumerator_udev.cpp      | 28 ++++++++-----------
 .../include/device_enumerator_udev.h          |  2 +-
 2 files changed, 13 insertions(+), 17 deletions(-)

Comments

Niklas Söderlund March 23, 2020, 11:24 a.m. UTC | #1 
Hi Laurent,

Thanks for your work.

On 2020-03-21 20:09:50 +0200, Laurent Pinchart wrote:
> Media devices that have unmet dependencies are added to the pending list
> in populateMediaDevice(). Move the code to the caller, addUdevDevice(),
> as it logically belongs there.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/device_enumerator_udev.cpp      | 28 ++++++++-----------
>  .../include/device_enumerator_udev.h          |  2 +-
>  2 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> index e55350544feb..ea3f6b7c9ae0 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -81,7 +81,8 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  		if (!media)
>  			return -ENODEV;
>  
> -		int ret = populateMediaDevice(media);
> +		DependencyMap deps;
> +		int ret = populateMediaDevice(media.get(), &deps);
>  		if (ret < 0) {
>  			LOG(DeviceEnumerator, Warning)
>  				<< "Failed to populate media device "
> @@ -90,10 +91,16 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  			return ret;
>  		}
>  
> -		if (ret) {
> +		if (!deps.empty()) {
>  			LOG(DeviceEnumerator, Debug)
>  				<< "Defer media device " << media->deviceNode()
>  				<< " due to " << ret << " missing dependencies";
> +
> +			pending_.emplace_back(media, deps);
> +			MediaDeviceDeps *mediaDeps = &pending_.back();
> +			for (const auto &dep : mediaDeps->deps_)
> +				devMap_[dep.first] = mediaDeps;
> +
>  			return 0;
>  		}
>  
> @@ -185,10 +192,9 @@ done:
>  	return 0;
>  }
>  
> -int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)
> +int DeviceEnumeratorUdev::populateMediaDevice(MediaDevice *media, DependencyMap *deps)
>  {
>  	std::set<dev_t> children;
> -	DependencyMap deps;
>  
>  	/* Associate entities to device node paths. */
>  	for (MediaEntity *entity : media->entities()) {
> @@ -203,7 +209,7 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
>  		 * dependencies.
>  		 */
>  		if (orphans_.find(devnum) == orphans_.end()) {
> -			deps[devnum].push_back(entity);
> +			(*deps)[devnum].push_back(entity);
>  			continue;
>  		}
>  
> @@ -231,17 +237,7 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
>  			++it;
>  	}
>  
> -	/*
> -	 * If the media device has unmet dependencies, add it to the pending
> -	 * list and update the devnum map accordingly.
> -	 */
> -	if (!deps.empty()) {
> -		pending_.emplace_back(media, deps);
> -		for (const auto &dep : deps)
> -			devMap_[dep.first] = &pending_.back();
> -	}
> -
> -	return deps.size();
> +	return 0;
>  }
>  
>  /**
> diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h
> index 6d8268620185..efaefe5dc4a3 100644
> --- a/src/libcamera/include/device_enumerator_udev.h
> +++ b/src/libcamera/include/device_enumerator_udev.h
> @@ -63,7 +63,7 @@ private:
>  	std::map<dev_t, MediaDeviceDeps *> devMap_;
>  
>  	int addUdevDevice(struct udev_device *dev);
> -	int populateMediaDevice(const std::shared_ptr<MediaDevice> &media);
> +	int populateMediaDevice(MediaDevice *media, DependencyMap *deps);
>  	std::string lookupDeviceNode(dev_t devnum);
>  
>  	int addV4L2Device(dev_t devnum);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham March 23, 2020, 2:14 p.m. UTC | #2 
Hi Laurent,

On 21/03/2020 18:09, Laurent Pinchart wrote:
> Media devices that have unmet dependencies are added to the pending list
> in populateMediaDevice(). Move the code to the caller, addUdevDevice(),
> as it logically belongs there.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/libcamera/device_enumerator_udev.cpp      | 28 ++++++++-----------
>  .../include/device_enumerator_udev.h          |  2 +-
>  2 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> index e55350544feb..ea3f6b7c9ae0 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -81,7 +81,8 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  		if (!media)
>  			return -ENODEV;
>  
> -		int ret = populateMediaDevice(media);
> +		DependencyMap deps;
> +		int ret = populateMediaDevice(media.get(), &deps);
>  		if (ret < 0) {
>  			LOG(DeviceEnumerator, Warning)
>  				<< "Failed to populate media device "
> @@ -90,10 +91,16 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  			return ret;
>  		}
>  
> -		if (ret) {
> +		if (!deps.empty()) {
>  			LOG(DeviceEnumerator, Debug)
>  				<< "Defer media device " << media->deviceNode()
>  				<< " due to " << ret << " missing dependencies";
> +
> +			pending_.emplace_back(media, deps);
> +			MediaDeviceDeps *mediaDeps = &pending_.back();
> +			for (const auto &dep : mediaDeps->deps_)
> +				devMap_[dep.first] = mediaDeps;
> +
>  			return 0;
>  		}
>  
> @@ -185,10 +192,9 @@ done:
>  	return 0;
>  }
>  
> -int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)
> +int DeviceEnumeratorUdev::populateMediaDevice(MediaDevice *media, DependencyMap *deps)
>  {
>  	std::set<dev_t> children;
> -	DependencyMap deps;
>  
>  	/* Associate entities to device node paths. */
>  	for (MediaEntity *entity : media->entities()) {
> @@ -203,7 +209,7 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
>  		 * dependencies.
>  		 */
>  		if (orphans_.find(devnum) == orphans_.end()) {
> -			deps[devnum].push_back(entity);
> +			(*deps)[devnum].push_back(entity);
>  			continue;
>  		}
>  
> @@ -231,17 +237,7 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
>  			++it;
>  	}
>  
> -	/*
> -	 * If the media device has unmet dependencies, add it to the pending
> -	 * list and update the devnum map accordingly.
> -	 */
> -	if (!deps.empty()) {
> -		pending_.emplace_back(media, deps);
> -		for (const auto &dep : deps)
> -			devMap_[dep.first] = &pending_.back();
> -	}
> -
> -	return deps.size();
> +	return 0;
>  }
>  
>  /**
> diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h
> index 6d8268620185..efaefe5dc4a3 100644
> --- a/src/libcamera/include/device_enumerator_udev.h
> +++ b/src/libcamera/include/device_enumerator_udev.h
> @@ -63,7 +63,7 @@ private:
>  	std::map<dev_t, MediaDeviceDeps *> devMap_;
>  
>  	int addUdevDevice(struct udev_device *dev);
> -	int populateMediaDevice(const std::shared_ptr<MediaDevice> &media);
> +	int populateMediaDevice(MediaDevice *media, DependencyMap *deps);
>  	std::string lookupDeviceNode(dev_t devnum);
>  
>  	int addV4L2Device(dev_t devnum);
>

Patch
diff mbox series 

diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
index e55350544feb..ea3f6b7c9ae0 100644
--- a/src/libcamera/device_enumerator_udev.cpp
+++ b/src/libcamera/device_enumerator_udev.cpp
@@ -81,7 +81,8 @@  int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
 		if (!media)
 			return -ENODEV;
 
-		int ret = populateMediaDevice(media);
+		DependencyMap deps;
+		int ret = populateMediaDevice(media.get(), &deps);
 		if (ret < 0) {
 			LOG(DeviceEnumerator, Warning)
 				<< "Failed to populate media device "
@@ -90,10 +91,16 @@  int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
 			return ret;
 		}
 
-		if (ret) {
+		if (!deps.empty()) {
 			LOG(DeviceEnumerator, Debug)
 				<< "Defer media device " << media->deviceNode()
 				<< " due to " << ret << " missing dependencies";
+
+			pending_.emplace_back(media, deps);
+			MediaDeviceDeps *mediaDeps = &pending_.back();
+			for (const auto &dep : mediaDeps->deps_)
+				devMap_[dep.first] = mediaDeps;
+
 			return 0;
 		}
 
@@ -185,10 +192,9 @@  done:
 	return 0;
 }
 
-int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)
+int DeviceEnumeratorUdev::populateMediaDevice(MediaDevice *media, DependencyMap *deps)
 {
 	std::set<dev_t> children;
-	DependencyMap deps;
 
 	/* Associate entities to device node paths. */
 	for (MediaEntity *entity : media->entities()) {
@@ -203,7 +209,7 @@  int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
 		 * dependencies.
 		 */
 		if (orphans_.find(devnum) == orphans_.end()) {
-			deps[devnum].push_back(entity);
+			(*deps)[devnum].push_back(entity);
 			continue;
 		}
 
@@ -231,17 +237,7 @@  int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice>
 			++it;
 	}
 
-	/*
-	 * If the media device has unmet dependencies, add it to the pending
-	 * list and update the devnum map accordingly.
-	 */
-	if (!deps.empty()) {
-		pending_.emplace_back(media, deps);
-		for (const auto &dep : deps)
-			devMap_[dep.first] = &pending_.back();
-	}
-
-	return deps.size();
+	return 0;
 }
 
 /**
diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h
index 6d8268620185..efaefe5dc4a3 100644
--- a/src/libcamera/include/device_enumerator_udev.h
+++ b/src/libcamera/include/device_enumerator_udev.h
@@ -63,7 +63,7 @@  private:
 	std::map<dev_t, MediaDeviceDeps *> devMap_;
 
 	int addUdevDevice(struct udev_device *dev);
-	int populateMediaDevice(const std::shared_ptr<MediaDevice> &media);
+	int populateMediaDevice(MediaDevice *media, DependencyMap *deps);
 	std::string lookupDeviceNode(dev_t devnum);
 
 	int addV4L2Device(dev_t devnum);