diff mbox series

[V2,4/5] arm64: dts: imx8mm-beacon: Enable OV5640 Camera

Message ID 20211106155427.753197-4-aford173@gmail.com (mailing list archive)
State New, archived
Headers show
Series [V2,1/5] soc: imx: imx8m-blk-ctrl: Fix imx8mm mipi reset | expand

Commit Message

Adam Ford Nov. 6, 2021, 3:54 p.m. UTC
The baseboard has support for a TDNext 5640 Camera which
uses an OV5640 connected to a 2-lane CSI2 interface.

With the CSI and mipi_csi2 drivers pointing to an OV5640 camera, the media
pipeline can be configured with the following:

    media-ctl --links "'ov5640 1-003c':0->'imx7-mipi-csis.0':0[1]"

The camera and various nodes in the pipeline can be configured for UYVY:
    media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/640x480 field:none]"
    media-ctl -v -V "'csi':0 [fmt:UYVY8_1X16/640x480 field:none]"

Signed-off-by: Adam Ford <aford173@gmail.com>
---
V2:  No change

 .../freescale/imx8mm-beacon-baseboard.dtsi    | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)

Comments

Fabio Estevam Nov. 7, 2021, 1:01 p.m. UTC | #1
On Sat, Nov 6, 2021 at 12:54 PM Adam Ford <aford173@gmail.com> wrote:
>
> The baseboard has support for a TDNext 5640 Camera which
> uses an OV5640 connected to a 2-lane CSI2 interface.
>
> With the CSI and mipi_csi2 drivers pointing to an OV5640 camera, the media
> pipeline can be configured with the following:
>
>     media-ctl --links "'ov5640 1-003c':0->'imx7-mipi-csis.0':0[1]"
>
> The camera and various nodes in the pipeline can be configured for UYVY:
>     media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/640x480 field:none]"
>     media-ctl -v -V "'csi':0 [fmt:UYVY8_1X16/640x480 field:none]"
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Fabio Estevam <festevam@gmail.com>
Laurent Pinchart Nov. 21, 2021, 11:18 p.m. UTC | #2
Hi Adam,

Thank you for the patch.

On Sat, Nov 06, 2021 at 10:54:26AM -0500, Adam Ford wrote:
> The baseboard has support for a TDNext 5640 Camera which
> uses an OV5640 connected to a 2-lane CSI2 interface.
> 
> With the CSI and mipi_csi2 drivers pointing to an OV5640 camera, the media
> pipeline can be configured with the following:
> 
>     media-ctl --links "'ov5640 1-003c':0->'imx7-mipi-csis.0':0[1]"
> 
> The camera and various nodes in the pipeline can be configured for UYVY:
>     media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/640x480 field:none]"
>     media-ctl -v -V "'csi':0 [fmt:UYVY8_1X16/640x480 field:none]"
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>

As the ov5640 is on an add-on module, would a DT overlay be better ?

