| Message ID | 20190912200330.19004-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 12/09/2019 21:03, Laurent Pinchart wrote: > The lookupDeviceNode() method is declared as pure virtual in the base > DeviceEnumerator class, but is only called by derived classes. Move it > to the DeviceEnumeratorSysfs and DeviceEnumeratorUdev. This allows > changing the udev version to take a dev_t instead of separate > major/minor, as that's what both the caller and the callee end up using. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/device_enumerator.cpp | 13 ------------- > src/libcamera/device_enumerator_sysfs.cpp | 11 +++++++++++ > src/libcamera/device_enumerator_udev.cpp | 19 ++++++++++++------- > src/libcamera/include/device_enumerator.h | 2 -- > .../include/device_enumerator_sysfs.h | 2 +- > .../include/device_enumerator_udev.h | 2 +- > 6 files changed, 25 insertions(+), 24 deletions(-) > > diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp > index e76438afdd65..0b596bcec363 100644 > --- a/src/libcamera/device_enumerator.cpp > +++ b/src/libcamera/device_enumerator.cpp > @@ -306,17 +306,4 @@ std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm) > return nullptr; > } > > -/** > - * \fn DeviceEnumerator::lookupDeviceNode(int major, int minor) > - * \brief Lookup device node path from device number > - * \param[in] major The device major number > - * \param[in] minor The device minor number > - * > - * Translate a device number given as \a major and \a minor to a device node > - * path. > - * > - * \return the device node path on success, or an empty string if the lookup > - * fails > - */ > - > } /* namespace libcamera */ > diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp > index 78a7da8d79e5..ad26affb38a3 100644 > --- a/src/libcamera/device_enumerator_sysfs.cpp > +++ b/src/libcamera/device_enumerator_sysfs.cpp > @@ -112,6 +112,17 @@ int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice > return 0; > } > > +/** > + * \brief Lookup device node path from device number > + * \param[in] major The device major number > + * \param[in] minor The device minor number > + * > + * Translate a device number given as \a major and \a minor to a device node > + * path. > + * > + * \return The device node path on success, or an empty string if the lookup > + * fails > + */ > std::string DeviceEnumeratorSysfs::lookupDeviceNode(int major, int minor) > { > std::string deviceNode; > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp > index 40853e77902f..e0c646c997de 100644 > --- a/src/libcamera/device_enumerator_udev.cpp > +++ b/src/libcamera/device_enumerator_udev.cpp > @@ -178,10 +178,9 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> > if (entity->deviceMajor() == 0 && entity->deviceMinor() == 0) > continue; > > - std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), > - entity->deviceMinor()); > 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()) { > @@ -205,14 +204,21 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> > return pendingNodes; > } > > -std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor) > +/** > + * \brief Lookup device node path from device number > + * \param[in] devnum The device number > + * > + * Translate a device number given as \a devnum to a device node path. > + * > + * \return The device node path on success, or an empty string if the lookup > + * fails > + */ > +std::string DeviceEnumeratorUdev::lookupDeviceNode(dev_t devnum) > { > struct udev_device *device; > const char *name; > - dev_t devnum; > std::string deviceNode = std::string(); > > - devnum = makedev(major, minor); > device = udev_device_new_from_devnum(udev_, 'c', devnum); > if (!device) > return std::string(); > @@ -246,8 +252,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum) > return 0; > } > > - std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), > - entity->deviceMinor()); > + std::string deviceNode = lookupDeviceNode(devnum); > if (deviceNode.empty()) > return -EINVAL; > > diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h > index c5d26f1a883e..770f42772270 100644 > --- a/src/libcamera/include/device_enumerator.h > +++ b/src/libcamera/include/device_enumerator.h > @@ -50,8 +50,6 @@ protected: > > private: > std::vector<std::shared_ptr<MediaDevice>> devices_; > - > - virtual std::string lookupDeviceNode(int major, int minor) = 0; > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h > index 242b22b289b0..9063f6a75e11 100644 > --- a/src/libcamera/include/device_enumerator_sysfs.h > +++ b/src/libcamera/include/device_enumerator_sysfs.h > @@ -24,7 +24,7 @@ public: > > private: > int populateMediaDevice(const std::shared_ptr<MediaDevice> &media); > - std::string lookupDeviceNode(int major, int minor) final; > + std::string lookupDeviceNode(int major, int minor); > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h > index 5bdcdea65c35..fb7cac8011a1 100644 > --- a/src/libcamera/include/device_enumerator_udev.h > +++ b/src/libcamera/include/device_enumerator_udev.h > @@ -47,7 +47,7 @@ private: > > int addUdevDevice(struct udev_device *dev); > int populateMediaDevice(const std::shared_ptr<MediaDevice> &media); > - std::string lookupDeviceNode(int major, int minor) final; > + std::string lookupDeviceNode(dev_t devnum); > > int addV4L2Device(dev_t devnum); > void udevNotify(EventNotifier *notifier); >
diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index e76438afdd65..0b596bcec363 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -306,17 +306,4 @@ std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm) return nullptr; } -/** - * \fn DeviceEnumerator::lookupDeviceNode(int major, int minor) - * \brief Lookup device node path from device number - * \param[in] major The device major number - * \param[in] minor The device minor number - * - * Translate a device number given as \a major and \a minor to a device node - * path. - * - * \return the device node path on success, or an empty string if the lookup - * fails - */ - } /* namespace libcamera */ diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp index 78a7da8d79e5..ad26affb38a3 100644 --- a/src/libcamera/device_enumerator_sysfs.cpp +++ b/src/libcamera/device_enumerator_sysfs.cpp @@ -112,6 +112,17 @@ int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice return 0; } +/** + * \brief Lookup device node path from device number + * \param[in] major The device major number + * \param[in] minor The device minor number + * + * Translate a device number given as \a major and \a minor to a device node + * path. + * + * \return The device node path on success, or an empty string if the lookup + * fails + */ std::string DeviceEnumeratorSysfs::lookupDeviceNode(int major, int minor) { std::string deviceNode; diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp index 40853e77902f..e0c646c997de 100644 --- a/src/libcamera/device_enumerator_udev.cpp +++ b/src/libcamera/device_enumerator_udev.cpp @@ -178,10 +178,9 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> if (entity->deviceMajor() == 0 && entity->deviceMinor() == 0) continue; - std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), - entity->deviceMinor()); 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()) { @@ -205,14 +204,21 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr<MediaDevice> return pendingNodes; } -std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor) +/** + * \brief Lookup device node path from device number + * \param[in] devnum The device number + * + * Translate a device number given as \a devnum to a device node path. + * + * \return The device node path on success, or an empty string if the lookup + * fails + */ +std::string DeviceEnumeratorUdev::lookupDeviceNode(dev_t devnum) { struct udev_device *device; const char *name; - dev_t devnum; std::string deviceNode = std::string(); - devnum = makedev(major, minor); device = udev_device_new_from_devnum(udev_, 'c', devnum); if (!device) return std::string(); @@ -246,8 +252,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum) return 0; } - std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), - entity->deviceMinor()); + std::string deviceNode = lookupDeviceNode(devnum); if (deviceNode.empty()) return -EINVAL; diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h index c5d26f1a883e..770f42772270 100644 --- a/src/libcamera/include/device_enumerator.h +++ b/src/libcamera/include/device_enumerator.h @@ -50,8 +50,6 @@ protected: private: std::vector<std::shared_ptr<MediaDevice>> devices_; - - virtual std::string lookupDeviceNode(int major, int minor) = 0; }; } /* namespace libcamera */ diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h index 242b22b289b0..9063f6a75e11 100644 --- a/src/libcamera/include/device_enumerator_sysfs.h +++ b/src/libcamera/include/device_enumerator_sysfs.h @@ -24,7 +24,7 @@ public: private: int populateMediaDevice(const std::shared_ptr<MediaDevice> &media); - std::string lookupDeviceNode(int major, int minor) final; + std::string lookupDeviceNode(int major, int minor); }; } /* namespace libcamera */ diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h index 5bdcdea65c35..fb7cac8011a1 100644 --- a/src/libcamera/include/device_enumerator_udev.h +++ b/src/libcamera/include/device_enumerator_udev.h @@ -47,7 +47,7 @@ private: int addUdevDevice(struct udev_device *dev); int populateMediaDevice(const std::shared_ptr<MediaDevice> &media); - std::string lookupDeviceNode(int major, int minor) final; + std::string lookupDeviceNode(dev_t devnum); int addV4L2Device(dev_t devnum); void udevNotify(EventNotifier *notifier);
The lookupDeviceNode() method is declared as pure virtual in the base DeviceEnumerator class, but is only called by derived classes. Move it to the DeviceEnumeratorSysfs and DeviceEnumeratorUdev. This allows changing the udev version to take a dev_t instead of separate major/minor, as that's what both the caller and the callee end up using. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/device_enumerator.cpp | 13 ------------- src/libcamera/device_enumerator_sysfs.cpp | 11 +++++++++++ src/libcamera/device_enumerator_udev.cpp | 19 ++++++++++++------- src/libcamera/include/device_enumerator.h | 2 -- .../include/device_enumerator_sysfs.h | 2 +- .../include/device_enumerator_udev.h | 2 +- 6 files changed, 25 insertions(+), 24 deletions(-)