[RFC,V2,5/5] arm64: dts: imx8mm-evk: Enable OV5640 Camera

Message ID 20211023203457.1217821-6-aford173@gmail.com (mailing list archive)
State Not Applicable, archived
Headers
Series arm64: dts: imx8mm: Enable CSI and OV5640 Camera |

Commit Message

Adam Ford Oct. 23, 2021, 8:34 p.m. UTC
  The schematic shows support for a camera interface, and the NXP
kernel shows it is an OV5640.

Signed-off-by: Adam Ford <aford173@gmail.com>
---
 arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
  

Comments

Laurent Pinchart Oct. 28, 2021, 1:31 a.m. UTC | #1
Hi Adam,

Thank you for the patch.

On Sat, Oct 23, 2021 at 03:34:56PM -0500, Adam Ford wrote:
> The schematic shows support for a camera interface, and the NXP
> kernel shows it is an OV5640.

The camera is an external module though. Should this be a DT overlay ?

> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> index e033d0257b5a..27217d30b8d8 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> @@ -239,6 +239,10 @@ ldo6_reg: LDO6 {
>  	};
>  };
>  
> +&csi {
> +	status = "okay";
> +};
> +
>  &i2c2 {
>  	clock-frequency = <400000>;
>  	pinctrl-names = "default";
> @@ -287,6 +291,38 @@ pca6416: gpio@20 {
>  		gpio-controller;
>  		#gpio-cells = <2>;
>  	};
> +
> +	camera@3c {
> +		compatible = "ovti,ov5640";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_ov5640>;
> +		reg = <0x3c>;
> +		clocks = <&clk IMX8MM_CLK_CLKO1>;
> +		clock-names = "xclk";
> +		assigned-clocks = <&clk IMX8MM_CLK_CLKO1>;
> +		assigned-clock-parents = <&clk IMX8MM_CLK_24M>;
> +		assigned-clock-rates = <24000000>;
> +		powerdown-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
> +		reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
> +
> +		port {
> +			/* MIPI CSI-2 bus endpoint */
> +			ov5640_to_mipi_csi2: endpoint {
> +				remote-endpoint = <&imx8mm_mipi_csi_in>;
> +				clock-lanes = <0>;
> +				data-lanes = <1 2>;
> +			};
> +		};
> +	};
> +};
> +
> +&imx8mm_mipi_csi_in {
> +	remote-endpoint = <&ov5640_to_mipi_csi2>;
> +	data-lanes = <1 2>;
> +};
> +
> +&mipi_csi2 {
> +	status = "okay";
>  };
>  
>  &sai3 {
> @@ -406,6 +442,14 @@ MX8MM_IOMUXC_I2C3_SDA_I2C3_SDA			0x400001c3
>  		>;
>  	};
>  
> +	pinctrl_ov5640: ov5640grp {
> +		fsl,pins = <
> +			MX8MM_IOMUXC_GPIO1_IO07_GPIO1_IO7		0x19
> +			MX8MM_IOMUXC_GPIO1_IO06_GPIO1_IO6		0x19
> +			MX8MM_IOMUXC_GPIO1_IO14_CCMSRCGPCMIX_CLKO1	0x59
> +		>;
> +	};
> +
>  	pinctrl_pmic: pmicirqgrp {
>  		fsl,pins = <
>  			MX8MM_IOMUXC_GPIO1_IO03_GPIO1_IO3		0x141
  
Tim Harvey Nov. 2, 2021, 5:49 p.m. UTC | #2
On Sat, Oct 23, 2021 at 1:39 PM Adam Ford <aford173@gmail.com> wrote:
>
> The schematic shows support for a camera interface, and the NXP
> kernel shows it is an OV5640.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> index e033d0257b5a..27217d30b8d8 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
> @@ -239,6 +239,10 @@ ldo6_reg: LDO6 {
>         };
>  };
>
> +&csi {
> +       status = "okay";
> +};
> +
>  &i2c2 {
>         clock-frequency = <400000>;
>         pinctrl-names = "default";
> @@ -287,6 +291,38 @@ pca6416: gpio@20 {
>                 gpio-controller;
>                 #gpio-cells = <2>;
>         };
> +
> +       camera@3c {
> +               compatible = "ovti,ov5640";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pinctrl_ov5640>;
> +               reg = <0x3c>;
> +               clocks = <&clk IMX8MM_CLK_CLKO1>;
> +               clock-names = "xclk";
> +               assigned-clocks = <&clk IMX8MM_CLK_CLKO1>;
> +               assigned-clock-parents = <&clk IMX8MM_CLK_24M>;
> +               assigned-clock-rates = <24000000>;
> +               powerdown-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
> +               reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
> +
> +               port {
> +                       /* MIPI CSI-2 bus endpoint */
> +                       ov5640_to_mipi_csi2: endpoint {
> +                               remote-endpoint = <&imx8mm_mipi_csi_in>;
> +                               clock-lanes = <0>;
> +                               data-lanes = <1 2>;
> +                       };
> +               };
> +       };

Adam,

On the imx8mm-evk the ov5640 is on i2c3

Tim
  
Tim Harvey Nov. 5, 2021, 4 p.m. UTC | #3
On Wed, Oct 27, 2021 at 6:34 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Adam,
>
> Thank you for the patch.
>
> On Sat, Oct 23, 2021 at 03:34:56PM -0500, Adam Ford wrote:
> > The schematic shows support for a camera interface, and the NXP
> > kernel shows it is an OV5640.
>
> The camera is an external module though. Should this be a DT overlay ?
>

Laurent,

I wanted to ask you about your comment here. I would agree that for
something like the OV5640 on the imx8mm-evk which is an add-in board
via a connector should be a dt overlay. I'm investigating using
overlays for features like this on my boards vs creating hierarchical
dts files and I see that the kernel allows building fragments with
'/plugin/' but I don't see any such overlays in the kernel tree
currently. Would overlay/fragments be accepted? Are there any examples
in the kernel tree already that I'm missing?

Best regards,

Tim
  
Laurent Pinchart Nov. 21, 2021, 11:18 p.m. UTC | #4
Hi Tim,

On Fri, Nov 05, 2021 at 09:00:03AM -0700, Tim Harvey wrote:
> On Wed, Oct 27, 2021 at 6:34 PM Laurent Pinchart wrote:
> >
> > Hi Adam,
> >
> > Thank you for the patch.
> >
> > On Sat, Oct 23, 2021 at 03:34:56PM -0500, Adam Ford wrote:
> > > The schematic shows support for a camera interface, and the NXP
> > > kernel shows it is an OV5640.
> >
> > The camera is an external module though. Should this be a DT overlay ?
> >
> 
> Laurent,
> 
> I wanted to ask you about your comment here. I would agree that for
> something like the OV5640 on the imx8mm-evk which is an add-in board
> via a connector should be a dt overlay. I'm investigating using
> overlays for features like this on my boards vs creating hierarchical
> dts files and I see that the kernel allows building fragments with
> '/plugin/' but I don't see any such overlays in the kernel tree
> currently. Would overlay/fragments be accepted?

I believe so (otherwise I'm not sure why the build system would allow
compiling overlays). Rob may have a more authoritative opinion on this
topic.

> Are there any examples
> in the kernel tree already that I'm missing?

Commit 7a4c31ee877a ("arm64: zynqmp: Add support for Xilinx Kria SOM
board") has been added recently.
  

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
index e033d0257b5a..27217d30b8d8 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
@@ -239,6 +239,10 @@  ldo6_reg: LDO6 {
 	};
 };
 
+&csi {
+	status = "okay";
+};
+
 &i2c2 {
 	clock-frequency = <400000>;
 	pinctrl-names = "default";
@@ -287,6 +291,38 @@  pca6416: gpio@20 {
 		gpio-controller;
 		#gpio-cells = <2>;
 	};
+
+	camera@3c {
+		compatible = "ovti,ov5640";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_ov5640>;
+		reg = <0x3c>;
+		clocks = <&clk IMX8MM_CLK_CLKO1>;
+		clock-names = "xclk";
+		assigned-clocks = <&clk IMX8MM_CLK_CLKO1>;
+		assigned-clock-parents = <&clk IMX8MM_CLK_24M>;
+		assigned-clock-rates = <24000000>;
+		powerdown-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
+
+		port {
+			/* MIPI CSI-2 bus endpoint */
+			ov5640_to_mipi_csi2: endpoint {
+				remote-endpoint = <&imx8mm_mipi_csi_in>;
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+			};
+		};
+	};
+};
+
+&imx8mm_mipi_csi_in {
+	remote-endpoint = <&ov5640_to_mipi_csi2>;
+	data-lanes = <1 2>;
+};
+
+&mipi_csi2 {
+	status = "okay";
 };
 
 &sai3 {
@@ -406,6 +442,14 @@  MX8MM_IOMUXC_I2C3_SDA_I2C3_SDA			0x400001c3
 		>;
 	};
 
+	pinctrl_ov5640: ov5640grp {
+		fsl,pins = <
+			MX8MM_IOMUXC_GPIO1_IO07_GPIO1_IO7		0x19
+			MX8MM_IOMUXC_GPIO1_IO06_GPIO1_IO6		0x19
+			MX8MM_IOMUXC_GPIO1_IO14_CCMSRCGPCMIX_CLKO1	0x59
+		>;
+	};
+
 	pinctrl_pmic: pmicirqgrp {
 		fsl,pins = <
 			MX8MM_IOMUXC_GPIO1_IO03_GPIO1_IO3		0x141