[libcamera-devel] [PATCH] libcamera: device_enumerator: Don't stop if one device fails
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Feb 17 11:41:23 CET 2020
On 17/02/2020 08:23, Kieran Bingham wrote:
> Hi Laurent,
>
> On 14/02/2020 12:37, Laurent Pinchart wrote:
>> If one device fails to enumerate, which isn't supposed to happen under
>> normal conditions, both the sysfs and the udev enumerators stop
>> enumeration of further devices. This potentially prevents working
>> devices from being detected and handled. Fix it by skipping the faulty
>> device.
>
> Thank you,
>
> This does indeed allow the system to continue and function with the
> available cameras.
>
> Interestingly, in the particular case that I was investigating - it
> wasn't the newly added device that failed ... it was the existing camera
> interface.
>
> That can be investigated separately.
>
> --
> Kieran
>
>
>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Thanks, indeed this now continues to function and produces an
appropriate message for the failing device components:
> [2:20:54.425174891] [3209] ERR MediaDevice media_device.cpp:242 Failed to enumerate topology: Bad address
> [2:20:54.425281647] [3209] INFO DeviceEnumerator device_enumerator.cpp:217 Unable to populate media device /dev/media0 (Invalid argument), skipping
> [2:20:54.425393663] [3209] WARN DeviceEnumerator device_enumerator_udev.cpp:149 Failed to add device for '/sys/devices/platform/soc/fe801000.csi/media0', skipping
> Using camera VF0520 Live! Cam Sync: VF0520 L
Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
And with one small comment considered below:
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>> src/libcamera/device_enumerator_sysfs.cpp | 15 +++++++--------
>> src/libcamera/device_enumerator_udev.cpp | 8 +++++---
>> 2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp
>> index ad26affb38a3..bde736226f95 100644
>> --- a/src/libcamera/device_enumerator_sysfs.cpp
>> +++ b/src/libcamera/device_enumerator_sysfs.cpp
>> @@ -33,7 +33,6 @@ int DeviceEnumeratorSysfs::enumerate()
>> {
>> struct dirent *ent;
>> DIR *dir;
>> - int ret = 0;
>>
>> static const char * const sysfs_dirs[] = {
>> "/sys/subsystem/media/devices",
>> @@ -74,14 +73,14 @@ int DeviceEnumeratorSysfs::enumerate()
>> }
>>
>> std::shared_ptr<MediaDevice> media = createDevice(devnode);
>> - if (!media) {
>> - ret = -ENODEV;
>> - break;
>> - }
>> + if (!media)
>> + continue;
>>
>> if (populateMediaDevice(media) < 0) {
>> - ret = -ENODEV;
>> - break;
>> + LOG(DeviceEnumerator, Warning)
>> + << "Failed to populate media device /dev/media"
>> + << idx << ", skipping";
Shouldn't this use something like media->deviceNode() ?
(Perhaps even printing media->driver() and media->model for guidance too?)
>> + continue;
>> }
>>
>> addDevice(media);
>> @@ -89,7 +88,7 @@ int DeviceEnumeratorSysfs::enumerate()
>>
>> closedir(dir);
>>
>> - return ret;
>> + return 0;
>> }
>>
>> int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)
>> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
>> index b2c5fd221dcd..3837aaf97e6a 100644
>> --- a/src/libcamera/device_enumerator_udev.cpp
>> +++ b/src/libcamera/device_enumerator_udev.cpp
>> @@ -145,10 +145,12 @@ int DeviceEnumeratorUdev::enumerate()
>> goto done;
>> }
>>
>> - ret = addUdevDevice(dev);
>> + if (addUdevDevice(dev) < 0)
>> + LOG(DeviceEnumerator, Warning)
>> + << "Failed to add device for '"
>> + << syspath << "', skipping";
>> +
>> udev_device_unref(dev);
>> - if (ret < 0)
>> - break;
>> }
>>
>> done:
>>
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list