linux-kernel - Re: [PATCH 5/5] arm64: dts: imx8qxp-mek: add parallel ov5640 camera support

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: <aGP56BUXrNnBCQu/@lizhi-Precision-Tower-5810>
Date: Tue, 1 Jul 2025 11:08:24 -0400
From: Frank Li <Frank.li@....com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	Rui Miguel Silva <rmfrfs@...il.com>,
	Martin Kepplinger <martink@...teo.de>,
	Purism Kernel Team <kernel@...i.sm>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Philipp Zabel <p.zabel@...gutronix.de>, linux-media@...r.kernel.org,
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 5/5] arm64: dts: imx8qxp-mek: add parallel ov5640 camera
 support

On Tue, Jul 01, 2025 at 02:06:51AM +0300, Laurent Pinchart wrote:
> Hi Frank,
>
> Thank you for the patch.
>
> On Mon, Jun 30, 2025 at 06:28:21PM -0400, Frank Li wrote:
> > Add parallel ov5640 nodes in imx8qxp-mek and create overlay file to enable
> > it because it can work at two mode: MIPI and parallel mode.
> >
> > Signed-off-by: Frank Li <Frank.Li@....com>
> > ---
> >  arch/arm64/boot/dts/freescale/Makefile             |  3 ++
> >  arch/arm64/boot/dts/freescale/imx8qxp-mek.dts      | 37 +++++++++++++++++++
> >  .../dts/freescale/imx8x-mek-ov5640-parallel.dtso   | 43 ++++++++++++++++++++++
> >  3 files changed, 83 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index 02ef35578dbc7..a9fb11ccd3dea 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -330,6 +330,9 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek-pcie-ep.dtb
> >  imx8qxp-mek-ov5640-csi-dtbs := imx8qxp-mek.dtb imx8qxp-mek-ov5640-csi.dtbo
> >  dtb-${CONFIG_ARCH_MXC} += imx8qxp-mek-ov5640-csi.dtb
> >
> > +imx8qxp-mek-ov5640-parallel-dtbs := imx8qxp-mek.dtb imx8x-mek-ov5640-parallel.dtbo
> > +dtb-${CONFIG_ARCH_MXC} += imx8qxp-mek-ov5640-parallel.dtb
> > +
> >  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-tqma8xqp-mba8xx.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-tqma8xqps-mb-smarc-2.dtb
> >  dtb-$(CONFIG_ARCH_MXC) += imx8ulp-evk.dtb
> > diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > index c95cb8acc360a..09eb85a9759e2 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> > @@ -487,6 +487,23 @@ pca6416: gpio@20 {
> >  		#gpio-cells = <2>;
> >  	};
> >
> > +	ov5640_pi: camera@3c {
> > +		compatible = "ovti,ov5640";
> > +		reg = <0x3c>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pinctrl_parallel_csi>;
> > +		clocks = <&pi0_misc_lpcg IMX_LPCG_CLK_0>;
> > +		assigned-clocks = <&pi0_misc_lpcg IMX_LPCG_CLK_0>;
> > +		assigned-clock-rates = <24000000>;
> > +		clock-names = "xclk";
> > +		powerdown-gpios = <&lsio_gpio3 2 GPIO_ACTIVE_HIGH>;
> > +		reset-gpios = <&lsio_gpio3 3 GPIO_ACTIVE_LOW>;
> > +		AVDD-supply = <&reg_2v8>;
> > +		DVDD-supply = <&reg_1v5>;
> > +		DOVDD-supply = <&reg_1v8>;
> > +		status = "disabled"; /* Overlay enable it */
> > +	};
> > +
>
> As far as I can tell, the sensor isn't soldered on the board, but is an
> external module connected through a cable. This DT node should therefore
> be moved to the overlay.

It is fine for i.MX8QXP. I put here try to reuse overlay file as much as
possible.

For example, imx93 have, 9x9, 11x11, 14x14 boards ...

Each board's reset-gpios have slice difference. If move whole to overlay
files, we have to create difference overlay for each boards.

