linux-kernel - Re: [RFC PATCH v4 09/11] media: uapi: Add V4L2_CID_USER_IMX_ASRC_RATIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fbedcbf1-d925-47d6-b9fb-c9e15263c117@xs4all.nl>
Date:   Wed, 20 Sep 2023 12:19:16 +0200
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Shengjiu Wang <shengjiu.wang@....com>, sakari.ailus@....fi,
        tfiga@...omium.org, m.szyprowski@...sung.com, mchehab@...nel.org,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        shengjiu.wang@...il.com, Xiubo.Lee@...il.com, festevam@...il.com,
        nicoleotsuka@...il.com, lgirdwood@...il.com, broonie@...nel.org,
        perex@...ex.cz, tiwai@...e.com, alsa-devel@...a-project.org,
        linuxppc-dev@...ts.ozlabs.org
Subject: Re: [RFC PATCH v4 09/11] media: uapi: Add
 V4L2_CID_USER_IMX_ASRC_RATIO_MOD control

On 20/09/2023 11:32, Shengjiu Wang wrote:
> The input clock and output clock may not be the accurate
> rate as the sample rate, there is some drift, so the convert
> ratio of i.MX ASRC module need to be changed according to
> actual clock rate.
> 
> Add V4L2_CID_USER_IMX_ASRC_RATIO_MOD control for user to
> adjust the ratio.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> ---
>  Documentation/userspace-api/media/v4l/control.rst | 5 +++++
>  drivers/media/v4l2-core/v4l2-ctrls-defs.c         | 1 +
>  include/uapi/linux/v4l2-controls.h                | 1 +
>  3 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/control.rst b/Documentation/userspace-api/media/v4l/control.rst
> index 4463fce694b0..2bc175900a34 100644
> --- a/Documentation/userspace-api/media/v4l/control.rst
> +++ b/Documentation/userspace-api/media/v4l/control.rst
> @@ -318,6 +318,11 @@ Control IDs
>      depending on particular custom controls should check the driver name
>      and version, see :ref:`querycap`.
>  
> +.. _v4l2-audio-imx:
> +
> +``V4L2_CID_USER_IMX_ASRC_RATIO_MOD``
> +    sets the rasampler ratio modifier of i.MX asrc module.

rasampler -> resampler (I think?)

This doesn't document at all what the type of the control is or how to interpret it.

> +
>  Applications can enumerate the available controls with the
>  :ref:`VIDIOC_QUERYCTRL` and
>  :ref:`VIDIOC_QUERYMENU <VIDIOC_QUERYCTRL>` ioctls, get and set a
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index 8696eb1cdd61..16f66f66198c 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -1242,6 +1242,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_COLORIMETRY_CLASS:	return "Colorimetry Controls";
>  	case V4L2_CID_COLORIMETRY_HDR10_CLL_INFO:		return "HDR10 Content Light Info";
>  	case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:	return "HDR10 Mastering Display";
> +	case V4L2_CID_USER_IMX_ASRC_RATIO_MOD:			return "ASRC RATIO MOD";

Let's stay consistent with the other control names:

"ASRC Ratio Modifier"

But if this is a driver specific control, then this doesn't belong here.

Driver specific controls are defined in the driver itself, including this
description.

Same for the control documentation: if it is driver specific, then that
typically is documented either in a driver-specific public header, or
possibly in driver-specific documentation (Documentation/admin-guide/media/).

But is this imx specific? Wouldn't other similar devices need this?

>  	default:
>  		return NULL;
>  	}
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index c3604a0a3e30..b1c319906d12 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -162,6 +162,7 @@ enum v4l2_colorfx {
>  /* The base for the imx driver controls.
>   * We reserve 16 controls for this driver. */
>  #define V4L2_CID_USER_IMX_BASE			(V4L2_CID_USER_BASE + 0x10b0)
> +#define V4L2_CID_USER_IMX_ASRC_RATIO_MOD	(V4L2_CID_USER_IMX_BASE + 0)
>  
>  /*
>   * The base for the atmel isc driver controls.

Regards,

	Hans