[PATCH v6] media: dt-bindings: aspeed,video-engine: Convert to json schema

[PATCH v6] media: dt-bindings: aspeed,video-engine: Convert to json schema

[Jammy Huang]

Convert aspeed-video.txt to yaml format.
Update aspeed-video.txt to aspeed,video-engine.yaml in MAINTAINER file.

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 v6:
  - Remove new properties in example.

 v5:
  - Don't put additional changes in this commit.

 v4:
  - Keep | after phandle description to fix dt_binding_check error.

 v3:
  - Add Additional changes into comments.
  - Remove | after phandle description

 v2:
  - Update patch subject
  - Add NOTE for false positive warning
---
 .../bindings/media/aspeed,video-engine.yaml   | 70 +++++++++++++++++++
 .../bindings/media/aspeed-video.txt           | 33 ---------
 MAINTAINERS                                   |  2 +-
 3 files changed, 71 insertions(+), 34 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
 delete mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt

diff --git a/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml b/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
new file mode 100644
index 000000000000..682bba20778c
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/aspeed,video-engine.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED Video Engine
+
+maintainers:
+  - Eddie James <eajames@linux.ibm.com>
+
+description:
+  The Video Engine (VE) embedded in the ASPEED SOCs can be configured to
+  capture and compress video data from digital or analog sources.
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2400-video-engine
+      - aspeed,ast2500-video-engine
+      - aspeed,ast2600-video-engine
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: vclk
+      - const: eclk
+
+  resets:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  memory-region:
+    maxItems: 1
+    description: |
+      Phandle to the reserved memory nodes to be associated with the
+      VE. VE will acquires memory space for 3 purposes:
+        1. JPEG header
+        2. Compressed result
+        3. Temporary transformed image data
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/ast2600-clock.h>
+
+    video@1e700000 {
+        compatible = "aspeed,ast2600-video-engine";
+        reg = <0x1e700000 0x1000>;
+        clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
+                 <&syscon ASPEED_CLK_GATE_ECLK>;
+        clock-names = "vclk", "eclk";
+        interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+    };
diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt
deleted file mode 100644
index d2ca32512272..000000000000
--- a/Documentation/devicetree/bindings/media/aspeed-video.txt
+++ /dev/null
@@ -1,33 +0,0 @@
-* Device tree bindings for Aspeed Video Engine
-
-The Video Engine (VE) embedded in the Aspeed AST2400/2500/2600 SOCs can
-capture and compress video data from digital or analog sources.
-
-Required properties:
- - compatible:		"aspeed,ast2400-video-engine" or
-			"aspeed,ast2500-video-engine" or
-			"aspeed,ast2600-video-engine"
- - reg:			contains the offset and length of the VE memory region
- - clocks:		clock specifiers for the syscon clocks associated with
-			the VE (ordering must match the clock-names property)
- - clock-names:		"vclk" and "eclk"
- - resets:		reset specifier for the syscon reset associated with
-			the VE
- - interrupts:		the interrupt associated with the VE on this platform
-
-Optional properties:
- - memory-region:
-	phandle to a memory region to allocate from, as defined in
-	Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
-
-Example:
-
-video-engine@1e700000 {
-    compatible = "aspeed,ast2500-video-engine";
-    reg = <0x1e700000 0x20000>;
-    clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>;
-    clock-names = "vclk", "eclk";
-    resets = <&syscon ASPEED_RESET_VIDEO>;
-    interrupts = <7>;
-    memory-region = <&video_engine_memory>;
-};
diff --git a/MAINTAINERS b/MAINTAINERS
index 896a307fa065..7e59daa1e89d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3549,7 +3549,7 @@ M:	Eddie James <eajames@linux.ibm.com>
 L:	linux-media@vger.kernel.org
 L:	openbmc@lists.ozlabs.org (moderated for non-subscribers)
 S:	Maintained
-F:	Documentation/devicetree/bindings/media/aspeed-video.txt
+F:	Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
 F:	drivers/media/platform/aspeed/
 
 ASUS EC HARDWARE MONITOR DRIVER

