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;
From:   Philippe CORNU <philippe.cornu@st.com>
To:     Benjamin GAIGNARD <benjamin.gaignard@st.com>,
        Maxime Ripard <maxime@cerno.tech>
CC:     "l.stach@pengutronix.de" <l.stach@pengutronix.de>,
        "linux+etnaviv@armlinux.org.uk" <linux+etnaviv@armlinux.org.uk>,
        "christian.gmeiner@gmail.com" <christian.gmeiner@gmail.com>,
        "airlied@linux.ie" <airlied@linux.ie>,
        "daniel@ffwll.ch" <daniel@ffwll.ch>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "etnaviv@lists.freedesktop.org" <etnaviv@lists.freedesktop.org>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Pierre Yves MORDRET <pierre-yves.mordret@st.com>
Subject: Re: [PATCH v2] dt-bindings: display: Convert etnaviv to json-schema
Thread-Topic: [PATCH v2] dt-bindings: display: Convert etnaviv to json-schema
Thread-Index: AQHV1bPFvwodev/kWkOXWkWfzChdiaf/6nAAgAAHB4CAAEgqAA==
Date:   Tue, 28 Jan 2020 16:49:35 +0000
Message-ID: <7acf71bb-092d-14f4-8208-6d906a993211@st.com>
References: <20200128082013.15951-1-benjamin.gaignard@st.com>
 <20200128120600.oagnindklixjyieo@gilmour.lan>
 <a7fa1b43-a188-9d06-73ec-16bcd4012207@st.com>
In-Reply-To: <a7fa1b43-a188-9d06-73ec-16bcd4012207@st.com>
Accept-Language: fr-FR, en-US
Content-Language: en-US
user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.4.1
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.75.127.49]
Content-Type: text/plain; charset="Windows-1252"
Content-ID: <85CDDBF742537543A477C23D8CA2A1C8@st.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138,18.0.572
 definitions=2020-01-28_05:2020-01-28,2020-01-28 signatures=0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Benjamin,


On 1/28/20 1:31 PM, Benjamin GAIGNARD wrote:
>=20
> On 1/28/20 1:06 PM, Maxime Ripard wrote:
>> Hi Benjamin,
>>
>> On Tue, Jan 28, 2020 at 09:20:13AM +0100, Benjamin Gaignard wrote:
>>> Convert etnaviv bindings to yaml format.
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>> ---
>>>    .../bindings/display/etnaviv/etnaviv-drm.txt       | 36 -----------
>>>    .../devicetree/bindings/gpu/vivante,gc.yaml        | 72 ++++++++++++=
++++++++++
>>>    2 files changed, 72 insertions(+), 36 deletions(-)
>>>    delete mode 100644 Documentation/devicetree/bindings/display/etnaviv=
/etnaviv-drm.txt
>>>    create mode 100644 Documentation/devicetree/bindings/gpu/vivante,gc.=
yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-=
drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
>>> deleted file mode 100644
>>> index 8def11b16a24..000000000000
>>> --- a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
>>> +++ /dev/null
>>> @@ -1,36 +0,0 @@
>>> -Vivante GPU core devices
>>> -=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D
>>> -
>>> -Required properties:
>>> -- compatible: Should be "vivante,gc"
>>> -  A more specific compatible is not needed, as the cores contain chip
>>> -  identification registers at fixed locations, which provide all the
>>> -  necessary information to the driver.
>>> -- reg: should be register base and length as documented in the
>>> -  datasheet
>>> -- interrupts: Should contain the cores interrupt line
>>> -- clocks: should contain one clock for entry in clock-names
>>> -  see Documentation/devicetree/bindings/clock/clock-bindings.txt
>>> -- clock-names:
>>> -   - "bus":    AXI/master interface clock
>>> -   - "reg":    AHB/slave interface clock
>>> -               (only required if GPU can gate slave interface independ=
ently)
>>> -   - "core":   GPU core clock
>>> -   - "shader": Shader clock (only required if GPU has feature PIPE_3D)
>>> -
>>> -Optional properties:
>>> -- power-domains: a power domain consumer specifier according to
>>> -  Documentation/devicetree/bindings/power/power_domain.txt
>>> -
>>> -example:
>>> -
>>> -gpu_3d: gpu@130000 {
>>> -	compatible =3D "vivante,gc";
>>> -	reg =3D <0x00130000 0x4000>;
>>> -	interrupts =3D <0 9 IRQ_TYPE_LEVEL_HIGH>;
>>> -	clocks =3D <&clks IMX6QDL_CLK_GPU3D_AXI>,
>>> -	         <&clks IMX6QDL_CLK_GPU3D_CORE>,
>>> -	         <&clks IMX6QDL_CLK_GPU3D_SHADER>;
>>> -	clock-names =3D "bus", "core", "shader";
>>> -	power-domains =3D <&gpc 1>;
>>> -};
>>> diff --git a/Documentation/devicetree/bindings/gpu/vivante,gc.yaml b/Do=
cumentation/devicetree/bindings/gpu/vivante,gc.yaml
>>> new file mode 100644
>>> index 000000000000..c4f549c0d750
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpu/vivante,gc.yaml
>>> @@ -0,0 +1,72 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/gpu/vivante,gc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Vivante GPU Bindings
>>> +
>>> +description: Vivante GPU core devices
>>> +
>>> +maintainers:
>>> +  -  Lucas Stach <l.stach@pengutronix.de>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: vivante,gc
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    items:
>>> +      - description: AXI/master interface clock
>>> +      - description: GPU core clock
>>> +      - description: Shader clock (only required if GPU has feature PI=
PE_3D)
>>> +      - description: AHB/slave interface clock (only required if GPU c=
an gate slave interface independently)
>> Can you have an AHB slave interface clock without a shader clock?
>=20
> No because the items in the list are ordered so you need to have, in
> order: "bus", "core", "shader", "reg"
>=20
> If it is needed to allow any number of clock in any order I could write
> it like this:
>=20
> clocks:
>=20
>   =A0 minItems: 1
>=20
>   =A0 maxItems: 4
>=20
> clock-names:
>=20
>   =A0 items:
>=20
>   =A0=A0=A0 enum: [ bus, core, shader, reg]
>=20
>   =A0 minItems: 1
>=20
>   =A0 maxItems: 4
>=20
> Benjamin


Thank you for your patch,

I confirm that your last proposal with enum would be better.

With that,
Reviewed-by: Philippe Cornu <philippe.cornu@st.com>

Philippe :-)


>=20
>>
>>> +    minItems: 2
>>> +    maxItems: 4
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: bus
>>> +      - const: core
>>> +      - const: shader
>>> +      - const: reg
>>> +    minItems: 2
>>> +    maxItems: 4
>> If so, that check will fail, since it would expect a clock named
>> shader on the 3rd item.
>>
>> It looks good otherwise, thanks!
>> Maxime=