[PATCH v2] dt-bindings: display: Convert etnaviv to json-schema

[PATCH v2] dt-bindings: display: Convert etnaviv to json-schema

[Benjamin Gaignard]

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
-========================
-
-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 independently)
-   - "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 = "vivante,gc";
-	reg = <0x00130000 0x4000>;
-	interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
-	clocks = <&clks IMX6QDL_CLK_GPU3D_AXI>,
-	         <&clks IMX6QDL_CLK_GPU3D_CORE>,
-	         <&clks IMX6QDL_CLK_GPU3D_SHADER>;
-	clock-names = "bus", "core", "shader";
-	power-domains = <&gpc 1>;
-};
diff --git a/Documentation/devicetree/bindings/gpu/vivante,gc.yaml b/Documentation/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 PIPE_3D)
+      - description: AHB/slave interface clock (only required if GPU can gate slave interface independently)
+    minItems: 2
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: bus
+      - const: core
+      - const: shader
+      - const: reg
+    minItems: 2
+    maxItems: 4
+
+  resets:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx6qdl-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    gpu@130000 {
+      compatible = "vivante,gc";
+      reg = <0x00130000 0x4000>;
+      interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&clks IMX6QDL_CLK_GPU3D_AXI>,
+               <&clks IMX6QDL_CLK_GPU3D_CORE>,
+               <&clks IMX6QDL_CLK_GPU3D_SHADER>;
+      clock-names = "bus", "core", "shader";
+      power-domains = <&gpc 1>;
+    };
+
+...
-- 
2.15.0

Re: [PATCH v2] dt-bindings: display: Convert etnaviv to json-schema

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3733 bytes --]

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
> -========================
> -
> -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 independently)
> -   - "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 = "vivante,gc";
> -	reg = <0x00130000 0x4000>;
> -	interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
> -	clocks = <&clks IMX6QDL_CLK_GPU3D_AXI>,
> -	         <&clks IMX6QDL_CLK_GPU3D_CORE>,
> -	         <&clks IMX6QDL_CLK_GPU3D_SHADER>;
> -	clock-names = "bus", "core", "shader";
> -	power-domains = <&gpc 1>;
> -};
> diff --git a/Documentation/devicetree/bindings/gpu/vivante,gc.yaml b/Documentation/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 PIPE_3D)
> +      - description: AHB/slave interface clock (only required if GPU can gate slave interface independently)

Can you have an AHB slave interface clock without a shader clock?

> +    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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] dt-bindings: display: Convert etnaviv to json-schema

[Benjamin GAIGNARD]

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
>> -========================
>> -
>> -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 independently)
>> -   - "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 = "vivante,gc";
>> -	reg = <0x00130000 0x4000>;
>> -	interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
>> -	clocks = <&clks IMX6QDL_CLK_GPU3D_AXI>,
>> -	         <&clks IMX6QDL_CLK_GPU3D_CORE>,
>> -	         <&clks IMX6QDL_CLK_GPU3D_SHADER>;
>> -	clock-names = "bus", "core", "shader";
>> -	power-domains = <&gpc 1>;
>> -};
>> diff --git a/Documentation/devicetree/bindings/gpu/vivante,gc.yaml b/Documentation/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 PIPE_3D)
>> +      - description: AHB/slave interface clock (only required if GPU can gate slave interface independently)
> Can you have an AHB slave interface clock without a shader clock?

No because the items in the list are ordered so you need to have, in 
order: "bus", "core", "shader", "reg"

If it is needed to allow any number of clock in any order I could write 
it like this:

clocks:

   minItems: 1

   maxItems: 4

clock-names:

   items:

     enum: [ bus, core, shader, reg]

   minItems: 1

   maxItems: 4

Benjamin

>
>> +    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

Re: [PATCH v2] dt-bindings: display: Convert etnaviv to json-schema

[Philippe CORNU]

Hi Benjamin,


On 1/28/20 1:31 PM, Benjamin GAIGNARD wrote:
> 
> 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
>>> -========================
>>> -
>>> -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 independently)
>>> -   - "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 = "vivante,gc";
>>> -	reg = <0x00130000 0x4000>;
>>> -	interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
>>> -	clocks = <&clks IMX6QDL_CLK_GPU3D_AXI>,
>>> -	         <&clks IMX6QDL_CLK_GPU3D_CORE>,
>>> -	         <&clks IMX6QDL_CLK_GPU3D_SHADER>;
>>> -	clock-names = "bus", "core", "shader";
>>> -	power-domains = <&gpc 1>;
>>> -};
>>> diff --git a/Documentation/devicetree/bindings/gpu/vivante,gc.yaml b/Documentation/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 PIPE_3D)
>>> +      - description: AHB/slave interface clock (only required if GPU can gate slave interface independently)
>> Can you have an AHB slave interface clock without a shader clock?
> 
> No because the items in the list are ordered so you need to have, in
> order: "bus", "core", "shader", "reg"
> 
> If it is needed to allow any number of clock in any order I could write
> it like this:
> 
> clocks:
> 
>     minItems: 1
> 
>     maxItems: 4
> 
> clock-names:
> 
>     items:
> 
>       enum: [ bus, core, shader, reg]
> 
>     minItems: 1
> 
>     maxItems: 4
> 
> 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 :-)


> 
>>
>>> +    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

Re: [PATCH v2] dt-bindings: display: Convert etnaviv to json-schema

[Rob Herring]