> ---
> V2:  No change
> 
>  .../freescale/imx8mm-beacon-baseboard.dtsi    | 58 +++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> index 6f5e63696ec0..0fb95f4a5e78 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> @@ -43,6 +43,16 @@ reg_audio: regulator-audio {
>  		enable-active-high;
>  	};
>  
> +	reg_camera: regulator-camera {
> +		compatible = "regulator-fixed";
> +		regulator-name = "mipi_pwr";
> +		regulator-min-microvolt = <2800000>;
> +		regulator-max-microvolt = <2800000>;
> +		gpio = <&pca6416_1 0 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		startup-delay-us = <100000>;
> +	};
> +
>  	reg_usdhc2_vmmc: regulator-usdhc2 {
>  		compatible = "regulator-fixed";
>  		regulator-name = "VSD_3V3";
> @@ -67,6 +77,10 @@ sound {
>  	};
>  };
>  
> +&csi {
> +	status = "okay";
> +};
> +
>  &ecspi2 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_espi2>;
> @@ -90,6 +104,30 @@ &i2c2 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_i2c2>;
>  	status = "okay";
> +
> +	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>;
> +		AVDD-supply = <&reg_camera>;  /* 2.8v */
> +		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>;
> +			};
> +		};
> +	};
>  };
>  
>  &i2c4 {
> @@ -141,6 +179,18 @@ pca6416_1: gpio@21 {
>  	};
>  };
>  
> +&mipi_csi {
> +	status = "okay";
> +	ports {
> +		port@0 {
> +			imx8mm_mipi_csi_in: endpoint {
> +				remote-endpoint = <&ov5640_to_mipi_csi2>;
> +				data-lanes = <1 2>;
> +			};
> +		};
> +	};
> +};
> +
>  &sai3 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_sai3>;
> @@ -209,6 +259,14 @@ MX8MM_IOMUXC_SAI3_RXFS_GPIO4_IO28	0x41
>  		>;
>  	};
>  
> +	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_pcal6414: pcal6414-gpiogrp {
>  		fsl,pins = <
>  			MX8MM_IOMUXC_SAI2_MCLK_GPIO4_IO27		0x19
Adam Ford Nov. 22, 2021, 3:07 a.m. UTC | #3
On Sun, Nov 21, 2021 at 5:18 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Adam,
>
> Thank you for the patch.
>
> On Sat, Nov 06, 2021 at 10:54:26AM -0500, Adam Ford wrote:
> > The baseboard has support for a TDNext 5640 Camera which
> > uses an OV5640 connected to a 2-lane CSI2 interface.
> >
> > With the CSI and mipi_csi2 drivers pointing to an OV5640 camera, the media
> > pipeline can be configured with the following:
> >
> >     media-ctl --links "'ov5640 1-003c':0->'imx7-mipi-csis.0':0[1]"
> >
> > The camera and various nodes in the pipeline can be configured for UYVY:
> >     media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/640x480 field:none]"
> >     media-ctl -v -V "'csi':0 [fmt:UYVY8_1X16/640x480 field:none]"
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> As the ov5640 is on an add-on module, would a DT overlay be better ?

At least for the Beacon / LogicPD boards, I would prefer to avoid the
overlays.  We have an i.M6Q and an OMAP3 board with cameras enabled in
our development kit device trees.  If the cameras are not connected,
they just display a message that the cameras are not communicating and
move on.  I'm OK with that.

