2024-05-10 14:35:40

by Andrea della Porta

[permalink] [raw]
Subject: [PATCH v2 0/4] Add minimal boot support for Raspberry Pi 5

Hi,

This patchset adds minimal support for the Broadcom BCM2712 SoC and for
the on-board SDHCI controller on Broadcom BCM2712 in order to make it
possible to boot (particularly) a Raspberry Pi 5 from SD card and get a
console through uart.
Changes to arm64/defconfig are not needed since the actual options work
as they are.
This work is heavily based on downstream contributions.

Tested on Tumbleweed substituting the stock kernel with upstream one,
either chainloading uboot+grub+kernel or directly booting the kernel
from 1st stage bootloader. Steps to reproduce:
- prepare an SD card from a Raspberry enabled raw image, mount the first
FAT partition.
- make sure the FAT partition is big enough to contain the kernel,
anything bigger than 64Mb is usually enough, depending on your kernel
config options.
- build the kernel and dtbs making sure that the support for your root
fs type is compiled as builtin.
- copy the kernel image in your FAT partition overwriting the older one
(e.g. kernel*.img for Raspberry Pi OS or u-boot.bin for Tumbleweed).
- copy arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb on FAT partition.
- make sure you have a cmdline.txt file in FAT partition with the
following content:
# cat /boot/efi/cmdline.txt
root=/dev/mmcblk0p3 rootwait rw console=tty ignore_loglevel earlycon
console=ttyAMA10,115200
- if you experience random SD issues during boot, try to set
initial_turbo=0 in config.txt.

Changes in V2:

- the patchshet has been considerably simplified, both in terms of dts and
driver code. Notably, the pinctrl/pinmux driver (and associated binding)
was not strictly needed to use the SD card so it has been dropped.
- dropped the optional SD express support patch
- the patches order has been revisited
- pass all checks (binding, dtb, checkpatch)

Many thanks,
Andrea

References:
- Link to V1: https://lore.kernel.org/all/[email protected]/

Andrea della Porta (4):
dt-bindings: arm: bcm: Add BCM2712 SoC support
dt-bindings: mmc: Add support for BCM2712 SD host controller
mmc: sdhci-brcmstb: Add BCM2712 support
arm64: dts: broadcom: Add support for BCM2712

.../devicetree/bindings/arm/bcm/bcm2835.yaml | 6 +
.../bindings/mmc/brcm,sdhci-brcmstb.yaml | 23 ++
arch/arm64/boot/dts/broadcom/Makefile | 1 +
.../boot/dts/broadcom/bcm2712-rpi-5-b.dts | 62 ++++
arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 302 ++++++++++++++++++
drivers/mmc/host/sdhci-brcmstb.c | 81 +++++
6 files changed, 475 insertions(+)
create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712.dtsi

--
2.35.3



2024-05-10 14:35:45

by Andrea della Porta

[permalink] [raw]
Subject: [PATCH v2 1/4] dt-bindings: arm: bcm: Add BCM2712 SoC support

The BCM2712 SoC is found on Raspberry Pi 5. Add compatible string to
acknowledge its new chipset.

Signed-off-by: Andrea della Porta <[email protected]>
---
Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml b/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
index 162a39dab218..e4ff71f006b8 100644
--- a/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/bcm2835.yaml
@@ -23,6 +23,12 @@ properties:
- raspberrypi,4-model-b
- const: brcm,bcm2711

+ - description: BCM2712 based Boards
+ items:
+ - enum:
+ - raspberrypi,5-model-b
+ - const: brcm,bcm2712
+
- description: BCM2835 based Boards
items:
- enum:
--
2.35.3


2024-05-10 14:36:16

by Andrea della Porta

[permalink] [raw]
Subject: [PATCH v2 3/4] mmc: sdhci-brcmstb: Add BCM2712 support

Broadcom BCM2712 SoC has an SDHCI card controller using the SDIO CFG
register block present on other STB chips. Add support for BCM2712
SD capabilities of this chipset.
The silicon is SD Express capable but this driver port does not currently
include that feature yet.
Based on downstream driver by raspberry foundation maintained kernel.

Signed-off-by: Andrea della Porta <[email protected]>
---
drivers/mmc/host/sdhci-brcmstb.c | 81 ++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)

diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
index 9053526fa212..13a1017d53c5 100644
--- a/drivers/mmc/host/sdhci-brcmstb.c
+++ b/drivers/mmc/host/sdhci-brcmstb.c
@@ -30,6 +30,24 @@

#define SDHCI_ARASAN_CQE_BASE_ADDR 0x200

