| Message ID | 20220811203306.179744-4-mail@conchuod.ie |
|---|---|
| State | New |
| Headers | show |
| Series | Fix RISC-V/PCI dt-schema issues with dt-schema v2022.08 | expand |
On 11/08/2022 23:33, Conor Dooley wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > v2022.08 of dt-schema improved checking of unevaluatedProperties, and > exposed a previously unseen warning for the PCIe controller's interrupt > controller node name: > > arch/riscv/boot/dts/microchip/mpfs-icicle-kit.dtb: pcie@2000000000: Unevaluated properties are not allowed ('clock-names', 'clocks', 'legacy-interrupt-controller', 'microchip,axi-m-atr0' were unexpected) > From schema: Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml > > Make the property in the binding match the node name actually used in > the dts. > > Fixes: dcd49679fb3a ("dt-bindings: PCI: Fix 'unevaluatedProperties' warnings") > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > This is another one Rob where I feel like I'm doing the wrong thing. > The Linux driver gets the child node without using the name, but > another OS etc could in theory (or reality), right? Yes and we had such cases when renaming device nodes caused regression. My interpretation is that node name is not part of ABI, so anyone depending on it made a mistake and they need to fix their stuff. I think actually that is really poor coding and poor solution to parse device node names and expect specific name. Other folks interpretation is that we never break the users of kernel, regardless what is documented in the ABI... so it depends. :) Here however it is not a device node name, but a property name (although still a node). Bindings require these to be specific, thus such name is a part of ABI. For your case, I wonder why it was called "legacy-interrupt-controller" in the first place? Node names - also for properties - should be generic, so generic name is just "interrupt-controller". > --- > .../devicetree/bindings/pci/microchip,pcie-host.yaml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml > index 2a2166f09e2c..9b123bcd034c 100644 > --- a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml > +++ b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml > @@ -71,7 +71,7 @@ properties: > msi-parent: > description: MSI controller the device is capable of using. > > - interrupt-controller: > + legacy-interrupt-controller: Best regards, Krzysztof
On 12/08/2022 08:42, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 11/08/2022 23:33, Conor Dooley wrote: >> From: Conor Dooley <conor.dooley@microchip.com> >> >> v2022.08 of dt-schema improved checking of unevaluatedProperties, and >> exposed a previously unseen warning for the PCIe controller's interrupt >> controller node name: >> >> arch/riscv/boot/dts/microchip/mpfs-icicle-kit.dtb: pcie@2000000000: Unevaluated properties are not allowed ('clock-names', 'clocks', 'legacy-interrupt-controller', 'microchip,axi-m-atr0' were unexpected) >> From schema: Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml >> >> Make the property in the binding match the node name actually used in >> the dts. >> >> Fixes: dcd49679fb3a ("dt-bindings: PCI: Fix 'unevaluatedProperties' warnings") >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >> --- >> This is another one Rob where I feel like I'm doing the wrong thing. >> The Linux driver gets the child node without using the name, but >> another OS etc could in theory (or reality), right? > > Yes and we had such cases when renaming device nodes caused regression. > My interpretation is that node name is not part of ABI, so anyone > depending on it made a mistake and they need to fix their stuff. I think > actually that is really poor coding and poor solution to parse device > node names and expect specific name. > > Other folks interpretation is that we never break the users of kernel, > regardless what is documented in the ABI... so it depends. :) > > Here however it is not a device node name, but a property name (although > still a node). Bindings require these to be specific, thus such name is > a part of ABI. Yup, pretty much aligned to my thoughts on this. > For your case, I wonder why it was called "legacy-interrupt-controller" > in the first place? Node names - also for properties - should be > generic, so generic name is just "interrupt-controller". I don't know. It's what we had in our internal tree prior to upstreaming. "We" don't rely on the name for the Linux driver, so I am not really that bothered if we change the binding or the dts. > >> --- >> .../devicetree/bindings/pci/microchip,pcie-host.yaml | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml >> index 2a2166f09e2c..9b123bcd034c 100644 >> --- a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml >> +++ b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml >> @@ -71,7 +71,7 @@ properties: >> msi-parent: >> description: MSI controller the device is capable of using. >> >> - interrupt-controller: >> + legacy-interrupt-controller: > > > Best regards, > Krzysztof > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 12/08/2022 10:55, Conor.Dooley@microchip.com wrote: > On 12/08/2022 08:42, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 11/08/2022 23:33, Conor Dooley wrote: >>> From: Conor Dooley <conor.dooley@microchip.com> >>> >>> v2022.08 of dt-schema improved checking of unevaluatedProperties, and >>> exposed a previously unseen warning for the PCIe controller's interrupt >>> controller node name: >>> >>> arch/riscv/boot/dts/microchip/mpfs-icicle-kit.dtb: pcie@2000000000: Unevaluated properties are not allowed ('clock-names', 'clocks', 'legacy-interrupt-controller', 'microchip,axi-m-atr0' were unexpected) >>> From schema: Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml >>> >>> Make the property in the binding match the node name actually used in >>> the dts. >>> >>> Fixes: dcd49679fb3a ("dt-bindings: PCI: Fix 'unevaluatedProperties' warnings") >>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >>> --- >>> This is another one Rob where I feel like I'm doing the wrong thing. >>> The Linux driver gets the child node without using the name, but >>> another OS etc could in theory (or reality), right? >> >> Yes and we had such cases when renaming device nodes caused regression. >> My interpretation is that node name is not part of ABI, so anyone >> depending on it made a mistake and they need to fix their stuff. I think >> actually that is really poor coding and poor solution to parse device >> node names and expect specific name. >> >> Other folks interpretation is that we never break the users of kernel, >> regardless what is documented in the ABI... so it depends. :) >> >> Here however it is not a device node name, but a property name (although >> still a node). Bindings require these to be specific, thus such name is >> a part of ABI. > > Yup, pretty much aligned to my thoughts on this. > >> For your case, I wonder why it was called "legacy-interrupt-controller" >> in the first place? Node names - also for properties - should be >> generic, so generic name is just "interrupt-controller". > > I don't know. It's what we had in our internal tree prior to upstreaming. > "We" don't rely on the name for the Linux driver, so I am not really that > bothered if we change the binding or the dts. Then I propose to change the name in DTS. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml index 2a2166f09e2c..9b123bcd034c 100644 --- a/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml +++ b/Documentation/devicetree/bindings/pci/microchip,pcie-host.yaml @@ -71,7 +71,7 @@ properties: msi-parent: description: MSI controller the device is capable of using. - interrupt-controller: + legacy-interrupt-controller: type: object properties: '#address-cells': @@ -125,7 +125,7 @@ examples: msi-controller; bus-range = <0x00 0x7f>; ranges = <0x03000000 0x0 0x78000000 0x0 0x78000000 0x0 0x04000000>; - pcie_intc0: interrupt-controller { + pcie_intc0: legacy-interrupt-controller { #address-cells = <0>; #interrupt-cells = <1>; interrupt-controller;