>
> > ---
> > V2:  No change
> >
> >  .../freescale/imx8mm-beacon-baseboard.dtsi    | 58 +++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> > index 6f5e63696ec0..0fb95f4a5e78 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> > @@ -43,6 +43,16 @@ reg_audio: regulator-audio {
> >               enable-active-high;
> >       };
> >
> > +     reg_camera: regulator-camera {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "mipi_pwr";
> > +             regulator-min-microvolt = <2800000>;
> > +             regulator-max-microvolt = <2800000>;
> > +             gpio = <&pca6416_1 0 GPIO_ACTIVE_HIGH>;
> > +             enable-active-high;
> > +             startup-delay-us = <100000>;
> > +     };
> > +
> >       reg_usdhc2_vmmc: regulator-usdhc2 {
> >               compatible = "regulator-fixed";
> >               regulator-name = "VSD_3V3";
> > @@ -67,6 +77,10 @@ sound {
> >       };
> >  };
> >
> > +&csi {
> > +     status = "okay";
> > +};
> > +
> >  &ecspi2 {
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&pinctrl_espi2>;
> > @@ -90,6 +104,30 @@ &i2c2 {
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&pinctrl_i2c2>;
> >       status = "okay";
> > +
> > +     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>;
> > +             AVDD-supply = <&reg_camera>;  /* 2.8v */
> > +             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>;
> > +                     };
> > +             };
> > +     };
> >  };
> >
> >  &i2c4 {
> > @@ -141,6 +179,18 @@ pca6416_1: gpio@21 {
> >       };
> >  };
> >
> > +&mipi_csi {
> > +     status = "okay";
> > +     ports {
> > +             port@0 {
> > +                     imx8mm_mipi_csi_in: endpoint {
> > +                             remote-endpoint = <&ov5640_to_mipi_csi2>;
> > +                             data-lanes = <1 2>;
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> >  &sai3 {
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&pinctrl_sai3>;
> > @@ -209,6 +259,14 @@ MX8MM_IOMUXC_SAI3_RXFS_GPIO4_IO28        0x41
> >               >;
> >       };
> >
> > +     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_pcal6414: pcal6414-gpiogrp {
> >               fsl,pins = <
> >                       MX8MM_IOMUXC_SAI2_MCLK_GPIO4_IO27               0x19
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart Nov. 23, 2021, 12:15 a.m. UTC | #4
Hi Adam,

On Sun, Nov 21, 2021 at 09:07:26PM -0600, Adam Ford wrote:
> On Sun, Nov 21, 2021 at 5:18 PM Laurent Pinchart wrote:
> > On Sat, Nov 06, 2021 at 10:54:26AM -0500, Adam Ford wrote:
> > > The baseboard has support for a TDNext 5640 Camera which
> > > uses an OV5640 connected to a 2-lane CSI2 interface.
> > >
> > > With the CSI and mipi_csi2 drivers pointing to an OV5640 camera, the media
> > > pipeline can be configured with the following:
> > >
> > >     media-ctl --links "'ov5640 1-003c':0->'imx7-mipi-csis.0':0[1]"
> > >
> > > The camera and various nodes in the pipeline can be configured for UYVY:
> > >     media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/640x480 field:none]"
> > >     media-ctl -v -V "'csi':0 [fmt:UYVY8_1X16/640x480 field:none]"
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > As the ov5640 is on an add-on module, would a DT overlay be better ?
> 
> At least for the Beacon / LogicPD boards, I would prefer to avoid the
> overlays.  We have an i.M6Q and an OMAP3 board with cameras enabled in
> our development kit device trees.  If the cameras are not connected,
> they just display a message that the cameras are not communicating and
> move on.  I'm OK with that.

You know the board better than I do, so I won't push against this, but I
still think it may not lead to the best user experience, especially if a
user wanted to connect a different sensor to the development board.

> > > ---
> > > V2:  No change
> > >
> > >  .../freescale/imx8mm-beacon-baseboard.dtsi    | 58 +++++++++++++++++++
> > >  1 file changed, 58 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> > > index 6f5e63696ec0..0fb95f4a5e78 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
> > > @@ -43,6 +43,16 @@ reg_audio: regulator-audio {
> > >               enable-active-high;
> > >       };
> > >
> > > +     reg_camera: regulator-camera {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "mipi_pwr";
> > > +             regulator-min-microvolt = <2800000>;
> > > +             regulator-max-microvolt = <2800000>;
> > > +             gpio = <&pca6416_1 0 GPIO_ACTIVE_HIGH>;
> > > +             enable-active-high;
> > > +             startup-delay-us = <100000>;
> > > +     };
> > > +
> > >       reg_usdhc2_vmmc: regulator-usdhc2 {
> > >               compatible = "regulator-fixed";
> > >               regulator-name = "VSD_3V3";
> > > @@ -67,6 +77,10 @@ sound {
> > >       };
> > >  };
> > >
> > > +&csi {
> > > +     status = "okay";
> > > +};
> > > +
> > >  &ecspi2 {
> > >       pinctrl-names = "default";
> > >       pinctrl-0 = <&pinctrl_espi2>;
> > > @@ -90,6 +104,30 @@ &i2c2 {
> > >       pinctrl-names = "default";
> > >       pinctrl-0 = <&pinctrl_i2c2>;
> > >       status = "okay";
> > > +
> > > +     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>;
> > > +             AVDD-supply = <&reg_camera>;  /* 2.8v */
> > > +             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>;
> > > +                     };
> > > +             };
> > > +     };
> > >  };
> > >
> > >  &i2c4 {
> > > @@ -141,6 +179,18 @@ pca6416_1: gpio@21 {
> > >       };
> > >  };
> > >
> > > +&mipi_csi {
> > > +     status = "okay";
> > > +     ports {
> > > +             port@0 {
> > > +                     imx8mm_mipi_csi_in: endpoint {
> > > +                             remote-endpoint = <&ov5640_to_mipi_csi2>;
> > > +                             data-lanes = <1 2>;
> > > +                     };
> > > +             };
> > > +     };
> > > +};
> > > +
> > >  &sai3 {
> > >       pinctrl-names = "default";
> > >       pinctrl-0 = <&pinctrl_sai3>;
> > > @@ -209,6 +259,14 @@ MX8MM_IOMUXC_SAI3_RXFS_GPIO4_IO28        0x41
> > >               >;
> > >       };
> > >
> > > +     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_pcal6414: pcal6414-gpiogrp {
> > >               fsl,pins = <
> > >                       MX8MM_IOMUXC_SAI2_MCLK_GPIO4_IO27               0x19
Alexander Stein Nov. 23, 2021, 7:38 a.m. UTC | #5
Am Dienstag, dem 23.11.2021 um 02:15 +0200 schrieb Laurent Pinchart:
> Hi Adam,
> 
> On Sun, Nov 21, 2021 at 09:07:26PM -0600, Adam Ford wrote:
> > On Sun, Nov 21, 2021 at 5:18 PM Laurent Pinchart wrote:
> > > On Sat, Nov 06, 2021 at 10:54:26AM -0500, Adam Ford wrote:
> > > > The baseboard has support for a TDNext 5640 Camera which
> > > > uses an OV5640 connected to a 2-lane CSI2 interface.
> > > > 
> > > > With the CSI and mipi_csi2 drivers pointing to an OV5640
> > > > camera, the media
> > > > pipeline can be configured with the following:
> > > > 
> > > >     media-ctl --links "'ov5640 1-003c':0->'imx7-mipi-
> > > > csis.0':0[1]"
> > > > 
> > > > The camera and various nodes in the pipeline can be configured
> > > > for UYVY:
> > > >     media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/640x480
> > > > field:none]"
> > > >     media-ctl -v -V "'csi':0 [fmt:UYVY8_1X16/640x480
> > > > field:none]"
> > > > 
> > > > Signed-off-by: Adam Ford <
> > > > aford173@gmail.com
> > > > >
> > > 
> > > As the ov5640 is on an add-on module, would a DT overlay be
> > > better ?
> > 
> > At least for the Beacon / LogicPD boards, I would prefer to avoid
> > the
> > overlays.  We have an i.M6Q and an OMAP3 board with cameras enabled
> > in
> > our development kit device trees.  If the cameras are not
> > connected,
> > they just display a message that the cameras are not communicating
> > and
> > move on.  I'm OK with that.
> 
> You know the board better than I do, so I won't push against this,
> but I
> still think it may not lead to the best user experience, especially
> if a
> user wanted to connect a different sensor to the development board.

I see the advantages of overlays compared to "stacked" .dts files. But
is there any general supported interface how to actually apply an
overlay?
Documentation/devicetree/overlay-notes.rst
states of_overlay_fdt_apply() but there is only exactly one user in-
kernel (rcar-du). Is it expected that the bootloader like u-boot shall
apply the .dtbo files?

Best regards,
Alexander
Laurent Pinchart Nov. 23, 2021, 9:47 a.m. UTC | #6
Hi Alexander,

On Tue, Nov 23, 2021 at 08:38:47AM +0100, Alexander Stein wrote:
> Am Dienstag, dem 23.11.2021 um 02:15 +0200 schrieb Laurent Pinchart:
> > On Sun, Nov 21, 2021 at 09:07:26PM -0600, Adam Ford wrote:
> > > On Sun, Nov 21, 2021 at 5:18 PM Laurent Pinchart wrote:
> > > > On Sat, Nov 06, 2021 at 10:54:26AM -0500, Adam Ford wrote:
> > > > > The baseboard has support for a TDNext 5640 Camera which
> > > > > uses an OV5640 connected to a 2-lane CSI2 interface.
> > > > > 
> > > > > With the CSI and mipi_csi2 drivers pointing to an OV5640 camera, the media
> > > > > pipeline can be configured with the following:
> > > > > 
> > > > >     media-ctl --links "'ov5640 1-003c':0->'imx7-mipi-csis.0':0[1]"
> > > > > 
> > > > > The camera and various nodes in the pipeline can be configured for UYVY:
> > > > >     media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/640x480 field:none]"
> > > > >     media-ctl -v -V "'csi':0 [fmt:UYVY8_1X16/640x480 field:none]"
> > > > > 
> > > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > 
> > > > As the ov5640 is on an add-on module, would a DT overlay be better ?
> > > 
> > > At least for the Beacon / LogicPD boards, I would prefer to avoid the
> > > overlays.  We have an i.M6Q and an OMAP3 board with cameras enabled in
> > > our development kit device trees.  If the cameras are not connected,
> > > they just display a message that the cameras are not communicating and
> > > move on.  I'm OK with that.
> > 
> > You know the board better than I do, so I won't push against this, but I
> > still think it may not lead to the best user experience, especially if a
> > user wanted to connect a different sensor to the development board.
> 
> I see the advantages of overlays compared to "stacked" .dts files. But
> is there any general supported interface how to actually apply an overlay?
> Documentation/devicetree/overlay-notes.rst
> states of_overlay_fdt_apply() but there is only exactly one user in-
> kernel (rcar-du). Is it expected that the bootloader like u-boot shall
> apply the .dtbo files?

