Re: [PATCH v2 05/19] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
On 01/04/2017 07:26 AM, Fabio Estevam wrote: On Tue, Jan 3, 2017 at 6:57 PM, Steve Longerbeamwrote: + camera: ov5642@3c { + compatible = "ovti,ov5642"; + pinctrl-names = "default"; + pinctrl-0 = <_ov5642>; + clocks = < IMX6QDL_CLK_CKO>; + clock-names = "xclk"; + reg = <0x3c>; + xclk = <2400>; + DOVDD-supply = <_reg>; /* 1.8v */ + AVDD-supply = <_reg>; /* 2.8v, rev C board is VGEN3 + rev B board is VGEN5 */ Please use vgen3 so that by default we have the valid AVDD-supply for revC boards which is more recent and more the users have access to. done. + mipi_camera: ov5640@3c { + compatible = "ovti,ov5640_mipi"; + pinctrl-names = "default"; + pinctrl-0 = <_ov5640>; + reg = <0x3c>; + clocks = < IMX6QDL_CLK_CKO>; + clock-names = "xclk"; + xclk = <2400>; + DOVDD-supply = <_reg>; /* 1.8v */ + AVDD-supply = <_reg>; /* 2.8v, rev C board is VGEN3 + rev B board is VGEN5 */ Same here. done. + pinctrl_ov5640: ov5640grp { + fsl,pins = < + MX6QDL_PAD_SD1_DAT2__GPIO1_IO19 0x8000 + MX6QDL_PAD_SD1_CLK__GPIO1_IO20 0x8000 Please avoid all the 0x8000 IOMUX settings and replace them by their real values. yeah, finally got around to this, done! Steve ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 05/19] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
On 01/04/2017 04:33 AM, Vladimir Zapolskiy wrote: + + camera: ov5642@3c { ov5642: camera@3c done. + pwdn-gpios = < 16 GPIO_ACTIVE_HIGH>; /* SD1_DAT0 */ + reset-gpios = < 17 GPIO_ACTIVE_LOW>; /* SD1_DAT1 */ Comments about SD1_* pad names are redundant. sure, removed. + status = "disabled"; Why is it disabled here? It's explained in the header. I don't yet have the OV5642 module for the sabresd for testing, so it is disabled for now. + + mipi_camera: ov5640@3c { ov5640: camera@3c done. + pwdn-gpios = < 19 GPIO_ACTIVE_HIGH>; /* SD1_DAT2 */ + reset-gpios = < 20 GPIO_ACTIVE_LOW>; /* SD1_CLK */ Comments about SD1_* pad names are redundant. removed. + + pinctrl_ipu1_csi0: ipu1grp-csi0 { Please rename the node name to ipu1csi0grp. Please add new pin control groups preserving the alphanimerical order. done and done. Steve ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 05/19] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
On Tue, Jan 3, 2017 at 6:57 PM, Steve Longerbeamwrote: > + camera: ov5642@3c { > + compatible = "ovti,ov5642"; > + pinctrl-names = "default"; > + pinctrl-0 = <_ov5642>; > + clocks = < IMX6QDL_CLK_CKO>; > + clock-names = "xclk"; > + reg = <0x3c>; > + xclk = <2400>; > + DOVDD-supply = <_reg>; /* 1.8v */ > + AVDD-supply = <_reg>; /* 2.8v, rev C board is VGEN3 > + rev B board is VGEN5 */ Please use vgen3 so that by default we have the valid AVDD-supply for revC boards which is more recent and more the users have access to. > + mipi_camera: ov5640@3c { > + compatible = "ovti,ov5640_mipi"; > + pinctrl-names = "default"; > + pinctrl-0 = <_ov5640>; > + reg = <0x3c>; > + clocks = < IMX6QDL_CLK_CKO>; > + clock-names = "xclk"; > + xclk = <2400>; > + DOVDD-supply = <_reg>; /* 1.8v */ > + AVDD-supply = <_reg>; /* 2.8v, rev C board is VGEN3 > + rev B board is VGEN5 */ Same here. > + pinctrl_ov5640: ov5640grp { > + fsl,pins = < > + MX6QDL_PAD_SD1_DAT2__GPIO1_IO19 0x8000 > + MX6QDL_PAD_SD1_CLK__GPIO1_IO20 0x8000 Please avoid all the 0x8000 IOMUX settings and replace them by their real values. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 05/19] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
On 01/03/2017 10:57 PM, Steve Longerbeam wrote: > Enables the OV5642 parallel-bus sensor, and the OV5640 MIPI CSI-2 sensor. > > The OV5642 connects to the parallel-bus mux input port on ipu1_csi0_mux. > > The OV5640 connects to the input port on the MIPI CSI-2 receiver on > mipi_csi. It is set to transmit over MIPI virtual channel 1. > > Until the OV5652 sensor module compatible with the SabreSD becomes > available for testing, the ov5642 node is currently disabled. > > Signed-off-by: Steve Longerbeam> --- [snip] > + > + camera: ov5642@3c { ov5642: camera@3c > + compatible = "ovti,ov5642"; > + pinctrl-names = "default"; > + pinctrl-0 = <_ov5642>; > + clocks = < IMX6QDL_CLK_CKO>; > + clock-names = "xclk"; > + reg = <0x3c>; > + xclk = <2400>; > + DOVDD-supply = <_reg>; /* 1.8v */ > + AVDD-supply = <_reg>; /* 2.8v, rev C board is VGEN3 > + rev B board is VGEN5 */ > + DVDD-supply = <_reg>; /* 1.5v*/ > + pwdn-gpios = < 16 GPIO_ACTIVE_HIGH>; /* SD1_DAT0 */ > + reset-gpios = < 17 GPIO_ACTIVE_LOW>; /* SD1_DAT1 */ Comments about SD1_* pad names are redundant. > + status = "disabled"; Why is it disabled here? > + > + port { > + ov5642_to_ipu1_csi0_mux: endpoint { > + remote-endpoint = > <_csi0_mux_from_parallel_sensor>; > + bus-width = <8>; > + hsync-active = <1>; > + vsync-active = <1>; > + }; > + }; > + }; > }; > > { > @@ -322,6 +376,34 @@ > }; > }; > }; > + > + mipi_camera: ov5640@3c { ov5640: camera@3c > + compatible = "ovti,ov5640_mipi"; > + pinctrl-names = "default"; > + pinctrl-0 = <_ov5640>; > + reg = <0x3c>; > + clocks = < IMX6QDL_CLK_CKO>; > + clock-names = "xclk"; > + xclk = <2400>; > + DOVDD-supply = <_reg>; /* 1.8v */ > + AVDD-supply = <_reg>; /* 2.8v, rev C board is VGEN3 > + rev B board is VGEN5 */ > + DVDD-supply = <_reg>; /* 1.5v*/ > + pwdn-gpios = < 19 GPIO_ACTIVE_HIGH>; /* SD1_DAT2 */ > + reset-gpios = < 20 GPIO_ACTIVE_LOW>; /* SD1_CLK */ Comments about SD1_* pad names are redundant. > + > + port { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ov5640_to_mipi_csi: endpoint@1 { > + reg = <1>; > + remote-endpoint = <_csi_from_mipi_sensor>; > + data-lanes = <0 1>; > + clock-lanes = <2>; > + }; > + }; > + }; > }; > > { > @@ -426,6 +508,36 @@ > >; > }; > > + pinctrl_ov5640: ov5640grp { > + fsl,pins = < > + MX6QDL_PAD_SD1_DAT2__GPIO1_IO19 0x8000 > + MX6QDL_PAD_SD1_CLK__GPIO1_IO20 0x8000 > + >; > + }; > + > + pinctrl_ov5642: ov5642grp { > + fsl,pins = < > + MX6QDL_PAD_SD1_DAT0__GPIO1_IO16 0x8000 > + MX6QDL_PAD_SD1_DAT1__GPIO1_IO17 0x8000 > + >; > + }; > + > + pinctrl_ipu1_csi0: ipu1grp-csi0 { Please rename the node name to ipu1csi0grp. Please add new pin control groups preserving the alphanimerical order. > + fsl,pins = < > + MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA12 > 0x8000 > + MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA13 > 0x8000 > + MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA14 > 0x8000 > + MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA15 > 0x8000 > + MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA16 > 0x8000 > + MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA17 > 0x8000 > + MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA18 > 0x8000 > + MX6QDL_PAD_CSI0_DAT19__IPU1_CSI0_DATA19 > 0x8000 > + MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK > 0x8000 > + MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC > 0x8000 > + MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC > 0x8000 > + >; > + }; > + > pinctrl_pcie: pciegrp { >