PCI schema accepts unevaluated properties

PCI schema accepts unevaluated properties

[Krzysztof Kozlowski]

Hi Rob,

Dmitry pointed out interesting issue when writing PCI schema. Consider
the patch for Exynos PCI schema (which includes SNPS schema and
unevaluatedProperties:false):
https://pastebin.ubuntu.com/p/4SzxVP4N3j/

This passes `make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/` without a
problem. It should not. The property is nowhere described, so
unevaluatedProperties should scream.

Other bindings using unevaluated:false and some ref, also has this
issue. The additionalProperties:false (when there is no ref for other
schema) works correct.

DT schema v2022.3.2.

Best regards,
Krzysztof

Re: PCI schema accepts unevaluated properties

[Rob Herring]

On Sat, Apr 23, 2022 at 5:46 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> Hi Rob,
>
> Dmitry pointed out interesting issue when writing PCI schema. Consider
> the patch for Exynos PCI schema (which includes SNPS schema and
> unevaluatedProperties:false):
> https://pastebin.ubuntu.com/p/4SzxVP4N3j/
>
> This passes `make dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/` without a
> problem. It should not. The property is nowhere described, so
> unevaluatedProperties should scream.
>
> Other bindings using unevaluated:false and some ref, also has this
> issue. The additionalProperties:false (when there is no ref for other
> schema) works correct.

This is because I never committed the fix for all this which explains the issue:

    dtschema: Strip 'additionalProperties: true' from schemas

    An 'additionalProperties: true' makes all properties evaluated, so an
    'unevaluatedProperties: false' schema doesn't work when referencing a
    schema with 'additionalProperties: true'.

    Really, 'additionalProperties: true' should be dropped, but it is used as
    a flag for common schemas which are usually incomplete and included by
    device specific schemas. Removing it would mean dropping the meta-schema
    requiring either 'additionalProperties' or 'unevaluatedProperties'.

    Signed-off-by: Rob Herring <robh@kernel.org>

Since snps,dw-pcie.yaml defines 'additionalProperties: true',
everything is evaluated...

