Re: [PATCH 1/2] dt-bindings: Add SONY Synaptics JDI panel

[Rob Herring]

On Tue, May 25, 2021 at 01:31:02PM +0200, Konrad Dybcio wrote:
> Add bindings for the SONY Synaptics JDI panel used in
> Xperia X, X Performance, X Compact, XZ and XZs smartphones.
> 
> Due to the nature of phone manufacturing and lack of any docs
> whatsoever, replacement names have been used to indicate the
> devices that this panel is used on.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx>
> ---
>  .../display/panel/sony,synaptics-jdi.yaml     | 100 ++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/sony,synaptics-jdi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/sony,synaptics-jdi.yaml b/Documentation/devicetree/bindings/display/panel/sony,synaptics-jdi.yaml
> new file mode 100644
> index 000000000000..81d841c049e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/sony,synaptics-jdi.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/sony,synaptics-jdi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SONY Synaptics JDI panel
> +
> +maintainers:
> +  - Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx>
> +
> +description: |+

Do you need the formatting? If not, drop '|+'.

> +  This panel seems to only be found in SONY Xperia
> +  X, X Performance, X Compact, XZ and XZs
> +  smartphones and we have no straightforward way of
> +  actually getting the correct model number,
> +  as no schematics are released publicly.

Odd choice of line break length. 80 char please.

> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - sony,synaptics-jdi-dora
> +          - sony,synaptics-jdi-kagura
> +          - sony,synaptics-jdi-keyaki
> +          - sony,synaptics-jdi-kugo
> +          - sony,synaptics-jdi-suzu
> +
> +  reg: true
> +
> +  reset-gpios: true
> +
> +  avdd-supply:
> +    description: avdd supply
> +
> +  vddio-supply:
> +    description: vddio supply
> +
> +  vsn-supply:
> +    description: voltage negative supply
> +
> +  vsp-supply:
> +    description: voltage positive supply
> +
> +  tvdd-supply:
> +    description: tvdd supply
> +
> +  preset-gpio:
> +    description: panel reset pin

What's reset-gpios then?

> +
> +  pvddio-gpio:
> +    description: panel vddio pin
> +
> +  treset-gpio:
> +    description: touch reset pin

Use '-gpios'

And need to define how many (maxItems: 1).

> +
> +required:
> +  - compatible
> +  - reg
> +  - preset-gpio
> +  - pvddio-gpio
> +  - treset-gpio
> +  - avdd-supply
> +  - vddio-supply
> +  - vsn-supply
> +  - vsp-supply
> +  - tvdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    dsi {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            panel: panel@0 {
> +                    reg = <0>;
> +
> +                    pvddio-gpio = <&tlmm 51 GPIO_ACTIVE_HIGH>;
> +                    preset-gpio = <&tlmm 8 GPIO_ACTIVE_HIGH>;
> +                    treset-gpio = <&tlmm 89 GPIO_ACTIVE_HIGH>;
> +
> +                    vddio-supply = <&pm8994_s4>;
> +                    avdd-supply = <&pm8994_l2>;
> +                    tvdd-supply = <&panel_tvdd>;
> +
> +                    backlight = <&pmi8994_wled>;
> +
> +                    port {
> +                      panel_in: endpoint {
> +                        remote-endpoint = <&dsi0_out>;
> +                      };

Consistent indentation please. 4 spaces is good.

> +                    };
> +            };
> +    };
> -- 
> 2.31.1