| Message ID | 20230123151302.368277-11-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/13] dt-bindings: serial: amlogic,meson-uart: allow other serial properties | expand |
On Mon, 23 Jan 2023 16:13:00 +0100, Krzysztof Kozlowski wrote: > The re-usable serial.yaml schema matches every property with ".*" > pattern, thus any other schema referencing it will not report unknown > (unevaluated) properties. This hides several wrong properties. It is > a limitation of dtschema, thus provide a simple workaround: expect > children to be only of few names matching upstream usage (Bluetooth, > GNSS, GPS and MCU). > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > Documentation/devicetree/bindings/serial/serial.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/intel,ixp4xx-expansion-bus-controller.example.dtb: serial@1,0: Unevaluated properties are not allowed ('intel,ixp4xx-eb-byte-access', 'intel,ixp4xx-eb-cycle-type', 'intel,ixp4xx-eb-t3', 'intel,ixp4xx-eb-write-enable' were unexpected) From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/serial/8250.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230123151302.368277-11-krzysztof.kozlowski@linaro.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 24/01/2023 00:05, Rob Herring wrote: > > On Mon, 23 Jan 2023 16:13:00 +0100, Krzysztof Kozlowski wrote: >> The re-usable serial.yaml schema matches every property with ".*" >> pattern, thus any other schema referencing it will not report unknown >> (unevaluated) properties. This hides several wrong properties. It is >> a limitation of dtschema, thus provide a simple workaround: expect >> children to be only of few names matching upstream usage (Bluetooth, >> GNSS, GPS and MCU). >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- >> Documentation/devicetree/bindings/serial/serial.yaml | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/intel,ixp4xx-expansion-bus-controller.example.dtb: serial@1,0: Unevaluated properties are not allowed ('intel,ixp4xx-eb-byte-access', 'intel,ixp4xx-eb-cycle-type', 'intel,ixp4xx-eb-t3', 'intel,ixp4xx-eb-write-enable' were unexpected) > From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/serial/8250.yaml Did I just forgot to test all the bindings (except the changed ones)? It seems so, therefoer note to myself: Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). :) Best regards, Krzysztof
On 24/01/2023 00:05, Rob Herring wrote: > > On Mon, 23 Jan 2023 16:13:00 +0100, Krzysztof Kozlowski wrote: >> The re-usable serial.yaml schema matches every property with ".*" >> pattern, thus any other schema referencing it will not report unknown >> (unevaluated) properties. This hides several wrong properties. It is >> a limitation of dtschema, thus provide a simple workaround: expect >> children to be only of few names matching upstream usage (Bluetooth, >> GNSS, GPS and MCU). >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- >> Documentation/devicetree/bindings/serial/serial.yaml | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/intel,ixp4xx-expansion-bus-controller.example.dtb: serial@1,0: Unevaluated properties are not allowed ('intel,ixp4xx-eb-byte-access', 'intel,ixp4xx-eb-cycle-type', 'intel,ixp4xx-eb-t3', 'intel,ixp4xx-eb-write-enable' were unexpected) > From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/serial/8250.yaml Eh, this is not trivial to solve. The "intel,ixp4xx-expansion-bus-controller.yaml" bindings add properties to children nodes, just like spi-peripheral-props: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/bus/intel,ixp4xx-expansion-bus-controller.yaml?h=v6.2-rc5#n147 Any node can be there: 1. serial, which was broken since beginning but errors not visible, 2. flash, which work fine just because mtd-physmap.yaml allows it, 3. more? Existing DTS of this ixp4xx controller have different nodes, for example: flash, shared-dma-pool, ns8250, arcom,vulcan-gpio, maxim,max6369: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/intel-ixp42x-arcom-vulcan.dts?h=v6.2-rc5#n40 Probably we need expansion-bus-controller-peripheral-props.yaml and include it in every possible child. Does it sound reasonable? Best regards, Krzysztof
On Tue, Jan 24, 2023 at 3:05 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 24/01/2023 00:05, Rob Herring wrote: > > > > On Mon, 23 Jan 2023 16:13:00 +0100, Krzysztof Kozlowski wrote: > >> The re-usable serial.yaml schema matches every property with ".*" > >> pattern, thus any other schema referencing it will not report unknown > >> (unevaluated) properties. This hides several wrong properties. It is > >> a limitation of dtschema, thus provide a simple workaround: expect > >> children to be only of few names matching upstream usage (Bluetooth, > >> GNSS, GPS and MCU). > >> > >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> --- > >> Documentation/devicetree/bindings/serial/serial.yaml | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > yamllint warnings/errors: > > > > dtschema/dtc warnings/errors: > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/bus/intel,ixp4xx-expansion-bus-controller.example.dtb: serial@1,0: Unevaluated properties are not allowed ('intel,ixp4xx-eb-byte-access', 'intel,ixp4xx-eb-cycle-type', 'intel,ixp4xx-eb-t3', 'intel,ixp4xx-eb-write-enable' were unexpected) > > From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/serial/8250.yaml > > Eh, this is not trivial to solve. The > "intel,ixp4xx-expansion-bus-controller.yaml" bindings add properties to > children nodes, just like spi-peripheral-props: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/bus/intel,ixp4xx-expansion-bus-controller.yaml?h=v6.2-rc5#n147 Yep, those properties probably need to go into mc-peripheral-props.yaml. > Any node can be there: > 1. serial, which was broken since beginning but errors not visible, 8250 only in all likelyhood. > 2. flash, which work fine just because mtd-physmap.yaml allows it, > 3. more? I guess it is *only* external parallel bus devices... > > Existing DTS of this ixp4xx controller have different nodes, for example: > flash, shared-dma-pool, ns8250, arcom,vulcan-gpio, maxim,max6369: 'shared-dma-pool' is odd. So is max6369. Looks like that h/w was creatively attached. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/intel-ixp42x-arcom-vulcan.dts?h=v6.2-rc5#n40 > > Probably we need expansion-bus-controller-peripheral-props.yaml and > include it in every possible child. Does it sound reasonable? What's the difference to mc-peripheral-props.yaml? Honestly, this stuff is somewhat rare that I'd rather worry about it later. Maybe ixp4xx will be removed by then... We could just comment out the problematic parts of the example for now. Rob
diff --git a/Documentation/devicetree/bindings/serial/serial.yaml b/Documentation/devicetree/bindings/serial/serial.yaml index 11e822bf09e2..21992e9a3c2b 100644 --- a/Documentation/devicetree/bindings/serial/serial.yaml +++ b/Documentation/devicetree/bindings/serial/serial.yaml @@ -96,7 +96,7 @@ then: rts-gpios: false patternProperties: - ".*": + "^bluetooth|gnss|gps|mcu$": if: type: object then:
The re-usable serial.yaml schema matches every property with ".*" pattern, thus any other schema referencing it will not report unknown (unevaluated) properties. This hides several wrong properties. It is a limitation of dtschema, thus provide a simple workaround: expect children to be only of few names matching upstream usage (Bluetooth, GNSS, GPS and MCU). Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Documentation/devicetree/bindings/serial/serial.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)