| Message ID | 20211202023021.3655384-1-robh@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: media: nxp,imx7-mipi-csi2: Drop bad if/then schema | expand |
On Wed, 01 Dec 2021 20:30:21 -0600, Rob Herring wrote: > 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 <rmfrfs@gmail.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: NXP Linux Team <linux-imx@nxp.com> > Cc: linux-media@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- > .../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
Hi Rob, Thank you for the patch. On Wed, Dec 01, 2021 at 08:30:21PM -0600, Rob Herring wrote: > 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 ? > 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 <rmfrfs@gmail.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: NXP Linux Team <linux-imx@nxp.com> > Cc: linux-media@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- > .../bindings/media/nxp,imx7-mipi-csi2.yaml | 12 ------------ > 1 file changed, 12 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml > index 877183cf4278..cb8d2afe52f7 100644 > --- a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml > +++ b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml > @@ -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:
Hi Rob, On Thu, Dec 02, 2021 at 07:58:09AM -0600, Rob Herring wrote: > On Wed, 01 Dec 2021 20:30:21 -0600, Rob Herring wrote: > > 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 <rmfrfs@gmail.com> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > > Cc: Shawn Guo <shawnguo@kernel.org> > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > > Cc: Fabio Estevam <festevam@gmail.com> > > Cc: NXP Linux Team <linux-imx@nxp.com> > > Cc: linux-media@vger.kernel.org > > Cc: linux-arm-kernel@lists.infradead.org > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > .../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. Fixes available in https://lore.kernel.org/all/20211202221903.21882-1-laurent.pinchart@ideasonboard.com :-) > 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 >
On Thu, Dec 2, 2021 at 3:06 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Rob, > > Thank you for the patch. > > On Wed, Dec 01, 2021 at 08:30:21PM -0600, Rob Herring wrote: > > 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
Hi Rob, On Thu, Dec 02, 2021 at 04:26:23PM -0600, Rob Herring wrote: > On Thu, Dec 2, 2021 at 3:06 PM Laurent Pinchart wrote: > > On Wed, Dec 01, 2021 at 08:30:21PM -0600, Rob Herring wrote: > > > 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. Looks good to me. > I assume it is 1 or 2 and not only 2, right? Correct.
diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml index 877183cf4278..cb8d2afe52f7 100644 --- a/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml +++ b/Documentation/devicetree/bindings/media/nxp,imx7-mipi-csi2.yaml @@ -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:
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 <rmfrfs@gmail.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Mauro Carvalho Chehab <mchehab@kernel.org> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> Cc: Fabio Estevam <festevam@gmail.com> Cc: NXP Linux Team <linux-imx@nxp.com> Cc: linux-media@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Rob Herring <robh@kernel.org> --- .../bindings/media/nxp,imx7-mipi-csi2.yaml | 12 ------------ 1 file changed, 12 deletions(-)