[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <786bbf35-e9fe-445c-b6f9-21119e60fb34@broadcom.com>
Date: Fri, 10 May 2024 09:14:50 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: Andrea della Porta <andrea.porta@...e.com>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Ray Jui <rjui@...adcom.com>,
Scott Branden <sbranden@...adcom.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>, Ulf Hansson
<ulf.hansson@...aro.org>, Adrian Hunter <adrian.hunter@...el.com>,
Kamal Dasu <kamal.dasu@...adcom.com>, Al Cooper <alcooperx@...il.com>,
Eric Anholt <eric@...olt.net>, Stefan Wahren <wahrenst@....net>,
devicetree@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mmc@...r.kernel.org
Subject: Re: [PATCH v2 4/4] arm64: dts: broadcom: Add support for BCM2712
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@...e.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@...0fff000 {
> + 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@...fff9000 {
> + 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@...c000000 {
> + 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@...03000 {
> + 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@...13880 {
> + compatible = "brcm,bcm2835-mbox";
> + reg = <0x7c013880 0x40>;
> + interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> + #mbox-cells = <0>;
> + };
> +
> + local_intc: local-intc@...00000 {
> + compatible = "brcm,bcm2836-l1-intc";
> + reg = <0x7cd00000 0x100>;
> + };
> +
> + uart0: serial@...01000 {
> + 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@...17000 {
> + 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@...17c00 {
> + 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
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4221 bytes)
Powered by blists - more mailing lists