| Message ID | 20230926155613.33904-3-robh@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Rob Herring |
| Headers | show |
| Series | [1/3] dt-bindings: PCI: brcm,iproc-pcie: Fix example indentation | expand |
On Tue, Sep 26, 2023 at 10:56:09AM -0500, Rob Herring wrote: > The 'msi' child node schema is missing constraints on additional properties. > It turns out it is incomplete and properties for it are documented in the > parent node by mistake. Move the reference to msi-controller.yaml and > the custom properties to the 'msi' node. Adding 'unevaluatedProperties' > ensures all the properties in the 'msi' node are documented. > > With the schema corrected, a minimal interrupt controller node is needed > to properly decode the interrupt properties since the example has > multiple interrupt parents. I suppose this is an ABI break, but the patch just makes the binding match the example and intent. Feels like of all the patches doing the unevaluatedProperty additions, this one is the most deserving of a fixes tag, since the original binding just seems to be completely wrong? Otherwise, Acked-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > > Signed-off-by: Rob Herring <robh@kernel.org> > --- > .../bindings/pci/brcm,iproc-pcie.yaml | 24 ++++++++++++------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml > index 6730d68fedc7..0e07ab61a48d 100644 > --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml > +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml > @@ -12,7 +12,6 @@ maintainers: > > allOf: > - $ref: /schemas/pci/pci-bus.yaml# > - - $ref: /schemas/interrupt-controller/msi-controller.yaml# > > properties: > compatible: > @@ -63,20 +62,24 @@ properties: > > msi: > type: object > + $ref: /schemas/interrupt-controller/msi-controller.yaml# > + unevaluatedProperties: false > + > properties: > compatible: > items: > - const: brcm,iproc-msi > > - msi-parent: true > + interrupts: > + maxItems: 4 > > - msi-controller: true > + brcm,pcie-msi-inten: > + type: boolean > + description: > + Needs to be present for some older iProc platforms that require the > + interrupt enable registers to be set explicitly to enable MSI > > - brcm,pcie-msi-inten: > - type: boolean > - description: > > - Needs to be present for some older iProc platforms that require the > - interrupt enable registers to be set explicitly to enable MSI > + msi-parent: true > > dependencies: > brcm,pcie-ob-axi-offset: ["brcm,pcie-ob"] > @@ -104,6 +107,11 @@ examples: > - | > #include <dt-bindings/interrupt-controller/arm-gic.h> > > + gic: interrupt-controller { > + interrupt-controller; > + #interrupt-cells = <3>; > + }; > + > pcie@18012000 { > compatible = "brcm,iproc-pcie"; > reg = <0x18012000 0x1000>; > -- > 2.40.1 >
On Wed, Sep 27, 2023 at 10:35 AM Conor Dooley <conor@kernel.org> wrote: > > On Tue, Sep 26, 2023 at 10:56:09AM -0500, Rob Herring wrote: > > The 'msi' child node schema is missing constraints on additional properties. > > It turns out it is incomplete and properties for it are documented in the > > parent node by mistake. Move the reference to msi-controller.yaml and > > the custom properties to the 'msi' node. Adding 'unevaluatedProperties' > > ensures all the properties in the 'msi' node are documented. > > > > With the schema corrected, a minimal interrupt controller node is needed > > to properly decode the interrupt properties since the example has > > multiple interrupt parents. > > I suppose this is an ABI break, but the patch just makes the binding > match the example and intent. It also matches what the in tree users do, so not an ABI break. I imagine the .txt binding just listed out properties and the conversion carried that over. > Feels like of all the patches doing the > unevaluatedProperty additions, this one is the most deserving of a fixes > tag, since the original binding just seems to be completely wrong? Yes, though the example fix is a dependency, so probably not worth backporting. Fixes: 905b986d099c ("dt-bindings: pci: Convert iProc PCIe to YAML") Rob
On 9/26/2023 5:56 PM, Rob Herring wrote: > The 'msi' child node schema is missing constraints on additional properties. > It turns out it is incomplete and properties for it are documented in the > parent node by mistake. Move the reference to msi-controller.yaml and > the custom properties to the 'msi' node. Adding 'unevaluatedProperties' > ensures all the properties in the 'msi' node are documented. > > With the schema corrected, a minimal interrupt controller node is needed > to properly decode the interrupt properties since the example has > multiple interrupt parents. > > Signed-off-by: Rob Herring <robh@kernel.org> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
On Tue, 26 Sep 2023 10:56:09 -0500, Rob Herring wrote: > The 'msi' child node schema is missing constraints on additional properties. > It turns out it is incomplete and properties for it are documented in the > parent node by mistake. Move the reference to msi-controller.yaml and > the custom properties to the 'msi' node. Adding 'unevaluatedProperties' > ensures all the properties in the 'msi' node are documented. > > With the schema corrected, a minimal interrupt controller node is needed > to properly decode the interrupt properties since the example has > multiple interrupt parents. > > Signed-off-by: Rob Herring <robh@kernel.org> > --- > .../bindings/pci/brcm,iproc-pcie.yaml | 24 ++++++++++++------- > 1 file changed, 16 insertions(+), 8 deletions(-) > Applied, thanks!
diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml index 6730d68fedc7..0e07ab61a48d 100644 --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml @@ -12,7 +12,6 @@ maintainers: allOf: - $ref: /schemas/pci/pci-bus.yaml# - - $ref: /schemas/interrupt-controller/msi-controller.yaml# properties: compatible: @@ -63,20 +62,24 @@ properties: msi: type: object + $ref: /schemas/interrupt-controller/msi-controller.yaml# + unevaluatedProperties: false + properties: compatible: items: - const: brcm,iproc-msi - msi-parent: true + interrupts: + maxItems: 4 - msi-controller: true + brcm,pcie-msi-inten: + type: boolean + description: + Needs to be present for some older iProc platforms that require the + interrupt enable registers to be set explicitly to enable MSI - brcm,pcie-msi-inten: - type: boolean - description: > - Needs to be present for some older iProc platforms that require the - interrupt enable registers to be set explicitly to enable MSI + msi-parent: true dependencies: brcm,pcie-ob-axi-offset: ["brcm,pcie-ob"] @@ -104,6 +107,11 @@ examples: - | #include <dt-bindings/interrupt-controller/arm-gic.h> + gic: interrupt-controller { + interrupt-controller; + #interrupt-cells = <3>; + }; + pcie@18012000 { compatible = "brcm,iproc-pcie"; reg = <0x18012000 0x1000>;
The 'msi' child node schema is missing constraints on additional properties. It turns out it is incomplete and properties for it are documented in the parent node by mistake. Move the reference to msi-controller.yaml and the custom properties to the 'msi' node. Adding 'unevaluatedProperties' ensures all the properties in the 'msi' node are documented. With the schema corrected, a minimal interrupt controller node is needed to properly decode the interrupt properties since the example has multiple interrupt parents. Signed-off-by: Rob Herring <robh@kernel.org> --- .../bindings/pci/brcm,iproc-pcie.yaml | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-)