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;
Date:   Thu, 23 Jun 2022 10:52:50 +0530
From:   Vinod Koul <vkoul@kernel.org>
To:     Bjorn Andersson <bjorn.andersson@linaro.org>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Manivannan Sadhasivam <mani@kernel.org>,
        Jassi Brar <jassisinghbrar@gmail.com>,
        Johan Hovold <johan+linaro@kernel.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] arm64: dts: qcom: add SC8280XP platform
Message-ID: <YrP4qs+GIlaS7Wzy@matsya>
References: <20220607214113.4057684-1-bjorn.andersson@linaro.org>
 <20220607214113.4057684-3-bjorn.andersson@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220607214113.4057684-3-bjorn.andersson@linaro.org>
Precedence: bulk

On 07-06-22, 14:41, Bjorn Andersson wrote:

> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		CPU0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "qcom,kryo";

Generic kryo and not kryoxxx?

> +	firmware {
> +		scm: scm {
> +			compatible = "qcom,scm-sc8280xp", "qcom,scm";

I dont see patch documenting this compatible, was it added earlier, if
not pls add..

> +		};
> +	};
> +
> +	aggre1_noc: interconncet-aggre1-noc {

s/interconncet/interconnect

Hmmm I thought it was required that node name should be interconnect@x

> +		qup1: geniqup@ac0000 {
> +			compatible = "qcom,geni-se-qup";
> +			reg = <0 0x00ac0000 0 0x6000>;
> +			clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
> +				 <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;
> +			clock-names = "m-ahb", "s-ahb";
> +			iommus = <&apps_smmu 0x83 0>;
> +
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +
> +			status = "disabled";
> +		};

why add this, if no i2c/spi/uarts are defined under this..?

> +		usb_0_hsphy: phy@88e5000 {
> +			compatible = "qcom,sc8280xp-usb-hs-phy",
> +				     "qcom,usb-snps-hs-5nm-phy";
> +			reg = <0 0x088e5000 0 0x400>;

this doesn't match with node address above (I think W=1 would warn you
of such mismatches, useful to run on new dts

> +
> +		spmi_bus: spmi@c440000 {

Is the new v7 spmi or older one?

-- 
~Vinod