linux-kernel - Re: [PATCH v10 13/14] media: imx-asrc: Add memory to memory driver

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]

Date:   Tue, 21 Nov 2023 10:20:10 +0100
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: [PATCH v10 13/14] media: imx-asrc: Add memory to memory driver

On 21/11/2023 08:37, Shengjiu Wang wrote:
> Implement the ASRC memory to memory function using
> the v4l2 framework, user can use this function with
> v4l2 ioctl interface.
> 
> User send the output and capture buffer to driver and
> driver store the converted data to the capture buffer.
> 
> This feature can be shared by ASRC and EASRC drivers
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> ---
>  drivers/media/platform/nxp/Kconfig    |   13 +
>  drivers/media/platform/nxp/Makefile   |    1 +
>  drivers/media/platform/nxp/imx-asrc.c | 1264 +++++++++++++++++++++++++
>  3 files changed, 1278 insertions(+)
>  create mode 100644 drivers/media/platform/nxp/imx-asrc.c
> 

<snip>

> +static const struct v4l2_ctrl_config asrc_src_rate_off_control = {
> +	.ops = &asrc_m2m_ctrl_ops,
> +	.id = V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET,
> +	.name = "Audio Source Sample Rate Offset",
> +	.type = V4L2_CTRL_TYPE_INTEGER64,

Do you need this to be INTEGER64? If the actual number of bits
of the fixed point value that the hardware uses fits in 32 bits,
then you can just use the INTEGER type.

> +	.min = 0xFFFFFF0000000000,
> +	.max = 0x7fffffffff,
> +	.def = 0,
> +	.step = 1,
> +	.flags = V4L2_CTRL_FLAG_UPDATE,
> +	.fraction_bits = 32,

Can't you use the actual number of fraction bits that the hardware
uses? I can't imagine that it is actually 32 bits, it is almost
certainly less.

I do think that we need a helper function to fill in the min/max values.

Regards,

	Hans

> +};
> +
> +static const struct v4l2_ctrl_config asrc_dst_rate_off_control = {
> +	.ops = &asrc_m2m_ctrl_ops,
> +	.id = V4L2_CID_M2M_AUDIO_DEST_RATE_OFFSET,
> +	.name = "Audio Dest Sample Rate Offset",
> +	.type = V4L2_CTRL_TYPE_INTEGER64,
> +	.min = 0xFFFFFF0000000000,
> +	.max = 0x7fffffffff,
> +	.def = 0,
> +	.step = 1,
> +	.flags = V4L2_CTRL_FLAG_UPDATE,
> +	.fraction_bits = 32,
> +};