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

Hi Harigovindan

Thanks for the follow-up. There are still a few things to improve. See below.

Sam

On Fri, Mar 06, 2020 at 04:06:27PM +0530, Harigovindan P wrote:

Add bindings for visionox rm69299 panel.

Signed-off-by: Harigovindan P harigovi@codeaurora.org

Changes in v2:

• Removed unwanted properties from description.
• Creating source files without execute permissions(Rob Herring).

Changes in v3:

• Changing txt file into yaml

Changes in v4:

• Updating license identifier.
• Moving yaml file inside panel directory.
• Removing pinctrl entries.
• Adding documentation for reset-gpios.

.../display/panel/visionox,rm69299.yaml | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/visionox,rm69299.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/visionox,rm69299.yaml b/Documentation/devicetree/bindings/display/panel/visionox,rm69299.yaml new file mode 100644 index 000000000000..93cae431207c --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/visionox,rm69299.yaml @@ -0,0 +1,85 @@ +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/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

+description: |

• This binding is for display panels using a Visionox RM692999 panel.

+allOf:

• • $ref: panel-common.yaml#

+patternProperties:

• "^(panel|panel-dsi)@[0-9]$":
• type: object

This looks wrong. None of the other panels needs to specify a patternProperties, and neither do we need to do so for this panel.

• properties:

•  compatible:
  

•    const: visionox,rm69299-1080p-display
  

The compatible is what we use to match this binding.

•  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.
  

•  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
  

I think you miss as " |" after "description:" Again, see other panel bindings.

•    properties:
  

•      port@0:
  

Please just use port: true

port is already documented in panel-common.yaml.

•        type: object
  

•        description:
  

•          DSI input port node.
  

•  reset-gpios:
  

reset-gpios is part of panel-common. So specify only: reset-gpios: true

No description, as it is in panel-common.

•    description:
  

•      a GPIO spec for the reset pin.
  

+required:

• • compatible
• • reg
• • vdda-supply
• • vdd3p3-supply
• • reset-gpios

+additionalProperties: false

+examples: +- |

• dsi@ae94000 {

No @ae94000 - this will warn with upcoming dt_binding_check improvements.

•    panel@0 {
  

•        compatible = "visionox,rm69299-1080p-display";
  

•        vdda-supply = <&src_pp1800_l8c>;
  

•        vdd3p3-supply = <&src_pp2800_l18a>;
  

•        reset-gpios = <&pm6150l_gpio 3 0>;
  

reg property is required, but missing in the example.

•        ports {
  

•                #address-cells = <1>;
  

•                #size-cells = <0>;
  

•                port@0 {
  

•                        reg = <0>;
  

•                        panel0_in: endpoint {
  

•                            remote-endpoint = <&dsi0_out>;
  

•                        };
  

•                };
  

•        };
  

•    };
  

• };

+...

-- 2.25.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel