Skip to contentSkip to conversation

dt-bindings: media: nxp, imx7-mipi-csi2: Drop bad if/then schema

PATCH Superseded Rob Herring 2 participants 6 messages Dec 2, 2021

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Rob Herring

author maintainer
#0 lore raw

PATCH 1/1 dt-bindings: media: nxp, imx7-mipi-csi2: Drop bad if/then schema

The if/then schema for 'data-lanes' doesn't work as 'compatible' is at a different level than 'data-lanes'. To make it work, the if/then schema would have to be moved to the top level and then whole hierarchy of nodes down to 'data-lanes' created. I don't think it is worth the complexity to do that, so let's just drop it.

The error in this schema is masked by a fixup in the tools causing the 'allOf' to get overwritten. Removing the fixup as part of moving to json-schema draft 2019-09 revealed the issue:

Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.example.dt.yaml: mipi-csi@30750000: ports:port@0:endpoint:data-lanes:0: [1] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.example.dt.yaml: mipi-csi@32e30000: ports:port@0:endpoint:data-lanes:0: [1, 2, 3, 4] is too long
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml

The if condition was always true because 'compatible' did not exist in 'endpoint' node and a non-existent property is true for json-schema.

Fixes 85b62ff2cb97 ("media: dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support")
Cc Rui Miguel Silva <[email protected]>
Cc Laurent Pinchart <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Pengutronix Kernel Team <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc NXP Linux Team <[email protected]>
---
 .../bindings/media/nxp,imx7-mipi-csi2.yaml           | 12 ------------
 1 file changed, 12 deletions(-)
Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml +0 -12
index 877183cf4278..cb8d2afe52f7 100644
@@ -91,18 +91,6 @@ properties:
             required:
               - data-lanes
 
-            allOf:
-              - if:
-                  properties:
-                    compatible:
-                      contains:
-                        const: fsl,imx7-mipi-csi2
-                then:
-                  properties:
-                    data-lanes:
-                      items:
-                        maxItems: 2
-
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description:
Signature · 1 line
2.32.0
author maintainer
#1 lore raw
Quoted 30 lines · Rob Herring

The if/then schema for 'data-lanes' doesn't work as 'compatible' is at a different level than 'data-lanes'. To make it work, the if/then schema would have to be moved to the top level and then whole hierarchy of nodes down to 'data-lanes' created. I don't think it is worth the complexity to do that, so let's just drop it.

The error in this schema is masked by a fixup in the tools causing the 'allOf' to get overwritten. Removing the fixup as part of moving to json-schema draft 2019-09 revealed the issue:

Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.example.dt.yaml: mipi-csi@30750000: ports:port@0:endpoint:data-lanes:0: [1] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.example.dt.yaml: mipi-csi@32e30000: ports:port@0:endpoint:data-lanes:0: [1, 2, 3, 4] is too long
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml

The if condition was always true because 'compatible' did not exist in 'endpoint' node and a non-existent property is true for json-schema.

Fixes 85b62ff2cb97 ("media: dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support")
Cc Rui Miguel Silva <[email protected]>
Cc Laurent Pinchart <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Pengutronix Kernel Team <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc NXP Linux Team <[email protected]>
Quoted 4 lines
---
 .../bindings/media/nxp,imx7-mipi-csi2.yaml           | 12 ------------
 1 file changed, 12 deletions(-)

Running 'make dtbs_check' with the schema in this patch gives the following warnings. Consider if they are expected or the schema is incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check. This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1562521

mipi-csi@30750000: '#address-cells', '#size-cells', 'fsl,csis-hs-settle', 'port@0', 'port@1', 'reset-names' do not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm/boot/dts/imx7s-warp.dt.yaml

