devicetree-org / dt-schema

Devicetree schema tools
http://www.devicetree.org
BSD 2-Clause "Simplified" License
67 stars 67 forks source link

`if:properties:foo:contains:` can be true if property is not present #136

Closed lumag closed 6 months ago

lumag commented 6 months ago

Reproducer: linux-next (e.g. 20240429). make ARCH=arm64 qcom/apq8016-sbc.dtb CHECK_DTBS=y DT_SCHEMA_FILES=qcom,pmic-gpio.yaml

I'm currently getting the following error:

/home/lumag/Projects/Qcomm/build-64/arch/arm64/boot/dts/qcom/apq8016-sbc.dtb: gpio@c000: gpio-line-names: ['USR_LED_3_CTRL', 'USR_LED_4_CTRL', 'USB_HUB_RESET_N_PM', 'USB_SW_SEL_PM'] is too short
    from schema $id: http://devicetree.org/schemas/pinctrl/qcom,pmic-gpio.yaml#

With the https://lore.kernel.org/linux-arm-msm/20240425185603.3295450-1-quic_amelende@quicinc.com/ I can no longer reproduce the issue. However the patch fixes a branch which should be false anyway.

I was able to limit it to the following reproducer:

allOf:
  - if:
      properties:
        compatible:
          contains:
            enum:
              - qcom,pm8916-gpio
    then:
      properties:
        gpio-line-names:
          minItems: 4
          maxItems: 4

  - if:
      properties:
        comptaible:
          contains:
            enum:
              - qcom,pmih0108-gpio
    then:
      properties:
        gpio-line-names:
          minItems: 18
          maxItems: 18

Note, changing allOf to oneOf results in the following warning, which shows that second branch evaluated to true:

/home/lumag/Projects/Qcomm/build-64/arch/arm64/boot/dts/qcom/apq8016-sbc.dtb: gpio@c000: More than one condition true in oneOf schema:

cc @krzk

robherring commented 6 months ago

That's defined behavior of json-schema. A not present property's schema will be true. Otherwise, that would be the property is required or the whole schema would fail. If a property cannot be assumed to be present, you also need {required: [propname]} in the 'if' schema.

Usually we don't need required in the if/then, because we can assume 'compatible' is present as we don't apply the schema at all if it is not.

lumag commented 6 months ago

I see, thank you for the explanation.