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;
MIME-Version: 1.0
References: <20221030194311.197909-1-a39.skl@gmail.com> <20221030194311.197909-2-a39.skl@gmail.com>
In-Reply-To: <20221030194311.197909-2-a39.skl@gmail.com>
From:   Rob Herring <robh+dt@kernel.org>
Date:   Sun, 30 Oct 2022 17:01:42 -0500
Message-ID: <CAL_JsqL3Yq53m6D_eELurkJjDCWZuvMtACbUnqpLymWAuFh=OQ@mail.gmail.com>
Subject: Re: [PATCH 1/1] arm64: dts: qcom: Add MSM8976 device tree
To:     Adam Skladowski <a39.skl@gmail.com>
Cc:     phone-devel@vger.kernel.org, ~postmarketos/upstreaming@lists.sr.ht,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Oct 30, 2022 at 2:43 PM Adam Skladowski <a39.skl@gmail.com> wrote:
>
> Add a base DT for MSM8976 SoC.
>
> Signed-off-by: Adam Skladowski <a39.skl@gmail.com>
> ---
>  arch/arm64/boot/dts/qcom/msm8976.dtsi | 1357 +++++++++++++++++++++++++
>  1 file changed, 1357 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8976.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8976.dtsi b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> new file mode 100644
> index 0000000000000..461593eb04635
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8976.dtsi
> @@ -0,0 +1,1357 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/* Copyright (c) 2022, The Linux Foundation. All rights reserved. */
> +
> +#include <dt-bindings/clock/qcom,gcc-msm8976.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/power/qcom-rpmpd.h>
> +#include <dt-bindings/thermal/thermal.h>
> +
> +/ {
> +       interrupt-parent = <&intc>;
> +
> +       #address-cells = <2>;
> +       #size-cells = <2>;
> +
> +       chosen { };
> +
> +       clocks {

Drop the container node.

> +               sleep_clk: sleep-clk {
> +                       compatible = "fixed-clock";
> +                       #clock-cells = <0>;
> +                       clock-frequency = <32768>;
> +               };
> +
> +               xo_board: xo-board {
> +                       compatible = "fixed-clock";
> +                       #clock-cells = <0>;
> +                       clock-frequency = <19200000>;
> +                       clock-output-names = "xo";
> +               };
> +       };
> +
> +       cpus {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               CPU0: cpu@0 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       reg = <0x0>;
> +                       enable-method = "psci";
> +                       capacity-dmips-mhz = <1024>;
> +                       next-level-cache = <&L2_0>;
> +                       #cooling-cells = <2>;
> +
> +                       l1-icache {
> +                               compatible = "cache";
> +                       };
> +                       l1-dcache {
> +                               compatible = "cache";
> +                       };

The cpu node is also the L1 cache(s). You've got a mixture here with
'next-level-cache' in the cpu node. Drop these nodes.

> +               };
> +
> +               CPU1: cpu@1 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       reg = <0x1>;
> +                       enable-method = "psci";
> +                       capacity-dmips-mhz = <1024>;
> +                       next-level-cache = <&L2_0>;
> +                       #cooling-cells = <2>;
> +
> +                       l1-icache {
> +                               compatible = "cache";
> +                       };
> +                       l1-dcache {
> +                               compatible = "cache";
> +                       };
> +               };
> +
> +               CPU2: cpu@2 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       reg = <0x2>;
> +                       enable-method = "psci";
> +                       capacity-dmips-mhz = <1024>;
> +                       next-level-cache = <&L2_0>;
> +                       #cooling-cells = <2>;
> +
> +                       l1-icache {
> +                               compatible = "cache";
> +                       };
> +                       l1-dcache {
> +                               compatible = "cache";
> +                       };
> +               };
> +
> +               CPU3: cpu@3 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a53";
> +                       reg = <0x3>;
> +                       enable-method = "psci";
> +                       capacity-dmips-mhz = <1024>;
> +                       next-level-cache = <&L2_0>;
> +                       #cooling-cells = <2>;
> +
> +                       l1-icache {
> +                               compatible = "cache";
> +                       };
> +                       l1-dcache {
> +                               compatible = "cache";
> +                       };
> +               };
> +
> +               CPU4: cpu@100 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x100>;
> +                       enable-method = "psci";
> +                       capacity-dmips-mhz = <1830>;
> +                       next-level-cache = <&L2_1>;
> +                       #cooling-cells = <2>;
> +
> +                       l1-icache {
> +                               compatible = "cache";
> +                       };
> +                       l1-dcache {
> +                               compatible = "cache";
> +                       };
> +               };
> +
> +               CPU5: cpu@101 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x101>;
> +                       enable-method = "psci";
> +                       capacity-dmips-mhz = <1830>;
> +                       next-level-cache = <&L2_1>;
> +                       #cooling-cells = <2>;
> +
> +                       l1-icache {
> +                               compatible = "cache";
> +                       };
> +                       l1-dcache {
> +                               compatible = "cache";
> +                       };
> +               };
> +
> +               CPU6: cpu@102 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x102>;
> +                       enable-method = "psci";
> +                       capacity-dmips-mhz = <1830>;
> +                       next-level-cache = <&L2_1>;
> +                       #cooling-cells = <2>;
> +
> +                       l1-icache {
> +                               compatible = "cache";
> +                       };
> +                       l1-dcache {
> +                               compatible = "cache";
> +                       };
> +               };
> +
> +               CPU7: cpu@103 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x103>;
> +                       enable-method = "psci";
> +                       capacity-dmips-mhz = <1830>;
> +                       next-level-cache = <&L2_1>;
> +                       #cooling-cells = <2>;
> +
> +                       l1-icache {
> +                               compatible = "cache";
> +                       };
> +                       l1-dcache {
> +                               compatible = "cache";
> +                       };
> +               };
> +
> +               cpu-map {
> +                       cluster0 {
> +                               core0 {
> +                                       cpu = <&CPU0>;
> +                               };
> +                               core1 {
> +                                       cpu = <&CPU1>;
> +                               };
> +                               core2 {
> +                                       cpu = <&CPU2>;
> +                               };
> +                               core3 {
> +                                       cpu = <&CPU3>;
> +                               };
> +                       };
> +
> +                       cluster1 {
> +                               core0 {
> +                                       cpu = <&CPU4>;
> +                               };
> +                               core1 {
> +                                       cpu = <&CPU5>;
> +                               };
> +                               core2 {
> +                                       cpu = <&CPU6>;
> +                               };
> +                               core3 {
> +                                       cpu = <&CPU7>;
> +                               };
> +                       };
> +               };
> +
> +               L2_0: l2-cache_0 {
> +                       compatible = "cache";
> +                       cache-level = <2>;

Unified? Needs 'unified-cache' property.

> +               };
> +
> +               L2_1: l2-cache_1 {
> +                       compatible = "cache";
> +                       cache-level = <2>;
> +               };
> +       };
> +
> +       firmware {
> +               scm: scm {
> +                       compatible = "qcom,scm-msm8976", "qcom,scm";
> +                       clocks = <&gcc GCC_CRYPTO_CLK>,
> +                                <&gcc GCC_CRYPTO_AXI_CLK>,
> +                                <&gcc GCC_CRYPTO_AHB_CLK>;
> +                       clock-names = "core", "bus", "iface";
> +                       #reset-cells = <1>;
> +               };
> +       };
> +
> +       memory {

Needs a unit-address.

> +               device_type = "memory";
> +               /* We expect the bootloader to fill in the reg */

Isn't the base address known?

> +               reg = <0 0 0 0>;
> +       };