+#define SDIO_CFG_CQ_CAPABILITY 0x4c
+#define SDIO_CFG_CQ_CAPABILITY_FMUL GENMASK(13, 12)
+
+#define SDIO_CFG_CTRL 0x0
+#define SDIO_CFG_CTRL_SDCD_N_TEST_EN BIT(31)
+#define SDIO_CFG_CTRL_SDCD_N_TEST_LEV BIT(30)
+
+#define SDIO_CFG_MAX_50MHZ_MODE 0x1ac
+#define SDIO_CFG_MAX_50MHZ_MODE_STRAP_OVERRIDE BIT(31)
+#define SDIO_CFG_MAX_50MHZ_MODE_ENABLE BIT(0)
+
+#define MMC_CAP_HSE_MASK (MMC_CAP2_HS200_1_8V_SDR | \
+ MMC_CAP2_HS200_1_2V_SDR | \
+ MMC_CAP2_HS400_1_8V | \
+ MMC_CAP2_HS400_1_2V)
+
+#define MMC_CAP_UHS_MASK (MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104)
+
struct sdhci_brcmstb_priv {
void __iomem *cfg_regs;
unsigned int flags;
@@ -38,6 +56,7 @@ struct sdhci_brcmstb_priv {
};

struct brcmstb_match_priv {
+ void (*cfginit)(struct sdhci_host *host);
void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios);
struct sdhci_ops *ops;
const unsigned int flags;
@@ -139,6 +158,17 @@ static void sdhci_brcmstb_set_clock(struct sdhci_host *host, unsigned int clock)
sdhci_enable_clk(host, clk);
}

+static void sdhci_brcmstb_set_power(struct sdhci_host *host, unsigned char mode,
+ unsigned short vdd)
+{
+ struct mmc_host *mmc = host->mmc;
+
+ if (!IS_ERR(mmc->supply.vmmc))
+ mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+
+ sdhci_set_power_noreg(host, mode, vdd);
+}
+
static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
unsigned int timing)
{
@@ -168,6 +198,40 @@ static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
}