mipi-csi@30750000: '#address-cells', '#size-cells', 'port@0', 'port@1', 'reset-names' do not match any of the regexes: 'pinctrl-[0-9]+'

	arch/arm/boot/dts/imx7d-cl-som-imx7.dt.yaml
	arch/arm/boot/dts/imx7d-colibri-aster.dt.yaml
	arch/arm/boot/dts/imx7d-colibri-emmc-aster.dt.yaml
	arch/arm/boot/dts/imx7d-colibri-emmc-eval-v3.dt.yaml
	arch/arm/boot/dts/imx7d-colibri-eval-v3.dt.yaml
	arch/arm/boot/dts/imx7d-flex-concentrator.dt.yaml
	arch/arm/boot/dts/imx7d-flex-concentrator-mfg.dt.yaml
	arch/arm/boot/dts/imx7d-mba7.dt.yaml
	arch/arm/boot/dts/imx7d-meerkat96.dt.yaml
	arch/arm/boot/dts/imx7d-nitrogen7.dt.yaml
	arch/arm/boot/dts/imx7d-pico-dwarf.dt.yaml
	arch/arm/boot/dts/imx7d-pico-hobbit.dt.yaml
	arch/arm/boot/dts/imx7d-pico-nymph.dt.yaml
	arch/arm/boot/dts/imx7d-pico-pi.dt.yaml
	arch/arm/boot/dts/imx7d-remarkable2.dt.yaml
	arch/arm/boot/dts/imx7d-sbc-imx7.dt.yaml
	arch/arm/boot/dts/imx7d-sdb.dt.yaml
	arch/arm/boot/dts/imx7d-sdb-reva.dt.yaml
	arch/arm/boot/dts/imx7d-sdb-sht11.dt.yaml
	arch/arm/boot/dts/imx7d-zii-rmu2.dt.yaml
	arch/arm/boot/dts/imx7d-zii-rpu2.dt.yaml
	arch/arm/boot/dts/imx7s-colibri-aster.dt.yaml
	arch/arm/boot/dts/imx7s-colibri-eval-v3.dt.yaml
	arch/arm/boot/dts/imx7s-mba7.dt.yaml

mipi-csi@30750000: 'ports' is a required property
arch/arm/boot/dts/imx7s-warp.dt.yaml

#2 lore raw

Hi Rob,

Quoted 36 lines · Rob Herring
Quoted 30 lines · Rob Herring

The if/then schema for 'data-lanes' doesn't work as 'compatible' is at a different level than 'data-lanes'. To make it work, the if/then schema would have to be moved to the top level and then whole hierarchy of nodes down to 'data-lanes' created. I don't think it is worth the complexity to do that, so let's just drop it.

The error in this schema is masked by a fixup in the tools causing the 'allOf' to get overwritten. Removing the fixup as part of moving to json-schema draft 2019-09 revealed the issue:

Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.example.dt.yaml: mipi-csi@30750000: ports:port@0:endpoint:data-lanes:0: [1] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.example.dt.yaml: mipi-csi@32e30000: ports:port@0:endpoint:data-lanes:0: [1, 2, 3, 4] is too long
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml

The if condition was always true because 'compatible' did not exist in 'endpoint' node and a non-existent property is true for json-schema.

Fixes 85b62ff2cb97 ("media: dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support")
Cc Rui Miguel Silva <[email protected]>
Cc Laurent Pinchart <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Pengutronix Kernel Team <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc NXP Linux Team <[email protected]>
Quoted 4 lines
---
 .../bindings/media/nxp,imx7-mipi-csi2.yaml           | 12 ------------
 1 file changed, 12 deletions(-)
Quoted 3 lines

Running 'make dtbs_check' with the schema in this patch gives the following warnings. Consider if they are expected or the schema is incorrect. These may not be new warnings.

Fixes available in
https://lore.kernel.org/all/[email protected]
:-)

Quoted 38 lines

Note that it is not yet a requirement to have 0 warnings for dtbs_check. This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1562521

mipi-csi@30750000: '#address-cells', '#size-cells', 'fsl,csis-hs-settle', 'port@0', 'port@1', 'reset-names' do not match any of the regexes: 'pinctrl-[0-9]+'
arch/arm/boot/dts/imx7s-warp.dt.yaml

mipi-csi@30750000: '#address-cells', '#size-cells', 'port@0', 'port@1', 'reset-names' do not match any of the regexes: 'pinctrl-[0-9]+'

	arch/arm/boot/dts/imx7d-cl-som-imx7.dt.yaml
	arch/arm/boot/dts/imx7d-colibri-aster.dt.yaml
	arch/arm/boot/dts/imx7d-colibri-emmc-aster.dt.yaml
	arch/arm/boot/dts/imx7d-colibri-emmc-eval-v3.dt.yaml
	arch/arm/boot/dts/imx7d-colibri-eval-v3.dt.yaml
	arch/arm/boot/dts/imx7d-flex-concentrator.dt.yaml
	arch/arm/boot/dts/imx7d-flex-concentrator-mfg.dt.yaml
	arch/arm/boot/dts/imx7d-mba7.dt.yaml
	arch/arm/boot/dts/imx7d-meerkat96.dt.yaml
	arch/arm/boot/dts/imx7d-nitrogen7.dt.yaml
	arch/arm/boot/dts/imx7d-pico-dwarf.dt.yaml
	arch/arm/boot/dts/imx7d-pico-hobbit.dt.yaml
	arch/arm/boot/dts/imx7d-pico-nymph.dt.yaml
	arch/arm/boot/dts/imx7d-pico-pi.dt.yaml
	arch/arm/boot/dts/imx7d-remarkable2.dt.yaml
	arch/arm/boot/dts/imx7d-sbc-imx7.dt.yaml
	arch/arm/boot/dts/imx7d-sdb.dt.yaml
	arch/arm/boot/dts/imx7d-sdb-reva.dt.yaml
	arch/arm/boot/dts/imx7d-sdb-sht11.dt.yaml
	arch/arm/boot/dts/imx7d-zii-rmu2.dt.yaml
	arch/arm/boot/dts/imx7d-zii-rpu2.dt.yaml
	arch/arm/boot/dts/imx7s-colibri-aster.dt.yaml
	arch/arm/boot/dts/imx7s-colibri-eval-v3.dt.yaml
	arch/arm/boot/dts/imx7s-mba7.dt.yaml

mipi-csi@30750000: 'ports' is a required property
arch/arm/boot/dts/imx7s-warp.dt.yaml

Signature · 3 lines
Regards,

Laurent Pinchart
#3 lore raw
In reply to Rob Herring

Hi Rob,

Thank you for the patch.

Quoted 5 lines · Rob Herring

The if/then schema for 'data-lanes' doesn't work as 'compatible' is at a different level than 'data-lanes'. To make it work, the if/then schema would have to be moved to the top level and then whole hierarchy of nodes down to 'data-lanes' created. I don't think it is worth the complexity to do that, so let's just drop it.

Fair enough, but then shouldn't there be a comment in the data-lanes property to tell that i.MX7 only supports up to 2 data lanes ?

Quoted 51 lines

The error in this schema is masked by a fixup in the tools causing the 'allOf' to get overwritten. Removing the fixup as part of moving to json-schema draft 2019-09 revealed the issue:

Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.example.dt.yaml: mipi-csi@30750000: ports:port@0:endpoint:data-lanes:0: [1] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml
Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.example.dt.yaml: mipi-csi@32e30000: ports:port@0:endpoint:data-lanes:0: [1, 2, 3, 4] is too long
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml

The if condition was always true because 'compatible' did not exist in 'endpoint' node and a non-existent property is true for json-schema.

Fixes 85b62ff2cb97 ("media: dt-bindings: media: nxp,imx7-mipi-csi2: Add i.MX8MM support")
Cc Rui Miguel Silva <[email protected]>
Cc Laurent Pinchart <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Pengutronix Kernel Team <[email protected]>
Cc Fabio Estevam <[email protected]>
Cc NXP Linux Team <[email protected]>
---
 .../bindings/media/nxp,imx7-mipi-csi2.yaml           | 12 ------------
 1 file changed, 12 deletions(-)
Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml +0 -12
index 877183cf4278..cb8d2afe52f7 100644
@@ -91,18 +91,6 @@ properties:
             required:
               - data-lanes
 
-            allOf:
-              - if:
-                  properties:
-                    compatible:
-                      contains:
-                        const: fsl,imx7-mipi-csi2
-                then:
-                  properties:
-                    data-lanes:
-                      items:
-                        maxItems: 2
-
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description:
Signature · 3 lines
Regards,

Laurent Pinchart
author maintainer
#4 lore raw
Quoted 6 lines · Laurent Pinchart

Hi Rob,

Thank you for the patch.

On Wed, Dec 01, 2021 at 08:30:21PM -0600, Rob Herring wrote:

Quoted 8 lines
Quoted 5 lines

The if/then schema for 'data-lanes' doesn't work as 'compatible' is at a different level than 'data-lanes'. To make it work, the if/then schema would have to be moved to the top level and then whole hierarchy of nodes down to 'data-lanes' created. I don't think it is worth the complexity to do that, so let's just drop it.

Fair enough, but then shouldn't there be a comment in the data-lanes property to tell that i.MX7 only supports up to 2 data lanes ?

I'll add this to data-lanes:

description: Note that 'fsl,imx7-mipi-csi2' only supports up to 2 data lines.

I assume it is 1 or 2 and not only 2, right?

Rob

#5 lore raw

Hi Rob,

Quoted 8 lines · Rob Herring
Quoted 7 lines · Laurent Pinchart
Quoted 5 lines · Rob Herring

The if/then schema for 'data-lanes' doesn't work as 'compatible' is at a different level than 'data-lanes'. To make it work, the if/then schema would have to be moved to the top level and then whole hierarchy of nodes down to 'data-lanes' created. I don't think it is worth the complexity to do that, so let's just drop it.

Quoted 6 lines
Quoted 2 lines

Fair enough, but then shouldn't there be a comment in the data-lanes property to tell that i.MX7 only supports up to 2 data lanes ?

I'll add this to data-lanes:

description: Note that 'fsl,imx7-mipi-csi2' only supports up to 2 data lines.

Looks good to me.

Quoted 1 line

I assume it is 1 or 2 and not only 2, right?

Correct.

Signature · 3 lines
Regards,

Laurent Pinchart