[v2,01/16] dt-bindings: media: fsl-pxp: convert to yaml

  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(-)
  

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