If keep here and set "status = okay" and update graphic links in overlay,
we can reuse this overlay file for different boards.

for example: imx93-pcam.dtso, which simialr with imx8x-mek-ov5640-parallel.dtso.

So we simple use
imx93-9x9-qsb.dtb + imx93-pcam.dtbo.
imx93-11x11-evk.dtb + imx93-pcam.dtbo.
imx93-14x14-evk.dtb + imx93-pcam.dtbo.

even for imx91 boards in future.

>
> >  	cs42888: audio-codec@48 {
> >  		compatible = "cirrus,cs42888";
> >  		reg = <0x48>;
> > @@ -865,6 +882,26 @@ IMX8QXP_MIPI_CSI0_MCLK_OUT_MIPI_CSI0_ACM_MCLK_OUT	0xC0000041
> >  		>;
> >  	};
> >
> > +	pinctrl_parallel_csi: parallelcsigrp {
> > +		fsl,pins = <
> > +			IMX8QXP_CSI_D00_CI_PI_D02				0xc0000041
> > +			IMX8QXP_CSI_D01_CI_PI_D03				0xc0000041
> > +			IMX8QXP_CSI_D02_CI_PI_D04				0xc0000041
> > +			IMX8QXP_CSI_D03_CI_PI_D05				0xc0000041
> > +			IMX8QXP_CSI_D04_CI_PI_D06				0xc0000041
> > +			IMX8QXP_CSI_D05_CI_PI_D07				0xc0000041
> > +			IMX8QXP_CSI_D06_CI_PI_D08				0xc0000041
> > +			IMX8QXP_CSI_D07_CI_PI_D09				0xc0000041
> > +
> > +			IMX8QXP_CSI_MCLK_CI_PI_MCLK				0xc0000041
> > +			IMX8QXP_CSI_PCLK_CI_PI_PCLK				0xc0000041
> > +			IMX8QXP_CSI_HSYNC_CI_PI_HSYNC				0xc0000041
> > +			IMX8QXP_CSI_VSYNC_CI_PI_VSYNC				0xc0000041
> > +			IMX8QXP_CSI_EN_LSIO_GPIO3_IO02				0xc0000041
> > +			IMX8QXP_CSI_RESET_LSIO_GPIO3_IO03			0xc0000041
> > +		>;
> > +	};
>
> Same for this one.
>
> > +
> >  	pinctrl_pcieb: pcieagrp {
> >  		fsl,pins = <
> >  			IMX8QXP_PCIE_CTRL0_PERST_B_LSIO_GPIO4_IO00		0x06000021
> > diff --git a/arch/arm64/boot/dts/freescale/imx8x-mek-ov5640-parallel.dtso b/arch/arm64/boot/dts/freescale/imx8x-mek-ov5640-parallel.dtso
> > new file mode 100644
> > index 0000000000000..927d6640662f3
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/imx8x-mek-ov5640-parallel.dtso
> > @@ -0,0 +1,43 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright 2025 NXP
> > + */
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/media/video-interfaces.h>
> > +
> > +&ov5640_pi {
> > +	status = "okay";
> > +
> > +	port {
> > +		ov5640_pi_ep: endpoint {
> > +			remote-endpoint = <&parallel_csi_in>;
> > +			data-lanes = <1 2>;
>
> data-lanes is not allowed for parallel buses.

Do you know there are method in dt_binding to restrict this?

Frank

>
> > +			bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
> > +			bus-width = <8>;
> > +			vsync-active = <0>;
> > +			hsync-active = <1>;
> > +			pclk-sample = <1>;
> > +		};
> > +	};
> > +};
> > +
> > +&parallel_csi {
> > +	status = "okay";
> > +
> > +	ports {
> > +		port@0 {
> > +			parallel_csi_in: endpoint {
> > +				remote-endpoint = <&ov5640_pi_ep>;
> > +			};
> > +		};
> > +
> > +	};
> > +};
> > +
> > +&isi {
> > +	status = "okay";
> > +};
>
> --
> Regards,
>
> Laurent Pinchart