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

From: Rob Herring
Date: Tue Jan 28 2020 - 17:08:25 EST


On Tue, Jan 28, 2020 at 1:58 PM Benjamin GAIGNARD
<benjamin.gaignard@xxxxxx> wrote:
>
>
> On 1/28/20 8:35 PM, Rob Herring wrote:
> > On Tue, Jan 28, 2020 at 6:31 AM Benjamin GAIGNARD
> > <benjamin.gaignard@xxxxxx> 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@xxxxxx>
> >>>> ---
> >>>> .../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@xxxxxxxxxxxxxx>
> >>>> +
> >>>> +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