[PATCH v2 0/2] dt-bindings: Convert GFX bindings to yaml

[PATCH v2 0/2] dt-bindings: Convert GFX bindings to yaml

[Joel Stanley]

v1: https://lore.kernel.org/all/20220302051056.678367-1-joel@jms.id.au/

This series cleans up the bindings for the ASPEED GFX unit.

The old text files are deleted for both the description under gpu, and the
placeholder one under mfd.

The mfd one existed because pinctrl for the 2500 depends on the gfx
bindings, and at the time we didn't have any support fo the gfx device,
so Andrew added the mfd ones.

The example in the pinctrl bindings is updated to prevent warnings about
missing properties that pop up when the gfx yaml bindings are added.

Joel Stanley (2):
  dt-bindings: pinctrl: aspeed: Update gfx node in example
  dt-bindings: gpu: Convert aspeed-gfx bindings to yaml

 .../devicetree/bindings/gpu/aspeed,gfx.yaml   | 69 +++++++++++++++++++
 .../devicetree/bindings/gpu/aspeed-gfx.txt    | 41 -----------
 .../devicetree/bindings/mfd/aspeed-gfx.txt    | 17 -----
 .../pinctrl/aspeed,ast2500-pinctrl.yaml       | 16 +++++
 4 files changed, 85 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
 delete mode 100644 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
 delete mode 100644 Documentation/devicetree/bindings/mfd/aspeed-gfx.txt

-- 
2.34.1

[PATCH v2 1/2] dt-bindings: pinctrl: aspeed: Update gfx node in example

[Joel Stanley]

The example needs updating to match the to be added yaml bindings for
the gfx node.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 .../bindings/pinctrl/aspeed,ast2500-pinctrl.yaml | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
index d316cc082107..9969997c2f1b 100644
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
@@ -73,6 +73,7 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/clock/aspeed-clock.h>
     apb {
         compatible = "simple-bus";
         #address-cells = <1>;
@@ -82,6 +83,8 @@ examples:
         syscon: scu@1e6e2000 {
             compatible = "aspeed,ast2500-scu", "syscon", "simple-mfd";
             reg = <0x1e6e2000 0x1a8>;
+            #clock-cells = <1>;
+            #reset-cells = <1>;
 
             pinctrl: pinctrl {
                 compatible = "aspeed,ast2500-pinctrl";
@@ -102,6 +105,12 @@ examples:
         gfx: display@1e6e6000 {
             compatible = "aspeed,ast2500-gfx", "syscon";
             reg = <0x1e6e6000 0x1000>;
+            reg-io-width = <4>;
+            clocks = <&syscon ASPEED_CLK_GATE_D1CLK>;
+            resets = <&syscon ASPEED_RESET_CRT1>;
+            interrupts = <0x19>;
+            syscon = <&syscon>;
+            memory-region = <&gfx_memory>;
         };
     };
 
@@ -128,3 +137,10 @@ examples:
             };
         };
     };
+
+    gfx_memory: framebuffer {
+        size = <0x01000000>;
+        alignment = <0x01000000>;
+        compatible = "shared-dma-pool";
+        reusable;
+    };
-- 
2.34.1

Re: [PATCH v2 1/2] dt-bindings: pinctrl: aspeed: Update gfx node in example

[Linus Walleij]

On Fri, Mar 4, 2022 at 1:03 AM Joel Stanley <joel@jms.id.au> wrote:

> The example needs updating to match the to be added yaml bindings for
> the gfx node.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I guess you will merge this elsewhere or shall I apply it to the pinctrl tree?

Yours,
Linus Walleij

Re: [PATCH v2 1/2] dt-bindings: pinctrl: aspeed: Update gfx node in example

[Joel Stanley]

On Tue, 15 Mar 2022 at 01:21, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Fri, Mar 4, 2022 at 1:03 AM Joel Stanley <joel@jms.id.au> wrote:
>
> > The example needs updating to match the to be added yaml bindings for
> > the gfx node.
> >
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> I guess you will merge this elsewhere or shall I apply it to the pinctrl tree?

Feel free to take this one.


>
> Yours,
> Linus Walleij

Re: [PATCH v2 1/2] dt-bindings: pinctrl: aspeed: Update gfx node in example

[Linus Walleij]

On Fri, Mar 4, 2022 at 1:03 AM Joel Stanley <joel@jms.id.au> wrote:

> The example needs updating to match the to be added yaml bindings for
> the gfx node.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Patch applied.

Yours,
Linus Walleij

[PATCH v2 2/2] dt-bindings: gpu: Convert aspeed-gfx bindings to yaml

[Joel Stanley]

Convert the bindings to yaml and add the ast2600 compatible string.

