[libcamera-devel] [PATCH] libcamera: device_enumerator: Don't stop if one device fails
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Feb 17 11:43:22 CET 2020
Hi Kieran,
On Mon, Feb 17, 2020 at 10:41:23AM +0000, Kieran Bingham wrote:
> On 17/02/2020 08:23, Kieran Bingham wrote:
> > 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.
> >
> >> 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?)
That's a good idea, I'll submit a v2 for you to test :-)
> >> + 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,
Laurent Pinchart
More information about the libcamera-devel
mailing list