| Message ID | 20241008-csi_dcmipp_mp25-v1-15-e3fd0ed54b31@foss.st.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: stm32: introduction of CSI / DCMIPP for STM32MP25 | expand |
On Tue, Oct 08, 2024 at 01:18:17PM +0200, Alain Volmat wrote: > Enable the camera pipeline with a imx335 sensor connected to the > dcmipp via the csi interface. > > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> > --- > arch/arm64/boot/dts/st/stm32mp257f-ev1.dts | 87 ++++++++++++++++++++++++++++++ > 1 file changed, 87 insertions(+) > > diff --git a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts > index 214191a8322b..599af4801d82 100644 > --- a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts > +++ b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts > @@ -27,6 +27,38 @@ chosen { > stdout-path = "serial0:115200n8"; > }; > > + clocks { > + clk_ext_camera: clk-ext-camera { > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <24000000>; > + }; > + }; > + > + imx335_2v9: imx335-2v9 { Please use name for all fixed regulators which matches current format recommendation: 'regulator-[0-9]v[0-9]' https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml?h=v6.11-rc1#n46 > + compatible = "regulator-fixed"; > + regulator-name = "imx335-avdd"; > + regulator-min-microvolt = <2900000>; > + regulator-max-microvolt = <2900000>; > + regulator-always-on; > + }; > + > + imx335_1v8: imx335-1v8 { > + compatible = "regulator-fixed"; > + regulator-name = "imx335-ovdd"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + regulator-always-on; > + }; > + > + imx335_1v2: imx335-1v2 { > + compatible = "regulator-fixed"; > + regulator-name = "imx335-dvdd"; > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <1200000>; > + regulator-always-on; > + }; > + > memory@80000000 { > device_type = "memory"; > reg = <0x0 0x80000000 0x1 0x0>; > @@ -50,6 +82,40 @@ &arm_wdt { > status = "okay"; > }; > > +&csi { > + vdd-supply = <&scmi_vddcore>; > + vdda18-supply = <&scmi_v1v8>; > + status = "okay"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@0 { > + reg = <0>; > + csi_sink: endpoint { > + remote-endpoint = <&imx335_ep>; > + data-lanes = <0 1>; > + bus-type = <4>; > + }; > + }; > + port@1 { > + reg = <1>; > + csi_source: endpoint { > + remote-endpoint = <&dcmipp_0>; > + }; > + }; > + }; > +}; > + > +&dcmipp { > + status = "okay"; > + port { > + dcmipp_0: endpoint { > + remote-endpoint = <&csi_source>; > + bus-type = <4>; > + }; > + }; > +}; > + > ðernet2 { > pinctrl-names = "default", "sleep"; > pinctrl-0 = <ð2_rgmii_pins_a>; > @@ -81,6 +147,27 @@ &i2c2 { > i2c-scl-falling-time-ns = <13>; > clock-frequency = <400000>; > status = "okay"; > + > + imx335: imx335@1a { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "sony,imx335"; > + reg = <0x1a>; > + clocks = <&clk_ext_camera>; > + avdd-supply = <&imx335_2v9>; > + ovdd-supply = <&imx335_1v8>; > + dvdd-supply = <&imx335_1v2>; > + reset-gpios = <&gpioi 7 (GPIO_ACTIVE_HIGH | GPIO_PUSH_PULL)>; > + powerdown-gpios = <&gpioi 0 (GPIO_ACTIVE_HIGH | GPIO_PUSH_PULL)>; > + status = "okay"; Why? Didi you disable it anywhere? Best regards, Krzysztof
Hi Krzysztof, On Tue, Oct 08, 2024 at 03:42:42PM +0200, Krzysztof Kozlowski wrote: > On Tue, Oct 08, 2024 at 01:18:17PM +0200, Alain Volmat wrote: > > Enable the camera pipeline with a imx335 sensor connected to the > > dcmipp via the csi interface. > > > > Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> > > --- > > arch/arm64/boot/dts/st/stm32mp257f-ev1.dts | 87 ++++++++++++++++++++++++++++++ > > 1 file changed, 87 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts > > index 214191a8322b..599af4801d82 100644 > > --- a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts > > +++ b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts > > @@ -27,6 +27,38 @@ chosen { > > stdout-path = "serial0:115200n8"; > > }; > > > > + clocks { > > + clk_ext_camera: clk-ext-camera { > > + #clock-cells = <0>; > > + compatible = "fixed-clock"; > > + clock-frequency = <24000000>; > > + }; > > + }; > > + > > + imx335_2v9: imx335-2v9 { > > Please use name for all fixed regulators which matches current format > recommendation: 'regulator-[0-9]v[0-9]' > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml?h=v6.11-rc1#n46 Done in v2 for all 3 fixed-regulators. > > > + compatible = "regulator-fixed"; > > + regulator-name = "imx335-avdd"; > > + regulator-min-microvolt = <2900000>; > > + regulator-max-microvolt = <2900000>; > > + regulator-always-on; > > + }; > > + > > + imx335_1v8: imx335-1v8 { > > + compatible = "regulator-fixed"; > > + regulator-name = "imx335-ovdd"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <1800000>; > > + regulator-always-on; > > + }; > > + > > + imx335_1v2: imx335-1v2 { > > + compatible = "regulator-fixed"; > > + regulator-name = "imx335-dvdd"; > > + regulator-min-microvolt = <1200000>; > > + regulator-max-microvolt = <1200000>; > > + regulator-always-on; > > + }; > > + > > memory@80000000 { > > device_type = "memory"; > > reg = <0x0 0x80000000 0x1 0x0>; > > @@ -50,6 +82,40 @@ &arm_wdt { > > status = "okay"; > > }; > > > > +&csi { > > + vdd-supply = <&scmi_vddcore>; > > + vdda18-supply = <&scmi_v1v8>; > > + status = "okay"; > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + port@0 { > > + reg = <0>; > > + csi_sink: endpoint { > > + remote-endpoint = <&imx335_ep>; > > + data-lanes = <0 1>; > > + bus-type = <4>; > > + }; > > + }; > > + port@1 { > > + reg = <1>; > > + csi_source: endpoint { > > + remote-endpoint = <&dcmipp_0>; > > + }; > > + }; > > + }; > > +}; > > + > > +&dcmipp { > > + status = "okay"; > > + port { > > + dcmipp_0: endpoint { > > + remote-endpoint = <&csi_source>; > > + bus-type = <4>; > > + }; > > + }; > > +}; > > + > > ðernet2 { > > pinctrl-names = "default", "sleep"; > > pinctrl-0 = <ð2_rgmii_pins_a>; > > @@ -81,6 +147,27 @@ &i2c2 { > > i2c-scl-falling-time-ns = <13>; > > clock-frequency = <400000>; > > status = "okay"; > > + > > + imx335: imx335@1a { > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Changed to camera@1a. > > > + compatible = "sony,imx335"; > > + reg = <0x1a>; > > + clocks = <&clk_ext_camera>; > > + avdd-supply = <&imx335_2v9>; > > + ovdd-supply = <&imx335_1v8>; > > + dvdd-supply = <&imx335_1v2>; > > + reset-gpios = <&gpioi 7 (GPIO_ACTIVE_HIGH | GPIO_PUSH_PULL)>; > > + powerdown-gpios = <&gpioi 0 (GPIO_ACTIVE_HIGH | GPIO_PUSH_PULL)>; > > + status = "okay"; > > Why? Didi you disable it anywhere? status property dropped in v2. powerdown-gpios property dropped as well since not necessary nor described in the sensor yaml. reset-gpios polarity is as well corrected in v2, following an change within the imx335 sensor. > > Best regards, > Krzysztof > Regards, Alain
diff --git a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts index 214191a8322b..599af4801d82 100644 --- a/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts +++ b/arch/arm64/boot/dts/st/stm32mp257f-ev1.dts @@ -27,6 +27,38 @@ chosen { stdout-path = "serial0:115200n8"; }; + clocks { + clk_ext_camera: clk-ext-camera { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <24000000>; + }; + }; + + imx335_2v9: imx335-2v9 { + compatible = "regulator-fixed"; + regulator-name = "imx335-avdd"; + regulator-min-microvolt = <2900000>; + regulator-max-microvolt = <2900000>; + regulator-always-on; + }; + + imx335_1v8: imx335-1v8 { + compatible = "regulator-fixed"; + regulator-name = "imx335-ovdd"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + }; + + imx335_1v2: imx335-1v2 { + compatible = "regulator-fixed"; + regulator-name = "imx335-dvdd"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + }; + memory@80000000 { device_type = "memory"; reg = <0x0 0x80000000 0x1 0x0>; @@ -50,6 +82,40 @@ &arm_wdt { status = "okay"; }; +&csi { + vdd-supply = <&scmi_vddcore>; + vdda18-supply = <&scmi_v1v8>; + status = "okay"; + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + csi_sink: endpoint { + remote-endpoint = <&imx335_ep>; + data-lanes = <0 1>; + bus-type = <4>; + }; + }; + port@1 { + reg = <1>; + csi_source: endpoint { + remote-endpoint = <&dcmipp_0>; + }; + }; + }; +}; + +&dcmipp { + status = "okay"; + port { + dcmipp_0: endpoint { + remote-endpoint = <&csi_source>; + bus-type = <4>; + }; + }; +}; + ðernet2 { pinctrl-names = "default", "sleep"; pinctrl-0 = <ð2_rgmii_pins_a>; @@ -81,6 +147,27 @@ &i2c2 { i2c-scl-falling-time-ns = <13>; clock-frequency = <400000>; status = "okay"; + + imx335: imx335@1a { + compatible = "sony,imx335"; + reg = <0x1a>; + clocks = <&clk_ext_camera>; + avdd-supply = <&imx335_2v9>; + ovdd-supply = <&imx335_1v8>; + dvdd-supply = <&imx335_1v2>; + reset-gpios = <&gpioi 7 (GPIO_ACTIVE_HIGH | GPIO_PUSH_PULL)>; + powerdown-gpios = <&gpioi 0 (GPIO_ACTIVE_HIGH | GPIO_PUSH_PULL)>; + status = "okay"; + + port { + imx335_ep: endpoint { + remote-endpoint = <&csi_sink>; + clock-lanes = <0>; + data-lanes = <1 2>; + link-frequencies = /bits/ 64 <594000000>; + }; + }; + }; }; &i2c8 {
Enable the camera pipeline with a imx335 sensor connected to the dcmipp via the csi interface. Signed-off-by: Alain Volmat <alain.volmat@foss.st.com> --- arch/arm64/boot/dts/st/stm32mp257f-ev1.dts | 87 ++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+)