This fix needed schema fixes to land before I committed this, but it
was the end of the year and I guess I forgot about it. Now, it looks
like we have more to fix. :(  25 to be exact:

Documentation/devicetree/bindings/mfd/samsung,exynos5433-lpass.example.dtb:
audio-subsystem@11400000: dma-controller@11420000: Unevaluated
properties are not allowed ('#dma-channels', '#dma-requests',
'power-domains' were unexpected)
Documentation/devicetree/bindings/mfd/samsung,exynos5433-lpass.example.dtb:
dma-controller@11420000: Unevaluated properties are not allowed
('#dma-channels', '#dma-requests', 'power-domains' were unexpected)
Documentation/devicetree/bindings/ufs/ti,j721e-ufs.example.dtb:
ufs-wrapper@4e80000: ufs@4000: Unevaluated properties are not allowed
('power-domains' was unexpected)
Documentation/devicetree/bindings/ufs/ti,j721e-ufs.example.dtb:
ufs-wrapper@4e80000: ufs@4000: Unevaluated properties are not allowed
('power-domains' was unexpected)
Documentation/devicetree/bindings/ufs/ti,j721e-ufs.example.dtb:
ufs@4000: Unevaluated properties are not allowed ('power-domains' was
unexpected)
Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.example.dtb:
hdmi@10180000: Unevaluated properties are not allowed ('ddc-i2c-bus'
was unexpected)
Documentation/devicetree/bindings/net/mctp-i2c-controller.example.dtb:
i2c-mux: i2c@1: Unevaluated properties are not allowed
('mctp-controller' was unexpected)
Documentation/devicetree/bindings/net/mediatek-dwmac.example.dtb:
ethernet@1101c000: Unevaluated properties are not allowed
('power-domains' was unexpected)
Documentation/devicetree/bindings/net/wireless/mediatek,mt76.example.dtb:
wifi@18000000: Unevaluated properties are not allowed ('reset-names'
was unexpected)
Documentation/devicetree/bindings/net/cdns,macb.example.dtb:
ethernet@ff0c0000: Unevaluated properties are not allowed
('#stream-id-cells' was unexpected)
Documentation/devicetree/bindings/net/dsa/microchip,ksz.example.dtb:
switch@0: Unevaluated properties are not allowed ('reg',
'spi-max-frequency' were unexpected)
Documentation/devicetree/bindings/net/dsa/microchip,ksz.example.dtb:
switch@1: Unevaluated properties are not allowed ('reg',
'spi-max-frequency' were unexpected)
Documentation/devicetree/bindings/net/dsa/realtek.example.dtb:
switch@29: Unevaluated properties are not allowed ('reg' was
unexpected)
Documentation/devicetree/bindings/pci/apple,pcie.example.dtb:
pcie@690000000: Unevaluated properties are not allowed
('power-domains' was unexpected)
Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.example.dtb:
pcie@66000000: Unevaluated properties are not allowed
('legacy-interrupt-controller' was unexpected)
Documentation/devicetree/bindings/mtd/mxc-nand.example.dtb:
nand-controller@d8000000: Unevaluated properties are not allowed
('nand-bus-width', 'nand-ecc-mode' were unexpected)
Documentation/devicetree/bindings/mtd/ingenic,nand.example.dtb:
nand-controller@1: Unevaluated properties are not allowed
('ingenic,nemc-tAS', 'ingenic,nemc-tAH', 'ingenic,nemc-tBP',
'ingenic,nemc-tAW', 'ingenic,nemc-tSTRV' were unexpected)
Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.example.dtb:
mmc@84b0000: Unevaluated properties are not allowed ('interrupt-names'
was unexpected)
Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.example.dtb:
mmc@84b1000: Unevaluated properties are not allowed ('interrupt-names'
was unexpected)
Documentation/devicetree/bindings/usb/dwc3-xilinx.example.dtb: usb@0:
usb@fe200000: Unevaluated properties are not allowed ('dma-coherent'
was unexpected)
Documentation/devicetree/bindings/usb/dwc3-xilinx.example.dtb:
usb@fe200000: Unevaluated properties are not allowed ('dma-coherent'
was unexpected)
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dtb:
flash@1: Unevaluated properties are not allowed ('rx-sample-delay-ns'
was unexpected)
Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dtb:
spi@20108000: Unevaluated properties are not allowed
('spi-max-frequency' was unexpected)
Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.example.dtb:
flash@0: Unevaluated properties are not allowed
('nvidia,tx-clk-tap-delay', 'nvidia,rx-clk-tap-delay' were unexpected)
Documentation/devicetree/bindings/spi/samsung,spi.example.dtb:
audio-codec@0: Unevaluated properties are not allowed
('spi-max-frequency', 'controller-data' were unexpected)

I'll look at it more tomorrow.

Rob

Re: PCI schema accepts unevaluated properties

[Krzysztof Kozlowski]

On 27/04/2022 04:36, Rob Herring wrote:
> On Sat, Apr 23, 2022 at 5:46 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> Hi Rob,
>>
>> Dmitry pointed out interesting issue when writing PCI schema. Consider
>> the patch for Exynos PCI schema (which includes SNPS schema and
>> unevaluatedProperties:false):
>> https://pastebin.ubuntu.com/p/4SzxVP4N3j/
>>
>> This passes `make dt_binding_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/` without a
>> problem. It should not. The property is nowhere described, so
>> unevaluatedProperties should scream.
>>
>> Other bindings using unevaluated:false and some ref, also has this
>> issue. The additionalProperties:false (when there is no ref for other
>> schema) works correct.
> 
> This is because I never committed the fix for all this which explains the issue:
> 
>     dtschema: Strip 'additionalProperties: true' from schemas
> 
>     An 'additionalProperties: true' makes all properties evaluated, so an
>     'unevaluatedProperties: false' schema doesn't work when referencing a
>     schema with 'additionalProperties: true'.
> 
>     Really, 'additionalProperties: true' should be dropped, but it is used as
>     a flag for common schemas which are usually incomplete and included by
>     device specific schemas. Removing it would mean dropping the meta-schema
>     requiring either 'additionalProperties' or 'unevaluatedProperties'.
> 
>     Signed-off-by: Rob Herring <robh@kernel.org>
> 
> Since snps,dw-pcie.yaml defines 'additionalProperties: true',
> everything is evaluated...

Thanks for explanation.

> 
> This fix needed schema fixes to land before I committed this, but it
> was the end of the year and I guess I forgot about it. Now, it looks
> like we have more to fix. :(  25 to be exact:
> 
> Documentation/devicetree/bindings/mfd/samsung,exynos5433-lpass.example.dtb:
> audio-subsystem@11400000: dma-controller@11420000: Unevaluated
> properties are not allowed ('#dma-channels', '#dma-requests',
> 'power-domains' were unexpected)

I'll fix the power-domains, but dma-channels/dma-requests should not
fail. This should be accepted through:
arm,pl330.yaml -> dma-controller.yaml -> dma-common.yaml

> Documentation/devicetree/bindings/mfd/samsung,exynos5433-lpass.example.dtb:
> dma-controller@11420000: Unevaluated properties are not allowed
> ('#dma-channels', '#dma-requests', 'power-domains' were unexpected)
> Documentation/devicetree/bindings/ufs/ti,j721e-ufs.example.dtb:
> ufs-wrapper@4e80000: ufs@4000: Unevaluated properties are not allowed
> ('power-domains' was unexpected)
> Documentation/devicetree/bindings/ufs/ti,j721e-ufs.example.dtb:
> ufs-wrapper@4e80000: ufs@4000: Unevaluated properties are not allowed
> ('power-domains' was unexpected)
> Documentation/devicetree/bindings/ufs/ti,j721e-ufs.example.dtb:
> ufs@4000: Unevaluated properties are not allowed ('power-domains' was
> unexpected)

I'll fix it.

> Documentation/devicetree/bindings/display/bridge/ingenic,jz4780-hdmi.example.dtb:
> hdmi@10180000: Unevaluated properties are not allowed ('ddc-i2c-bus'
> was unexpected)
> Documentation/devicetree/bindings/net/mctp-i2c-controller.example.dtb:
> i2c-mux: i2c@1: Unevaluated properties are not allowed
> ('mctp-controller' was unexpected)
> Documentation/devicetree/bindings/net/mediatek-dwmac.example.dtb:
> ethernet@1101c000: Unevaluated properties are not allowed
> ('power-domains' was unexpected)
> Documentation/devicetree/bindings/net/wireless/mediatek,mt76.example.dtb:
> wifi@18000000: Unevaluated properties are not allowed ('reset-names'
> was unexpected)
> Documentation/devicetree/bindings/net/cdns,macb.example.dtb:
> ethernet@ff0c0000: Unevaluated properties are not allowed
> ('#stream-id-cells' was unexpected)
> Documentation/devicetree/bindings/net/dsa/microchip,ksz.example.dtb:
> switch@0: Unevaluated properties are not allowed ('reg',
> 'spi-max-frequency' were unexpected)
> Documentation/devicetree/bindings/net/dsa/microchip,ksz.example.dtb:
> switch@1: Unevaluated properties are not allowed ('reg',
> 'spi-max-frequency' were unexpected)
> Documentation/devicetree/bindings/net/dsa/realtek.example.dtb:
> switch@29: Unevaluated properties are not allowed ('reg' was
> unexpected)
> Documentation/devicetree/bindings/pci/apple,pcie.example.dtb:
> pcie@690000000: Unevaluated properties are not allowed
> ('power-domains' was unexpected)
> Documentation/devicetree/bindings/pci/socionext,uniphier-pcie.example.dtb:
> pcie@66000000: Unevaluated properties are not allowed
> ('legacy-interrupt-controller' was unexpected)
> Documentation/devicetree/bindings/mtd/mxc-nand.example.dtb:
> nand-controller@d8000000: Unevaluated properties are not allowed
> ('nand-bus-width', 'nand-ecc-mode' were unexpected)
> Documentation/devicetree/bindings/mtd/ingenic,nand.example.dtb:
> nand-controller@1: Unevaluated properties are not allowed
> ('ingenic,nemc-tAS', 'ingenic,nemc-tAH', 'ingenic,nemc-tBP',
> 'ingenic,nemc-tAW', 'ingenic,nemc-tSTRV' were unexpected)
> Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.example.dtb:
> mmc@84b0000: Unevaluated properties are not allowed ('interrupt-names'
> was unexpected)
> Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.example.dtb:
> mmc@84b1000: Unevaluated properties are not allowed ('interrupt-names'
> was unexpected)
> Documentation/devicetree/bindings/usb/dwc3-xilinx.example.dtb: usb@0:
> usb@fe200000: Unevaluated properties are not allowed ('dma-coherent'
> was unexpected)
> Documentation/devicetree/bindings/usb/dwc3-xilinx.example.dtb:
> usb@fe200000: Unevaluated properties are not allowed ('dma-coherent'
> was unexpected)
> Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dtb:
> flash@1: Unevaluated properties are not allowed ('rx-sample-delay-ns'
> was unexpected)
> Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dtb:
> spi@20108000: Unevaluated properties are not allowed
> ('spi-max-frequency' was unexpected)
> Documentation/devicetree/bindings/spi/nvidia,tegra210-quad.example.dtb:
> flash@0: Unevaluated properties are not allowed
> ('nvidia,tx-clk-tap-delay', 'nvidia,rx-clk-tap-delay' were unexpected)

> Documentation/devicetree/bindings/spi/samsung,spi.example.dtb:
> audio-codec@0: Unevaluated properties are not allowed
> ('spi-max-frequency', 'controller-data' were unexpected)

I'll fix the audio-codec.

Best regards,
Krzysztof

Re: PCI schema accepts unevaluated properties

[Rob Herring]

On Wed, Apr 27, 2022 at 1:29 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 27/04/2022 04:36, Rob Herring wrote:
> > On Sat, Apr 23, 2022 at 5:46 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> Hi Rob,
> >>
> >> Dmitry pointed out interesting issue when writing PCI schema. Consider
> >> the patch for Exynos PCI schema (which includes SNPS schema and
> >> unevaluatedProperties:false):
> >> https://pastebin.ubuntu.com/p/4SzxVP4N3j/
> >>
> >> This passes `make dt_binding_check
> >> DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/` without a
> >> problem. It should not. The property is nowhere described, so
> >> unevaluatedProperties should scream.
> >>
> >> Other bindings using unevaluated:false and some ref, also has this
> >> issue. The additionalProperties:false (when there is no ref for other
> >> schema) works correct.
> >
> > This is because I never committed the fix for all this which explains the issue:
> >
> >     dtschema: Strip 'additionalProperties: true' from schemas
> >
> >     An 'additionalProperties: true' makes all properties evaluated, so an
> >     'unevaluatedProperties: false' schema doesn't work when referencing a
> >     schema with 'additionalProperties: true'.
> >
> >     Really, 'additionalProperties: true' should be dropped, but it is used as
> >     a flag for common schemas which are usually incomplete and included by
> >     device specific schemas. Removing it would mean dropping the meta-schema
> >     requiring either 'additionalProperties' or 'unevaluatedProperties'.
> >
> >     Signed-off-by: Rob Herring <robh@kernel.org>
> >
> > Since snps,dw-pcie.yaml defines 'additionalProperties: true',
> > everything is evaluated...
>
> Thanks for explanation.
>
> >
> > This fix needed schema fixes to land before I committed this, but it
> > was the end of the year and I guess I forgot about it. Now, it looks
> > like we have more to fix. :(  25 to be exact:
> >
> > Documentation/devicetree/bindings/mfd/samsung,exynos5433-lpass.example.dtb:
> > audio-subsystem@11400000: dma-controller@11420000: Unevaluated
> > properties are not allowed ('#dma-channels', '#dma-requests',
> > 'power-domains' were unexpected)
>
> I'll fix the power-domains, but dma-channels/dma-requests should not
> fail. This should be accepted through:
> arm,pl330.yaml -> dma-controller.yaml -> dma-common.yaml

Except that the standard property names are 'dma-channels' and
'dma-requests' without the '#'.

Thanks for the fixes. Unfortunately, it looks like NAND needs the same
parent/child schema split that was done for SPI.

Rob

Re: PCI schema accepts unevaluated properties

[Krzysztof Kozlowski]

On 27/04/2022 16:53, Rob Herring wrote:
>> I'll fix the power-domains, but dma-channels/dma-requests should not
>> fail. This should be accepted through:
>> arm,pl330.yaml -> dma-controller.yaml -> dma-common.yaml
> 
> Except that the standard property names are 'dma-channels' and
> 'dma-requests' without the '#'.
> 
> Thanks for the fixes. Unfortunately, it looks like NAND needs the same
> parent/child schema split that was done for SPI.

Oh, few drivers indeed parse '#dma-xxx' variant. For pl330 schema it was
introduced with 42cf20980cde ("ARM: dts: pl330: Add #dma-cells for
generic dma binding support")
 which clearly defines something different than uses in example. I can
imagine that editor added '#' automatically after pressing <enter> when
editing the example.

This wrong variant spread all over and in practice was not detected
because pl330 driver does not parse this field.

I'll fix it.

Best regards,
Krzysztof