On Sat, 20 Aug 2022 12:24:55 -0500 Samuel Holland <samuel@xxxxxxxxxxxx> wrote: Hi, > On 8/15/22 12:01 PM, Conor.Dooley@xxxxxxxxxxxxx wrote: > > On 15/08/2022 14:11, Andre Przywara wrote: > >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >> > >> On Mon, 15 Aug 2022 00:08:09 -0500 > >> Samuel Holland <samuel@xxxxxxxxxxxx> wrote: > >> > >> Hi, > >> > >> thanks for all the efforts in getting those SoC peripherals supported! > >> > >>> D1 is a SoC containing a single-core T-HEAD Xuantie C906 CPU, as well as > >>> one HiFi 4 DSP. The SoC is based on a design that additionally contained > >>> a pair of Cortex A7's. For that reason, some peripherals are duplicated. > >> > >> So because of this, the Allwinner R528 and T113 SoCs would share almost > >> everything in this file. Would it be useful to already split this DT up? > >> To have a base .dtsi, basically this file without /cpus and /soc/plic, > >> then have a RISC-V specific file with just those, including the base? > >> There is precedence for this across-arch(-directories) sharing with the > >> Raspberry Pi and Allwinner H3/H5 SoCs. > > > > For those playing along at home, one example is the arm64 bananapi m2 > > dts which looks like: > >> /dts-v1/; > >> #include "sun50i-h5.dtsi" > >> #include "sun50i-h5-cpu-opp.dtsi" > >> #include <arm/sunxi-bananapi-m2-plus-v1.2.dtsi> > >> > >> / { > >> model = "Banana Pi BPI-M2-Plus v1.2 H5"; > >> compatible = "bananapi,bpi-m2-plus-v1.2", "allwinner,sun50i-h5"; > >> }; > > > > I think this is a pretty good idea, and putting in the modularity up > > front seems logical to me, so when the arm one does eventually get > > added it can be done by only touching a single arch. > > This is not feasible, due to the different #interrupt-cells. See > https://lore.kernel.org/linux-riscv/CAMuHMdXHSMcrVOH+vcrdRRF+i2TkMcFisGxHMBPUEa8nTMFpzw@xxxxxxxxxxxxxx/ > > Even if we share some file across architectures, you still have to update files > in both places to get the interrupts properties correct. 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>; This is completely untested, but I have all the files spelt out there, and dtc seems happy for both architectures (outside of the kernel tree for now). > 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. So is this the only thing that prevents sharing? The above paragraph sounds a bit you are not very fond of the idea to begin with? Cheers, Andre