I believe the boot loader is expected to apply overlays nowadays, yes.
That's my personal workflow.
Tim Harvey Nov. 29, 2021, 6:56 p.m. UTC | #7
On Tue, Nov 23, 2021 at 1:47 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Alexander,
>
> On Tue, Nov 23, 2021 at 08:38:47AM +0100, Alexander Stein wrote:
> > Am Dienstag, dem 23.11.2021 um 02:15 +0200 schrieb Laurent Pinchart:
> > > On Sun, Nov 21, 2021 at 09:07:26PM -0600, Adam Ford wrote:
> > > > On Sun, Nov 21, 2021 at 5:18 PM Laurent Pinchart wrote:
> > > > > On Sat, Nov 06, 2021 at 10:54:26AM -0500, Adam Ford wrote:
> > > > > > The baseboard has support for a TDNext 5640 Camera which
> > > > > > uses an OV5640 connected to a 2-lane CSI2 interface.
> > > > > >
> > > > > > With the CSI and mipi_csi2 drivers pointing to an OV5640 camera, the media
> > > > > > pipeline can be configured with the following:
> > > > > >
> > > > > >     media-ctl --links "'ov5640 1-003c':0->'imx7-mipi-csis.0':0[1]"
> > > > > >
> > > > > > The camera and various nodes in the pipeline can be configured for UYVY:
> > > > > >     media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_1X16/640x480 field:none]"
> > > > > >     media-ctl -v -V "'csi':0 [fmt:UYVY8_1X16/640x480 field:none]"
> > > > > >
> > > > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > >
> > > > > As the ov5640 is on an add-on module, would a DT overlay be better ?
> > > >
> > > > At least for the Beacon / LogicPD boards, I would prefer to avoid the
> > > > overlays.  We have an i.M6Q and an OMAP3 board with cameras enabled in
> > > > our development kit device trees.  If the cameras are not connected,
> > > > they just display a message that the cameras are not communicating and
> > > > move on.  I'm OK with that.
> > >
> > > You know the board better than I do, so I won't push against this, but I
> > > still think it may not lead to the best user experience, especially if a
> > > user wanted to connect a different sensor to the development board.
> >
> > I see the advantages of overlays compared to "stacked" .dts files. But
> > is there any general supported interface how to actually apply an overlay?
> > Documentation/devicetree/overlay-notes.rst
> > states of_overlay_fdt_apply() but there is only exactly one user in-
> > kernel (rcar-du). Is it expected that the bootloader like u-boot shall
> > apply the .dtbo files?
>
> I believe the boot loader is expected to apply overlays nowadays, yes.
> That's my personal workflow.
>

