[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ddda4f98-2402-04ab-108d-a1ee4beb33bd@gmail.com>
Date: Wed, 18 Sep 2024 20:54:14 +0300
From: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>,
Sylwester Nawrocki <s.nawrocki@...sung.com>,
Linus Walleij <linus.walleij@...aro.org>, Rob Herring <robh+dt@...nel.org>,
linux-samsung-soc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
devicetree@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 08/10] arm64: dts: exynos: Add initial support for
exynos8895 SoC
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@...il.com>
>> + */
>> +
>> +#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@...00000 {
>> + compatible = "samsung,exynos8895-chipid",
>> + "samsung,exynos850-chipid";
>> + reg = <0x10000000 0x24>;
>> + };
>> +
>> + gic: interrupt-controller@...01000 {
>> + 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
>
Powered by blists - more mailing lists