Re: [PATCH 12/12] arm64: dts: exynos: Add Exynos850 SoC support

From: Marc Zyngier
Date: Fri Jul 30 2021 - 12:51:00 EST


On 2021-07-30 15:49, Sam Protsenko wrote:
Samsung Exynos850 is ARMv8-based mobile-oriented SoC.

Features:
* CPU: Cortex-A55 Octa (8 cores), up to 2 GHz
* Memory interface: LPDDR4/4x 2 channels (12.8 GB/s)
* SD/MMC: SD 3.0, eMMC5.1 DDR 8-bit
* Modem: 4G LTE, 3G, GSM/GPRS/EDGE
* RF: Quad GNSS, WiFi 5 (802.11ac), Bluetooth 5.0
* GPU: Mali-G52 MP1
* Codec: 1080p 60fps H64, HEVC, JPEG HW Codec
* Display: Full HD+ (2520x1080)@60fps LCD
* Camera: 16+5MP/13+8MP ISP, MIPI CSI 4/4/2, FD, DRC
* Connectivity: USB 2.0 DRD, USI (SPI/UART/I2C), HSI2C, I3C, ADC, Audio

This patch adds minimal SoC support. Particular board device tree files
can include exynos850.dtsi file to get SoC related nodes, and then
reference those nodes further as needed.

Signed-off-by: Sam Protsenko <semen.protsenko@xxxxxxxxxx>
---
.../boot/dts/exynos/exynos850-pinctrl.dtsi | 782 ++++++++++++++++++
arch/arm64/boot/dts/exynos/exynos850-usi.dtsi | 30 +
arch/arm64/boot/dts/exynos/exynos850.dtsi | 245 ++++++
3 files changed, 1057 insertions(+)
create mode 100644 arch/arm64/boot/dts/exynos/exynos850-pinctrl.dtsi
create mode 100644 arch/arm64/boot/dts/exynos/exynos850-usi.dtsi
create mode 100644 arch/arm64/boot/dts/exynos/exynos850.dtsi

diff --git a/arch/arm64/boot/dts/exynos/exynos850-pinctrl.dtsi
b/arch/arm64/boot/dts/exynos/exynos850-pinctrl.dtsi
new file mode 100644
index 000000000000..4cf0a22cc6db

[...]

+ gic: interrupt-controller@12a00000 {
+ compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";

One thing for sure, it cannot be both. And given that it is
an A55-based SoC, it isn't either. It is more likely a GIC400.

+ #interrupt-cells = <3>;
+ #address-cells = <0>;
+ interrupt-controller;
+ reg = <0x0 0x12a01000 0x1000>,
+ <0x0 0x12a02000 0x1000>,

This is wrong. It is architecturally set to 8kB.

+ <0x0 0x12a04000 0x2000>,
+ <0x0 0x12a06000 0x2000>;
+ interrupts = <GIC_PPI 9
+ (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;

4? With 8 CPUs?

I also find it curious that you went through the unusual
(and IMO confusing) effort to allocate a name to each and
every SPI in the system, but didn't do it for any on the PPIs...


+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupts = <GIC_PPI 13
+ (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 14
+ (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 11
+ (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
+ <GIC_PPI 10
+ (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
+ clock-frequency = <26000000>;

No, please. Fix the firmware to program CNTFRQ_EL0 on each
and every CPU. This isn't 2012 anymore.

You are also missing the hypervisor virtual timer interrupt.

+ use-clocksource-only;
+ use-physical-timer;

Thankfully, these two properties do not exist.

Thanks,

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