On Tue, Jan 28, 2020 at 6:31 AM Benjamin GAIGNARD
<benjamin.gaignard@st.com> wrote:
>
>
> 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
> >> -========================
> >> -
> >> -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 independently)
> >> -   - "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 = "vivante,gc";
> >> -    reg = <0x00130000 0x4000>;
> >> -    interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
> >> -    clocks = <&clks IMX6QDL_CLK_GPU3D_AXI>,
> >> -             <&clks IMX6QDL_CLK_GPU3D_CORE>,
> >> -             <&clks IMX6QDL_CLK_GPU3D_SHADER>;
> >> -    clock-names = "bus", "core", "shader";
> >> -    power-domains = <&gpc 1>;
> >> -};
> >> diff --git a/Documentation/devicetree/bindings/gpu/vivante,gc.yaml b/Documentation/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 PIPE_3D)
> >> +      - description: AHB/slave interface clock (only required if GPU can gate slave interface independently)
> > Can you have an AHB slave interface clock without a shader clock?
>
> No because the items in the list are ordered so you need to have, in
> order: "bus", "core", "shader", "reg"
>
> If it is needed to allow any number of clock in any order I could write
> it like this:

Yes, but I prefer we don't allow any order if we don't have to. Did
you run this schema against dtbs_check or just audit the dts files
with vivante?

Rob

Re: [PATCH v2] dt-bindings: display: Convert etnaviv to json-schema

[Benjamin GAIGNARD]

On 1/28/20 8:35 PM, Rob Herring wrote:
> On Tue, Jan 28, 2020 at 6:31 AM Benjamin GAIGNARD
> <benjamin.gaignard@st.com> wrote:
>>
>> 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
>>>> -========================
>>>> -
>>>> -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 independently)
>>>> -   - "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 = "vivante,gc";
>>>> -    reg = <0x00130000 0x4000>;
>>>> -    interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
>>>> -    clocks = <&clks IMX6QDL_CLK_GPU3D_AXI>,
>>>> -             <&clks IMX6QDL_CLK_GPU3D_CORE>,
>>>> -             <&clks IMX6QDL_CLK_GPU3D_SHADER>;
>>>> -    clock-names = "bus", "core", "shader";
>>>> -    power-domains = <&gpc 1>;
>>>> -};
>>>> diff --git a/Documentation/devicetree/bindings/gpu/vivante,gc.yaml b/Documentation/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 PIPE_3D)
>>>> +      - description: AHB/slave interface clock (only required if GPU can gate slave interface independently)
>>> Can you have an AHB slave interface clock without a shader clock?
>> No because the items in the list are ordered so you need to have, in
>> order: "bus", "core", "shader", "reg"
>>
>> If it is needed to allow any number of clock in any order I could write
>> it like this:
> Yes, but I prefer we don't allow any order if we don't have to. Did
> you run this schema against dtbs_check or just audit the dts files
> with vivante?

Both, I found these mix of reg-names:

"core"

"bus","core"

"bus","core","shader"

That not really match with original bindings description...


>
> Rob

Re: [PATCH v2] dt-bindings: display: Convert etnaviv to json-schema

[Rob Herring]

On Tue, Jan 28, 2020 at 1:58 PM Benjamin GAIGNARD
<benjamin.gaignard@st.com> wrote:
>
>
> On 1/28/20 8:35 PM, Rob Herring wrote:
> > On Tue, Jan 28, 2020 at 6:31 AM Benjamin GAIGNARD
> > <benjamin.gaignard@st.com> wrote:
> >>
> >> 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
> >>>> -========================
> >>>> -
> >>>> -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 independently)
> >>>> -   - "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 = "vivante,gc";
> >>>> -    reg = <0x00130000 0x4000>;
> >>>> -    interrupts = <0 9 IRQ_TYPE_LEVEL_HIGH>;
> >>>> -    clocks = <&clks IMX6QDL_CLK_GPU3D_AXI>,
> >>>> -             <&clks IMX6QDL_CLK_GPU3D_CORE>,
> >>>> -             <&clks IMX6QDL_CLK_GPU3D_SHADER>;
> >>>> -    clock-names = "bus", "core", "shader";
> >>>> -    power-domains = <&gpc 1>;
> >>>> -};
> >>>> diff --git a/Documentation/devicetree/bindings/gpu/vivante,gc.yaml b/Documentation/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 PIPE_3D)
> >>>> +      - description: AHB/slave interface clock (only required if GPU can gate slave interface independently)
> >>> Can you have an AHB slave interface clock without a shader clock?
> >> No because the items in the list are ordered so you need to have, in
> >> order: "bus", "core", "shader", "reg"
> >>
> >> If it is needed to allow any number of clock in any order I could write
> >> it like this:
> > Yes, but I prefer we don't allow any order if we don't have to. Did
> > you run this schema against dtbs_check or just audit the dts files
> > with vivante?
>
> Both, I found these mix of reg-names:
>
> "core"
>
> "bus","core"
>
> "bus","core","shader"

You missed a couple:

arch/arc/boot/dts/hsdk.dts-                     clock-names = "bus",
"reg", "core", "shader";
arch/arm/boot/dts/dove.dtsi-                            clock-names = "core";
arch/arm/boot/dts/imx6q.dtsi-                   clock-names = "bus", "core";
arch/arm/boot/dts/imx6qdl.dtsi-                 clock-names = "bus",
"core", "shader";
arch/arm/boot/dts/imx6qdl.dtsi-                 clock-names = "bus", "core";
arch/arm/boot/dts/imx6sl.dtsi-                  clock-names = "bus", "core";
arch/arm/boot/dts/imx6sl.dtsi-                  clock-names = "bus", "core";
arch/arm/boot/dts/imx6sx.dtsi-                  clock-names = "bus",
"core", "shader";
arch/arm/boot/dts/stm32mp157c.dtsi-                     clock-names =
"bus" ,"core";
arch/arm64/boot/dts/freescale/imx8mq.dtsi-
clock-names = "core", "shader", "bus", "reg";

imx8mq is probably new enough to change if we wanted to.

I guess just do an enum...

Rob