+static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host);
+ u32 hsemmc_mask = MMC_CAP_HSE_MASK;
+ u32 uhs_mask = MMC_CAP_UHS_MASK;
+ u32 reg, base_clk_mhz;
+
+ /*
+ * If we support a speed that requires tuning,
+ * then select the delay line PHY as the clock source.
+ */
+ if ((host->mmc->caps & uhs_mask) || (host->mmc->caps2 & hsemmc_mask)) {
+ reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_MAX_50MHZ_MODE);
+ reg &= ~SDIO_CFG_MAX_50MHZ_MODE_ENABLE;
+ reg |= SDIO_CFG_MAX_50MHZ_MODE_STRAP_OVERRIDE;
+ writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_MAX_50MHZ_MODE);
+ }
+
+ if ((host->mmc->caps & MMC_CAP_NONREMOVABLE) ||
+ (host->mmc->caps & MMC_CAP_NEEDS_POLL)) {
+ /* Force presence */
+ reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_CTRL);
+ reg &= ~SDIO_CFG_CTRL_SDCD_N_TEST_LEV;
+ reg |= SDIO_CFG_CTRL_SDCD_N_TEST_EN;
+ writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_CTRL);
+ }
+
+ /* Guesstimate the timer frequency (controller base clock) */
+ base_clk_mhz = max_t(u32, clk_get_rate(pltfm_host->clk) / (1000 * 1000), 1);
+ reg = SDIO_CFG_CQ_CAPABILITY_FMUL | base_clk_mhz;
+ writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_CQ_CAPABILITY);
+}
+
static void sdhci_brcmstb_dumpregs(struct mmc_host *mmc)
{
sdhci_dumpregs(mmc_priv(mmc));
@@ -200,6 +264,14 @@ static struct sdhci_ops sdhci_brcmstb_ops = {
.set_uhs_signaling = sdhci_set_uhs_signaling,
};

+static struct sdhci_ops sdhci_brcmstb_ops_2712 = {
+ .set_clock = sdhci_set_clock,
+ .set_power = sdhci_brcmstb_set_power,
+ .set_bus_width = sdhci_set_bus_width,
+ .reset = sdhci_reset,
+ .set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
static struct sdhci_ops sdhci_brcmstb_ops_7216 = {
.set_clock = sdhci_brcmstb_set_clock,
.set_bus_width = sdhci_set_bus_width,
@@ -214,6 +286,11 @@ static struct sdhci_ops sdhci_brcmstb_ops_74165b0 = {
.set_uhs_signaling = sdhci_brcmstb_set_uhs_signaling,
};

+static const struct brcmstb_match_priv match_priv_2712 = {
+ .cfginit = sdhci_brcmstb_cfginit_2712,
+ .ops = &sdhci_brcmstb_ops_2712,
+};
+
static struct brcmstb_match_priv match_priv_7425 = {
.flags = BRCMSTB_MATCH_FLAGS_NO_64BIT |
BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT,
@@ -238,6 +315,7 @@ static struct brcmstb_match_priv match_priv_74165b0 = {
};

static const struct of_device_id __maybe_unused sdhci_brcm_of_match[] = {
+ { .compatible = "brcm,bcm2712-sdhci", .data = &match_priv_2712 },
{ .compatible = "brcm,bcm7425-sdhci", .data = &match_priv_7425 },
{ .compatible = "brcm,bcm7445-sdhci", .data = &match_priv_7445 },
{ .compatible = "brcm,bcm7216-sdhci", .data = &match_priv_7216 },
@@ -370,6 +448,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
(host->mmc->caps2 & MMC_CAP2_HS400_ES))
host->mmc_host_ops.hs400_enhanced_strobe = match_priv->hs400es;

+ if (match_priv->cfginit)
+ match_priv->cfginit(host);
+
/*
* Supply the existing CAPS, but clear the UHS modes. This
* will allow these modes to be specified by device tree
--
2.35.3


2024-05-10 14:36:48

by Andrea della Porta

[permalink] [raw]
Subject: [PATCH v2 2/4] dt-bindings: mmc: Add support for BCM2712 SD host controller

The BCM2712 has an SDHCI capable host interface similar to the one found
in other STB chipsets. Add the relevant compatible string and relative
example.

Signed-off-by: Andrea della Porta <[email protected]>
---
.../bindings/mmc/brcm,sdhci-brcmstb.yaml | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
index cbd3d6c6c77f..404b75fa7adb 100644
--- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
+++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
@@ -13,6 +13,10 @@ maintainers:
properties:
compatible:
oneOf:
+ - items:
+ - enum:
+ - brcm,bcm2712-sdhci
+ - const: brcm,sdhci-brcmstb
- items:
- enum:
- brcm,bcm7216-sdhci
@@ -114,3 +118,22 @@ examples:
clocks = <&scmi_clk 245>;
clock-names = "sw_sdio";
};
+
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ mmc@1000fff000 {
+ compatible = "brcm,bcm2712-sdhci",
+ "brcm,sdhci-brcmstb";
+ reg = <0x10 0x00fff000 0x0 0x260>,
+ <0x10 0x00fff400 0x0 0x200>;
+ reg-names = "host", "cfg";
+ mmc-ddr-3_3v;
+ interrupts = <GIC_SPI 0x111 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk_emmc2>;
+ clock-names = "sw_sdio";
+ };
+ };
--
2.35.3


2024-05-10 14:37:03

by Andrea della Porta

[permalink] [raw]
Subject: [PATCH v2 4/4] arm64: dts: broadcom: Add support for BCM2712

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 <[email protected]>
---
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
+/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>;
+ };
+
+ 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>;
+ };
+
+ 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>;
+
+ interrupt-parent = <&gicv2>;
+
+ axi: axi {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ ranges;
+
+ 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)>;
+ };
+ };
+
+ 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>;
+ };
+
+ 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";
+ };
+
+ 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;
+ };
+};
--
2.35.3


2024-05-10 16:13:25

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] arm64: dts: broadcom: Add support for BCM2712

On 2024-05-10 15: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 <[email protected]>
> ---
> 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
>

[...]

> + psci {
> + method = "smc";
> + compatible = "arm,psci-1.0", "arm,psci-0.2", "arm,psci";
> + cpu_on = <0xc4000003>;
> + cpu_suspend = <0xc4000001>;
> + cpu_off = <0x84000002>;

Please drop the PSCI function numbers. From PSCI-0.2, these are
standard,
and what is implemented is discoverable.

> + };
> +

[...]

> + 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 is missing the EL2 virtual timer interrupt, as this is a
VHE-capable
system.


> + /* This only applies to the ARMv7 stub */
> + arm,cpu-registers-not-fw-configured;

Please drop this. It makes no sense on a modern platform.

> + };
> +};

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2024-05-10 16:15:08

by Florian Fainelli

[permalink] [raw]
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 <[email protected]>
> ---
> 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


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2024-05-10 18:46:27

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt-bindings: arm: bcm: Add BCM2712 SoC support

Am 10.05.24 um 16:35 schrieb Andrea della Porta:
> The BCM2712 SoC is found on Raspberry Pi 5. Add compatible string to
> acknowledge its new chipset.
>
> Signed-off-by: Andrea della Porta <[email protected]>
Reviewed-by: Stefan Wahren <[email protected]>

Thanks

2024-05-10 19:02:04

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: mmc: Add support for BCM2712 SD host controller

Hi Andrea,

please drop E. Anholt from the recipients. AFAIK she is not interested
in kernel development anymore.

Am 10.05.24 um 16:35 schrieb Andrea della Porta:
> The BCM2712 has an SDHCI capable host interface similar to the one found
> in other STB chipsets. Add the relevant compatible string and relative
> example.
>
> Signed-off-by: Andrea della Porta <[email protected]>
> ---
> .../bindings/mmc/brcm,sdhci-brcmstb.yaml | 23 +++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
> index cbd3d6c6c77f..404b75fa7adb 100644
> --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
> +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
> @@ -13,6 +13,10 @@ maintainers:
> properties:
> compatible:
> oneOf:
> + - items:
> + - enum:
> + - brcm,bcm2712-sdhci
> + - const: brcm,sdhci-brcmstb
> - items:
> - enum:
> - brcm,bcm7216-sdhci
> @@ -114,3 +118,22 @@ examples:
> clocks = <&scmi_clk 245>;
> clock-names = "sw_sdio";
> };
> +
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
Would be nice to have a short explanation in the commit message why this
is necessary.
> +
> + mmc@1000fff000 {
> + compatible = "brcm,bcm2712-sdhci",
> + "brcm,sdhci-brcmstb";
> + reg = <0x10 0x00fff000 0x0 0x260>,
> + <0x10 0x00fff400 0x0 0x200>;
> + reg-names = "host", "cfg";
> + mmc-ddr-3_3v;
> + interrupts = <GIC_SPI 0x111 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk_emmc2>;
> + clock-names = "sw_sdio";
> + };
> + };


