Re: [PATCH 1/2] drm/panel: Add DT bindings for Samsung S6D27A1 display panel

[Sam Ravnborg <sam@ravnborg.org>]

Hi Markuss,

On Thu, Aug 05, 2021 at 04:33:41PM +0300, Markuss Broks wrote:
> This adds device-tree bindings for the Samsung S6D27A1 RGB
> DPI display panel.
> 
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  .../display/panel/samsung,s6d27a1.yaml        | 97 +++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml b/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
> new file mode 100644
> index 0000000000..c6c534aa87
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/samsung,s6d27a1.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung S6D27A1 display panel
> +
> +description: The S6D27A1 is a 480x800 DPI display panel from Samsung Mobile
> +  Displays (SMD). The panel must obey the rules for a SPI slave device
> +  as specified in spi/spi-controller.yaml
> +
> +maintainers:
> +  - Markuss Broks <markuss.broks@gmail.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: samsung,s6d27a1
> +
> +  reg: true
> +
> +  interrupts:
> +    description: provides an optional ESD (electrostatic discharge)
> +      interrupt that signals abnormalities in the display hardware.
> +      This can also be raised for other reasons like erroneous
> +      configuration.
> +    maxItems: 1
> +
> +  reset-gpios: true
> +
> +  vci-supply:
> +    description: regulator that supplies the VCI analog voltage
> +      usually around 3.0 V
> +
> +  vccio-supply:
> +    description: regulator that supplies the VCCIO voltage usually
> +      around 1.8 V
> +
> +  backlight: true
> +
> +  spi-cpha: true
> +
> +  spi-cpol: true
> +
> +  spi-max-frequency:
> +    maximum: 1200000
> +
> +  port: true
> +
> +required:
> +  - compatible
> +  - reg

> +  - spi-cpha
> +  - spi-cpol
> +  - port
Do the HW really require the above three properties?

Can the HW work without a supply (hint: I think vci-supply and
vccio-supply are required)

> +
> +additionalProperties: false

As this device sits on the SPI bus you need to use:
unevaluatedProterties: false
(Check spelling from other bindings)

This allows DT files to add more spi related properties, but do not
allow DT files to add unknown properties.

With the current scheme you restrict the DT file to only the three
mentioned SPI properties that may be OK for your HW but maybe not OK for
the next HW using the same panel.

	Sam



> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +        compatible = "spi-gpio";
> +        sck-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
> +        miso-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
> +        mosi-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
> +        cs-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
> +        num-chipselects = <1>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        panel@0 {
> +            compatible = "samsung,s6d27a1";
> +            spi-max-frequency = <1200000>;
> +            spi-cpha;
> +            spi-cpol;
> +            reg = <0>;
> +            vci-supply = <&lcd_3v0_reg>;
> +            vccio-supply = <&lcd_1v8_reg>;
> +            reset-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <5 IRQ_TYPE_EDGE_RISING>;
> +
> +            port {
> +                panel_in: endpoint {
> +                    remote-endpoint = <&display_out>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.32.0