Re: [PATCH v4 3/3] arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: Add navigation mezzanine dts

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 12/11/2022 13:17, Konrad Dybcio wrote:
> 
> 
> On 12/11/2022 13:41, Bryan O'Donoghue wrote:
>> Move the dts data for the rb3 navigation mezzanine into its own dts file.
>>
>> Suggested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/Makefile             |   1 +
>>   .../sdm845-db845c-navigation-mezzanine.dts    | 109 ++++++++++++++++++
>>   arch/arm64/boot/dts/qcom/sdm845-db845c.dts    | 101 ----------------
>>   3 files changed, 110 insertions(+), 101 deletions(-)
>>   create mode 100644 
>> arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dts
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile 
>> b/arch/arm64/boot/dts/qcom/Makefile
>> index cd88efa19e750..5eadd251a0a16 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -132,6 +132,7 @@ dtb-$(CONFIG_ARCH_QCOM)    += sdm845-cheza-r1.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)    += sdm845-cheza-r2.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)    += sdm845-cheza-r3.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)    += sdm845-db845c.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)    += sdm845-db845c-navigation-mezzanine.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)    += sdm845-lg-judyln.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)    += sdm845-lg-judyp.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)    += sdm845-mtp.dtb
>> diff --git 
>> a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dts 
>> b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dts
>> new file mode 100644
>> index 0000000000000..0862ca30c8963
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dts
>> @@ -0,0 +1,109 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2022, Linaro Ltd.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "sdm845-db845c.dts"
>> +
>> +&cci {
>> +    status = "okay";
>> +};
>> +
>> +&camss {
>> +    vdda-phy-supply = <&vreg_l1a_0p875>;
>> +    vdda-pll-supply = <&vreg_l26a_1p2>;
>> +
>> +    status = "ok";
> "okay" is preferred.
> 
>> +
>> +    ports {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
> Looks like the -cells properties are defined in 845 dtsi already.
> 
> 
>> +        port@0 {
>> +            reg = <0>;
>> +            csiphy0_ep: endpoint {
>> +                data-lanes = <0 1 2 3>;
>> +                remote-endpoint = <&ov8856_ep>;
>> +            };
>> +        };
>> +    };
>> +};
>> +
>> +&cci_i2c0 {
>> +    camera@10 {
>> +        compatible = "ovti,ov8856";
>> +        reg = <0x10>;
>> +
>> +        /* CAM0_RST_N */
>> +        reset-gpios = <&tlmm 9 GPIO_ACTIVE_LOW>;
>> +        pinctrl-names = "default";
>> +        pinctrl-0 = <&cam0_default>;
>> +
>> +        clocks = <&clock_camcc CAM_CC_MCLK0_CLK>;
>> +        clock-names = "xvclk";
>> +        clock-frequency = <19200000>;
>> +
>> +        /*
>> +         * The &vreg_s4a_1p8 trace is powered on as a,
>> +         * so it is represented by a fixed regulator.
>> +         *
>> +         * The 2.8V vdda-supply and 1.2V vddd-supply regulators
>> +         * both have to be enabled through the power management
>> +         * gpios.
>> +         */
>> +        dovdd-supply = <&vreg_lvs1a_1p8>;
>> +        avdd-supply = <&cam0_avdd_2v8>;
>> +        dvdd-supply = <&cam0_dvdd_1v2>;
>> +
>> +        status = "ok";
> "okay" is prefered.
> 
>> +
>> +        port {
>> +            ov8856_ep: endpoint {
>> +                link-frequencies = /bits/ 64
>> +                    <360000000 180000000>;
>> +                data-lanes = <1 2 3 4>;
>> +                remote-endpoint = <&csiphy0_ep>;
>> +            };
>> +        };
>> +    };
>> +};
>> +
>> +&cci_i2c1 {
>> +    camera@60 {
>> +        compatible = "ovti,ov7251";
>> +
>> +        /* I2C address as per ov7251.txt linux documentation */
>> +        reg = <0x60>;
>> +
>> +        /* CAM3_RST_N */
>> +        enable-gpios = <&tlmm 21 GPIO_ACTIVE_HIGH>;
>> +        pinctrl-names = "default";
>> +        pinctrl-0 = <&cam3_default>;
>> +
>> +        clocks = <&clock_camcc CAM_CC_MCLK3_CLK>;
>> +        clock-names = "xclk";
>> +        clock-frequency = <24000000>;
>> +
>> +        /*
>> +         * The &vreg_s4a_1p8 trace always powered on.
>> +         *
>> +         * The 2.8V vdda-supply regulator is enabled when the
>> +         * vreg_s4a_1p8 trace is pulled high.
>> +         * It too is represented by a fixed regulator.
>> +         *
>> +         * No 1.2V vddd-supply regulator is used.
>> +         */
>> +        vdddo-supply = <&vreg_lvs1a_1p8>;
>> +        vdda-supply = <&cam3_avdd_2v8>;
>> +
>> +        status = "disable";
> Missing d
> 
> Konrad
>> +
>> +        port {
>> +            ov7251_ep: endpoint {
>> +                data-lanes = <0 1>;
>> +/*                remote-endpoint = <&csiphy3_ep>; */
>> +            };
>> +        };
>> +    };
>> +};
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts 
>> b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
>> index 3e7ceb0861eb0..f41c6d600ea8c 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
>> @@ -1174,107 +1174,6 @@ &pm8998_gpio {
>>   };
>> -&cci {
>> -    status = "okay";
>> -};
>> -
>> -&camss {
>> -    vdda-phy-supply = <&vreg_l1a_0p875>;
>> -    vdda-pll-supply = <&vreg_l26a_1p2>;
>> -
>> -    status = "ok";
>> -
>> -    ports {
>> -        #address-cells = <1>;
>> -        #size-cells = <0>;
>> -        port@0 {
>> -            reg = <0>;
>> -            csiphy0_ep: endpoint {
>> -                data-lanes = <0 1 2 3>;
>> -                remote-endpoint = <&ov8856_ep>;
>> -            };
>> -        };
>> -    };
>> -};
>> -
>> -&cci_i2c0 {
>> -    camera@10 {
>> -        compatible = "ovti,ov8856";
>> -        reg = <0x10>;
>> -
>> -        /* CAM0_RST_N */
>> -        reset-gpios = <&tlmm 9 GPIO_ACTIVE_LOW>;
>> -        pinctrl-names = "default";
>> -        pinctrl-0 = <&cam0_default>;
>> -
>> -        clocks = <&clock_camcc CAM_CC_MCLK0_CLK>;
>> -        clock-names = "xvclk";
>> -        clock-frequency = <19200000>;
>> -
>> -        /*
>> -         * The &vreg_s4a_1p8 trace is powered on as a,
>> -         * so it is represented by a fixed regulator.
>> -         *
>> -         * The 2.8V vdda-supply and 1.2V vddd-supply regulators
>> -         * both have to be enabled through the power management
>> -         * gpios.
>> -         */
>> -        dovdd-supply = <&vreg_lvs1a_1p8>;
>> -        avdd-supply = <&cam0_avdd_2v8>;
>> -        dvdd-supply = <&cam0_dvdd_1v2>;
>> -
>> -        status = "ok";
>> -
>> -        port {
>> -            ov8856_ep: endpoint {
>> -                link-frequencies = /bits/ 64
>> -                    <360000000 180000000>;
>> -                data-lanes = <1 2 3 4>;
>> -                remote-endpoint = <&csiphy0_ep>;
>> -            };
>> -        };
>> -    };
>> -};
>> -
>> -&cci_i2c1 {
>> -    camera@60 {
>> -        compatible = "ovti,ov7251";
>> -
>> -        /* I2C address as per ov7251.txt linux documentation */
>> -        reg = <0x60>;
>> -
>> -        /* CAM3_RST_N */
>> -        enable-gpios = <&tlmm 21 GPIO_ACTIVE_HIGH>;
>> -        pinctrl-names = "default";
>> -        pinctrl-0 = <&cam3_default>;
>> -
>> -        clocks = <&clock_camcc CAM_CC_MCLK3_CLK>;
>> -        clock-names = "xclk";
>> -        clock-frequency = <24000000>;
>> -
>> -        /*
>> -         * The &vreg_s4a_1p8 trace always powered on.
>> -         *
>> -         * The 2.8V vdda-supply regulator is enabled when the
>> -         * vreg_s4a_1p8 trace is pulled high.
>> -         * It too is represented by a fixed regulator.
>> -         *
>> -         * No 1.2V vddd-supply regulator is used.
>> -         */
>> -        vdddo-supply = <&vreg_lvs1a_1p8>;
>> -        vdda-supply = <&cam3_avdd_2v8>;
>> -
>> -        status = "disable";
>> -
>> -        port {
>> -            ov7251_ep: endpoint {
>> -                data-lanes = <0 1>;
>> -/*                remote-endpoint = <&csiphy3_ep>; */
>> -            };
>> -        };
>> -    };
>> -};
>> -
>>   /* PINCTRL - additions to nodes defined in sdm845.dtsi */
>>   &qup_spi0_default {
>>       config {

Hmm.

I wasn't really looking to do anything other than move existing upstream 
stuff from one location to another.

But, since you flag it I will generate a number of Fixes: and append 
them to the head of this series.

---
bod