From: Florian Fainelli <florian.fainelli@broadcom.com>
To: Andrea della Porta <andrea.porta@suse.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Conor Dooley <conor+dt@kernel.org>, Ray Jui <rjui@broadcom.com>,
Scott Branden <sbranden@broadcom.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@broadcom.com>,
Ulf Hansson <ulf.hansson@linaro.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Kamal Dasu <kamal.dasu@broadcom.com>,
Al Cooper <alcooperx@gmail.com>, Eric Anholt <eric@anholt.net>,
Stefan Wahren <wahrenst@gmx.net>,
devicetree@vger.kernel.org, linux-rpi-kernel@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org
Subject: Re: [PATCH v2 4/4] arm64: dts: broadcom: Add support for BCM2712
Date: Fri, 10 May 2024 09:14:50 -0700 [thread overview]
Message-ID: <786bbf35-e9fe-445c-b6f9-21119e60fb34@broadcom.com> (raw)
In-Reply-To: <59a3015c3a6f2f0b70a38c030274a163773e7757.1715332922.git.andrea.porta@suse.com>
[-- Attachment #1: Type: text/plain, Size: 15276 bytes --]
On 5/10/24 07:35, Andrea della Porta wrote:
> The BCM2712 SoC family can be found on Raspberry Pi 5.
> Add minimal SoC and board (Rpi5 specific) dts file to be able to
> boot from SD card and use console on debug UART.
>
> Signed-off-by: Andrea della Porta <andrea.porta@suse.com>
> ---
> arch/arm64/boot/dts/broadcom/Makefile | 1 +
> .../boot/dts/broadcom/bcm2712-rpi-5-b.dts | 62 ++++
> arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 302 ++++++++++++++++++
> 3 files changed, 365 insertions(+)
> create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712.dtsi
>
> diff --git a/arch/arm64/boot/dts/broadcom/Makefile b/arch/arm64/boot/dts/broadcom/Makefile
> index 8b4591ddd27c..92565e9781ad 100644
> --- a/arch/arm64/boot/dts/broadcom/Makefile
> +++ b/arch/arm64/boot/dts/broadcom/Makefile
> @@ -6,6 +6,7 @@ DTC_FLAGS := -@
> dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
> bcm2711-rpi-4-b.dtb \
> bcm2711-rpi-cm4-io.dtb \
> + bcm2712-rpi-5-b.dtb \
> bcm2837-rpi-3-a-plus.dtb \
> bcm2837-rpi-3-b.dtb \
> bcm2837-rpi-3-b-plus.dtb \
> diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> new file mode 100644
> index 000000000000..b5921437e09f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
New DTS files should be under GPL-2.0-or-MIT AFAICT.
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include "bcm2712.dtsi"
> +
> +/ {
> + compatible = "raspberrypi,5-model-b", "brcm,bcm2712";
> + model = "Raspberry Pi 5";
> +
> + aliases {
> + serial10 = &uart0;
> + };
> +
> + chosen: chosen {
> + stdout-path = "serial10:115200n8";
> + };
> +
> + /* Will be filled by the bootloader */
> + memory@0 {
> + device_type = "memory";
> + reg = <0 0 0x28000000>;
That should be a 4 cell member, see my comment on bcm2712.dtsi.
> + };
> +
> + sd_io_1v8_reg: sd-io-1v8-reg {
> + compatible = "regulator-gpio";
> + regulator-name = "vdd-sd-io";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-boot-on;
> + regulator-always-on;
> + regulator-settling-time-us = <5000>;
> + gpios = <&gio_aon 3 GPIO_ACTIVE_HIGH>;
> + states = <1800000 0x1>,
> + <3300000 0x0>;
Can we use decimal for the second cell as well?
> + };
> +
> + sd_vcc_reg: sd-vcc-reg {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc-sd";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-boot-on;
> + enable-active-high;
> + gpios = <&gio_aon 4 GPIO_ACTIVE_HIGH>;
> + };
> +};
> +
> +/* The system UART */
> +&uart0 {
> + status = "okay";
> +};
> +
> +/* SDIO1 is used to drive the SD card */
> +&sdio1 {
> + vqmmc-supply = <&sd_io_1v8_reg>;
> + vmmc-supply = <&sd_vcc_reg>;
> + bus-width = <4>;
> + sd-uhs-sdr50;
> + sd-uhs-ddr50;
> + sd-uhs-sdr104;
> +};
> diff --git a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
> new file mode 100644
> index 000000000000..398df13148bd
> --- /dev/null
> +++ b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
> @@ -0,0 +1,302 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> + compatible = "brcm,bcm2712";
> +
> + #address-cells = <2>;
> + #size-cells = <1>;
This should be #size-cells = <2> to be future proof and support over 4GB
of DRAM, because the DDR controller and the memory map on that chip have
been designed with that requirement.
> +
> + interrupt-parent = <&gicv2>;
> +
> + axi: axi {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <1>;
> + ranges;
The AXI peripheral window should be defined in the ranges property. The
aperture is from 0x10_0000_0000 to 0x10_3FFF_FFFF.
From that point on you can define all peripherals under the axi node to
be relative to that axi aperture, just like what you did for the legacy
Pi peripherals in the subsequent bus node.
> +
> + sdio1: mmc@1000fff000 {
> + compatible = "brcm,bcm2712-sdhci",
> + "brcm,sdhci-brcmstb";
> + reg = <0x10 0x00fff000 0x260>,
> + <0x10 0x00fff400 0x200>;
> + reg-names = "host", "cfg";
> + interrupts = <GIC_SPI 273 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk_emmc2>;
> + clock-names = "sw_sdio";
> + mmc-ddr-3_3v;
> + };
> +
> + gicv2: interrupt-controller@107fff9000 {
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + compatible = "arm,gic-400";
> + reg = <0x10 0x7fff9000 0x1000>,
> + <0x10 0x7fffa000 0x2000>,
> + <0x10 0x7fffc000 0x2000>,
> + <0x10 0x7fffe000 0x2000>;
> + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
> + IRQ_TYPE_LEVEL_HIGH)>;
I would omit this interrupt property, because it is not clear to me
whether the VGIC maintenance interrupt has actually been wired up.
> + };
> + };
> +
> + clocks {
> + /* The oscillator is the root of the clock tree. */
> + clk_osc: clk-osc {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-output-names = "osc";
> + clock-frequency = <54000000>;
> + };
> +
> + clk_vpu: clk-vpu {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <750000000>;
> + clock-output-names = "vpu-clock";
> + };
> +
> + clk_uart: clk-uart {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <9216000>;
> + clock-output-names = "uart-clock";
> + };
> +
> + clk_emmc2: clk-emmc2 {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <200000000>;
> + clock-output-names = "emmc2-clock";
> + };
> + };
> +
> + cpus: cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* Source for d/i cache-line-size, cache-sets, cache-size
> + * https://developer.arm.com/documentation/100798/0401
> + * /L1-memory-system/About-the-L1-memory-system?lang=en
> + */
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a76";
> + reg = <0x000>;
> + enable-method = "psci";
> + d-cache-size = <0x10000>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <256>; // 64KiB(size)/64(line-size)=1024ways/4-way set
> + i-cache-size = <0x10000>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <256>; // 64KiB(size)/64(line-size)=1024ways/4-way set
> + next-level-cache = <&l2_cache_l0>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a76";
> + reg = <0x100>;
> + enable-method = "psci";
> + d-cache-size = <0x10000>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <256>; // 64KiB(size)/64(line-size)=1024ways/4-way set
> + i-cache-size = <0x10000>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <256>; // 64KiB(size)/64(line-size)=1024ways/4-way set
> + next-level-cache = <&l2_cache_l1>;
> + };
> +
> + cpu2: cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a76";
> + reg = <0x200>;
> + enable-method = "psci";
> + d-cache-size = <0x10000>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <256>; // 64KiB(size)/64(line-size)=1024ways/4-way set
> + i-cache-size = <0x10000>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <256>; // 64KiB(size)/64(line-size)=1024ways/4-way set
> + next-level-cache = <&l2_cache_l2>;
> + };
> +
> + cpu3: cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a76";
> + reg = <0x300>;
> + enable-method = "psci";
> + d-cache-size = <0x10000>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <256>; // 64KiB(size)/64(line-size)=1024ways/4-way set
> + i-cache-size = <0x10000>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <256>; // 64KiB(size)/64(line-size)=1024ways/4-way set
> + next-level-cache = <&l2_cache_l3>;
> + };
> +
> + /* Source for cache-line-size and cache-sets:
> + * https://developer.arm.com/documentation/100798/0401
> + * /L2-memory-system/About-the-L2-memory-system?lang=en
> + * and for cache-size:
> + * https://www.raspberrypi.com/documentation/computers
> + * /processors.html#bcm2712
> + */
> + l2_cache_l0: l2-cache-l0 {
> + compatible = "cache";
> + cache-size = <0x80000>;
> + cache-line-size = <128>;
> + cache-sets = <1024>; // 512KiB(size)/64(line-size)=8192ways/8-way set
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
Since each of these L2 caches are per-CPU core, they should be under
their respective CPU node. This is what I did for the BCM2712 sister
chip, and it looks like this:
cpu@0 {
operating-points = <0x0 0x0 0x0 0x0 0x0 0x0 0x0
0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
d-cache-sets = <0x100>;
d-cache-line-size = <0x40>;
d-cache-size = <0x10000>;
i-cache-sets = <0x100>;
i-cache-line-size = <0x40>;
i-cache-size = <0x10000>;
clock-frequency = <0x95aad1c0>;
device_type = "cpu";
enable-method = "psci";
compatible = "arm,cortex-a76";
reg = <0x0 0x0>;
next-level-cache = <0x2>;
cpu-idle-states = <0x3>;
clocks = <0x4 0x0>;
clock-names = "cpu";
phandle = <0x9>;
cache-controller-0 {
cache-sets = <0x400>;
cache-line-size = <0x40>;
cache-size = <0x80000>;
compatible = "cache";
cache-unified;
cache-level = <0x2>;
next-level-cache = <0x5>;
phandle = <0x2>;
};
};
Also, the values are actually populated by the boot loader from reading
the CLIDR registers, and they yield the same results as yours, so this
part looks good to me. We should consider decimal numbers however for a
source level Device Tree (the output I provided is rendered via libfdt).
> +
> + l2_cache_l1: l2-cache-l1 {
> + compatible = "cache";
> + cache-size = <0x80000>;
> + cache-line-size = <128>;
> + cache-sets = <1024>; // 512KiB(size)/64(line-size)=8192ways/8-way set
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
> +
> + l2_cache_l2: l2-cache-l2 {
> + compatible = "cache";
> + cache-size = <0x80000>;
> + cache-line-size = <128>;
> + cache-sets = <1024>; // 512KiB(size)/64(line-size)=8192ways/8-way set
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
> +
> + l2_cache_l3: l2-cache-l3 {
> + compatible = "cache";
> + cache-size = <0x80000>;
> + cache-line-size = <128>;
> + cache-sets = <1024>; // 512KiB(size)/64(line-size)=8192ways/8-way set
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
> +
> + /* Source for cache-line-size and cache-sets:
> + * https://developer.arm.com/documentation/100453/0401/L3-cache?lang=en
> + * Source for cache-size:
> + * https://www.raspberrypi.com/documentation/computers/processors.html#bcm2712
> + */
> + l3_cache: l3-cache {
> + compatible = "cache";
> + cache-size = <0x200000>;
> + cache-line-size = <64>;
> + cache-sets = <2048>; // 2MiB(size)/64(line-size)=32768ways/16-way set
> + cache-level = <3>;
> + cache-unified;
> + };
> + };
> +
> + psci {
> + method = "smc";
> + compatible = "arm,psci-1.0", "arm,psci-0.2", "arm,psci";
> + cpu_on = <0xc4000003>;
> + cpu_suspend = <0xc4000001>;
> + cpu_off = <0x84000002>;
> + };
> +
> + rmem: reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <1>;
> + ranges;
> +
> + atf@0 {
> + reg = <0x0 0x0 0x80000>;
> + no-map;
> + };
> +
> + cma: linux,cma {
> + compatible = "shared-dma-pool";
> + size = <0x4000000>; /* 64MB */
> + reusable;
> + linux,cma-default;
> + alloc-ranges = <0x0 0x00000000 0x40000000>;
> + };
> + };
> +
> + soc: soc@107c000000 {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + ranges = <0x7c000000 0x10 0x7c000000 0x04000000>;
> + /* Emulate a contiguous 30-bit address range for DMA */
> + dma-ranges = <0xc0000000 0x00 0x00000000 0x40000000>,
> + <0x7c000000 0x10 0x7c000000 0x04000000>;
> +
> + system_timer: timer@7c003000 {
> + compatible = "brcm,bcm2835-system-timer";
> + reg = <0x7c003000 0x1000>;
> + interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
> + clock-frequency = <1000000>;
> + };
> +
> + mailbox: mailbox@7c013880 {
> + compatible = "brcm,bcm2835-mbox";
> + reg = <0x7c013880 0x40>;
> + interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> + #mbox-cells = <0>;
> + };
> +
> + local_intc: local-intc@7cd00000 {
> + compatible = "brcm,bcm2836-l1-intc";
> + reg = <0x7cd00000 0x100>;
> + };
> +
> + uart0: serial@7d001000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0x7d001000 0x200>;
> + interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk_uart>, <&clk_vpu>;
> + clock-names = "uartclk", "apb_pclk";
> + arm,primecell-periphid = <0x00241011>;
> + status = "disabled";
> + };
> +
> + interrupt-controller@7d517000 {
> + compatible = "brcm,bcm7271-l2-intc";
> + reg = <0x7d517000 0x10>;
> + interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + status = "disabled";
> + };
I see no reason for an interrupt controller to be disabled by default,
if the interrupts are not used, we are not going to be consuming resources.
> +
> + gio_aon: gpio@7d517c00 {
> + compatible = "brcm,bcm7445-gpio", "brcm,brcmstb-gpio";
> + reg = <0x7d517c00 0x40>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + // Don't use GIO_AON as an interrupt controller because it will
> + // clash with the firmware monitoring the PMIC interrupt via the VPU.
> + brcm,gpio-bank-widths = <17 6>;
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
> + IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
> + IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
> + IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
> + IRQ_TYPE_LEVEL_LOW)>;
> + /* This only applies to the ARMv7 stub */
> + arm,cpu-registers-not-fw-configured;
And the A76 cannot boot an AArch32 kernel at EL1, is that comment still
relevant? It looks like there is an ARM trusted reserved region, can we
assume that it will have done the configuration properly?
--
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]
next prev parent reply other threads:[~2024-05-10 16:14 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-10 14:35 [PATCH v2 0/4] Add minimal boot support for Raspberry Pi 5 Andrea della Porta
2024-05-10 14:35 ` [PATCH v2 1/4] dt-bindings: arm: bcm: Add BCM2712 SoC support Andrea della Porta
2024-05-10 18:40 ` Stefan Wahren
2024-05-10 21:33 ` Conor Dooley
2024-05-20 13:37 ` Krzysztof Kozlowski
2024-05-10 14:35 ` [PATCH v2 2/4] dt-bindings: mmc: Add support for BCM2712 SD host controller Andrea della Porta
2024-05-10 19:01 ` Stefan Wahren
2024-05-13 18:58 ` Rob Herring
2024-05-10 14:35 ` [PATCH v2 3/4] mmc: sdhci-brcmstb: Add BCM2712 support Andrea della Porta
2024-05-10 19:22 ` Stefan Wahren
2024-05-10 14:35 ` [PATCH v2 4/4] arm64: dts: broadcom: Add support for BCM2712 Andrea della Porta
2024-05-10 16:13 ` Marc Zyngier
2024-05-10 16:14 ` Florian Fainelli [this message]
2024-05-16 8:36 ` Andrea della Porta
2024-05-20 18:03 ` Florian Fainelli
2024-05-10 19:43 ` Stefan Wahren
2024-05-13 13:27 ` [PATCH v2 0/4] Add minimal boot support for Raspberry Pi 5 Rob Herring (Arm)
2024-05-16 10:32 ` Andrea della Porta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=786bbf35-e9fe-445c-b6f9-21119e60fb34@broadcom.com \
--to=florian.fainelli@broadcom.com \
--cc=adrian.hunter@intel.com \
--cc=alcooperx@gmail.com \
--cc=andrea.porta@suse.com \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=eric@anholt.net \
--cc=kamal.dasu@broadcom.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-rpi-kernel@lists.infradead.org \
--cc=rjui@broadcom.com \
--cc=robh@kernel.org \
--cc=sbranden@broadcom.com \
--cc=ulf.hansson@linaro.org \
--cc=wahrenst@gmx.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).