Re: [PATCH v1] dt-bindings: display: Convert fsl,imx-fb.txt to dt-schema

[Rob Herring <robh@kernel.org>]

On Thu, Nov 10, 2022 at 10:49:45AM +0100, Uwe Kleine-König wrote:
> This is a straight forward conversion. Note that fsl,imx-lcdc was picked
> as the new name as this is the compatible that should supersede the
> legacy fb binding.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> the eventual goal is to add drm support for this hardware. That one will
> use a different (and more sensible) binding. However fsl,imx*-fb won't
> go away directly, and Rob requested to describe both bindings in the
> same file given that it describes a single hardware type.
> 
> As a first step I convert the old binding to yaml. I tried to put the
> new binding on top of that but I'm not sure about a few things in this
> patch and so post only this first patch and once it's accepted add the
> new binding which I guess is less overall work.
> 
> What I'm unsure about is the description of the display node (Is there a
> better description? I didn't find a schema for that.)

That's going to be a challenge to describe because every panel binding 
will need a reference to those custom properties. It's a similar problem 
that spi-peripheral-props.yaml solved. But here, there may not be enough 
instances to do such a general solution. Do the panels used even have 
schemas yet?

It's kind of a separate problem. You could start with just creating a 
schema just listing the custom properties.


> Further I didn't find documentation about additionalProperties and
> unevaluatedProperties. Did I pick the right one here?

example-schema.yaml talks about it some. In general, if there's a 
$ref to other properties for a node not defined locally, then you need 
unevaluatedProperties. Otherwise, additionalProperties is fine.


> Best regards
> Uwe
> 
>  .../bindings/display/imx/fsl,imx-fb.txt       |  57 ---------
>  .../bindings/display/imx/fsl,imx-lcdc.yaml    | 110 ++++++++++++++++++
>  2 files changed, 110 insertions(+), 57 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt
>  create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml

[...]

> diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
> new file mode 100644
> index 000000000000..c3cf6f92a766
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/imx/fsl,imx-lcdc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX LCD Controller, found on i.MX1, i.MX21, i.MX25 and i.MX27
> +
> +maintainers:
> +  - Sascha Hauer <s.hauer@pengutronix.de>
> +  - Pengutronix Kernel Team <kernel@pengutronix.de>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - fsl,imx1-fb
> +              - fsl,imx21-fb
> +      - items:
> +          - enum:
> +              - fsl,imx25-fb
> +              - fsl,imx27-fb
> +          - const: fsl,imx21-fb
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: ahb
> +      - const: per
> +
> +  display:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      Display hardware node
> +      It needs to define the following properties:
> +        - bits-per-pixel
> +        - fsl,pcr: LCDC PCR value
> +      And optionally:
> +        - fsl,aus-mode: boolean to enable AUS mode (only for imx21)
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +  lcd-supply:
> +    description:
> +      Regulator for LCD supply voltage.
> +
> +  fsl,dmacr:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'

Drop quotes.

> +    description:
> +      Override value for DMA Control Register
> +
> +  fsl,lpccr:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'

Drop quotes.

> +    description:
> +      Contrast Control Register value.
> +
> +  fsl,lscr1:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'

Drop quotes.

> +    description:
> +      LCDC Sharp Configuration Register value.
> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - display
> +  - interrupts
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    imxfb: fb@10021000 {

lcd-controller@...

> +        compatible = "fsl,imx21-fb";
> +        interrupts = <61>;
> +        reg = <0x10021000 0x1000>;
> +        display = <&display0>;
> +        clocks = <&clks 103>, <&clks 49>, <&clks 66>;
> +        clock-names = "ipg", "ahb", "per";
> +    };
> +
> +    display0: display0 {
> +        model = "Primeview-PD050VL1";
> +        bits-per-pixel = <16>;
> +        fsl,pcr = <0xf0c88080>; /* non-standard but required */
> +
> +        display-timings {
> +            native-mode = <&timing_disp0>;
> +            timing_disp0: timing0 {
> +                hactive = <640>;
> +                vactive = <480>;
> +                hback-porch = <112>;
> +                hfront-porch = <36>;
> +                hsync-len = <32>;
> +                vback-porch = <33>;
> +                vfront-porch = <33>;
> +                vsync-len = <2>;
> +                clock-frequency = <25000000>;
> +            };
> +        };
> +    };
> -- 
> 2.38.1
> 
>