On 12/02/2025 18:44, Alexander Sverdlin wrote:
> Hi
>
> On Wed, 2025-02-12 at 17:46 +0100, Krzysztof Kozlowski wrote:
>>>>> Make the peripheral device tree re-usable on ARM64 platform by moving CPU
>>>>> core and interrupt controllers' parts into the respective per-SoC .dtsi
>>>>> files.
>>>>>
>>>>> Add SOC_PERIPHERAL_IRQ() macro which explicitly maps peripheral nubering
>>>>> into "plic" interrupt-controller numbering.
>>>>>
>>>>> Have a nice refactoring side-effect that "plic" and "clint" "compatible"
>>>>> property is not specified outside of the corresponding device itself.
>>>>>
>>>>> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
>>>>> ---
>>>>> Changelog:
>>>>> v2:
>>>>> - instead of carving out peripherals' part, carve out ARCH-specifics (CPU
>>>>> core, interrupt controllers) and spread them among 3 SoC .dtsi files which
>>>>> included cv18xx.dtsi;
>>>>> - define a label for the "soc" node and use it in the newly introduced DTs;
>>>>>
>>>>> arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 64 ++++++++++++---
>>>>> arch/riscv/boot/dts/sophgo/cv1812h.dtsi | 64 ++++++++++++---
>>>>> arch/riscv/boot/dts/sophgo/cv181x.dtsi | 2 +-
>>>>> arch/riscv/boot/dts/sophgo/cv18xx-cpu.dtsi | 57 ++++++++++++++
>>>>> arch/riscv/boot/dts/sophgo/cv18xx.dtsi | 91 ++++++----------------
>>>>> arch/riscv/boot/dts/sophgo/sg2002.dtsi | 64 ++++++++++++---
>>>>> 6 files changed, 240 insertions(+), 102 deletions(-)
>>>>> create mode 100644 arch/riscv/boot/dts/sophgo/cv18xx-cpu.dtsi
>>>>>
>>>>> diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
>>>>> index aa1f5df100f0..eef2884b36f9 100644
>>>>> --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
>>>>> +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
>>>>> @@ -3,6 +3,8 @@
>>>>> * Copyright (C) 2023 Jisheng Zhang <jszhang@xxxxxxxxxx>
>>>>> */
>>>>>
>>>>> +#define SOC_PERIPHERAL_IRQ(nr) ((nr) + 16)
>>>>> +
>>>>> #include <dt-bindings/pinctrl/pinctrl-cv1800b.h>
>>>>> #include "cv18xx.dtsi"
>>>>>
>>>>> @@ -14,22 +16,62 @@ memory@80000000 {
>>>>> reg = <0x80000000 0x4000000>;
>>>>> };
>>>>>
>>>>
>>>>> - soc {
>>>>> - pinctrl: pinctrl@3001000 {
>>>>> - compatible = "sophgo,cv1800b-pinctrl";
>>>>> - reg = <0x03001000 0x1000>,
>>>>> - <0x05027000 0x1000>;
>>>>> - reg-names = "sys", "rtc";
>>>>
>>>>
>>>>> + cpus: cpus {
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> + timebase-frequency = <25000000>;
>>>>> +
>>>>> + cpu0: cpu@0 {
>>>>> + compatible = "thead,c906", "riscv";
>>>>> + device_type = "cpu";
>>>>> + reg = <0>;
>>>>> + d-cache-block-size = <64>;
>>>>> + d-cache-sets = <512>;
>>>>> + d-cache-size = <65536>;
>>>>> + i-cache-block-size = <64>;
>>>>> + i-cache-sets = <128>;
>>>>> + i-cache-size = <32768>;
>>>>> + mmu-type = "riscv,sv39";
>>>>> + riscv,isa = "rv64imafdc";
>>>>> + riscv,isa-base = "rv64i";
>>>>> + riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
>>>>> + "zifencei", "zihpm";
>>>>> +
>>>>> + cpu0_intc: interrupt-controller {
>>>>> + compatible = "riscv,cpu-intc";
>>>>> + interrupt-controller;
>>>>> + #interrupt-cells = <1>;
>>>>> + };
>>>>> };
>>>>> };
>>>>> };
>>>>
>>>> Make all soc definition include the common cpu file.
>>>> Not just copy it.
>>>
>>> I was acting according to Krzysztof's suggestion:
>>> https://lore.kernel.org/soc/d3ba0ea5-0491-42d5-a18e-64cf21df696c@xxxxxxxxxx/
>>>
>>> Krzysztof, I can name the file cv18xx-cpu-intc.dtsi and pack CPU core + interrupt
>>> controllers into it. Would it make sense?
>>
>>
>> I don't understand the original suggestion.
>
> This is the snippet in question:
>
> ---[ cut ]---
> #define SOC_PERIPHERAL_IRQ(nr) ((nr) + 16)
>
> / {
> cpus: cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> timebase-frequency = <25000000>;
>
> cpu0: cpu@0 {
> compatible = "thead,c906", "riscv";
> device_type = "cpu";
> reg = <0>;
> d-cache-block-size = <64>;
> d-cache-sets = <512>;
> d-cache-size = <65536>;
> i-cache-block-size = <64>;
> i-cache-sets = <128>;
> i-cache-size = <32768>;
> mmu-type = "riscv,sv39";
> riscv,isa = "rv64imafdc";
> riscv,isa-base = "rv64i";
> riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> "zifencei", "zihpm";
>
> cpu0_intc: interrupt-controller {
> compatible = "riscv,cpu-intc";
> interrupt-controller;
> #interrupt-cells = <1>;
> };
> };
> };
> };
>
> &soc {
> interrupt-parent = <&plic>;
> dma-noncoherent;
>
> plic: interrupt-controller@70000000 {
> reg = <0x70000000 0x4000000>;
> interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
> interrupt-controller;
> #address-cells = <0>;
> #interrupt-cells = <2>;
> riscv,ndev = <101>;
> };
>
> clint: timer@74000000 {
> reg = <0x74000000 0x10000>;
> interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> };
> };
> ---[ cut ]---
>
> Inochi's proposal is to put it into separate cv18xx-cpu-intc.dtsi and
> include the latter in 3 other SoC-specific .dtsis. In v2 I've just
> duplicated the above snippet 3 times (refer to diffstat above).
>
> What are your thoughts? In Renesas everything is duplicated, I believe.
> Sophgo outsources much smaller snippets into .dtsi (refer to cv181x.dtsi).
If it represents some shared design/part, then it feels good.
Best regards,
Krzysztof
