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@xxxxxxxxxxx>
> > ---
> > 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