base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
-- 
2.25.1

Re: [PATCH v6] media: dt-bindings: aspeed,video-engine: Convert to json schema

[Krzysztof Kozlowski]

On Thu, Feb 13, 2025 at 09:53:38AM +0800, Jammy Huang wrote:
> Convert aspeed-video.txt to yaml format.
> Update aspeed-video.txt to aspeed,video-engine.yaml in MAINTAINER file.
> 
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v6] media: dt-bindings: aspeed,video-engine: Convert to json schema

[Andrew Jeffery]

On Thu, 2025-02-13 at 09:53 +0800, Jammy Huang wrote:
> Convert aspeed-video.txt to yaml format.
> Update aspeed-video.txt to aspeed,video-engine.yaml in MAINTAINER file.
> 
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>  v6:
>   - Remove new properties in example.
> 
>  v5:
>   - Don't put additional changes in this commit.
> 
>  v4:
>   - Keep | after phandle description to fix dt_binding_check error.
> 
>  v3:
>   - Add Additional changes into comments.
>   - Remove | after phandle description
> 
>  v2:
>   - Update patch subject
>   - Add NOTE for false positive warning
> ---
>  .../bindings/media/aspeed,video-engine.yaml   | 70 +++++++++++++++++++
>  .../bindings/media/aspeed-video.txt           | 33 ---------
>  MAINTAINERS                                   |  2 +-
>  3 files changed, 71 insertions(+), 34 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
>  delete mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml b/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
> new file mode 100644
> index 000000000000..682bba20778c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/aspeed,video-engine.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASPEED Video Engine
> +
> +maintainers:
> +  - Eddie James <eajames@linux.ibm.com>
> +
> +description:
> +  The Video Engine (VE) embedded in the ASPEED SOCs can be configured to
> +  capture and compress video data from digital or analog sources.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - aspeed,ast2400-video-engine
> +      - aspeed,ast2500-video-engine
> +      - aspeed,ast2600-video-engine
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: vclk
> +      - const: eclk
> +
> +  resets:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  memory-region:
> +    maxItems: 1
> +    description: |
> +      Phandle to the reserved memory nodes to be associated with the
> +      VE. VE will acquires memory space for 3 purposes:
> +        1. JPEG header
> +        2. Compressed result
> +        3. Temporary transformed image data
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts

This should list `resets` as well, as that wasn't optional in the text
binding.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/ast2600-clock.h>
> +
> +    video@1e700000 {
> +        compatible = "aspeed,ast2600-video-engine";

I guess it's not much of a concern that the example is different...

Andrew

> +        reg = <0x1e700000 0x1000>;
> +        clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
> +                 <&syscon ASPEED_CLK_GATE_ECLK>;
> +        clock-names = "vclk", "eclk";
> +        interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> +    };
> diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt b/Documentation/devicetree/bindings/media/aspeed-video.txt
> deleted file mode 100644
> index d2ca32512272..000000000000
> --- a/Documentation/devicetree/bindings/media/aspeed-video.txt
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -* Device tree bindings for Aspeed Video Engine
> -
> -The Video Engine (VE) embedded in the Aspeed AST2400/2500/2600 SOCs can
> -capture and compress video data from digital or analog sources.
> -
> -Required properties:
> - - compatible:         "aspeed,ast2400-video-engine" or
> -                       "aspeed,ast2500-video-engine" or
> -                       "aspeed,ast2600-video-engine"
> - - reg:                        contains the offset and length of the VE memory region
> - - clocks:             clock specifiers for the syscon clocks associated with
> -                       the VE (ordering must match the clock-names property)
> - - clock-names:                "vclk" and "eclk"
> - - resets:             reset specifier for the syscon reset associated with
> -                       the VE
> - - interrupts:         the interrupt associated with the VE on this platform
> -
> -Optional properties:
> - - memory-region:
> -       phandle to a memory region to allocate from, as defined in
> -       Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> -
> -Example:
> -
> -video-engine@1e700000 {
> -    compatible = "aspeed,ast2500-video-engine";
> -    reg = <0x1e700000 0x20000>;
> -    clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon ASPEED_CLK_GATE_ECLK>;
> -    clock-names = "vclk", "eclk";
> -    resets = <&syscon ASPEED_RESET_VIDEO>;
> -    interrupts = <7>;
> -    memory-region = <&video_engine_memory>;
> -};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 896a307fa065..7e59daa1e89d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3549,7 +3549,7 @@ M:        Eddie James <eajames@linux.ibm.com>
>  L:     linux-media@vger.kernel.org
>  L:     openbmc@lists.ozlabs.org (moderated for non-subscribers)
>  S:     Maintained
> -F:     Documentation/devicetree/bindings/media/aspeed-video.txt
> +F:     Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
>  F:     drivers/media/platform/aspeed/
>  
>  ASUS EC HARDWARE MONITOR DRIVER
> 
> base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b

