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

[Frank Li <Frank.li@nxp.com>]

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@nxp.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