| Message ID | 20200804161358.1628962-5-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Tue, Aug 04, 2020 at 06:13:53PM +0200, Niklas Söderlund wrote: > The ID is generated from information in the firmware description of the > sensor if available or from module and model information if the sensor > is virtual (for example VIMC). > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > * Changes sinve v6 > - Do not allow for subclassing > - Add support for virtual sensors > - Use new sysfs:: helper. > > * Changes since v5 > - Use new utils:: helper. > - Allow for subclassing > > * Changes since v4 > - Fix spelling. > > * Changes since v3 > - Update commit message. > - Add description of how ID are generated to comment. > --- > include/libcamera/internal/camera_sensor.h | 3 ++ > src/libcamera/camera_sensor.cpp | 38 ++++++++++++++++++++++ > 2 files changed, 41 insertions(+) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index 06c8292ca30129de..0f65fca46877e89a 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -67,11 +67,14 @@ protected: > std::string logPrefix() const override; > > private: > + int generateId(); > + > const MediaEntity *entity_; > std::unique_ptr<V4L2Subdevice> subdev_; > unsigned int pad_; > > std::string model_; > + std::string id_; > > V4L2Subdevice::Formats formats_; > Size resolution_; > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 350f49accad99c7b..3952d16b053ae20e 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -9,6 +9,7 @@ > > #include <algorithm> > #include <float.h> > +#include <fstream> > #include <iomanip> > #include <limits.h> > #include <math.h> > @@ -16,7 +17,9 @@ > > #include <libcamera/property_ids.h> > > +#include "libcamera/internal/file.h" > #include "libcamera/internal/formats.h" > +#include "libcamera/internal/sysfs.h" > #include "libcamera/internal/utils.h" > > /** > @@ -204,6 +207,11 @@ int CameraSensor::init() > if (ret < 0) > return ret; > > + /* Generate a unique ID for the sensor. */ > + ret = generateId(); > + if (ret) > + return ret; > + > /* Retrieve and store the camera sensor properties. */ > const ControlInfoMap &controls = subdev_->controls(); > int32_t propertyValue; > @@ -541,4 +549,34 @@ std::string CameraSensor::logPrefix() const > return "'" + entity_->name() + "'"; > } > > +int CameraSensor::generateId() > +{ > + const std::string devPath = subdev_->devicePath(); > + > + /* Try to get ID from firmware description. */ > + if (!sysfs::firmwareId(devPath, &id_)) > + return 0; > + > + /* Virtual sensors not described in firmware, use modalias and model. */ > + std::string path = devPath + "/modalias"; > + if (File::exists(path)) { > + std::ifstream file(path.c_str()); > + > + if (!file.is_open()) { > + LOG(CameraSensor, Error) << "Failed to read modalias"; > + return -EINVAL; > + } > + > + std::string modalias; > + std::getline(file, modalias); > + file.close(); > + > + id_ = modalias + " " + model(); > + return 0; > + } > + > + LOG(CameraSensor, Error) << "Can't generate sensor ID"; > + return -EINVAL; > +} > + > } /* namespace libcamera */
Hi Niklas, Another comment I'm afraid. On Tue, Aug 04, 2020 at 09:22:12PM +0300, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Tue, Aug 04, 2020 at 06:13:53PM +0200, Niklas Söderlund wrote: > > The ID is generated from information in the firmware description of the > > sensor if available or from module and model information if the sensor > > is virtual (for example VIMC). > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > * Changes sinve v6 > > - Do not allow for subclassing > > - Add support for virtual sensors > > - Use new sysfs:: helper. > > > > * Changes since v5 > > - Use new utils:: helper. > > - Allow for subclassing > > > > * Changes since v4 > > - Fix spelling. > > > > * Changes since v3 > > - Update commit message. > > - Add description of how ID are generated to comment. > > --- > > include/libcamera/internal/camera_sensor.h | 3 ++ > > src/libcamera/camera_sensor.cpp | 38 ++++++++++++++++++++++ > > 2 files changed, 41 insertions(+) > > > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > > index 06c8292ca30129de..0f65fca46877e89a 100644 > > --- a/include/libcamera/internal/camera_sensor.h > > +++ b/include/libcamera/internal/camera_sensor.h > > @@ -67,11 +67,14 @@ protected: > > std::string logPrefix() const override; > > > > private: > > + int generateId(); > > + > > const MediaEntity *entity_; > > std::unique_ptr<V4L2Subdevice> subdev_; > > unsigned int pad_; > > > > std::string model_; > > + std::string id_; > > > > V4L2Subdevice::Formats formats_; > > Size resolution_; > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > > index 350f49accad99c7b..3952d16b053ae20e 100644 > > --- a/src/libcamera/camera_sensor.cpp > > +++ b/src/libcamera/camera_sensor.cpp > > @@ -9,6 +9,7 @@ > > > > #include <algorithm> > > #include <float.h> > > +#include <fstream> > > #include <iomanip> > > #include <limits.h> > > #include <math.h> > > @@ -16,7 +17,9 @@ > > > > #include <libcamera/property_ids.h> > > > > +#include "libcamera/internal/file.h" > > #include "libcamera/internal/formats.h" > > +#include "libcamera/internal/sysfs.h" > > #include "libcamera/internal/utils.h" > > > > /** > > @@ -204,6 +207,11 @@ int CameraSensor::init() > > if (ret < 0) > > return ret; > > > > + /* Generate a unique ID for the sensor. */ > > + ret = generateId(); > > + if (ret) > > + return ret; > > + > > /* Retrieve and store the camera sensor properties. */ > > const ControlInfoMap &controls = subdev_->controls(); > > int32_t propertyValue; > > @@ -541,4 +549,34 @@ std::string CameraSensor::logPrefix() const > > return "'" + entity_->name() + "'"; > > } > > > > +int CameraSensor::generateId() > > +{ > > + const std::string devPath = subdev_->devicePath(); > > + > > + /* Try to get ID from firmware description. */ > > + if (!sysfs::firmwareId(devPath, &id_)) > > + return 0; > > + > > + /* Virtual sensors not described in firmware, use modalias and model. */ > > + std::string path = devPath + "/modalias"; > > + if (File::exists(path)) { > > + std::ifstream file(path.c_str()); > > + > > + if (!file.is_open()) { > > + LOG(CameraSensor, Error) << "Failed to read modalias"; > > + return -EINVAL; > > + } > > + > > + std::string modalias; > > + std::getline(file, modalias); > > + file.close(); > > + > > + id_ = modalias + " " + model(); > > + return 0; > > + } This won't generate a unique name if the same driver handles multiple devices. Would it make sense to instead use devPath stripped of the /sys/devices/ prefix ? That would return platform/vimc.0 for vimc for instance. If you want to restrict this to platform devices, you could further check that the ID starts with platform/. > > + > > + LOG(CameraSensor, Error) << "Can't generate sensor ID"; > > + return -EINVAL; > > +} > > + > > } /* namespace libcamera */
Hi Niklas, On Tue, Aug 04, 2020 at 06:13:53PM +0200, Niklas Söderlund wrote: > The ID is generated from information in the firmware description of the > sensor if available or from module and model information if the sensor > is virtual (for example VIMC). > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > * Changes sinve v6 > - Do not allow for subclassing > - Add support for virtual sensors > - Use new sysfs:: helper. > > * Changes since v5 > - Use new utils:: helper. > - Allow for subclassing > > * Changes since v4 > - Fix spelling. > > * Changes since v3 > - Update commit message. > - Add description of how ID are generated to comment. > --- > include/libcamera/internal/camera_sensor.h | 3 ++ > src/libcamera/camera_sensor.cpp | 38 ++++++++++++++++++++++ > 2 files changed, 41 insertions(+) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index 06c8292ca30129de..0f65fca46877e89a 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -67,11 +67,14 @@ protected: > std::string logPrefix() const override; > > private: > + int generateId(); > + > const MediaEntity *entity_; > std::unique_ptr<V4L2Subdevice> subdev_; > unsigned int pad_; > > std::string model_; > + std::string id_; > > V4L2Subdevice::Formats formats_; > Size resolution_; > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 350f49accad99c7b..3952d16b053ae20e 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -9,6 +9,7 @@ > > #include <algorithm> > #include <float.h> > +#include <fstream> > #include <iomanip> > #include <limits.h> > #include <math.h> > @@ -16,7 +17,9 @@ > > #include <libcamera/property_ids.h> > > +#include "libcamera/internal/file.h" > #include "libcamera/internal/formats.h" > +#include "libcamera/internal/sysfs.h" > #include "libcamera/internal/utils.h" > > /** > @@ -204,6 +207,11 @@ int CameraSensor::init() > if (ret < 0) > return ret; > > + /* Generate a unique ID for the sensor. */ > + ret = generateId(); > + if (ret) > + return ret; > + > /* Retrieve and store the camera sensor properties. */ > const ControlInfoMap &controls = subdev_->controls(); > int32_t propertyValue; > @@ -541,4 +549,34 @@ std::string CameraSensor::logPrefix() const > return "'" + entity_->name() + "'"; > } > > +int CameraSensor::generateId() > +{ > + const std::string devPath = subdev_->devicePath(); > + > + /* Try to get ID from firmware description. */ > + if (!sysfs::firmwareId(devPath, &id_)) > + return 0; > + > + /* Virtual sensors not described in firmware, use modalias and model. */ > + std::string path = devPath + "/modalias"; > + if (File::exists(path)) { > + std::ifstream file(path.c_str()); > + > + if (!file.is_open()) { > + LOG(CameraSensor, Error) << "Failed to read modalias"; > + return -EINVAL; > + } > + > + std::string modalias; > + std::getline(file, modalias); > + file.close(); > + > + id_ = modalias + " " + model(); > + return 0; > + } > + > + LOG(CameraSensor, Error) << "Can't generate sensor ID"; > + return -EINVAL; > +} > + > } /* namespace libcamera */ > -- > 2.28.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index 06c8292ca30129de..0f65fca46877e89a 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -67,11 +67,14 @@ protected: std::string logPrefix() const override; private: + int generateId(); + const MediaEntity *entity_; std::unique_ptr<V4L2Subdevice> subdev_; unsigned int pad_; std::string model_; + std::string id_; V4L2Subdevice::Formats formats_; Size resolution_; diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 350f49accad99c7b..3952d16b053ae20e 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -9,6 +9,7 @@ #include <algorithm> #include <float.h> +#include <fstream> #include <iomanip> #include <limits.h> #include <math.h> @@ -16,7 +17,9 @@ #include <libcamera/property_ids.h> +#include "libcamera/internal/file.h" #include "libcamera/internal/formats.h" +#include "libcamera/internal/sysfs.h" #include "libcamera/internal/utils.h" /** @@ -204,6 +207,11 @@ int CameraSensor::init() if (ret < 0) return ret; + /* Generate a unique ID for the sensor. */ + ret = generateId(); + if (ret) + return ret; + /* Retrieve and store the camera sensor properties. */ const ControlInfoMap &controls = subdev_->controls(); int32_t propertyValue; @@ -541,4 +549,34 @@ std::string CameraSensor::logPrefix() const return "'" + entity_->name() + "'"; } +int CameraSensor::generateId() +{ + const std::string devPath = subdev_->devicePath(); + + /* Try to get ID from firmware description. */ + if (!sysfs::firmwareId(devPath, &id_)) + return 0; + + /* Virtual sensors not described in firmware, use modalias and model. */ + std::string path = devPath + "/modalias"; + if (File::exists(path)) { + std::ifstream file(path.c_str()); + + if (!file.is_open()) { + LOG(CameraSensor, Error) << "Failed to read modalias"; + return -EINVAL; + } + + std::string modalias; + std::getline(file, modalias); + file.close(); + + id_ = modalias + " " + model(); + return 0; + } + + LOG(CameraSensor, Error) << "Can't generate sensor ID"; + return -EINVAL; +} + } /* namespace libcamera */
The ID is generated from information in the firmware description of the sensor if available or from module and model information if the sensor is virtual (for example VIMC). Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- * Changes sinve v6 - Do not allow for subclassing - Add support for virtual sensors - Use new sysfs:: helper. * Changes since v5 - Use new utils:: helper. - Allow for subclassing * Changes since v4 - Fix spelling. * Changes since v3 - Update commit message. - Add description of how ID are generated to comment. --- include/libcamera/internal/camera_sensor.h | 3 ++ src/libcamera/camera_sensor.cpp | 38 ++++++++++++++++++++++ 2 files changed, 41 insertions(+)