On Wed, Nov 16, 2022 at 04:04:45PM +0530, Anup Patel wrote: > On Wed, Nov 16, 2022 at 2:30 PM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > > On 15/11/2022 23:34, Conor Dooley wrote: > > > On Mon, Nov 14, 2022 at 05:59:00PM +0530, Anup Patel wrote: > > >> On Sun, Nov 13, 2022 at 8:18 PM Conor Dooley <conor@xxxxxxxxxx> wrote: > > > > > >>> Also, the file name says "riscv,imsic", the description says "IMSIC" but > > >>> you've used "imsics" in the compatible. Is this a typo, or a plural? > > >> > > >> Yes, the file name should be consistent. I will update the file name. > > > > > > Is there a reason why the compatible is plural when all of the other > > > mentions etc do not have an "s"? It really did look like a typo to me. > > > > > > It's the "incoming MSI controller", so I am unsure as to where the "s" > > > actually even comes from. Why not just use "riscv,imsic"? > > > > Yep, should be rather consistent with all others, and IMSIC stands for > > Integrated Circuit? > > This is intentionally plural because even though we have one > IMSIC per-CPU, Linux (and various OSes) expect one DT node > as MSI controller targeting all CPUs. Even still, calling it "riscv,imsic" would seem fair to me given the multiple regs make the distinct regions clear. I think I must have missed the bit at the end of the description though: + The device tree of a RISC-V platform will have one IMSIC device tree node + for each privilege level (machine or supervisor) which collectively describe + IMSIC interrupt files at that privilege level across CPUs (or HARTs). Perhaps, for eejits like me, that paragraph should become paragraph 3 instead of hiding it below the register layout etc? Anyways, existing name seems fine to me then w/ the filename update & increased prominence of the many-controllers-in-one statement. Maybe the devicetree gods think differently! > The plural compatible string "riscv,imsics" was chosen based > on consensus on RISC-V AIA Task Group meetings. btw, I see the following in the example: + reg = <0x28000000 0x2000>, /* Group0 IMSICs */ + <0x29000000 0x2000>; /* Group1 IMSICs */ And in the property: + reg: + minItems: 1 + maxItems: 128 + description: + Base address of each IMSIC group. It would appear that the comment there conflicts with the description of the reg property itself & it's that lack of consistency me confused (: Should the comments be in the singular form? Thanks, Conor.
