linux-kernel - Re: [PATCH v2 04/32] media: v4l2-common: Add helper function media_bus_fmt_to_csi2

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] [day] [month] [year] [list]

Message-ID: <20250820191651.GB1722@pendragon.ideasonboard.com>
Date: Wed, 20 Aug 2025 22:16:51 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Frank Li <Frank.Li@....com>
Cc: Rui Miguel Silva <rmfrfs@...il.com>,
	Martin Kepplinger <martink@...teo.de>,
	Purism Kernel Team <kernel@...i.sm>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Eugen Hristev <eugen.hristev@...aro.org>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>, Peng Fan <peng.fan@....com>,
	Alice Yuan <alice.yuan@....com>, Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Steve Longerbeam <slongerbeam@...il.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-media@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, linux-phy@...ts.infradead.org,
	linux-staging@...ts.linux.dev
Subject: Re: [PATCH v2 04/32] media: v4l2-common: Add helper function
 media_bus_fmt_to_csi2_dt()

Hi Frank,

Thank you for the patch.

On Fri, Aug 08, 2025 at 06:39:07PM -0400, Frank Li wrote:
> CSI2 data type is defined by MIPI Camera Serial Interface 2 Spec Ver4.1.
> See section 9.4.
> 
> Add helper function media_bus_fmt_to_csi2_dt() to convert media bus fmt to
> MIPI defined data type and avoid below duplicated static array in each CSI2
> drivers.
> 
> 	{
> 		.code = MEDIA_BUS_FMT_UYVY8_1X16,
> 		.data_type = MIPI_CSI2_DT_YUV422_8B,
> 	}
> 
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 44 +++++++++++++++++++++++++++++++++++
>  include/media/mipi-csi2.h             |  3 +++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 3a48b6a55c6e322696b910dd519def4f0b4a58fb..fcc01030beb347499da2a3c8539793d20f6f512c 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -701,6 +701,50 @@ int v4l2_link_freq_to_bitmap(struct device *dev, const u64 *fw_link_freqs,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_link_freq_to_bitmap);
>  
> +int media_bus_fmt_to_csi2_dt(int bus_fmt)
> +{
> +	switch (bus_fmt) {
> +	case MEDIA_BUS_FMT_UYVY8_1X16:
> +		return MIPI_CSI2_DT_YUV422_8B;
> +	case MEDIA_BUS_FMT_RGB565_1X16:
> +		return MIPI_CSI2_DT_RGB565;
> +	case MEDIA_BUS_FMT_BGR888_1X24:
> +		return MIPI_CSI2_DT_RGB888;
> +	case MEDIA_BUS_FMT_SBGGR8_1X8:
> +	case MEDIA_BUS_FMT_SGBRG8_1X8:
> +	case MEDIA_BUS_FMT_SGRBG8_1X8:
> +	case MEDIA_BUS_FMT_SRGGB8_1X8:
> +	case MEDIA_BUS_FMT_Y8_1X8:
> +		return MIPI_CSI2_DT_RAW8;
> +	case MEDIA_BUS_FMT_SBGGR10_1X10:
> +	case MEDIA_BUS_FMT_SGBRG10_1X10:
> +	case MEDIA_BUS_FMT_SGRBG10_1X10:
> +	case MEDIA_BUS_FMT_SRGGB10_1X10:
> +	case MEDIA_BUS_FMT_Y10_1X10:
> +		return MIPI_CSI2_DT_RAW10;
> +	case MEDIA_BUS_FMT_SBGGR12_1X12:
> +	case MEDIA_BUS_FMT_SGBRG12_1X12:
> +	case MEDIA_BUS_FMT_SGRBG12_1X12:
> +	case MEDIA_BUS_FMT_SRGGB12_1X12:
> +	case MEDIA_BUS_FMT_Y12_1X12:
> +		return MIPI_CSI2_DT_RAW12;
> +	case MEDIA_BUS_FMT_SBGGR14_1X14:
> +	case MEDIA_BUS_FMT_SGBRG14_1X14:
> +	case MEDIA_BUS_FMT_SGRBG14_1X14:
> +	case MEDIA_BUS_FMT_SRGGB14_1X14:
> +		return MIPI_CSI2_DT_RAW14;
> +	case MEDIA_BUS_FMT_SBGGR16_1X16:
> +	case MEDIA_BUS_FMT_SGBRG16_1X16:
> +	case MEDIA_BUS_FMT_SGRBG16_1X16:
> +	case MEDIA_BUS_FMT_SRGGB16_1X16:
> +		return MIPI_CSI2_DT_RAW16;
> +
> +	default:
> +		return MIPI_CSI2_DT_NULL;

I would use 0 here. MIPI_CSI2_DT_NULL is equal to 0x10, and I'm
concerned about driver comparing the return value of the function to 0.

> +	}
> +}
> +EXPORT_SYMBOL_GPL(media_bus_fmt_to_csi2_dt);
> +

We have something similar for pixel formats, with the v4l2_format_info
structure and the v4l2_format_info() lookup function. Adding information
about media bus formats to the V4L2 core seems to be a good idea.

Regarding the implementation, I think you should mimick what we do for
the pixel formats, and add the data to an info structure with a lookup
function.

>  int media_bus_fmt_to_csi2_bpp(int bus_fmt)
>  {
>  	switch (bus_fmt) {
> diff --git a/include/media/mipi-csi2.h b/include/media/mipi-csi2.h
> index c5b4e8e1ca93803568b1eee85f7f99c3a45a0b6e..35de536b9b65b49ad9e2914437d26d0e4240cf38 100644
> --- a/include/media/mipi-csi2.h
> +++ b/include/media/mipi-csi2.h
> @@ -8,6 +8,8 @@
>  #ifndef _MEDIA_MIPI_CSI2_H
>  #define _MEDIA_MIPI_CSI2_H
>  
> +/* DT value ref to MIPI Camera Serial Interface 2 Spec Ver4.1 section 9.4 */
> +
>  /* Short packet data types */
>  #define MIPI_CSI2_DT_FS			0x00
>  #define MIPI_CSI2_DT_FE			0x01
> @@ -44,6 +46,7 @@
>  #define MIPI_CSI2_DT_RAW20		0x2f
>  #define MIPI_CSI2_DT_USER_DEFINED(n)	(0x30 + (n))	/* 0..7 */
>  
> +int media_bus_fmt_to_csi2_dt(int buf_fmt);
>  int media_bus_fmt_to_csi2_bpp(int bus_fmt);
>  
>  #endif /* _MEDIA_MIPI_CSI2_H */
> 
> 

-- 
Regards,

Laurent Pinchart