[libcamera-devel] [PATCH 2/2] libcamera: device_enumerator_udev: Don't add media device twice

[Kieran Bingham]

Hi Laurent,

On 23/03/2020 11:05, Niklas Söderlund wrote:
> Hi Laurent,
> 
> Thanks for your patch.
> 
> On 2020-03-20 23:04:35 +0200, Laurent Pinchart wrote:
>> Commit 68daa9302f90 ("libcamera: device_enumerator: Don't stop if one
>> device fails") introduced a bug in device enumeration. Previously, a
>> media device would only be added when all its dependencies were present,
>> as indicated by populateMediaDevice() returning 0. The commit changed
>> the logic to propagate the populateMediaDevice() return code up, but
>> mistakenly added the media device to the enumerator even when
>> dependencies were missing. This causes media devices enumerated with
>> missing dependencies to be added twice, once when enumerating the media
>> device itself, and a second time when all the dependencies are found.
>>
>> Fix it by deferring addition of the media device when dependencies are
>> missing, and add debug messages to track this process.
>>
>> Fixes: 68daa9302f90 ("libcamera: device_enumerator: Don't stop if one device fails")
>> Reported-by: Andrey Konovalov <andrey.konovalov at linaro.org>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

Sure, looks ok.

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

> 
>> ---
>>  src/libcamera/device_enumerator_udev.cpp | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
>> index 87638c59761c..e55350544feb 100644
>> --- a/src/libcamera/device_enumerator_udev.cpp
>> +++ b/src/libcamera/device_enumerator_udev.cpp
>> @@ -90,6 +90,13 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>>  			return ret;
>>  		}
>>  
>> +		if (ret) {
>> +			LOG(DeviceEnumerator, Debug)
>> +				<< "Defer media device " << media->deviceNode()
>> +				<< " due to " << ret << " missing dependencies";
>> +			return 0;
>> +		}
>> +
>>  		addDevice(media);
>>  		return 0;
>>  	}
>> @@ -313,6 +320,9 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
>>  	deps->deps_.erase(devnum);
>>  
>>  	if (deps->deps_.empty()) {
>> +		LOG(DeviceEnumerator, Debug)
>> +			<< "All dependencies for media device "
>> +			<< deps->media_->deviceNode() << " found";
>>  		addDevice(deps->media_);
>>  		pending_.remove(*deps);
>>  	}
>> -- 
>> Regards,
>>
>> Laurent Pinchart
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
> 

-- 
Regards
--
Kieran