Re: [PATCH v4 2/3] dt-bindings: add binding for tft displays based on ilitek,ili9486

[Sam Ravnborg <sam@xxxxxxxxxxxx>]

Hi Kamlesh

Thanks for the conversion to DT Schema format.

On Mon, Jan 27, 2020 at 07:56:14PM +0530, Kamlesh Gurudasani wrote:
> This binding is for the tft displays based on ilitek,ili9486.
> ozzmaker,piscreen and waveshare,rpi-lcd-35 are such displays
> 
> Signed-off-by: Kamlesh Gurudasani <kamlesh.gurudasani@xxxxxxxxx>
> ---
> 
> v2 changes:
> * Changing file from txt to yaml format
> * removed ilitek,ili9486 from compatible string
> 
> v3 changes:
> * no changes
> 
> v4 changes:
> * no changes
> ---
>  .../bindings/display/ilitek,ili9486.yaml           | 79 ++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/ilitek,ili9486.yaml b/Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
> new file mode 100644
> index 0000000..dd306c88
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/ilitek,ili9486.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ilitek ILI9486 display panels device tree bindings
> +
> +maintainers:
> +  - Kamlesh Gurudasani <kamlesh.gurudasani@xxxxxxxxx>
> +
> +description:
> +  This binding is for display panels using an Ilitek ILI9486 controller in SPI
> +  mode.
> +
> +allOf:
> +  - $ref: panel/panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description:
> +          Waveshare 3.5" 320x480 Color TFT LCD
> +        items:
> +          - enum:
> +              - waveshare,rpi-lcd-35
> +          - const: ilitek,ili9486
> +      - description:
> +          Ozzmaker 3.5" 320x480 Color TFT LCD
> +        items:
> +          - enum:
> +              - ozzmaker,piscreen
> +          - const: ilitek,ili9486

This parts looks a bit too complicated.
I think the following would do the trick:

    compatible:
      items:
        - enum:
            # Waveshare 3.5" 320x480 Color TFT LCD
          - waveshare,rpi-lcd-35
            # Ozzmaker 3.5" 320x480 Color TFT LCD
          - ozzmaker,piscreen
        - const: ilitek,ili9486

This makes it more obvious that you try to say that the binding should
include one of the enum values and ilitek,ili9486

With this your example would also rightfully fail as it is missing
the ilitek,ili9486 compatible.


	Sam

> +
> +  spi-max-frequency:
> +    maximum: 32000000
> +
> +  dc-gpios:
> +    maxItems: 1
> +    description: Display data/command selection (D/CX)
> +
> +  backlight: true
> +  reg: true
> +  reset-gpios: true
> +  rotation: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - dc-gpios
> +  - reset-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    backlight: backlight {
> +            compatible = "gpio-backlight";
> +            gpios = <&gpio 22 GPIO_ACTIVE_HIGH>;
> +    };
> +    spi {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +
> +            display@0{
> +                    compatible = "waveshare,rpi-lcd-35";
> +                    reg = <0>;
> +                    spi-max-frequency = <32000000>;
> +                    dc-gpios = <&gpio0 24 GPIO_ACTIVE_HIGH>;
> +                    reset-gpios = <&gpio0 25 GPIO_ACTIVE_HIGH>;
> +                    rotation = <180>;
> +                    backlight = <&backlight>;
> +            };
> +    };
> +
> +...
> -- 
> 2.7.4