Re: [PATCHv3 1/2] dt-bindings: display: add visionox rm69299 panel variant

[Sam Ravnborg <sam@ravnborg.org>]

Hi Harigovindan

On Wed, Feb 05, 2020 at 06:36:29PM +0530, Harigovindan P wrote:
> Add bindings for visionox rm69299 panel.
> 
> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> ---
> 
> Changes in v1:
> 	- Added a compatible string to support sc7180 panel version.
> Changes in v2:
> 	- Removed unwanted properties from description.
> 	- Creating source files without execute permissions(Rob Herring).
> Changes in v3:
> 	- Changing txt file into yaml
Thanks for changing to DT Schema.


> 
>  .../bindings/display/visionox,rm69299.yaml         | 109 +++++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/visionox,rm69299.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/visionox,rm69299.yaml b/Documentation/devicetree/bindings/display/visionox,rm69299.yaml
> new file mode 100644
> index 0000000..bfcd46d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/visionox,rm69299.yaml

This is a binding for a panel - and thus belongs in the panel/
directory.

> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0-only
The preferred license for new bindings are (GPL-2.0-only or BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/visionox,rm69299.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Visionox model RM69299 Panels Device Tree Bindings
> +
> +maintainers:
> +  - Harigovindan P <harigovi@codeaurora.org>
> +  - Kalyan Thota <kalyan_t@codeaurora.org>
> +  - Vishnuvardhan Prodduturi <vproddut@codeaurora.org>
> +
> +description:
> +  This binding is for display panels using a Visionox RM692999 panel.
> +

Use:
allOf:
  - $ref: panel-common.yaml#

Then you have access to all properties described there.


> +patternProperties:
> +  "^(panel|panel-dsi)@[0-9]$":
> +    type: object
> +    description:
> +      A node containing the panel or bridge description as documented in
> +      Documentation/devicetree/bindings/display/mipi-dsi-bus.txt

The panel shall not document the parent node.
See how other panels do it.
In general - start with the compatible.

This will also simplify your list of required properties.

> +    properties:
> +      compatible:
> +        const: visionox,rm69299-1080p-display
> +
> +      reg:
> +        maxItems: 1
> +
> +      vdda-supply:
> +        description:
> +          Phandle of the regulator that provides the vdda supply voltage.
> +
> +      vdd3p3-supply:
> +        description:
> +          Phandle of the regulator that provides the vdd3p3 supply voltage.
> +
> +      pinctrl-names:
> +        items:
> +          - const: default
> +          - const: suspend
> +
> +      pinctrl-0:
> +        items:
> +          - const: Display default pin
> +          - const: Display default pin

pinctrl are not needed for the panel. They belong to the display driver.

> +
> +      ports:
> +        type: object
> +        description:
> +          A node containing DSI input & output port nodes with endpoint
> +          definitions as documented in
> +          Documentation/devicetree/bindings/media/video-interfaces.txt
> +          Documentation/devicetree/bindings/graph.txt
> +        properties:
> +          port@0:
> +            type: object
> +            description:
> +              DSI input port node.
> +
A
       port: true
should do the trick here.




> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
Not required.
> +
> +required:
> +  - "#address-cells"
> +  - "#size-cells"
> +  - compatible
> +  - reg
> +  - vdda-supply
> +  - vdd3p3-supply
> +  - pinctrl-names
> +  - pinctrl-0
> +  - pinctrl-1
> +  - reset-gpios

reset-gpios are not documented in the above.

Add
      reset-gpios: true


Please give it a spin more.
And preferably give it a "make dt_binding_check" whirl.
If you cannot get this part working I can do so for you in next
reviision.

	Sam