That is my understanding as well. I believe the support to apply dt
overlays within Linux (which the rpi kernel still uses) never got
merged due to race conditions so the focus was moved to bootloader.

I also have begun submitting some dt overlay files [1] [2] which I
will likely repost later this week removing the RFC.

My understanding is that these should be '.dtbo' files in the Linux
Makefile which are handled. My boards use the U-Boot bootloader and to
handle the dt overlays there you need to:
- set CONFIG_OF_LIBFDT_OVERLAY=y which gives you the 'fdt apply' command
- use 'fdt addr <addr> && fdt resize && fdt apply <loadaddr>' prior to
booting with booti
- Note that there is some support at the FIT level as well for
overlays if you need them applied to U-Boot's live dt (I don't for my
needs)

In my U-Boot environment I use scripts for loading the fdt and
applying the overlays. For example for booting kernel/dtb from network
I use:
boot_net setenv fsload tftpboot; run loadfdt && run apply_overlays &&
$fsload $kernel_addr_r venice/Image && booti $kernel_addr_r -
$fdt_addr_r
loadfdt if $fsload $fdt_addr_r $dir/$fdt_file1; then echo loaded
$fdt_file1; elif $fsload $fdt_addr_r $dir/$fdt_file2; then echo loaded
$fdt_file2; elif $fsload $fdt_addr_r $dir/$fdt_file3; then echo loaded
$fdt_file3; elif $fsload $fdt_addr_r $dir/$fdt_file4; then echo loaded
$fdt_file4; elif $fsload $fdt_addr_r $dir/$fdt_file5; then echo loaded
$fdt_file5; fi
apply_overlays fdt addr $fdt_addr_r && fdt resize && for i in
"$fdt_overlays"; do $fsload $loadaddr $dir/$i && fdt apply $loadaddr
&& echo applied $i...; done