2024-05-10 19:22:57

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] mmc: sdhci-brcmstb: Add BCM2712 support

Hi Andrea,

Am 10.05.24 um 16:35 schrieb Andrea della Porta:
> Broadcom BCM2712 SoC has an SDHCI card controller using the SDIO CFG
> register block present on other STB chips. Add support for BCM2712
> SD capabilities of this chipset.
> The silicon is SD Express capable but this driver port does not currently
> include that feature yet.
> Based on downstream driver by raspberry foundation maintained kernel.
>
> Signed-off-by: Andrea della Porta <[email protected]>
> ---
> drivers/mmc/host/sdhci-brcmstb.c | 81 ++++++++++++++++++++++++++++++++
> 1 file changed, 81 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> index 9053526fa212..13a1017d53c5 100644
> --- a/drivers/mmc/host/sdhci-brcmstb.c
> +++ b/drivers/mmc/host/sdhci-brcmstb.c
> @@ -30,6 +30,24 @@
>
> #define SDHCI_ARASAN_CQE_BASE_ADDR 0x200
>
> +#define SDIO_CFG_CQ_CAPABILITY 0x4c
> +#define SDIO_CFG_CQ_CAPABILITY_FMUL GENMASK(13, 12)
> +
> +#define SDIO_CFG_CTRL 0x0
> +#define SDIO_CFG_CTRL_SDCD_N_TEST_EN BIT(31)
> +#define SDIO_CFG_CTRL_SDCD_N_TEST_LEV BIT(30)
> +
> +#define SDIO_CFG_MAX_50MHZ_MODE 0x1ac
> +#define SDIO_CFG_MAX_50MHZ_MODE_STRAP_OVERRIDE BIT(31)
> +#define SDIO_CFG_MAX_50MHZ_MODE_ENABLE BIT(0)
> +
> +#define MMC_CAP_HSE_MASK (MMC_CAP2_HS200_1_8V_SDR | \
> + MMC_CAP2_HS200_1_2V_SDR | \
> + MMC_CAP2_HS400_1_8V | \
> + MMC_CAP2_HS400_1_2V)
This could be simplified either with MMC_CAP2_HSX00_1_2V |
MMC_CAP2_HSX00_1_8V or MMC_CAP2_HS200 | MMC_CAP2_HS400
> +
> +#define MMC_CAP_UHS_MASK (MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104)
There is already a MMC_CAP_UHS define, but according to the code this is
not intended. It looks more like a mask of all UHS modes equal and above
50 MHz. The name or at least a comment should reflect this.
> +
> struct sdhci_brcmstb_priv {
> void __iomem *cfg_regs;
> unsigned int flags;
> @@ -38,6 +56,7 @@ struct sdhci_brcmstb_priv {
> };
>
> struct brcmstb_match_priv {
> + void (*cfginit)(struct sdhci_host *host);
> void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios);
> struct sdhci_ops *ops;
> const unsigned int flags;
> @@ -139,6 +158,17 @@ static void sdhci_brcmstb_set_clock(struct sdhci_host *host, unsigned int clock)
> sdhci_enable_clk(host, clk);
> }
>
> +static void sdhci_brcmstb_set_power(struct sdhci_host *host, unsigned char mode,
> + unsigned short vdd)
> +{
> + struct mmc_host *mmc = host->mmc;
> +
> + if (!IS_ERR(mmc->supply.vmmc))
> + mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> +
> + sdhci_set_power_noreg(host, mode, vdd);
> +}
> +
> static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
> unsigned int timing)
> {
> @@ -168,6 +198,40 @@ static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> }
>
> +static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host);
> + u32 hsemmc_mask = MMC_CAP_HSE_MASK;
> + u32 uhs_mask = MMC_CAP_UHS_MASK;
> + u32 reg, base_clk_mhz;
> +
> + /*
> + * If we support a speed that requires tuning,
> + * then select the delay line PHY as the clock source.
> + */
> + if ((host->mmc->caps & uhs_mask) || (host->mmc->caps2 & hsemmc_mask)) {
> + reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_MAX_50MHZ_MODE);
> + reg &= ~SDIO_CFG_MAX_50MHZ_MODE_ENABLE;
> + reg |= SDIO_CFG_MAX_50MHZ_MODE_STRAP_OVERRIDE;
> + writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_MAX_50MHZ_MODE);
> + }
> +
> + if ((host->mmc->caps & MMC_CAP_NONREMOVABLE) ||
> + (host->mmc->caps & MMC_CAP_NEEDS_POLL)) {
> + /* Force presence */
> + reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_CTRL);
> + reg &= ~SDIO_CFG_CTRL_SDCD_N_TEST_LEV;
> + reg |= SDIO_CFG_CTRL_SDCD_N_TEST_EN;
> + writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_CTRL);
> + }
> +
> + /* Guesstimate the timer frequency (controller base clock) */
> + base_clk_mhz = max_t(u32, clk_get_rate(pltfm_host->clk) / (1000 * 1000), 1);
> + reg = SDIO_CFG_CQ_CAPABILITY_FMUL | base_clk_mhz;
> + writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_CQ_CAPABILITY);
> +}
> +
> static void sdhci_brcmstb_dumpregs(struct mmc_host *mmc)
> {
> sdhci_dumpregs(mmc_priv(mmc));
> @@ -200,6 +264,14 @@ static struct sdhci_ops sdhci_brcmstb_ops = {
> .set_uhs_signaling = sdhci_set_uhs_signaling,
> };
>
> +static struct sdhci_ops sdhci_brcmstb_ops_2712 = {
> + .set_clock = sdhci_set_clock,
> + .set_power = sdhci_brcmstb_set_power,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = sdhci_reset,
> + .set_uhs_signaling = sdhci_set_uhs_signaling,
> +};
> +
> static struct sdhci_ops sdhci_brcmstb_ops_7216 = {
> .set_clock = sdhci_brcmstb_set_clock,
> .set_bus_width = sdhci_set_bus_width,
> @@ -214,6 +286,11 @@ static struct sdhci_ops sdhci_brcmstb_ops_74165b0 = {
> .set_uhs_signaling = sdhci_brcmstb_set_uhs_signaling,
> };
>
> +static const struct brcmstb_match_priv match_priv_2712 = {
> + .cfginit = sdhci_brcmstb_cfginit_2712,
> + .ops = &sdhci_brcmstb_ops_2712,
> +};
> +
> static struct brcmstb_match_priv match_priv_7425 = {
> .flags = BRCMSTB_MATCH_FLAGS_NO_64BIT |
> BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT,
> @@ -238,6 +315,7 @@ static struct brcmstb_match_priv match_priv_74165b0 = {
> };
>
> static const struct of_device_id __maybe_unused sdhci_brcm_of_match[] = {
> + { .compatible = "brcm,bcm2712-sdhci", .data = &match_priv_2712 },
> { .compatible = "brcm,bcm7425-sdhci", .data = &match_priv_7425 },
> { .compatible = "brcm,bcm7445-sdhci", .data = &match_priv_7445 },
> { .compatible = "brcm,bcm7216-sdhci", .data = &match_priv_7216 },
> @@ -370,6 +448,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> (host->mmc->caps2 & MMC_CAP2_HS400_ES))
> host->mmc_host_ops.hs400_enhanced_strobe = match_priv->hs400es;
>
> + if (match_priv->cfginit)
> + match_priv->cfginit(host);
> +
> /*
> * Supply the existing CAPS, but clear the UHS modes. This
> * will allow these modes to be specified by device tree


2024-05-10 19:51:23

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] arm64: dts: broadcom: Add support for BCM2712

Hi Andrea,

Am 10.05.24 um 16:35 schrieb Andrea della Porta:
> 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 <[email protected]>
> ---
> 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
> +/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>;
> + };
> +
> + 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>;
> + };
> +
> + 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 */
Not sure this comment is helpful. Debug 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>;
> +
> + interrupt-parent = <&gicv2>;
> +
> + axi: axi {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <1>;
> + ranges;
> +
> + 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)>;
Please take care of the DTS coding style [1].
> + };
> + };
> +
> + 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
I think we should try to keep URLs in one line, even checkpatch
complains about it.

[1] - https://docs.kernel.org/devicetree/bindings/dts-coding-style.html

2024-05-10 21:33:30

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt-bindings: arm: bcm: Add BCM2712 SoC support

On Fri, May 10, 2024 at 04:35:27PM +0200, Andrea della Porta wrote:
> The BCM2712 SoC is found on Raspberry Pi 5. Add compatible string to
> acknowledge its new chipset.
>
> Signed-off-by: Andrea della Porta <[email protected]>

Acked-by: Conor Dooley <[email protected]>

Cheers,
Conor.


Attachments:
(No filename) (312.00 B)
signature.asc (235.00 B)
Download all attachments

2024-05-13 13:27:11

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Add minimal boot support for Raspberry Pi 5


On Fri, 10 May 2024 16:35:26 +0200, Andrea della Porta wrote:
> Hi,
>
> This patchset adds minimal support for the Broadcom BCM2712 SoC and for
> the on-board SDHCI controller on Broadcom BCM2712 in order to make it
> possible to boot (particularly) a Raspberry Pi 5 from SD card and get a
> console through uart.
> Changes to arm64/defconfig are not needed since the actual options work
> as they are.
> This work is heavily based on downstream contributions.
>
> Tested on Tumbleweed substituting the stock kernel with upstream one,
> either chainloading uboot+grub+kernel or directly booting the kernel
> from 1st stage bootloader. Steps to reproduce:
> - prepare an SD card from a Raspberry enabled raw image, mount the first
> FAT partition.
> - make sure the FAT partition is big enough to contain the kernel,
> anything bigger than 64Mb is usually enough, depending on your kernel
> config options.
> - build the kernel and dtbs making sure that the support for your root
> fs type is compiled as builtin.
> - copy the kernel image in your FAT partition overwriting the older one
> (e.g. kernel*.img for Raspberry Pi OS or u-boot.bin for Tumbleweed).
> - copy arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb on FAT partition.
> - make sure you have a cmdline.txt file in FAT partition with the
> following content:
> # cat /boot/efi/cmdline.txt
> root=/dev/mmcblk0p3 rootwait rw console=tty ignore_loglevel earlycon
> console=ttyAMA10,115200
> - if you experience random SD issues during boot, try to set
> initial_turbo=0 in config.txt.
>
> Changes in V2:
>
> - the patchshet has been considerably simplified, both in terms of dts and
> driver code. Notably, the pinctrl/pinmux driver (and associated binding)
> was not strictly needed to use the SD card so it has been dropped.
> - dropped the optional SD express support patch
> - the patches order has been revisited
> - pass all checks (binding, dtb, checkpatch)
>
> Many thanks,
> Andrea
>
> References:
> - Link to V1: https://lore.kernel.org/all/[email protected]/
>
> Andrea della Porta (4):
> dt-bindings: arm: bcm: Add BCM2712 SoC support
> dt-bindings: mmc: Add support for BCM2712 SD host controller
> mmc: sdhci-brcmstb: Add BCM2712 support
> arm64: dts: broadcom: Add support for BCM2712
>
> .../devicetree/bindings/arm/bcm/bcm2835.yaml | 6 +
> .../bindings/mmc/brcm,sdhci-brcmstb.yaml | 23 ++
> arch/arm64/boot/dts/broadcom/Makefile | 1 +
> .../boot/dts/broadcom/bcm2712-rpi-5-b.dts | 62 ++++
> arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 302 ++++++++++++++++++
> drivers/mmc/host/sdhci-brcmstb.c | 81 +++++
> 6 files changed, 475 insertions(+)
> create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712.dtsi
>
> --
> 2.35.3
>
>
>


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y broadcom/bcm2712-rpi-5-b.dtb' for [email protected]:

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: /soc@107c000000/timer@7c003000: failed to match any schema with compatible: ['brcm,bcm2835-system-timer']
arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: /soc@107c000000/local-intc@7cd00000: failed to match any schema with compatible: ['brcm,bcm2836-l1-intc']






2024-05-13 18:58:23

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] dt-bindings: mmc: Add support for BCM2712 SD host controller

