[libcamera-devel] [PATCH v2 3/7] libcamera: camera_sensor: Discover VCMs through ancillary links

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Fri, Dec 03, 2021 at 10:42:26PM +0000, Daniel Scally wrote:
> Add a function to check for and initialise any VCMs linked to the
> CameraSensor's entity by ancillary links. This should initialise
> the lens_ member with the linked entity. Call the new function
> during CameraSensor::init().
> 
> Signed-off-by: Daniel Scally <djrscally at gmail.com>
> ---
> Changes in v2:
> 
> 	- Moved this functionality to CameraSensor::init() instead of the
> 	pipeline's ::configure() (Laurent)
> 
>  include/libcamera/internal/camera_sensor.h |  1 +
>  src/libcamera/camera_sensor.cpp            | 42 ++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 788c2fa3..d329c512 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -76,6 +76,7 @@ private:
>  	void initTestPatternModes(
>  		const std::map<int32_t, int32_t> &testPatternModeMap);
>  	int initProperties();
> +	int discoverAncillaryDevices();
>  
>  	const MediaEntity *entity_;
>  	std::unique_ptr<V4L2Subdevice> subdev_;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 4c142a58..e1d4d424 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -161,6 +161,10 @@ int CameraSensor::init()
>  	if (ret)
>  		return ret;
>  
> +	ret = discoverAncillaryDevices();
> +	if (ret)
> +		return ret;
> +
>  	return 0;
>  }
>  
> @@ -423,6 +427,44 @@ int CameraSensor::initProperties()
>  	return 0;
>  }
>  
> +/**
> + * \brief Check for and initialise any ancillary devices
> + *
> + * Sensors sometimes have ancillary devices such as a Lens or Flash that could
> + * be linked to their MediaEntity by the kernel. Search for and handle any
> + * such device.
> + *
> + * \todo Handle MEDIA_ENT_F_FLASH too.
> + */
> +int CameraSensor::discoverAncillaryDevices()
> +{
> +	int ret;
> +
> +	for (MediaEntity *ancillary : entity_->ancillaryEntities()) {
> +
> +		switch (ancillary->function()) {
> +		case MEDIA_ENT_F_LENS:
> +			focusLens_ = std::make_unique<CameraLens>(ancillary);
> +			ret = focusLens_->init();
> +			if (ret) {
> +				LOG(CameraSensor, Error)
> +					<< "Error during CameraLens init";

I'd write "CameraLens initialization failed" (although we don't really
have rules when it comes to error messages).

> +				return ret;
> +			}
> +			break;

Missing blank line.

> +		case MEDIA_ENT_F_FLASH:
> +			LOG(CameraSensor, Warning)
> +				<< "Flash devices not yet supported";
> +			break;

Same here.

> +		default:
> +			LOG(CameraSensor, Warning)
> +				<< "Unsupported ancillary entity function";

It could be useful to print the function. You could then possibly drop
the flash case as it will be caught here.

> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * \fn CameraSensor::model()
>   * \brief Retrieve the sensor model name

-- 
Regards,

Laurent Pinchart