On 06/04/2023 20:45, Conor Dooley wrote: > On Thu, Apr 06, 2023 at 07:35:09PM +0100, Conor Dooley wrote: >> On Thu, Apr 06, 2023 at 08:24:55PM +0200, Krzysztof Kozlowski wrote: >>> On 06/04/2023 13:11, Minda Chen wrote: >>>> + >>>> + interrupt-controller: >>>> + type: object >>>> + properties: >>>> + '#address-cells': >>>> + const: 0 >>>> + >>>> + '#interrupt-cells': >>>> + const: 1 >>>> + >>>> + interrupt-controller: true >>>> + >>>> + required: >>>> + - '#address-cells' >>>> + - '#interrupt-cells' >>>> + - interrupt-controller >>>> + >>>> + additionalProperties: false >>>> + >>>> +required: >>>> + - reg >>>> + - reg-names >>>> + - "#interrupt-cells" >>> >>> Keep consistent quotes - either ' or " >>> >>> Are you sure this is correct? You have interrupt controller as child node. >> >> I know existing stuff in-tree is far from a guarantee that it'll be >> right, but this does at least follow what we've got for PolarFire SoC: >> Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml >> >> Both PLDA and both RISC-V w/ a PLIC as the interrupt controller, so in >> similar waters. >> This note existed in the original text form binding of the Microchip >> PCI controller: >> | +NOTE: >> | +The core provides a single interrupt for both INTx/MSI messages. So, >> | +create an interrupt controller node to support 'interrupt-map' DT >> | +functionality. The driver will create an IRQ domain for this map, decode >> | +the four INTx interrupts in ISR and route them to this domain. >> >> Given the similarities, I figure the same requirement applies here too. >> Minda? > > Further, if, as I currently suspect, there's a lot of commonality here, > should the binding as well as the driver be split into common pdla bits > and microchip/starfive specific ones? > > Suppose that's more one for you Krzysztof. Yeah, looks like only clocks and resets are different. At the end it depends how much code you would remove... Best regards, Krzysztof