| Message ID | 20190414013506.10515-4-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Sun, Apr 14, 2019 at 03:34:58AM +0200, Niklas Söderlund wrote: > There is no reason to reread the MEDIA_IOC_DEVICE_INFO information every > time the media device is opened. Move it populate() where it will be > read once together the other information about the media device. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/media_device.cpp | 31 ++++++++++++++----------------- > 1 file changed, 14 insertions(+), 17 deletions(-) > > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > index ecb00a1d5abffe80..54706bb73a7591d5 100644 > --- a/src/libcamera/media_device.cpp > +++ b/src/libcamera/media_device.cpp > @@ -131,9 +131,6 @@ bool MediaDevice::acquire() > * with the device node associated with the media device, the device node must > * be opened. > * > - * This function also retrieves media device information from the device node, > - * which can be queried through driver(). This should go to the documentation of the populate() function (in a modified form to integrate nicely with it). > - * > * If the device is already open the function returns -EBUSY. > * > * \return 0 on success or a negative error code otherwise > @@ -155,20 +152,6 @@ int MediaDevice::open() > } > fd_ = ret; > > - struct media_device_info info = { }; > - ret = ioctl(fd_, MEDIA_IOC_DEVICE_INFO, &info); > - if (ret) { > - ret = -errno; > - LOG(MediaDevice, Error) > - << "Failed to get media device info " > - << ": " << strerror(-ret); > - return ret; > - } > - > - driver_ = info.driver; > - model_ = info.model; > - version_ = info.media_version; > - > return 0; > } > > @@ -224,6 +207,20 @@ int MediaDevice::populate() > if (ret) > return ret; > > + struct media_device_info info = {}; > + ret = ioctl(fd_, MEDIA_IOC_DEVICE_INFO, &info); > + if (ret) { > + ret = -errno; > + LOG(MediaDevice, Error) > + << "Failed to get media device info " While at it you could merge the ": " wit the previous line (and remove the space after "info"). Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + << ": " << strerror(-ret); > + goto err_out; > + } > + > + driver_ = info.driver; > + model_ = info.model; > + version_ = info.media_version; > + > /* > * Keep calling G_TOPOLOGY until the version number stays stable. > */
diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index ecb00a1d5abffe80..54706bb73a7591d5 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -131,9 +131,6 @@ bool MediaDevice::acquire() * with the device node associated with the media device, the device node must * be opened. * - * This function also retrieves media device information from the device node, - * which can be queried through driver(). - * * If the device is already open the function returns -EBUSY. * * \return 0 on success or a negative error code otherwise @@ -155,20 +152,6 @@ int MediaDevice::open() } fd_ = ret; - struct media_device_info info = { }; - ret = ioctl(fd_, MEDIA_IOC_DEVICE_INFO, &info); - if (ret) { - ret = -errno; - LOG(MediaDevice, Error) - << "Failed to get media device info " - << ": " << strerror(-ret); - return ret; - } - - driver_ = info.driver; - model_ = info.model; - version_ = info.media_version; - return 0; } @@ -224,6 +207,20 @@ int MediaDevice::populate() if (ret) return ret; + struct media_device_info info = {}; + ret = ioctl(fd_, MEDIA_IOC_DEVICE_INFO, &info); + if (ret) { + ret = -errno; + LOG(MediaDevice, Error) + << "Failed to get media device info " + << ": " << strerror(-ret); + goto err_out; + } + + driver_ = info.driver; + model_ = info.model; + version_ = info.media_version; + /* * Keep calling G_TOPOLOGY until the version number stays stable. */
There is no reason to reread the MEDIA_IOC_DEVICE_INFO information every time the media device is opened. Move it populate() where it will be read once together the other information about the media device. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/media_device.cpp | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-)