Re: [PATCH v2 2/5] media: v4l: ctrls: Add a control for temperature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

adding linux-hwmon in CC for a wider feedback.

Le vendredi 15 avril 2022 à 13:18 +0200, Benjamin Mugnier a écrit :
> Add V4L2_CID_TEMPERATURE control to get temperature from sensor in
> celsius as a volatile and read-only control, and its documentation.
> Useful to monitor thermals from v4l controls for sensors that support
> this.

Any justification to expose a temperature sensor outside of the dedicated kernel
API hwmon ? I know if may makes it harder to use, as the sensor isn't bound to
the camera driver, and also the sensor may not work if the camera is not
streaming. But in the end, the API in hwmon does not look that complex, and is
perhaps more precise ?

All in all, I think we need a strong justification to implement a custom
thermometer interface, something described here and documented with care to
prevent abuse. I would also see opinion from folks outside of the linux-media,
hence why I have CCed hwmon mailing list.

regards,
Nicolas

> 
> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@xxxxxxxxxxx>
> ---
>  Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst | 3 +++
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c                  | 4 ++++
>  include/uapi/linux/v4l2-controls.h                         | 2 ++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> index 4c5061aa9cd4..26fa21f5c45a 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst
> @@ -661,3 +661,6 @@ enum v4l2_scene_mode -
>  .. [#f1]
>     This control may be changed to a menu control in the future, if more
>     options are required.
> +
> +``V4L2_CID_TEMPERATURE (integer)``
> +    The temperature of the sensor in celsius. This is a read-only control.
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 54ca4e6b820b..45ad3edd59e0 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1042,6 +1042,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_UNIT_CELL_SIZE:		return "Unit Cell Size";
>  	case V4L2_CID_CAMERA_ORIENTATION:	return "Camera Orientation";
>  	case V4L2_CID_CAMERA_SENSOR_ROTATION:	return "Camera Sensor Rotation";
> +	case V4L2_CID_TEMPERATURE:		return "Temperature in °C";
>  
>  	/* FM Radio Modulator controls */
>  	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
> @@ -1597,6 +1598,9 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_RF_TUNER_PLL_LOCK:
>  		*flags |= V4L2_CTRL_FLAG_VOLATILE;
>  		break;
> +	case V4L2_CID_TEMPERATURE:
> +		*flags |= V4L2_CTRL_FLAG_READ_ONLY |
> +			  V4L2_CTRL_FLAG_VOLATILE;
>  	}
>  }
>  EXPORT_SYMBOL(v4l2_ctrl_fill);
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index bb40129446d4..705b4043c2de 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1008,6 +1008,8 @@ enum v4l2_auto_focus_range {
>  
>  #define V4L2_CID_CAMERA_SENSOR_ROTATION		(V4L2_CID_CAMERA_CLASS_BASE+35)
>  
> +#define V4L2_CID_TEMPERATURE			(V4L2_CID_CAMERA_CLASS_BASE+36)
> +
>  /* FM Modulator class control IDs */
>  
>  #define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)





[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux