| Message ID | 20230112-imx-pxp-v2-1-e2281da1db55@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx-pxp: add support for i.MX7D | expand |
Hi Michael, Thank you for the patch. On Fri, Jan 13, 2023 at 10:54:07AM +0100, Michael Tretter wrote: > Convert the bindings of the Freescale Pixel Pipeline to YAML. > > The conversion drops the previously listed compatibles for several SoCs. > It is unclear, if the PXP on these SoCs is compatible to any of the PXPs > on the existing SoCs and would allow to reuse the already defined > compatibles. The missing compatibles should be brought back when the > support for the PXP on these SoCs is added. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > Changelog: > > v2: > > - add fsl,imx6sll-pxp and fsl,imx6sx-pxp compatibles > - restrict number of interrupts per variant > - cleanup syntax > --- > .../devicetree/bindings/media/fsl,imx6ull-pxp.yaml | 82 ++++++++++++++++++++++ > .../devicetree/bindings/media/fsl-pxp.txt | 26 ------- > 2 files changed, 82 insertions(+), 26 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml > new file mode 100644 > index 000000000000..c1232689a261 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml > @@ -0,0 +1,82 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > + > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale Pixel Pipeline > + > +maintainers: > + - Philipp Zabel <p.zabel@pengutronix.de> > + - Michael Tretter <m.tretter@pengutronix.de> > + > +description: > + The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine > + that supports scaling, colorspace conversion, alpha blending, rotation, and > + pixel conversion via lookup table. Different versions are present on various > + i.MX SoCs from i.MX23 to i.MX7. > + > +properties: > + compatible: > + oneOf: > + - const: fsl,imx6ul-pxp > + - const: fsl,imx6ull-pxp > + - const: fsl,imx7d-pxp > + - items: > + - enum: > + - fsl,imx6sll-pxp > + - fsl,imx6sx-pxp > + - const: fsl,imx6ull-pxp > + > + reg: > + maxItems: 1 > + > + interrupts: > + minItems: 1 > + maxItems: 2 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + const: axi > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - fsl,imx6sx-pxp > + then: > + properties: > + interrupts: > + numItems: 1 > + else: > + properties: > + interrupts: > + numItems: 2 > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/imx6ul-clock.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + pxp: pxp@21cc000 { > + compatible = "fsl,imx6ull-pxp"; > + reg = <0x021cc000 0x4000>; > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>; > + clock-names = "axi"; > + clocks = <&clks IMX6UL_CLK_PXP>; > + }; > diff --git a/Documentation/devicetree/bindings/media/fsl-pxp.txt b/Documentation/devicetree/bindings/media/fsl-pxp.txt > deleted file mode 100644 > index f8090e06530d..000000000000 > --- a/Documentation/devicetree/bindings/media/fsl-pxp.txt > +++ /dev/null > @@ -1,26 +0,0 @@ > -Freescale Pixel Pipeline > -======================== > - > -The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine > -that supports scaling, colorspace conversion, alpha blending, rotation, and > -pixel conversion via lookup table. Different versions are present on various > -i.MX SoCs from i.MX23 to i.MX7. > - > -Required properties: > -- compatible: should be "fsl,<soc>-pxp", where SoC can be one of imx23, imx28, > - imx6dl, imx6sl, imx6sll, imx6ul, imx6sx, imx6ull, or imx7d. > -- reg: the register base and size for the device registers > -- interrupts: the PXP interrupt, two interrupts for imx6ull and imx7d. > -- clock-names: should be "axi" > -- clocks: the PXP AXI clock > - > -Example: > - > -pxp@21cc000 { > - compatible = "fsl,imx6ull-pxp"; > - reg = <0x021cc000 0x4000>; > - interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, > - <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>; > - clock-names = "axi"; > - clocks = <&clks IMX6UL_CLK_PXP>; > -}; >
On Fr, 2023-01-13 at 10:54 +0100, Michael Tretter wrote: > Convert the bindings of the Freescale Pixel Pipeline to YAML. > > The conversion drops the previously listed compatibles for several SoCs. > It is unclear, if the PXP on these SoCs is compatible to any of the PXPs > on the existing SoCs and would allow to reuse the already defined > compatibles. The missing compatibles should be brought back when the > support for the PXP on these SoCs is added. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp
On 13/01/2023 10:54, Michael Tretter wrote: > Convert the bindings of the Freescale Pixel Pipeline to YAML. > > The conversion drops the previously listed compatibles for several SoCs. > It is unclear, if the PXP on these SoCs is compatible to any of the PXPs > on the existing SoCs and would allow to reuse the already defined > compatibles. The missing compatibles should be brought back when the > support for the PXP on these SoCs is added. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > --- > Changelog: > > v2: > > - add fsl,imx6sll-pxp and fsl,imx6sx-pxp compatibles > - restrict number of interrupts per variant > - cleanup syntax > --- > .../devicetree/bindings/media/fsl,imx6ull-pxp.yaml | 82 ++++++++++++++++++++++ > .../devicetree/bindings/media/fsl-pxp.txt | 26 ------- > 2 files changed, 82 insertions(+), 26 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml > new file mode 100644 > index 000000000000..c1232689a261 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml > @@ -0,0 +1,82 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > + > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale Pixel Pipeline > + > +maintainers: > + - Philipp Zabel <p.zabel@pengutronix.de> > + - Michael Tretter <m.tretter@pengutronix.de> > + > +description: > + The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine > + that supports scaling, colorspace conversion, alpha blending, rotation, and > + pixel conversion via lookup table. Different versions are present on various > + i.MX SoCs from i.MX23 to i.MX7. > + > +properties: > + compatible: > + oneOf: > + - const: fsl,imx6ul-pxp > + - const: fsl,imx6ull-pxp > + - const: fsl,imx7d-pxp These three are an enum. > + - items: > + - enum: > + - fsl,imx6sll-pxp > + - fsl,imx6sx-pxp > + - const: fsl,imx6ull-pxp > + > + reg: > + maxItems: 1 > + > + interrupts: > + minItems: 1 > + maxItems: 2 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + const: axi > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - fsl,imx6sx-pxp > + then: > + properties: > + interrupts: > + numItems: 1 That's not correct syntax... I am surprised that it works. Did you test the bindings? Best regards, Krzysztof
On Fri, 13 Jan 2023 12:56:12 +0100, Krzysztof Kozlowski wrote: > On 13/01/2023 10:54, Michael Tretter wrote: > > Convert the bindings of the Freescale Pixel Pipeline to YAML. > > > > The conversion drops the previously listed compatibles for several SoCs. > > It is unclear, if the PXP on these SoCs is compatible to any of the PXPs > > on the existing SoCs and would allow to reuse the already defined > > compatibles. The missing compatibles should be brought back when the > > support for the PXP on these SoCs is added. > > > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > > --- > > Changelog: > > > > v2: > > > > - add fsl,imx6sll-pxp and fsl,imx6sx-pxp compatibles > > - restrict number of interrupts per variant > > - cleanup syntax > > --- > > .../devicetree/bindings/media/fsl,imx6ull-pxp.yaml | 82 ++++++++++++++++++++++ > > .../devicetree/bindings/media/fsl-pxp.txt | 26 ------- > > 2 files changed, 82 insertions(+), 26 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml > > new file mode 100644 > > index 000000000000..c1232689a261 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml > > @@ -0,0 +1,82 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > + > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Freescale Pixel Pipeline > > + > > +maintainers: > > + - Philipp Zabel <p.zabel@pengutronix.de> > > + - Michael Tretter <m.tretter@pengutronix.de> > > + > > +description: > > + The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine > > + that supports scaling, colorspace conversion, alpha blending, rotation, and > > + pixel conversion via lookup table. Different versions are present on various > > + i.MX SoCs from i.MX23 to i.MX7. > > + > > +properties: > > + compatible: > > + oneOf: > > + - const: fsl,imx6ul-pxp > > + - const: fsl,imx6ull-pxp > > + - const: fsl,imx7d-pxp > > These three are an enum. These are alternatives to the 'items:' entry below. Are you suggesting to use the following statement? oneOf: - enum: - fsl,imx6ul-pxp - fsl,imx6ull-pxp - fsl,imx7d-pxp - items: - enum: - fsl,imx6sll-pxp - fsl,imx6sx-pxp Why is this better than the one that I used? > > > + - items: > > + - enum: > > + - fsl,imx6sll-pxp > > + - fsl,imx6sx-pxp > > + - const: fsl,imx6ull-pxp > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + minItems: 1 > > + maxItems: 2 > > + > > + clocks: > > + maxItems: 1 > > + > > + clock-names: > > + const: axi > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + - clock-names > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx6sx-pxp > > + then: > > + properties: > > + interrupts: > > + numItems: 1 > > That's not correct syntax... I am surprised that it works. Did you test > the bindings? I copied this syntax from renesas,wdt.yaml and ran make ARCH=arm dtbs_check DT_SCHEMA_FILES=fsl,imx6ull-pxp.yaml with SOC_IMX7D=y, SOC_IMX6UL=y, SOC_IMX6SLL=y, and SOC_IMX6SX=y. The latter two were not enabled in the v1, which is why it didn't catch the missing compatibles. On a closer look, I just saw that the checker ignored the schema due to the incorrect syntax and didn't produce any further errors. With the syntax fixed, the checker now produces also a few more errors about power-domains, which I will fix in v3. Is this syntax correct? allOf: - if: properties: compatible: contains: enum: - fsl,imx6sx-pxp then: properties: interrupts: minItems: 1 maxItems: 1 else: properties: interrupts: minItems: 2 maxItems: 2 Michael
On Fri, 13 Jan 2023 10:54:07 +0100, Michael Tretter wrote: > Convert the bindings of the Freescale Pixel Pipeline to YAML. > > The conversion drops the previously listed compatibles for several SoCs. > It is unclear, if the PXP on these SoCs is compatible to any of the PXPs > on the existing SoCs and would allow to reuse the already defined > compatibles. The missing compatibles should be brought back when the > support for the PXP on these SoCs is added. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > --- > Changelog: > > v2: > > - add fsl,imx6sll-pxp and fsl,imx6sx-pxp compatibles > - restrict number of interrupts per variant > - cleanup syntax > --- > .../devicetree/bindings/media/fsl,imx6ull-pxp.yaml | 82 ++++++++++++++++++++++ > .../devicetree/bindings/media/fsl-pxp.txt | 26 ------- > 2 files changed, 82 insertions(+), 26 deletions(-) > 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/media/fsl,imx6ull-pxp.yaml: allOf:0:else:properties:interrupts: 'anyOf' conditional failed, one must be fixed: 'numItems' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems'] 'type' was expected from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml: allOf:0:then:properties:interrupts: 'anyOf' conditional failed, one must be fixed: 'numItems' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems'] 'type' was expected from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230112-imx-pxp-v2-1-e2281da1db55@pengutronix.de 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 13/01/2023 16:09, Michael Tretter wrote: >>> +properties: >>> + compatible: >>> + oneOf: >>> + - const: fsl,imx6ul-pxp >>> + - const: fsl,imx6ull-pxp >>> + - const: fsl,imx7d-pxp >> >> These three are an enum. > > These are alternatives to the 'items:' entry below. > > Are you suggesting to use the following statement? > > oneOf: > - enum: > - fsl,imx6ul-pxp > - fsl,imx6ull-pxp > - fsl,imx7d-pxp > - items: > - enum: > - fsl,imx6sll-pxp > - fsl,imx6sx-pxp Yes. > > Why is this better than the one that I used? Because that's the convention - use enum for enumeration which nicely groups all of them and is the easiest to read. > >> >>> + - items: >>> + - enum: >>> + - fsl,imx6sll-pxp >>> + - fsl,imx6sx-pxp >>> + - const: fsl,imx6ull-pxp >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + minItems: 1 >>> + maxItems: 2 >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> + clock-names: >>> + const: axi >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + - clocks >>> + - clock-names >>> + >>> +allOf: >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - fsl,imx6sx-pxp >>> + then: >>> + properties: >>> + interrupts: >>> + numItems: 1 >> >> That's not correct syntax... I am surprised that it works. Did you test >> the bindings? > > I copied this syntax from renesas,wdt.yaml and ran > > make ARCH=arm dtbs_check DT_SCHEMA_FILES=fsl,imx6ull-pxp.yaml > > with SOC_IMX7D=y, SOC_IMX6UL=y, SOC_IMX6SLL=y, and SOC_IMX6SX=y. The latter > two were not enabled in the v1, which is why it didn't catch the missing > compatibles. > > On a closer look, I just saw that the checker ignored the schema due to the > incorrect syntax and didn't produce any further errors. With the syntax fixed, > the checker now produces also a few more errors about power-domains, which I > will fix in v3. > > Is this syntax correct? > > allOf: > - if: > properties: > compatible: > contains: > enum: > - fsl,imx6sx-pxp > then: > properties: > interrupts: > minItems: 1 Drop this one > maxItems: 1 > else: > properties: > interrupts: > minItems: 2 > maxItems: 2 > > Michael Rest is ok Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml new file mode 100644 index 000000000000..c1232689a261 --- /dev/null +++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml @@ -0,0 +1,82 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) + +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Freescale Pixel Pipeline + +maintainers: + - Philipp Zabel <p.zabel@pengutronix.de> + - Michael Tretter <m.tretter@pengutronix.de> + +description: + The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine + that supports scaling, colorspace conversion, alpha blending, rotation, and + pixel conversion via lookup table. Different versions are present on various + i.MX SoCs from i.MX23 to i.MX7. + +properties: + compatible: + oneOf: + - const: fsl,imx6ul-pxp + - const: fsl,imx6ull-pxp + - const: fsl,imx7d-pxp + - items: + - enum: + - fsl,imx6sll-pxp + - fsl,imx6sx-pxp + - const: fsl,imx6ull-pxp + + reg: + maxItems: 1 + + interrupts: + minItems: 1 + maxItems: 2 + + clocks: + maxItems: 1 + + clock-names: + const: axi + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + +allOf: + - if: + properties: + compatible: + contains: + enum: + - fsl,imx6sx-pxp + then: + properties: + interrupts: + numItems: 1 + else: + properties: + interrupts: + numItems: 2 + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/imx6ul-clock.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + pxp: pxp@21cc000 { + compatible = "fsl,imx6ull-pxp"; + reg = <0x021cc000 0x4000>; + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>; + clock-names = "axi"; + clocks = <&clks IMX6UL_CLK_PXP>; + }; diff --git a/Documentation/devicetree/bindings/media/fsl-pxp.txt b/Documentation/devicetree/bindings/media/fsl-pxp.txt deleted file mode 100644 index f8090e06530d..000000000000 --- a/Documentation/devicetree/bindings/media/fsl-pxp.txt +++ /dev/null @@ -1,26 +0,0 @@ -Freescale Pixel Pipeline -======================== - -The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine -that supports scaling, colorspace conversion, alpha blending, rotation, and -pixel conversion via lookup table. Different versions are present on various -i.MX SoCs from i.MX23 to i.MX7. - -Required properties: -- compatible: should be "fsl,<soc>-pxp", where SoC can be one of imx23, imx28, - imx6dl, imx6sl, imx6sll, imx6ul, imx6sx, imx6ull, or imx7d. -- reg: the register base and size for the device registers -- interrupts: the PXP interrupt, two interrupts for imx6ull and imx7d. -- clock-names: should be "axi" -- clocks: the PXP AXI clock - -Example: - -pxp@21cc000 { - compatible = "fsl,imx6ull-pxp"; - reg = <0x021cc000 0x4000>; - interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>, - <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>; - clock-names = "axi"; - clocks = <&clks IMX6UL_CLK_PXP>; -};
Convert the bindings of the Freescale Pixel Pipeline to YAML. The conversion drops the previously listed compatibles for several SoCs. It is unclear, if the PXP on these SoCs is compatible to any of the PXPs on the existing SoCs and would allow to reuse the already defined compatibles. The missing compatibles should be brought back when the support for the PXP on these SoCs is added. Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> --- Changelog: v2: - add fsl,imx6sll-pxp and fsl,imx6sx-pxp compatibles - restrict number of interrupts per variant - cleanup syntax --- .../devicetree/bindings/media/fsl,imx6ull-pxp.yaml | 82 ++++++++++++++++++++++ .../devicetree/bindings/media/fsl-pxp.txt | 26 ------- 2 files changed, 82 insertions(+), 26 deletions(-)