| Message ID | 20220910195659.11843-4-Sergey.Semin@baikalelectronics.ru |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | EDAC/synopsys: Add generic resources and Baikal-T1 support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
On Sat, Sep 10, 2022 at 10:56:47PM +0300, Serge Semin wrote: > At the current state the DW uMCTL2 DDRC DT-schema can't be used as the > common one for all the IP-core-based devices due to the compatible string > property constraining the list of the supported device names. In order to > fix that we suggest to update the compatible property constraints so one > would permit having any value aside with the generic device names. At the > same time the generic DT-schema selection must be restricted to the > denoted generic devices only so not to permit the generic fallback > compatibles. Finally since the generic schema will be referenced from the > vendor-specific DT-bindings with possibly non-standard properties defined > it must permit having additional properties specified. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Note alternatively we could drop the "additionalProperties" keyword > modification since currently there is no actual device available with the > properties not listed in the generic DT-schema. Normally, this has required 2 schema files. However, I think you can do something like this: if: compatible: enum: - snps,ddrc-3.80a - snps,dw-umctl2-ddrc - xlnx,zynqmp-ddrc-2.40a then: unevaluatedProperties: false But please make sure that actually catches undocumented properties because unevaluateProperties under 'then' is not something I've tried. Rob
On Mon, Sep 12, 2022 at 09:32:19AM -0500, Rob Herring wrote: > On Sat, Sep 10, 2022 at 10:56:47PM +0300, Serge Semin wrote: > > At the current state the DW uMCTL2 DDRC DT-schema can't be used as the > > common one for all the IP-core-based devices due to the compatible string > > property constraining the list of the supported device names. In order to > > fix that we suggest to update the compatible property constraints so one > > would permit having any value aside with the generic device names. At the > > same time the generic DT-schema selection must be restricted to the > > denoted generic devices only so not to permit the generic fallback > > compatibles. Finally since the generic schema will be referenced from the > > vendor-specific DT-bindings with possibly non-standard properties defined > > it must permit having additional properties specified. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > --- > > > > Note alternatively we could drop the "additionalProperties" keyword > > modification since currently there is no actual device available with the > > properties not listed in the generic DT-schema. > > Normally, this has required 2 schema files. However, I think you can > do something like this: > > if: > compatible: > enum: > - snps,ddrc-3.80a > - snps,dw-umctl2-ddrc > - xlnx,zynqmp-ddrc-2.40a > then: > unevaluatedProperties: false > > > But please make sure that actually catches undocumented properties > because unevaluateProperties under 'then' is not something I've tried. Oh, I wish this would work! Alas it doesn't. AFAIU the schemas under the "then" and "else" keywords are considered as separate schemas and are independently applied to the DT node. As soon as I added the construction suggested by you the schema evaluation started failing with error as none of the DT-node properties in the examples are valid: < ... /snps,dw-umctl2-ddrc.example.dtb: memory-controller@fd070000: < Unevaluated properties are not allowed ('compatible', 'reg', interrupts', 'interrupt-names', '$nodename' were unexpected) < ... /snps,dw-umctl2-ddrc.example.dtb: memory-controller@3d400000: < Unevaluated properties are not allowed ('compatible', 'reg', 'interrupts', 'interrupt-names', 'clocks', 'clock-names', '$nodename' were unexpected) Any suggestion of how this could be fixed? Perhaps updating the dtschema tool anyhow? (I failed to find a quick-fix for it) Creating an additional separate schema with the common properties seems a bit overkill in this case. On the other hand is there a decent alternative? What about accepting what I suggested in this patch? It does permit additional properties, but we won't need to have a separate schema with just several common properties. -Sergey > > Rob
On Mon, Sep 26, 2022 at 5:56 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > On Mon, Sep 12, 2022 at 09:32:19AM -0500, Rob Herring wrote: > > On Sat, Sep 10, 2022 at 10:56:47PM +0300, Serge Semin wrote: > > > At the current state the DW uMCTL2 DDRC DT-schema can't be used as the > > > common one for all the IP-core-based devices due to the compatible string > > > property constraining the list of the supported device names. In order to > > > fix that we suggest to update the compatible property constraints so one > > > would permit having any value aside with the generic device names. At the > > > same time the generic DT-schema selection must be restricted to the > > > denoted generic devices only so not to permit the generic fallback > > > compatibles. Finally since the generic schema will be referenced from the > > > vendor-specific DT-bindings with possibly non-standard properties defined > > > it must permit having additional properties specified. > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > > > --- > > > > > > Note alternatively we could drop the "additionalProperties" keyword > > > modification since currently there is no actual device available with the > > > properties not listed in the generic DT-schema. > > > > > Normally, this has required 2 schema files. However, I think you can > > do something like this: > > > > if: > > compatible: > > enum: > > - snps,ddrc-3.80a > > - snps,dw-umctl2-ddrc > > - xlnx,zynqmp-ddrc-2.40a > > then: > > unevaluatedProperties: false > > > > > > But please make sure that actually catches undocumented properties > > because unevaluateProperties under 'then' is not something I've tried. > > Oh, I wish this would work! Alas it doesn't. AFAIU the schemas under > the "then" and "else" keywords are considered as separate schemas > and are independently applied to the DT node. As soon as I added the > construction suggested by you the schema evaluation started failing > with error as none of the DT-node properties in the examples are valid: > > < ... /snps,dw-umctl2-ddrc.example.dtb: memory-controller@fd070000: > < Unevaluated properties are not allowed ('compatible', 'reg', interrupts', 'interrupt-names', '$nodename' were unexpected) > > < ... /snps,dw-umctl2-ddrc.example.dtb: memory-controller@3d400000: > < Unevaluated properties are not allowed ('compatible', 'reg', 'interrupts', 'interrupt-names', 'clocks', 'clock-names', '$nodename' were unexpected) Indeed. While unevaluatedProperties takes if/then/else into account, flipping it around doesn't. > Any suggestion of how this could be fixed? Perhaps updating the > dtschema tool anyhow? (I failed to find a quick-fix for it) Creating > an additional separate schema with the common properties seems a bit > overkill in this case. On the other hand is there a decent > alternative? I don't think there is any other fix. > What about accepting what I suggested in this patch? It does permit > additional properties, but we won't need to have a separate schema > with just several common properties. No. You can't have it both ways. Either it is a common schema or a specific device schema. Rob
On Tue, Sep 27, 2022 at 05:02:40PM -0500, Rob Herring wrote: > On Mon, Sep 26, 2022 at 5:56 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > > > On Mon, Sep 12, 2022 at 09:32:19AM -0500, Rob Herring wrote: > > > On Sat, Sep 10, 2022 at 10:56:47PM +0300, Serge Semin wrote: > > > > At the current state the DW uMCTL2 DDRC DT-schema can't be used as the > > > > common one for all the IP-core-based devices due to the compatible string > > > > property constraining the list of the supported device names. In order to > > > > fix that we suggest to update the compatible property constraints so one > > > > would permit having any value aside with the generic device names. At the > > > > same time the generic DT-schema selection must be restricted to the > > > > denoted generic devices only so not to permit the generic fallback > > > > compatibles. Finally since the generic schema will be referenced from the > > > > vendor-specific DT-bindings with possibly non-standard properties defined > > > > it must permit having additional properties specified. > > > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > > > > > --- > > > > > > > > Note alternatively we could drop the "additionalProperties" keyword > > > > modification since currently there is no actual device available with the > > > > properties not listed in the generic DT-schema. > > > > > > > > Normally, this has required 2 schema files. However, I think you can > > > do something like this: > > > > > > if: > > > compatible: > > > enum: > > > - snps,ddrc-3.80a > > > - snps,dw-umctl2-ddrc > > > - xlnx,zynqmp-ddrc-2.40a > > > then: > > > unevaluatedProperties: false > > > > > > > > > But please make sure that actually catches undocumented properties > > > because unevaluateProperties under 'then' is not something I've tried. > > > > Oh, I wish this would work! Alas it doesn't. AFAIU the schemas under > > the "then" and "else" keywords are considered as separate schemas > > and are independently applied to the DT node. As soon as I added the > > construction suggested by you the schema evaluation started failing > > with error as none of the DT-node properties in the examples are valid: > > > > < ... /snps,dw-umctl2-ddrc.example.dtb: memory-controller@fd070000: > > < Unevaluated properties are not allowed ('compatible', 'reg', interrupts', 'interrupt-names', '$nodename' were unexpected) > > > > < ... /snps,dw-umctl2-ddrc.example.dtb: memory-controller@3d400000: > > < Unevaluated properties are not allowed ('compatible', 'reg', 'interrupts', 'interrupt-names', 'clocks', 'clock-names', '$nodename' were unexpected) > > Indeed. While unevaluatedProperties takes if/then/else into account, > flipping it around doesn't. > > > Any suggestion of how this could be fixed? Perhaps updating the > > dtschema tool anyhow? (I failed to find a quick-fix for it) Creating > > an additional separate schema with the common properties seems a bit > > overkill in this case. On the other hand is there a decent > > alternative? > > I don't think there is any other fix. > > > What about accepting what I suggested in this patch? It does permit > > additional properties, but we won't need to have a separate schema > > with just several common properties. > > No. You can't have it both ways. Either it is a common schema or a > specific device schema. Sigh... I see. Will fix it in the next patchset round. -Sergey > > Rob
diff --git a/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml b/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml index e68c4306025a..a3394b4600ef 100644 --- a/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml @@ -21,9 +21,21 @@ description: | controller. It has an optional SEC/DEC ECC support in 64- and 32-bits bus width configurations. +# Please create a separate DT-schema for your DW uMCTL2 DDR controller +# and make sure it's assigned with the vendor-specific compatible string. +select: + properties: + compatible: + enum: + - snps,ddrc-3.80a + - snps,dw-umctl2-ddrc + - xlnx,zynqmp-ddrc-2.40a + required: + - compatible + properties: compatible: - oneOf: + anyOf: - deprecated: true description: Synopsys DW uMCTL2 DDR controller v3.80a const: snps,ddrc-3.80a @@ -31,6 +43,7 @@ properties: const: snps,dw-umctl2-ddrc - description: Xilinx ZynqMP DDR controller v2.40a const: xlnx,zynqmp-ddrc-2.40a + - {} interrupts: description: @@ -87,7 +100,7 @@ required: - reg - interrupts -additionalProperties: false +additionalProperties: true examples: - |
At the current state the DW uMCTL2 DDRC DT-schema can't be used as the common one for all the IP-core-based devices due to the compatible string property constraining the list of the supported device names. In order to fix that we suggest to update the compatible property constraints so one would permit having any value aside with the generic device names. At the same time the generic DT-schema selection must be restricted to the denoted generic devices only so not to permit the generic fallback compatibles. Finally since the generic schema will be referenced from the vendor-specific DT-bindings with possibly non-standard properties defined it must permit having additional properties specified. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> --- Note alternatively we could drop the "additionalProperties" keyword modification since currently there is no actual device available with the properties not listed in the generic DT-schema. Changelog v2: - This is a new patch created on v2 cycle of the patchset. (@Krzysztof) --- .../memory-controllers/snps,dw-umctl2-ddrc.yaml | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)