Re: [PATCH 2/3] dt-bindings: Add Mantix MLAF057WE51-X panel bindings

Hi Sam, On Sat, Aug 15, 2020 at 10:39:17AM +0200, Sam Ravnborg wrote:

Hi Guido.

On Fri, Aug 14, 2020 at 03:36:22PM +0200, Guido Günther wrote:

...

The panel uses a Focaltech FT8006p, the touch part is handled by the already existing edt-ft5x06.

Signed-off-by: Guido Günther agx@sigxcpu.org

A few trivialities.

Thanks for having a look. One remark inline:

...

---

.../display/panel/mantix,mlaf057we51-x.yaml | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml new file mode 100644 index 0000000000000..349f3380ac940 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/mantix,mlaf057we51-x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#

+title: Mantix MLAF057WE51-X 5.7" 720x1440 TFT LCD panel

+maintainers:

• • Guido Günther agx@sigxcpu.org

+description: |

•         Mantix MLAF057WE51 X is a 720x1440 TFT LCD panel
  

•         connected using a MIPI-DSI video interface.
  

Indent text with two spaces only. And I have learned that '|' is only needed to preserve formatting - so it can be dropped.

...

+allOf:

• • $ref: panel-common.yaml#

+properties:

• compatible:
• enum:

•  - mantix,mlaf057we51-x
  

This is a list - so needs an extra 2 spaces indent. See https://lore.kernel.org/linux-devicetree/f1963eb9-283f-e903-2a3a-4f324d71d41... for examples where Rob fixes this.

Doesn't this only apply if the 'outer element' is a list too so e.g.:

- enum - foo

trips up yamllint but

enum - foo

doesn't. Since yamllint was happy i kept it as is (looking at your reference suggests that too).

All the rest made sense and i fixed that for the upcoming v2. Thanks for having a look! -- Guido

...

• port: true
• reg:
• maxItems: 1
• description: DSI virtual channel
• avdd-supply:
• description: Positive analog power supply
• avee-supply:
• description: Negative analog power supply
• vddi-supply:
• description: 1.8V I/O voltage supply
• reset-gpios:
• description: GPIO used for the reset pin
• maxItems: 1

Use reset-gpios: true as we already have it in panel-common.yaml

...

• backlight:
• description: Backlight used by the panel
• $ref: "/schemas/types.yaml#/definitions/phandle"

Use backlight from panel-common.yaml.

...

+required:

• • compatible
• • reg
• • avdd-supply
• • avee-supply
• • vddi-supply
• • reset-gpios

+additionalProperties: false

+examples:

• • |
• #include <dt-bindings/gpio/gpio.h>
• dsi {

My personal preference is indent with 4 spaces in examples but there are no rules so feel free to ignore.

...

•  #address-cells = <1>;
  

•  #size-cells = <0>;
  

•  panel@0 {
  

•    compatible = "mantix,mlaf057we51-x";
  

•    reg = <0>;
  

•    avdd-supply = <&reg_avdd>;
  

•    avee-supply = <&reg_avee>;
  

•    vddi-supply = <&reg_1v8_p>;
  

•    reset-gpios = <&gpio1 29 GPIO_ACTIVE_LOW>;
  

•    backlight = <&backlight>;
  

•  };
  

• };

I think we need an ampty line here.

...

+...

2.26.2

---

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