Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 10 Jan 2020 18:12:26 +0100
From:   Sam Ravnborg <sam@ravnborg.org>
To:     Icenowy Zheng <icenowy@aosc.io>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Maxime Ripard <mripard@kernel.org>,
        Chen-Yu Tsai <wens@csie.org>,
        Thierry Reding <thierry.reding@gmail.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linux-sunxi@googlegroups.com
Subject: Re: [PATCH 2/5] dt-bindings: panel: add Feixin K101 IM2BA02 MIPI-DSI
 panel
Message-ID: <20200110171226.GB25099@ravnborg.org>
References: <20200110155225.1051749-1-icenowy@aosc.io>
 <20200110155225.1051749-3-icenowy@aosc.io>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200110155225.1051749-3-icenowy@aosc.io>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Icenowy.

On Fri, Jan 10, 2020 at 11:52:22PM +0800, Icenowy Zheng wrote:
> Feixin K101 IM2BA02 is a 10.1" 800x1280 4-lane MIPI-DSI panel.
> 
> Add device tree binding for it.
Thanks for using the new meta-schema format and with the correct
license.


The example fails dt_binding_check - I had to do the following to make
it pass:

diff --git a/Documentation/devicetree/bindings/display/panel/feixin,k101-im2ba02.yaml b/Documentation/devicetree/bindings/display/panel/feixin,k101-im2ba02.yaml
index 7176d14893ff..c43f884de6ac 100644
--- a/Documentation/devicetree/bindings/display/panel/feixin,k101-im2ba02.yaml
+++ b/Documentation/devicetree/bindings/display/panel/feixin,k101-im2ba02.yaml
@@ -37,7 +37,9 @@ additionalProperties: false
 
 examples:
   - |
-    &dsi {
+    #include <dt-bindings/gpio/gpio.h>
+
+    dsi {
         #address-cells = <1>;
         #size-cells = <0>;
         panel@0 {

Added include to get definition of GPIO_...
And dropped the "&" as it is not OK in the example here.

And please consider if backlight really is a required property.

	Sam
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  .../display/panel/feixin,k101-im2ba02.yaml    | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/feixin,k101-im2ba02.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/feixin,k101-im2ba02.yaml b/Documentation/devicetree/bindings/display/panel/feixin,k101-im2ba02.yaml
> new file mode 100644
> index 000000000000..7176d14893ff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/feixin,k101-im2ba02.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/feixin,k101-im2ba02.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Feixin K101 IM2BA02 10.1" MIPI-DSI LCD panel
> +
> +maintainers:
> +  - Icenowy Zheng <icenowy@aosc.io>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: feixin,k101-im2ba02
> +  reg: true
> +  backlight: true
> +  reset-gpios: true
> +  avdd-supply:
> +     description: regulator that supplies the AVDD voltage
> +  dvdd-supply:
> +     description: regulator that supplies the DVDD voltage
> +  cvdd-supply:
> +     description: regulator that supplies the CVDD voltage
> +
> +required:
> +  - compatible
> +  - reg
> +  - backlight
> +  - avdd-supply
> +  - dvdd-supply
> +  - cvdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    &dsi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        panel@0 {
> +            compatible = "feixin,k101-im2ba02";
> +            reg = <0>;
> +            avdd-supply = <&reg_dc1sw>;
> +            dvdd-supply = <&reg_dc1sw>;
> +            cvdd-supply = <&reg_ldo_io1>;
> +            reset-gpios = <&pio 3 24 GPIO_ACTIVE_HIGH>;
> +            backlight = <&backlight>;
> +        };
> +    };
> +
> +...
> -- 
> 2.23.0