The legacy mfd description was put in place before the gfx bindings
existed, to document the compatible that is used in the pinctrl
bindings.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 .../devicetree/bindings/gpu/aspeed,gfx.yaml   | 69 +++++++++++++++++++
 .../devicetree/bindings/gpu/aspeed-gfx.txt    | 41 -----------
 .../devicetree/bindings/mfd/aspeed-gfx.txt    | 17 -----
 3 files changed, 69 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
 delete mode 100644 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
 delete mode 100644 Documentation/devicetree/bindings/mfd/aspeed-gfx.txt

diff --git a/Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml b/Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
new file mode 100644
index 000000000000..8ddc4fa6e8e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpu/aspeed,gfx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED GFX display device
+
+maintainers:
+  - Joel Stanley <joel@jms.id.au>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - aspeed,ast2400-gfx
+          - aspeed,ast2500-gfx
+          - aspeed,ast2600-gfx
+      - const: syscon
+
+  reg:
+    minItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  memory-region: true
+
+  syscon: true
+
+  reg-io-width: true
+
+required:
+  - reg
+  - compatible
+  - interrupts
+  - clocks
+  - resets
+  - memory-region
+  - syscon
+
+additionalProperties: false
+
+examples:
+  - |
+   #include <dt-bindings/clock/aspeed-clock.h>
+   gfx: display@1e6e6000 {
+       compatible = "aspeed,ast2500-gfx", "syscon";
+       reg = <0x1e6e6000 0x1000>;
+       reg-io-width = <4>;
+       clocks = <&syscon ASPEED_CLK_GATE_D1CLK>;
+       resets = <&syscon ASPEED_RESET_CRT1>;
+       interrupts = <0x19>;
+       syscon = <&syscon>;
+       memory-region = <&gfx_memory>;
+   };
+
+   gfx_memory: framebuffer {
+       size = <0x01000000>;
+       alignment = <0x01000000>;
+       compatible = "shared-dma-pool";
+       reusable;
+   };
diff --git a/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt b/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
deleted file mode 100644
index 958bdf962339..000000000000
--- a/Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
+++ /dev/null
@@ -1,41 +0,0 @@
-Device tree configuration for the GFX display device on the ASPEED SoCs
-
-Required properties:
-  - compatible
-    * Must be one of the following:
-      + aspeed,ast2500-gfx
-      + aspeed,ast2400-gfx
-    * In addition, the ASPEED pinctrl bindings require the 'syscon' property to
-      be present
-
-  - reg: Physical base address and length of the GFX registers
-
-  - interrupts: interrupt number for the GFX device
-
-  - clocks: clock number used to generate the pixel clock
-
-  - resets: reset line that must be released to use the GFX device
-
-  - memory-region:
-    Phandle to a memory region to allocate from, as defined in
-    Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
-
-
-Example:
-
-gfx: display@1e6e6000 {
-	compatible = "aspeed,ast2500-gfx", "syscon";
-	reg = <0x1e6e6000 0x1000>;
-	reg-io-width = <4>;
-	clocks = <&syscon ASPEED_CLK_GATE_D1CLK>;
-	resets = <&syscon ASPEED_RESET_CRT1>;
-	interrupts = <0x19>;
-	memory-region = <&gfx_memory>;
-};
-
-gfx_memory: framebuffer {
-	size = <0x01000000>;
-	alignment = <0x01000000>;
-	compatible = "shared-dma-pool";
-	reusable;
-};
diff --git a/Documentation/devicetree/bindings/mfd/aspeed-gfx.txt b/Documentation/devicetree/bindings/mfd/aspeed-gfx.txt
deleted file mode 100644
index aea5370efd97..000000000000
--- a/Documentation/devicetree/bindings/mfd/aspeed-gfx.txt
+++ /dev/null
@@ -1,17 +0,0 @@
-* Device tree bindings for Aspeed SoC Display Controller (GFX)
-
-The Aspeed SoC Display Controller primarily does as its name suggests, but also
-participates in pinmux requests on the g5 SoCs. It is therefore considered a
-syscon device.
-
-Required properties:
-- compatible:		"aspeed,ast2500-gfx", "syscon"
-- reg:			contains offset/length value of the GFX memory
-			region.
-
-Example:
-
-gfx: display@1e6e6000 {
-	compatible = "aspeed,ast2500-gfx", "syscon";
-	reg = <0x1e6e6000 0x1000>;
-};
-- 
2.34.1

Re: [PATCH v2 2/2] dt-bindings: gpu: Convert aspeed-gfx bindings to yaml

[Rob Herring]

