| Message ID | 20250121130037.237947-1-dan.scally@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Dan, Thank you for the patch. On Tue, Jan 21, 2025 at 01:00:37PM +0000, Daniel Scally wrote: > In the ::init() function there are two places that return values they MaliC55CameraData::init() ? ::init() in C++ means global namespace. > shouldn't; the ret variable is returned after checking a pointer is > not null instead of an explicit -ENODEV and later the boolean value > false is returned on failure instead of the error value returned by > V4L2Subdevice::open() - fix both problems. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > index 5abd6b20..6aa2f2d9 100644 > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > @@ -159,15 +159,16 @@ int MaliC55CameraData::init() > */ > sensor_ = CameraSensorFactoryBase::create(entity_); > if (!sensor_) > - return ret; > + return -ENODEV; > > const MediaPad *sourcePad = entity_->getPadByIndex(0); > MediaEntity *csiEntity = sourcePad->links()[0]->sink()->entity(); > > csi_ = std::make_unique<V4L2Subdevice>(csiEntity); > - if (csi_->open()) { > + ret = csi_->open(); > + if (ret) { > LOG(MaliC55, Error) << "Failed to open CSI-2 subdevice"; > - return false; > + return ret; > } > > return 0;
Hi Laurent On 21/01/2025 13:18, Laurent Pinchart wrote: > Hi Dan, > > Thank you for the patch. > > On Tue, Jan 21, 2025 at 01:00:37PM +0000, Daniel Scally wrote: >> In the ::init() function there are two places that return values they > MaliC55CameraData::init() ? ::init() in C++ means global namespace. Yes, perhaps that wasn't clear - my bad. > >> shouldn't; the ret variable is returned after checking a pointer is >> not null instead of an explicit -ENODEV and later the boolean value >> false is returned on failure instead of the error value returned by >> V4L2Subdevice::open() - fix both problems. >> >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks Dan > >> --- >> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >> index 5abd6b20..6aa2f2d9 100644 >> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp >> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >> @@ -159,15 +159,16 @@ int MaliC55CameraData::init() >> */ >> sensor_ = CameraSensorFactoryBase::create(entity_); >> if (!sensor_) >> - return ret; >> + return -ENODEV; >> >> const MediaPad *sourcePad = entity_->getPadByIndex(0); >> MediaEntity *csiEntity = sourcePad->links()[0]->sink()->entity(); >> >> csi_ = std::make_unique<V4L2Subdevice>(csiEntity); >> - if (csi_->open()) { >> + ret = csi_->open(); >> + if (ret) { >> LOG(MaliC55, Error) << "Failed to open CSI-2 subdevice"; >> - return false; >> + return ret; >> } >> >> return 0;
diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index 5abd6b20..6aa2f2d9 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -159,15 +159,16 @@ int MaliC55CameraData::init() */ sensor_ = CameraSensorFactoryBase::create(entity_); if (!sensor_) - return ret; + return -ENODEV; const MediaPad *sourcePad = entity_->getPadByIndex(0); MediaEntity *csiEntity = sourcePad->links()[0]->sink()->entity(); csi_ = std::make_unique<V4L2Subdevice>(csiEntity); - if (csi_->open()) { + ret = csi_->open(); + if (ret) { LOG(MaliC55, Error) << "Failed to open CSI-2 subdevice"; - return false; + return ret; } return 0;
In the ::init() function there are two places that return values they shouldn't; the ret variable is returned after checking a pointer is not null instead of an explicit -ENODEV and later the boolean value false is returned on failure instead of the error value returned by V4L2Subdevice::open() - fix both problems. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)