[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e39b5259-db92-4269-84c9-d51e8e4f327e@kernel.org>
Date: Mon, 16 Jun 2025 08:15:08 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Ryan Chen <ryan_chen@...eedtech.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Joel Stanley <joel@....id.au>,
Andrew Jeffery <andrew@...econstruct.com.au>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Arnd Bergmann <arnd@...db.de>,
Bjorn Andersson <bjorn.andersson@....qualcomm.com>,
Geert Uytterhoeven <geert@...ux-m68k.org>, Nishanth Menon <nm@...com>,
"nfraprado@...labora.com" <nfraprado@...labora.com>,
Taniya Das <quic_tdas@...cinc.com>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
Eric Biggers <ebiggers@...gle.com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"soc@...ts.linux.dev" <soc@...ts.linux.dev>, Mo Elbadry
<elbadrym@...gle.com>, Rom Lemarchand <romlem@...gle.com>,
William Kennington <wak@...gle.com>, Yuxiao Zhang <yuxiaozhang@...gle.com>,
"wthai@...dia.com" <wthai@...dia.com>, "leohu@...dia.com"
<leohu@...dia.com>, "dkodihalli@...dia.com" <dkodihalli@...dia.com>,
"spuranik@...dia.com" <spuranik@...dia.com>
Subject: Re: [PATCH v0 3/5] arm64: dts: aspeed: Add initial AST2700 SoC device
tree
On 16/06/2025 04:24, Ryan Chen wrote:
>> Subject: Re: [PATCH v0 3/5] arm64: dts: aspeed: Add initial AST2700 SoC device
>> tree
>>
>> On 13/06/2025 04:29, Ryan Chen wrote:
>>>> Subject: Re: [PATCH v0 3/5] arm64: dts: aspeed: Add initial AST2700
>>>> SoC device tree
>>>>
>>>> On 12/06/2025 12:09, Ryan Chen wrote:
>>>>> This add the initial device tree support for the ASPEED AST2700 SoC.
>>>>>
>>>>> - Add top-level compatible string "aspeed,ast2700" and set up
>>>>> address-cells/size-cells for 64-bit address space.
>>>>> - Describe a quad-core ARM Cortex-A35 CPU cluster with L2 cache,
>>>>> including cache properties and PSCI enable-method.
>>>>> - Add PMU and ARMv8 timer nodes with correct PPI interrupt wiring.
>>>>> - Model the dual-SoC architecture with two simple-bus nodes:
>>>>> soc0 (@0x10000000) and soc1 (@0x14000000).
>>>>> - Add syscon nodes for both SoCs (syscon0, syscon1) with clock/reset
>>>>> cell definitions and address mapping.
>>>>> - Add GICv3 interrupt controller node under soc0, with full register
>>>>> mapping and interrupt properties.
>>>>> - Hierarchical interrupt controller structure:
>>>>> - intc0 under soc0, with child intc0_11 node.
>>>>> - intc1 under soc1, with child intc1_0~intc1_5 nodes.
>>>>> - Add serial4 node under soc0, others serial node under soc1.
>>>>>
>>>>> Signed-off-by: Ryan Chen <ryan_chen@...eedtech.com>
>>>>> ---
>>>>> arch/arm64/boot/dts/aspeed/aspeed-g7.dtsi | 380
>>>>> ++++++++++++++++++++++
>>>>> 1 file changed, 380 insertions(+)
>>>>> create mode 100644 arch/arm64/boot/dts/aspeed/aspeed-g7.dtsi
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/aspeed/aspeed-g7.dtsi
>>>>> b/arch/arm64/boot/dts/aspeed/aspeed-g7.dtsi
>>>>> new file mode 100644
>>>>> index 000000000000..d197187bcf9f
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/boot/dts/aspeed/aspeed-g7.dtsi
>>>>> @@ -0,0 +1,380 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0-or-later #include
>>>>> +<dt-bindings/clock/aspeed,ast2700-scu.h>
>>>>> +#include <dt-bindings/reset/aspeed,ast2700-scu.h>
>>>>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> +
>>>>> +/ {
>>>>> + #address-cells = <2>;
>>>>> + #size-cells = <1>;
>>>>> + interrupt-parent = <&gic>;
>>>>> +
>>>>> + cpus {
>>>>> + #address-cells = <2>;
>>>>> + #size-cells = <0>;
>>>>> +
>>>>> + cpu0: cpu@0 {
>>>>> + device_type = "cpu";
>>>>> + compatible = "arm,cortex-a35";
>>>>> + reg = <0x0 0x0>;
>>>>> + enable-method = "psci";
>>>>> + i-cache-size = <0x8000>;
>>>>> + i-cache-line-size = <64>;
>>>>> + i-cache-sets = <256>;
>>>>> + d-cache-size = <0x8000>;
>>>>> + d-cache-line-size = <64>;
>>>>> + d-cache-sets = <128>;
>>>>> + next-level-cache = <&l2>;
>>>>> + };
>>>>> +
>>>>> + cpu1: cpu@1 {
>>>>> + device_type = "cpu";
>>>>> + compatible = "arm,cortex-a35";
>>>>> + enable-method = "psci";
>>>>> + reg = <0x0 0x1>;
>>>>> + i-cache-size = <0x8000>;
>>>>> + i-cache-line-size = <64>;
>>>>> + i-cache-sets = <256>;
>>>>> + d-cache-size = <0x8000>;
>>>>> + d-cache-line-size = <64>;
>>>>> + d-cache-sets = <128>;
>>>>> + next-level-cache = <&l2>;
>>>>> + };
>>>>> +
>>>>> + cpu2: cpu@2 {
>>>>> + device_type = "cpu";
>>>>> + compatible = "arm,cortex-a35";
>>>>> + enable-method = "psci";
>>>>> + reg = <0x0 0x2>;
>>>>> + i-cache-size = <0x8000>;
>>>>> + i-cache-line-size = <64>;
>>>>> + i-cache-sets = <256>;
>>>>> + d-cache-size = <0x8000>;
>>>>> + d-cache-line-size = <64>;
>>>>> + d-cache-sets = <128>;
>>>>> + next-level-cache = <&l2>;
>>>>> + };
>>>>> +
>>>>> + cpu3: cpu@3 {
>>>>> + device_type = "cpu";
>>>>> + compatible = "arm,cortex-a35";
>>>>> + enable-method = "psci";
>>>>> + reg = <0x0 0x3>;
>>>>> + i-cache-size = <0x8000>;
>>>>> + i-cache-line-size = <64>;
>>>>> + i-cache-sets = <256>;
>>>>> + d-cache-size = <0x8000>;
>>>>> + d-cache-line-size = <64>;
>>>>> + d-cache-sets = <128>;
>>>>> + next-level-cache = <&l2>;
>>>>> + };
>>>>> +
>>>>> + l2: l2-cache0 {
>>>>> + compatible = "cache";
>>>>> + cache-level = <2>;
>>>>> + cache-unified;
>>>>> + cache-size = <0x80000>;
>>>>> + cache-line-size = <64>;
>>>>> + cache-sets = <1024>;
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + arm-pmu {
>>>>> + compatible = "arm,cortex-a35-pmu";
>>>>> + interrupts = <GIC_PPI 7 (GIC_CPU_MASK_SIMPLE(4) |
>>>> IRQ_TYPE_LEVEL_HIGH)>;
>>>>> + };
>>>>> +
>>>>> + psci {
>>>>> + compatible = "arm,psci-1.0";
>>>>> + method = "smc";
>>>>> + };
>>>>> +
>>>>> + 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)>;
>>>>> + arm,cpu-registers-not-fw-configured;
>>>>> + always-on;
>>>>> + };
>>>>> +
>>>>> + soc0: soc@...00000 {
>>>>> + compatible = "simple-bus";
>>>>> + reg = <0x0 0x10000000 0x10000000>;
>>>>> + #address-cells = <2>;
>>>>> + #size-cells = <1>;
>>>>> + ranges;
>>>>> +
>>>>> + syscon0: syscon@...02000 {
>>>>> + compatible = "aspeed,ast2700-scu0", "syscon",
>> "simple-mfd";
>>>>> + reg = <0x0 0x12c02000 0x1000>;
>>>>> + ranges = <0x0 0x0 0 0x12c02000 0x1000>;
>>>>> + #address-cells = <2>;
>>>>> + #size-cells = <1>;
>>>>> + #clock-cells = <1>;
>>>>> + #reset-cells = <1>;
>>>>> + };
>>>>> +
>>>>> + gic: interrupt-controller@...00000 {
>>>>> + compatible = "arm,gic-v3";
>>>>> + reg = <0 0x12200000 0x10000>, /* GICD */
>>>>> + <0 0x12280000 0x80000>, /* GICR */
>>>>> + <0 0x40440000 0x1000>; /* GICC */
>>>>> + #interrupt-cells = <3>;
>>>>> + interrupt-controller;
>>>>> + interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) |
>>>> IRQ_TYPE_LEVEL_HIGH)>;
>>>>> + interrupt-parent = <&gic>;
>>>>> + };
>>>>> +
>>>>> + serial4: serial@...1a000 {
>>>>> + compatible = "ns16550a";
>>>>> + reg = <0x0 0x12c1a000 0x1000>;
>>>>> + clocks = <&syscon0 SCU0_CLK_GATE_UART4CLK>;
>>>>> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
>>>>> + reg-shift = <2>;
>>>>> + status = "disabled";
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + soc1: soc@...00000 {
>>>>> + compatible = "simple-bus";
>>>>> + reg = <0x0 0x14000000 0x10000000>;
>>>>> + #address-cells = <2>;
>>>>> + #size-cells = <1>;
>>>>> + ranges;
>>>>> +
>>>>> + syscon1: syscon@...02000 {
>>>>> + compatible = "aspeed,ast2700-scu1", "syscon",
>> "simple-mfd";
>>>>> + reg = <0x0 0x14c02000 0x1000>;
>>>>> + ranges = <0x0 0x0 0x0 0x14c02000 0x1000>;
>>>>> + #address-cells = <2>;
>>>>> + #size-cells = <1>;
>>>>> + #clock-cells = <1>;
>>>>> + #reset-cells = <1>;
>>>>> + };
>>>>> +
>>>>> + serial12: serial@...33b00 {
>>>>> + compatible = "ns16550a";
>>>>> + reg = <0x0 0x14c33b00 0x100>;
>>>>> + clocks = <&syscon1 SCU1_CLK_GATE_UART12CLK>;
>>>>> + interrupts-extended =
>>>>> + <&intc1_4 18 (GIC_CPU_MASK_SIMPLE(4) |
>>>> IRQ_TYPE_LEVEL_HIGH)>;
>>>>> + reg-shift = <2>;
>>>>> + status = "disabled";
>>>>> + };
>>>>> + };
>>>>> +};
>>>>> +
>>>>> +&soc0 {
>>>>
>>>> This is the base DTSI, there is no existing node to override. Just
>>>> define complete SoC node in one place like every other vendor.
>>>
>>> My original is use this way, but when I do checkpatch, get
>>> CHECK: line length of 106 exceeds 100 columns.
>>> interrupts = <GIC_SPI 192 (GIC_CPU_MASK_SIMPLE(4) |
>>> IRQ_TYPE_LEVEL_HIGH)>, That the reason modify by this way.
>>
>> Look how other recent, most developed platforms do it and learn from them
>> instead of coming with own, confusing style.
>
> Thanks, I will refer this to modify.
> https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/broadcom/bcm2712.dtsi#L580-L589
>>
>>>
>>>>
>>>>
>>>>> + intc0: interrupt-controller@...00000 {
>>>>> + compatible = "simple-mfd";
>>>>
>>>> NAK, never tested.
>>>>
>>>> Not allowed, see bindings. And test it next time.
>>>
>>> Got it, will update by following.
>>> Intc0: bus@...00000 {
>>> compatible = "simple-bus";
>>> #address-cells = <2>;
>>> #size-cells = <1>;
>>> reg = <0 0x12100000 0x4000>;
>>> ranges = <0x0 0x0 0x0 0x12100000 0x4000>;
>>> #address-cells = <2>;
>>
>> Does not follow DTS coding style and anyway, what sort of bus is this?
>
> Sorry, for miss-lead. intc0 is like the global base for inte0_11 interrupt-controller.
So that's not a bus, thus do not use simple-bus for that. Define
bindings for your device.
> So, I use simple-mfd.
> intc0: interrupt-controller@...00000 {
> compatible = "simple-mfd";
> .....
> intc0_11: interrupt-controller@...0 {
> ......
> };
> };
>
> But I don't know your previous "NAK, never tested" mean.
> I did make CHECK_DTBS=y arch/arm64/boot/dts/aspeed/ don't see the fail with
> intc0: interrupt-controller@...00000 {
> compatible = "simple-mfd";
>
> So, could you point me more test instruction for this?
See syscon.yaml. And writing bindings or talks on conferences:
simple-mfd cannot be alone.
Best regards,
Krzysztof
Powered by blists - more mailing lists