Re: [PATCH 06/12] riscv: dts: allwinner: Add the D1 SoC base devicetree
From: Andre Przywara <andre.przywara@arm.com>
Date: 2022-08-22 12:29:58
Also in:
linux-riscv, linux-sunxi, lkml
On Mon, 22 Aug 2022 12:13:42 +0000 [off-list ref] wrote: Hi,
On 22/08/2022 12:46, Geert Uytterhoeven wrote:quoted
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe Hi Conor, Andre, On Sun, Aug 21, 2022 at 12:07 PM [off-list ref] wrote:quoted
On 21/08/2022 07:45, Icenowy Zheng wrote:quoted
在 2022-08-20星期六的 17:29 +0000,Conor.Dooley@microchip.com写道:quoted
On 20/08/2022 18:24, Samuel Holland wrote:quoted
quoted
quoted
quoted
quoted
This is not feasible, due to the different #interrupt-cells. See https://lore.kernel.org/linux-riscv/CAMuHMdXHSMcrVOH+vcrdRRF+i2TkMcFisGxHMBPUEa8nTMFpzw@mail.gmail.com/ (local) Even if we share some file across architectures, you still have to update files in both places to get the interrupts properties correct. I get the desire to deduplicate things, but we already deal with updating the same/similar nodes across several SoCs, so that is nothing new. I think it would be more confusing/complicated to have all of the interrupts properties overridden in a separate file.Yeah, should maybe have circled back after that conversation, would have been nice but if the DTC can't do it nicely then w/e.Well, maybe we can overuse the facility of C preprocessor? e.g.// For ARM #define SOC_PERIPHERAL_IRQ(n) GIC_SPI n // For RISC-V #define SOC_PERIPHERAL_IRQ(n) nGeert pointed out that this is not possible (at least on the Renesas stuff) because the GIC interrupt numbers are not the same as the PLIC's & the DTC is not able to handle the addition: https://lore.kernel.org/linux-riscv/CAMuHMdXHSMcrVOH+vcrdRRF+i2TkMcFisGxHMBPUEa8nTMFpzw@mail.gmail.com/ (local)Without the ability to do additions in DTC, we could e.g. list both interrupts in the macro, like: // For ARM #define SOC_PERIPHERAL_IRQ(na, nr) GIC_SPI na // For RISC-V #define SOC_PERIPHERAL_IRQ(na, nr) nrDo you think this is worth doing? Or are you just providing an example of what could be done? Where would you envisage putting these macros? I forget the order of the CPP operations that are done, can they be put in the dts?quoted
On Mon, Aug 22, 2022 at 12:52 PM Andre Przywara [off-list ref] wrote:quoted
There are interrupt-maps for that: sun8i-r528.dtsi: soc { #interrupt-cells = <1>; interrupt-map = <0 18 &gic GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>, <0 19 &gic GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>, .... sun20i-d1.dtsi: soc { #interrupt-cells = <1>; interrupt-map = <0 18 &plic 18 IRQ_TYPE_LEVEL_HIGH>, <0 19 &plic 19 IRQ_TYPE_LEVEL_HIGH>, then, in the shared .dtsi: uart0: serial@2500000 { compatible = "snps,dw-apb-uart"; ... interrupts = <18>;Nice! But it's gonna be a very large interrupt-map.I quite like the idea of not duplicating files across the archs if it can be helped, but not at the expense of making them hard to understand & I feel like unfortunately the large interrupt map is in that territory.
Well, I don't know about the Renesas case, but as far as we know the Allwinner D1 and R528 are using the exact same die, just fused differently. So expressing this in a common .dtsi sounds very desirable, especially since a devicetree is an architecture agnostic data structure. And while it's true that a DT interrupt-map is not for the faint of heart, I think even the casual reader gets the idea quickly by looking at it, possibly guided by a comment. And it doesn't need to be very large. grep counted 32 genuine interrupts in the current .dtsi file, so I just put those ones needed in. If we need more IRQs later (quite likely), they are easily added, using copy&paste. Cheers, Andre