On 9/16/24 17:23, Krzysztof Kozlowski wrote:
> On Mon, Sep 09, 2024 at 02:00:15PM +0300, Ivaylo Ivanov wrote:
>
>> +&pinctrl_peric0 {
>> + gpd0: gpd0-gpio-bank {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpd1: gpd1-gpio-bank {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpd2: gpd2-gpio-bank {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpd3: gpd3-gpio-bank {
> These should be ordered by node name. Same in peric1.
Alright.
>
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpb1: gpb1-gpio-bank {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpe7: gpe7-gpio-bank {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>> +
>> + gpf1: gpf1-gpio-bank {
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> +
>> + interrupt-controller;
>> + #interrupt-cells = <2>;
>> + };
>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos8895.dtsi b/arch/arm64/boot/dts/exynos/exynos8895.dtsi
>> new file mode 100644
>> index 000000000..59af33420
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/exynos/exynos8895.dtsi
>> @@ -0,0 +1,249 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
>> +/*
>> + * Samsung's Exynos 8895 SoC device tree source
>> + *
>> + * Copyright (c) 2024, Ivaylo Ivanov <ivo.ivanov.ivanov1@xxxxxxxxx>
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +/ {
>> + compatible = "samsung,exynos8895";
>> + #address-cells = <2>;
>> + #size-cells = <1>;
>> +
>> + interrupt-parent = <&gic>;
>> +
>> + aliases {
>> + pinctrl0 = &pinctrl_alive;
>> + pinctrl1 = &pinctrl_abox;
>> + pinctrl2 = &pinctrl_vts;
>> + pinctrl3 = &pinctrl_fsys0;
>> + pinctrl4 = &pinctrl_fsys1;
>> + pinctrl5 = &pinctrl_busc;
>> + pinctrl6 = &pinctrl_peric0;
>> + pinctrl7 = &pinctrl_peric1;
>> + };
>> +
>> + arm-a53-pmu {
>> + compatible = "arm,cortex-a53-pmu";
>> + interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-affinity = <&cpu0>,
>> + <&cpu1>,
>> + <&cpu2>,
>> + <&cpu3>;
>> + };
>> +
>> + /* There's no PMU model for the Mongoose cores */
>> +
>> + cpus {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + cpu-map {
>> + cluster0 {
>> + core0 {
>> + cpu = <&cpu0>;
>> + };
>> + core1 {
>> + cpu = <&cpu1>;
>> + };
>> + core2 {
>> + cpu = <&cpu2>;
>> + };
>> + core3 {
>> + cpu = <&cpu3>;
>> + };
>> + };
>> +
>> + cluster1 {
>> + core0 {
>> + cpu = <&cpu4>;
>> + };
>> + core1 {
>> + cpu = <&cpu5>;
>> + };
>> + core2 {
>> + cpu = <&cpu6>;
>> + };
>> + core3 {
>> + cpu = <&cpu7>;
>> + };
>> + };
>> + };
>> +
>> + cpu0: cpu@100 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a53";
>> + reg = <0x100>;
>> + enable-method = "psci";
>> + };
>> +
>> + cpu1: cpu@101 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a53";
>> + reg = <0x101>;
>> + enable-method = "psci";
>> + };
>> +
>> + cpu2: cpu@102 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a53";
>> + reg = <0x102>;
>> + enable-method = "psci";
>> + };
>> +
>> + cpu3: cpu@103 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a53";
>> + reg = <0x103>;
>> + enable-method = "psci";
>> + };
>> +
>> + cpu4: cpu@0 {
> Why cpu@0 is cpu4 not cpu0? Anyway, these should be ordered by unit
> address.
cpu@100 is the boot core of the first cluster consisting of cortex-a53
cores, hence why it's labelled as cpu0. The second cluster contains
the Mongoose cores, labelled and ordered after the first cluster.
It's ordered like so on a lot of SoCs for sanity's sake, hence why I
believe it should stay like that.
If you still think that they must be ordered by unit address, please
explicitly let me know so that I include that change in the v5.
Regards, Ivo.
>
>> +
>> + oscclk: osc-clock {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + clock-output-names = "oscclk";
>> + };
>> +
>> + psci {
>> + compatible = "arm,psci";
>> + method = "smc";
>> + cpu_suspend = <0xc4000001>;
>> + cpu_off = <0x84000002>;
>> + cpu_on = <0xc4000003>;
>> + };
>> +
>> + timer {
> Keep order by names, timer goes after soc.
>
>> + compatible = "arm,armv8-timer";
>> + /* Hypervisor Virtual Timer interrupt is not wired to GIC */
>> + 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)>;
>> + /*
>> + * Non-updatable, broken stock Samsung bootloader does not
>> + * configure CNTFRQ_EL0
>> + */
>> + clock-frequency = <26000000>;
>> + };
>> +
>> + soc: soc@0 {
>> + compatible = "simple-bus";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0x0 0x0 0x0 0x20000000>;
> ranges is the second property (see DTS coding style).
>
>> +
>> + chipid@10000000 {
>> + compatible = "samsung,exynos8895-chipid",
>> + "samsung,exynos850-chipid";
>> + reg = <0x10000000 0x24>;
>> + };
>> +
>> + gic: interrupt-controller@10201000 {
>> + compatible = "arm,gic-400";
>> + #interrupt-cells = <3>;
>> + #address-cells = <0>;
>> + interrupt-controller;
>> + reg = <0x10201000 0x1000>,
> reg is second, please follow DTS coding style for new submissions.
>
>> + <0x10202000 0x1000>,
>> + <0x10204000 0x2000>,
>> + <0x10206000 0x2000>;
>> + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) |
>> + IRQ_TYPE_LEVEL_HIGH)>;
>> + };
> Best regards,
> Krzysztof
>
