[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8355dc9f-ec3b-4738-b4e1-41351af2fa91@oss.qualcomm.com>
Date: Thu, 31 Oct 2024 21:39:06 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: Krishna Kurapati <quic_kriskura@...cinc.com>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/4] arm64: dts: qcom: sar2130p: add support for
SAR2130P
On 30.10.2024 12:50 PM, Dmitry Baryshkov wrote:
> Add DT file for the Qualcomm SAR2130P platform.
>
> Co-developed-by: Konrad Dybcio <konrad.dybcio@...aro.org>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> ---
> arch/arm64/boot/dts/qcom/sar2130p.dtsi | 3091 ++++++++++++++++++++++++++++++++
> 1 file changed, 3091 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sar2130p.dtsi b/arch/arm64/boot/dts/qcom/sar2130p.dtsi
> new file mode 100644
> index 0000000000000000000000000000000000000000..d4f5514cbdaf2d0a1c1cd367be2d7a08246d203e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sar2130p.dtsi
> @@ -0,0 +1,3091 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2024, Linaro Limited
> + */
> +
> +#include <dt-bindings/clock/qcom,rpmh.h>
> +#include <dt-bindings/clock/qcom,sar2130p-gcc.h>
> +#include <dt-bindings/clock/qcom,sar2130p-gpucc.h>
> +#include <dt-bindings/clock/qcom,sm8550-tcsr.h>
> +#include <dt-bindings/dma/qcom-gpi.h>
> +#include <dt-bindings/interconnect/qcom,icc.h>
> +#include <dt-bindings/interconnect/qcom,sar2130p-rpmh.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/mailbox/qcom-ipcc.h>
> +#include <dt-bindings/phy/phy-qcom-qmp.h>
> +#include <dt-bindings/power/qcom-rpmpd.h>
> +#include <dt-bindings/power/qcom,rpmhpd.h>
> +#include <dt-bindings/soc/qcom,gpr.h>
> +#include <dt-bindings/soc/qcom,rpmh-rsc.h>
> +#include <dt-bindings/thermal/thermal.h>
> +
> +/ {
> + interrupt-parent = <&intc>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen { };
> +
> + clocks {
> + xo_board: xo-board {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <19200000>;
> + };
> +
> + sleep_clk: sleep-clk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32000>;
> + };
> + };
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + CPU0: cpu@0 {
Krzysztof recently turned these lowercase, please follow suit
[...]
> + /* secdata region can be reused by apps */
This comment isn't very useful
> + smem: smem@...00000 {
> + compatible = "qcom,smem";
> + reg = <0x0 0x80900000 0x0 0x200000>;
> + hwlocks = <&tcsr_mutex 3>;
> + no-map;
> + };
> +
> + cpucp_fw_mem: cpucp-fw-region@...00000 {
Not sure if we want -region everywhere. Krzysztof?
[...]
> + soc: soc@0 {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0 0 0 0 0x10 0>;
> + dma-ranges = <0 0 0 0 0x10 0>;
> + compatible = "simple-bus";
Compatible first, please
[...]
> + #size-cells = <2>;
> + ranges;
> + status = "disabled";
A newline before status would be very cool
[...]
> + pdc: interrupt-controller@...0000 {
> + compatible = "qcom,sar2130p-pdc", "qcom,pdc";
> + reg = <0x0 0x0b220000 0x0 0x30000>, <0x0 0x174000f0 0x0 0x64>;
> + qcom,pdc-ranges = <0 480 94>,
> + <94 609 31>, <125 63 1>, <126 716 12>;
Super weird line break
[...]
> +
> + apps_smmu: iommu@...00000 {
> + compatible = "qcom,sar2130p-smmu-500", "qcom,smmu-500", "arm,mmu-500";
> + reg = <0x0 0x15000000 0x0 0x100000>;
> + #iommu-cells = <2>;
> + #global-interrupts = <1>;
> + interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 185 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 188 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 190 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 191 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 315 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 318 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 319 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 321 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 322 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 323 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 324 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 325 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 326 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 327 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 328 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 332 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 333 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 338 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 339 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 340 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 341 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 342 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 344 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 345 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 395 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 396 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 397 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 398 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 399 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 401 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 402 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 403 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 405 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 407 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 408 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 419 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 412 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 707 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 690 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 691 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 692 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 693 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 695 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 696 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 697 IRQ_TYPE_LEVEL_HIGH>;
dma-coherent?
Please check if this is the order they appear in originally, as they're
ordered (although on some recent socs it appears they're accidentally
sorted by design)
> + };
> +
> + intc: interrupt-controller@...00000 {
> + compatible = "arm,gic-v3";
> + #interrupt-cells = <3>;
> + interrupt-controller;
> + #redistributor-regions = <1>;
> + redistributor-stride = <0x0 0x20000>;
> + reg = <0x0 0x17200000 0x0 0x10000>, /* GICD */
> + <0x0 0x17260000 0x0 0x100000>; /* GICR * 8 */
Please remove these comments
[...]
> +
> + /*
> + * Bootloader expects just cache-controller node instead of
> + * the typical system-cache-controller
> + */
Uh-oh..
> + llcc: cache-controller@...00000 {
Konrad
Powered by blists - more mailing lists