Best regards,

Tim
[1] https://www.spinics.net/lists/arm-kernel/msg933447.html
[2] https://www.spinics.net/lists/arm-kernel/msg933638.html
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
index 6f5e63696ec0..0fb95f4a5e78 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-beacon-baseboard.dtsi
@@ -43,6 +43,16 @@  reg_audio: regulator-audio {
 		enable-active-high;
 	};
 
+	reg_camera: regulator-camera {
+		compatible = "regulator-fixed";
+		regulator-name = "mipi_pwr";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		gpio = <&pca6416_1 0 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+		startup-delay-us = <100000>;
+	};
+
 	reg_usdhc2_vmmc: regulator-usdhc2 {
 		compatible = "regulator-fixed";
 		regulator-name = "VSD_3V3";
@@ -67,6 +77,10 @@  sound {
 	};
 };
 
+&csi {
+	status = "okay";
+};
+
 &ecspi2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_espi2>;
@@ -90,6 +104,30 @@  &i2c2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_i2c2>;
 	status = "okay";
+
+	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>;
+		AVDD-supply = <&reg_camera>;  /* 2.8v */
+		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>;
+			};
+		};
+	};
 };
 
 &i2c4 {
@@ -141,6 +179,18 @@  pca6416_1: gpio@21 {
 	};
 };
 
+&mipi_csi {
+	status = "okay";
+	ports {
+		port@0 {
+			imx8mm_mipi_csi_in: endpoint {
+				remote-endpoint = <&ov5640_to_mipi_csi2>;
+				data-lanes = <1 2>;
+			};
+		};
+	};
+};
+
 &sai3 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_sai3>;
@@ -209,6 +259,14 @@  MX8MM_IOMUXC_SAI3_RXFS_GPIO4_IO28	0x41
 		>;
 	};
 
+	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_pcal6414: pcal6414-gpiogrp {
 		fsl,pins = <
 			MX8MM_IOMUXC_SAI2_MCLK_GPIO4_IO27		0x19