On Fri, 04 Mar 2022 10:33:11 +1030, Joel Stanley wrote:
> Convert the bindings to yaml and add the ast2600 compatible string.
> 
> The legacy mfd description was put in place before the gfx bindings
> existed, to document the compatible that is used in the pinctrl
> bindings.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  .../devicetree/bindings/gpu/aspeed,gfx.yaml   | 69 +++++++++++++++++++
>  .../devicetree/bindings/gpu/aspeed-gfx.txt    | 41 -----------
>  .../devicetree/bindings/mfd/aspeed-gfx.txt    | 17 -----
>  3 files changed, 69 insertions(+), 58 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
>  delete mode 100644 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
>  delete mode 100644 Documentation/devicetree/bindings/mfd/aspeed-gfx.txt
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1600723


display@1e6e6000: 'memory-region' is a required property
	arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dt.yaml
	arch/arm/boot/dts/aspeed-bmc-arm-stardragon4800-rep2.dt.yaml
	arch/arm/boot/dts/aspeed-bmc-intel-s2600wf.dt.yaml
	arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dt.yaml
	arch/arm/boot/dts/aspeed-bmc-opp-zaius.dt.yaml
	arch/arm/boot/dts/aspeed-bmc-supermicro-x11spi.dt.yaml

Re: [PATCH v2 2/2] dt-bindings: gpu: Convert aspeed-gfx bindings to yaml

[Krzysztof Kozlowski]

On 04/03/2022 01:03, Joel Stanley wrote:
> Convert the bindings to yaml and add the ast2600 compatible string.
> 
> The legacy mfd description was put in place before the gfx bindings
> existed, to document the compatible that is used in the pinctrl
> bindings.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  .../devicetree/bindings/gpu/aspeed,gfx.yaml   | 69 +++++++++++++++++++
>  .../devicetree/bindings/gpu/aspeed-gfx.txt    | 41 -----------
>  .../devicetree/bindings/mfd/aspeed-gfx.txt    | 17 -----
>  3 files changed, 69 insertions(+), 58 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
>  delete mode 100644 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
>  delete mode 100644 Documentation/devicetree/bindings/mfd/aspeed-gfx.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml b/Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
> new file mode 100644
> index 000000000000..8ddc4fa6e8e4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpu/aspeed,gfx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASPEED GFX display device
> +
> +maintainers:
> +  - Joel Stanley <joel@jms.id.au>
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - aspeed,ast2400-gfx
> +          - aspeed,ast2500-gfx
> +          - aspeed,ast2600-gfx

That's different than original bindings - new patch. It's not currently
supported, so adding it is more than just updating bindings to current
state.

> +      - const: syscon
> +
> +  reg:
> +    minItems: 1

maxItems?

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  memory-region: true
> +
> +  syscon: true

You need at least description. I see old bindings did not mention it
(except that it is required)... I think you also need a type, because it
is not a standard property.

> +
> +  reg-io-width: true

Some constraints? Can it be anything from syscon schema?

Best regards,
Krzysztof

Re: [PATCH v2 2/2] dt-bindings: gpu: Convert aspeed-gfx bindings to yaml

[Rob Herring]

On Sat, Mar 05, 2022 at 10:49:05PM +0100, Krzysztof Kozlowski wrote:
> On 04/03/2022 01:03, Joel Stanley wrote:
> > Convert the bindings to yaml and add the ast2600 compatible string.
> > 
> > The legacy mfd description was put in place before the gfx bindings
> > existed, to document the compatible that is used in the pinctrl
> > bindings.
> > 
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >  .../devicetree/bindings/gpu/aspeed,gfx.yaml   | 69 +++++++++++++++++++
> >  .../devicetree/bindings/gpu/aspeed-gfx.txt    | 41 -----------
> >  .../devicetree/bindings/mfd/aspeed-gfx.txt    | 17 -----
> >  3 files changed, 69 insertions(+), 58 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/gpu/aspeed-gfx.txt
> >  delete mode 100644 Documentation/devicetree/bindings/mfd/aspeed-gfx.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml b/Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
> > new file mode 100644
> > index 000000000000..8ddc4fa6e8e4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpu/aspeed,gfx.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/gpu/aspeed,gfx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ASPEED GFX display device
> > +
> > +maintainers:
> > +  - Joel Stanley <joel@jms.id.au>
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - aspeed,ast2400-gfx
> > +          - aspeed,ast2500-gfx
> > +          - aspeed,ast2600-gfx
> 
> That's different than original bindings - new patch. It's not currently
> supported, so adding it is more than just updating bindings to current
> state.
> 
> > +      - const: syscon
> > +
> > +  reg:
> > +    minItems: 1
> 
> maxItems?
> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  memory-region: true

Also need to define how many (maxItems: 1).

> > +
> > +  syscon: true
> 
> You need at least description. I see old bindings did not mention it
> (except that it is required)... I think you also need a type, because it
> is not a standard property.
> 
> > +
> > +  reg-io-width: true
> 
> Some constraints? Can it be anything from syscon schema?
> 
> Best regards,
> Krzysztof
>