RE: [PATCH v6] media: dt-bindings: aspeed,video-engine: Convert to json schema

[Jammy Huang]

Hi Andrew,

Thanks for your feedback, please find my explanation below.

> 
> On Thu, 2025-02-13 at 09:53 +0800, Jammy Huang wrote:
> > Convert aspeed-video.txt to yaml format.
> > Update aspeed-video.txt to aspeed,video-engine.yaml in MAINTAINER file.
> >
> > Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> > ---
> >  v6:
> >   - Remove new properties in example.
> >
> >  v5:
> >   - Don't put additional changes in this commit.
> >
> >  v4:
> >   - Keep | after phandle description to fix dt_binding_check error.
> >
> >  v3:
> >   - Add Additional changes into comments.
> >   - Remove | after phandle description
> >
> >  v2:
> >   - Update patch subject
> >   - Add NOTE for false positive warning
> > ---
> >  .../bindings/media/aspeed,video-engine.yaml   | 70
> > +++++++++++++++++++
> >  .../bindings/media/aspeed-video.txt           | 33 ---------
> >  MAINTAINERS                                   |  2
> +-
> >  3 files changed, 71 insertions(+), 34 deletions(-)
> >  create mode 100644
> > Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
> >  delete mode 100644
> > Documentation/devicetree/bindings/media/aspeed-video.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
> > b/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
> > new file mode 100644
> > index 000000000000..682bba20778c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
> > @@ -0,0 +1,70 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/aspeed,video-engine.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ASPEED Video Engine
> > +
> > +maintainers:
> > +  - Eddie James <eajames@linux.ibm.com>
> > +
> > +description:
> > +  The Video Engine (VE) embedded in the ASPEED SOCs can be configured
> > +to
> > +  capture and compress video data from digital or analog sources.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - aspeed,ast2400-video-engine
> > +      - aspeed,ast2500-video-engine
> > +      - aspeed,ast2600-video-engine
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    items:
> > +      - const: vclk
> > +      - const: eclk
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  memory-region:
> > +    maxItems: 1
> > +    description: |
> > +      Phandle to the reserved memory nodes to be associated with the
> > +      VE. VE will acquires memory space for 3 purposes:
> > +        1. JPEG header
> > +        2. Compressed result
> > +        3. Temporary transformed image data
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - interrupts
> 
> This should list `resets` as well, as that wasn't optional in the text binding.
> 
'resets' is not listed as required here is because ASPEED handle reset in the clock driver
in ast2600/ast2500/..., etc. So I keep this part identical as previous aspeed-video.txt.

> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/ast2600-clock.h>
> > +
> > +    video@1e700000 {
> > +        compatible = "aspeed,ast2600-video-engine";
> 
> I guess it's not much of a concern that the example is different...
> 
> Andrew
> 
> > +        reg = <0x1e700000 0x1000>;
> > +        clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
> > +                 <&syscon ASPEED_CLK_GATE_ECLK>;
> > +        clock-names = "vclk", "eclk";
> > +        interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> > +    };
> > diff --git a/Documentation/devicetree/bindings/media/aspeed-video.txt
> > b/Documentation/devicetree/bindings/media/aspeed-video.txt
> > deleted file mode 100644
> > index d2ca32512272..000000000000
> > --- a/Documentation/devicetree/bindings/media/aspeed-video.txt
> > +++ /dev/null
> > @@ -1,33 +0,0 @@
> > -* Device tree bindings for Aspeed Video Engine
> > -
> > -The Video Engine (VE) embedded in the Aspeed AST2400/2500/2600 SOCs
> > can -capture and compress video data from digital or analog sources.
> > -
> > -Required properties:
> > - - compatible:         "aspeed,ast2400-video-engine" or
> > -                       "aspeed,ast2500-video-engine" or
> > -                       "aspeed,ast2600-video-engine"
> > - - reg:                        contains the offset and length of
> the
> > VE memory region
> > - - clocks:             clock specifiers for the syscon clocks
> > associated with
> > -                       the VE (ordering must match the
> clock-names
> > property)
> > - - clock-names:                "vclk" and "eclk"
> > - - resets:             reset specifier for the syscon reset
> > associated with
> > -                       the VE
> > - - interrupts:         the interrupt associated with the VE on this
> > platform
> > -
> > -Optional properties:
> > - - memory-region:
> > -       phandle to a memory region to allocate from, as defined in
> > -
> > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > -
> > -Example:
> > -
> > -video-engine@1e700000 {
> > -    compatible = "aspeed,ast2500-video-engine";
> > -    reg = <0x1e700000 0x20000>;
> > -    clocks = <&syscon ASPEED_CLK_GATE_VCLK>, <&syscon
> > ASPEED_CLK_GATE_ECLK>;
> > -    clock-names = "vclk", "eclk";
> > -    resets = <&syscon ASPEED_RESET_VIDEO>;
> > -    interrupts = <7>;
> > -    memory-region = <&video_engine_memory>; -}; diff --git
> > a/MAINTAINERS b/MAINTAINERS index 896a307fa065..7e59daa1e89d
> 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3549,7 +3549,7 @@ M:        Eddie James
> <eajames@linux.ibm.com>
> >  L:     linux-media@vger.kernel.org
> >  L:     openbmc@lists.ozlabs.org (moderated for non-subscribers)
> >  S:     Maintained
> > -F:     Documentation/devicetree/bindings/media/aspeed-video.txt
> > +F:
> > +Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
> >  F:     drivers/media/platform/aspeed/
> >
> >  ASUS EC HARDWARE MONITOR DRIVER
> >
> > base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b

Re: [PATCH v6] media: dt-bindings: aspeed,video-engine: Convert to json schema

[Andrew Jeffery]

On Mon, 2025-02-17 at 02:11 +0000, Jammy Huang wrote:
> Hi Andrew,
> 
> Thanks for your feedback, please find my explanation below.
> 
> > 
> > On Thu, 2025-02-13 at 09:53 +0800, Jammy Huang wrote:
> > > Convert aspeed-video.txt to yaml format.
> > > Update aspeed-video.txt to aspeed,video-engine.yaml in MAINTAINER file.
> > > 
> > > Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> > > 
...
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - clocks
> > > +  - clock-names
> > > +  - interrupts
> > 
> > This should list `resets` as well, as that wasn't optional in the text binding.
> > 
> 'resets' is not listed as required here is because ASPEED handle reset in the clock driver
> in ast2600/ast2500/..., etc. So I keep this part identical as previous aspeed-video.txt.

It's not identical though. The text binding listed the _optional_
properties, while the yaml binding lists the _required_ properties. The
reset property appears in neither list across two binding definitions,
but it has to be in one or the other for consistency.

However, it sounds like the text binding was not accurate, and this is
reflected in the video nodes in the SoC DTSIs. The yaml binding
definition at least matches that reality.

Andrew

Re: [PATCH v6] media: dt-bindings: aspeed,video-engine: Convert to json schema

[Andrew Jeffery]

On Thu, 2025-02-13 at 09:53 +0800, Jammy Huang wrote:
> Convert aspeed-video.txt to yaml format.
> Update aspeed-video.txt to aspeed,video-engine.yaml in MAINTAINER
> file.
> 
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>

Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>

Thanks Jammy.

Andrew