2020-01-28 08:21:51

by Benjamin GAIGNARD

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

Convert etnaviv bindings to yaml format.

Signed-off-by: Benjamin Gaignard <[email protected]>
---
.../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 <[email protected]>
+
+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


2020-01-28 12:07:26

by Maxime Ripard

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

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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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


Attachments:
(No filename) (3.75 kB)
signature.asc (235.00 B)
Download all attachments

2020-01-28 12:33:04

by Benjamin GAIGNARD

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


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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> +
>> +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

2020-01-28 16:51:08

by Philippe Cornu

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

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 <[email protected]>
>>> ---
>>> .../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 <[email protected]>
>>> +
>>> +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 <[email protected]>

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

2020-01-28 19:37:22

by Rob Herring

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

On Tue, Jan 28, 2020 at 6:31 AM Benjamin GAIGNARD
<[email protected]> 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 <[email protected]>
> >> ---
> >> .../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 <[email protected]>
> >> +
> >> +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

2020-01-28 19:59:23

by Benjamin GAIGNARD

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


On 1/28/20 8:35 PM, Rob Herring wrote:
> On Tue, Jan 28, 2020 at 6:31 AM Benjamin GAIGNARD
> <[email protected]> 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 <[email protected]>
>>>> ---
>>>> .../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 <[email protected]>
>>>> +
>>>> +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

2020-01-28 22:09:28

by Rob Herring

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

On Tue, Jan 28, 2020 at 1:58 PM Benjamin GAIGNARD
<[email protected]> wrote:
>
>
> On 1/28/20 8:35 PM, Rob Herring wrote:
> > On Tue, Jan 28, 2020 at 6:31 AM Benjamin GAIGNARD
> > <[email protected]> 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 <[email protected]>
> >>>> ---
> >>>> .../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 <[email protected]>
> >>>> +
> >>>> +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