Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e0ab19b3-0ae4-efd4-4035-44c8a5aec443@linaro.org>
Date:   Wed, 13 Apr 2022 11:56:21 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH v3 17/18] ARM: dts: qcom: add ipq8064-v2.0 dtsi
Content-Language: en-GB
To:     Ansuel Smith <ansuelsmth@gmail.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Jonathan McDowell <noodles@earth.li>
References: <20220309190152.7998-1-ansuelsmth@gmail.com>
 <20220309190152.7998-18-ansuelsmth@gmail.com>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
In-Reply-To: <20220309190152.7998-18-ansuelsmth@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 09/03/2022 22:01, Ansuel Smith wrote:
> Many devices are based on the v2.0 of the ipq8064 SoC. Main difference
> is a change in the pci compatible and different way to configre the usb
> phy.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> Tested-by: Jonathan McDowell <noodles@earth.li>
> ---
>   arch/arm/boot/dts/qcom-ipq8064-v2.0.dtsi | 70 ++++++++++++++++++++++++
>   1 file changed, 70 insertions(+)
>   create mode 100644 arch/arm/boot/dts/qcom-ipq8064-v2.0.dtsi
> 
> diff --git a/arch/arm/boot/dts/qcom-ipq8064-v2.0.dtsi b/arch/arm/boot/dts/qcom-ipq8064-v2.0.dtsi
> new file mode 100644
> index 000000000000..c082c3cd1a19
> --- /dev/null
> +++ b/arch/arm/boot/dts/qcom-ipq8064-v2.0.dtsi
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "qcom-ipq8064.dtsi"
> +
> +/ {
> +	aliases {
> +		serial0 = &gsbi4_serial;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};

I'd expect that /aliases and /chosen should go to the board dts files.
I see that ipq8064-v1.0.dtsi also is a mixture of SoC-specific nodes and 
board details (gpio, leds, aliases, etc.). I think it should be split 
into ipq8064-v1.0.dtsi and ipq8064-common.dtsi (or 
ipq8064-v1.0-common.dtsi). This file also should contain just SoC 
specifics, not the enablement of individual devices.

> +
> +	reserved-memory {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		rsvd@41200000 {
> +			reg = <0x41200000 0x300000>;
> +			no-map;
> +		};
> +	};
> +};
> +
> +&gsbi4 {
> +	qcom,mode = <GSBI_PROT_I2C_UART>;
> +	status = "okay";
> +
> +	serial@16340000 {
> +		status = "okay";
> +	};
> +	/*
> +	 * The i2c device on gsbi4 should not be enabled.
> +	 * On ipq806x designs gsbi4 i2c is meant for exclusive
> +	 * RPM usage. Turning this on in kernel manifests as
> +	 * i2c failure for the RPM.
> +	 */
> +};
> +
> +&CPU_SPC {
> +	status = "okay";
> +};
> +
> +&pcie0 {
> +	compatible = "qcom,pcie-ipq8064-v2";
> +};
> +
> +&pcie1 {
> +	compatible = "qcom,pcie-ipq8064-v2";
> +};
> +
> +&pcie2 {
> +	compatible = "qcom,pcie-ipq8064-v2";
> +};
> +
> +&sata {
> +	ports-implemented = <0x1>;
> +};
> +
> +&ss_phy_0 {
> +	qcom,rx-eq = <2>;
> +	qcom,tx-deamp_3_5db = <32>;
> +	qcom,mpll = <5>;
> +};
> +
> +&ss_phy_1 {
> +	qcom,rx-eq = <2>;
> +	qcom,tx-deamp_3_5db = <32>;
> +	qcom,mpll = <5>;
> +};


-- 
With best wishes
Dmitry