On Fri, May 10, 2024 at 09:01:33PM +0200, Stefan Wahren wrote:
> Hi Andrea,
>
> please drop E. Anholt from the recipients. AFAIK she is not interested
> in kernel development anymore.
>
> Am 10.05.24 um 16:35 schrieb Andrea della Porta:
> > The BCM2712 has an SDHCI capable host interface similar to the one found
> > in other STB chipsets. Add the relevant compatible string and relative
> > example.
> >
> > Signed-off-by: Andrea della Porta <[email protected]>
> > ---
> > .../bindings/mmc/brcm,sdhci-brcmstb.yaml | 23 +++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
> > index cbd3d6c6c77f..404b75fa7adb 100644
> > --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml
> > @@ -13,6 +13,10 @@ maintainers:
> > properties:
> > compatible:
> > oneOf:
> > + - items:
> > + - enum:
> > + - brcm,bcm2712-sdhci
> > + - const: brcm,sdhci-brcmstb
> > - items:
> > - enum:
> > - brcm,bcm7216-sdhci
> > @@ -114,3 +118,22 @@ examples:
> > clocks = <&scmi_clk 245>;
> > clock-names = "sw_sdio";
> > };
> > +
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> Would be nice to have a short explanation in the commit message why this
> is necessary.

That has nothing to do with *this* patch. It's needed because 'reg' is
dependent on it and the examples default to 1 cell.

In any case, please just drop the example. A new compatible doesn't
justify another example.

Rob

2024-05-16 08:37:27

by Andrea della Porta

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] arm64: dts: broadcom: Add support for BCM2712

On 09:14 Fri 10 May , Florian Fainelli wrote:
> On 5/10/24 07:35, Andrea della Porta wrote:
>
> 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.

This doesn't seem to match with what I have here:
- some axi peripherals goes beyond 0x10_3FFF_FFFF (e.g. the interrupt
controller is @0x107fff9000)
- downstream dts have that ranges going from 0x10 0x00000000 0x11 0x00000000,
so to span all the peripheral (included e.g. the above int controller)
- another comment in downstream dts says: // 10_00000000-1x_xxxxxxxx = up to
64GB system RAM

I'm a little confused here, of course we could also define multiple ranges
but I don't really know what the boundaries are. Anyway, I would opt for the
extended range (0x10 0x00000000 - 0x11 0x00000000) unless there is concern
about it. Any thoughts?

Many thanks,
Andrea

2024-05-16 10:32:13

by Andrea della Porta

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Add minimal boot support for Raspberry Pi 5

On 08:27 Mon 13 May , Rob Herring (Arm) wrote:
>
> On Fri, 10 May 2024 16:35:26 +0200, Andrea della Porta wrote:
> > Hi,
> >
> > This patchset adds minimal support for the Broadcom BCM2712 SoC and for
> > the on-board SDHCI controller on Broadcom BCM2712 in order to make it
> > possible to boot (particularly) a Raspberry Pi 5 from SD card and get a
> > console through uart.
> > Changes to arm64/defconfig are not needed since the actual options work
> > as they are.
> > This work is heavily based on downstream contributions.
> >
> > Tested on Tumbleweed substituting the stock kernel with upstream one,
> > either chainloading uboot+grub+kernel or directly booting the kernel
> > from 1st stage bootloader. Steps to reproduce:
> > - prepare an SD card from a Raspberry enabled raw image, mount the first
> > FAT partition.
> > - make sure the FAT partition is big enough to contain the kernel,
> > anything bigger than 64Mb is usually enough, depending on your kernel
> > config options.
> > - build the kernel and dtbs making sure that the support for your root
> > fs type is compiled as builtin.
> > - copy the kernel image in your FAT partition overwriting the older one
> > (e.g. kernel*.img for Raspberry Pi OS or u-boot.bin for Tumbleweed).
> > - copy arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb on FAT partition.
> > - make sure you have a cmdline.txt file in FAT partition with the
> > following content:
> > # cat /boot/efi/cmdline.txt
> > root=/dev/mmcblk0p3 rootwait rw console=tty ignore_loglevel earlycon
> > console=ttyAMA10,115200
> > - if you experience random SD issues during boot, try to set
> > initial_turbo=0 in config.txt.
> >
> > Changes in V2:
> >
> > - the patchshet has been considerably simplified, both in terms of dts and
> > driver code. Notably, the pinctrl/pinmux driver (and associated binding)
> > was not strictly needed to use the SD card so it has been dropped.
> > - dropped the optional SD express support patch
> > - the patches order has been revisited
> > - pass all checks (binding, dtb, checkpatch)
> >
> > Many thanks,
> > Andrea
> >
> > References:
> > - Link to V1: https://lore.kernel.org/all/[email protected]/
> >
> > Andrea della Porta (4):
> > dt-bindings: arm: bcm: Add BCM2712 SoC support
> > dt-bindings: mmc: Add support for BCM2712 SD host controller
> > mmc: sdhci-brcmstb: Add BCM2712 support
> > arm64: dts: broadcom: Add support for BCM2712
> >
> > .../devicetree/bindings/arm/bcm/bcm2835.yaml | 6 +
> > .../bindings/mmc/brcm,sdhci-brcmstb.yaml | 23 ++
> > arch/arm64/boot/dts/broadcom/Makefile | 1 +
> > .../boot/dts/broadcom/bcm2712-rpi-5-b.dts | 62 ++++
> > arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 302 ++++++++++++++++++
> > drivers/mmc/host/sdhci-brcmstb.c | 81 +++++
> > 6 files changed, 475 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> > create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712.dtsi
> >
> > --
> > 2.35.3
> >
> >
> >
>
>
> My bot found new DTB warnings on the .dts files added or changed in this
> series.
>
> Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to reply
> unless the platform maintainer has comments.
>
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
>
> pip3 install dtschema --upgrade
>
>
> New warnings running 'make CHECK_DTBS=y broadcom/bcm2712-rpi-5-b.dtb' for [email protected]:
>
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: /soc@107c000000/timer@7c003000: failed to match any schema with compatible: ['brcm,bcm2835-system-timer']
> arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtb: /soc@107c000000/local-intc@7cd00000: failed to match any schema with compatible: ['brcm,bcm2836-l1-intc']

Those two property come respectively from:
- Documentation/devicetree/bindings/timer/brcm,bcm2835-system-timer.txt
- Documentation/devicetree/bindings/interrupt-controller/brcm,bcm2836-l1-intc.txt

that are old style txt bindings. So unfortunally they will output that
kind of warnings: converting txt to yaml schema wouldbe great, but I
think a little outside the scope of this patchset. I would say it is
a good cancidate for a separate patch.

Many thanks,
Andrea

2024-05-20 13:37:59

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] dt-bindings: arm: bcm: Add BCM2712 SoC support

On 10/05/2024 16:34, Andrea della Porta wrote:
> The BCM2712 SoC is found on Raspberry Pi 5. Add compatible string to
> acknowledge its new chipset.
>
> Signed-off-by: Andrea della Porta <[email protected]>
> ---

Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2024-05-20 18:04:01

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] arm64: dts: broadcom: Add support for BCM2712

On 5/16/24 01:36, Andrea della Porta wrote:
> On 09:14 Fri 10 May , Florian Fainelli wrote:
>> On 5/10/24 07:35, Andrea della Porta wrote:
>>
>> 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.
>
> This doesn't seem to match with what I have here:
> - some axi peripherals goes beyond 0x10_3FFF_FFFF (e.g. the interrupt
> controller is @0x107fff9000)

The interrupt controller (GIC) is technically its own dedicated bus to
the CPU, that's why it is typically added to the root node.

> - downstream dts have that ranges going from 0x10 0x00000000 0x11 0x00000000,
> so to span all the peripheral (included e.g. the above int controller)
> - another comment in downstream dts says: // 10_00000000-1x_xxxxxxxx = up to
> 64GB system RAM

Yes, up to 64GB of DRAM is correct.

>
> I'm a little confused here, of course we could also define multiple ranges
> but I don't really know what the boundaries are. Anyway, I would opt for the
> extended range (0x10 0x00000000 - 0x11 0x00000000) unless there is concern
> about it. Any thoughts?

A single range is fine with me, those sort of details should not matter.

>
> Many thanks,
> Andrea
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
Florian


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature