diff mbox series

[v6,1/2] dt-bindings: media: convert aspeed-video.txt to dt-schema

Message ID 20240829064508.3706672-2-jammy_huang@aspeedtech.com
State New
Headers show
Series media: aspeed: Allow to capture from SoC display (GFX) | expand

Commit Message

Jammy Huang Aug. 29, 2024, 6:45 a.m. UTC
Convert the ASPEED SoCs video txt bindings to dt-schema.

Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
---
 .../bindings/media/aspeed,video-engine.yaml   | 78 +++++++++++++++++++
 .../bindings/media/aspeed-video.txt           | 33 --------
 2 files changed, 78 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
 delete mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt

Comments

Krzysztof Kozlowski Aug. 29, 2024, 7:55 a.m. UTC | #1
On 29/08/2024 08:45, Jammy Huang wrote:
> Convert the ASPEED SoCs video txt bindings to dt-schema.
> 
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>  .../bindings/media/aspeed,video-engine.yaml   | 78 +++++++++++++++++++
>  .../bindings/media/aspeed-video.txt           | 33 --------
>  2 files changed, 78 insertions(+), 33 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
>  delete mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt

Fix the paths in kernel (git grep).

...

> +
> +  resets:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  memory-region:
> +    description: |
> +      Phandle to a memory region to allocate from, as defined in
> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

Useless description, completely redundant. Please say something useful
about this particular memory region and its usage.

Missing maxItems.

> +
> +  aspeed,scu:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      Specifies the scu node that is needed if video wants to capture
> +      from sources other than Host VGA.
> +
> +  aspeed,gfx:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      Specifies the Soc Display(gfx) node that needs to be queried to get
> +      related information if video wants to use gfx as capture source.

These two were not in the binding. Mention in the commit msg any changes
from pure conversion with rationale WHY you are changing the binding.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +
> +additionalProperties: false
Best regards,
Krzysztof
Rob Herring Aug. 29, 2024, 8:23 a.m. UTC | #2
On Thu, 29 Aug 2024 14:45:07 +0800, Jammy Huang wrote:
> Convert the ASPEED SoCs video txt bindings to dt-schema.
> 
> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> ---
>  .../bindings/media/aspeed,video-engine.yaml   | 78 +++++++++++++++++++
>  .../bindings/media/aspeed-video.txt           | 33 --------
>  2 files changed, 78 insertions(+), 33 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
>  delete mode 100644 Documentation/devicetree/bindings/media/aspeed-video.txt
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/media/aspeed,video-engine.example.dts:27.29-30 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:442: Documentation/devicetree/bindings/media/aspeed,video-engine.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1432: dt_binding_check] Error 2
make: *** [Makefile:224: __sub-make] Error 2

doc reference errors (make refcheckdocs):
Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/media/aspeed-video.txt
MAINTAINERS: Documentation/devicetree/bindings/media/aspeed-video.txt

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240829064508.3706672-2-jammy_huang@aspeedtech.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
kernel test robot Aug. 30, 2024, 12:21 a.m. UTC | #3
Hi Jammy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 47ac09b91befbb6a235ab620c32af719f8208399]

url:    https://github.com/intel-lab-lkp/linux/commits/Jammy-Huang/dt-bindings-media-convert-aspeed-video-txt-to-dt-schema/20240829-144721
base:   47ac09b91befbb6a235ab620c32af719f8208399
patch link:    https://lore.kernel.org/r/20240829064508.3706672-2-jammy_huang%40aspeedtech.com
patch subject: [PATCH v6 1/2] dt-bindings: media: convert aspeed-video.txt to dt-schema
reproduce: (https://download.01.org/0day-ci/archive/20240830/202408300813.2RN73Kn4-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408300813.2RN73Kn4-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Warning: Documentation/devicetree/bindings/power/wakeup-source.txt references a file that doesn't exist: Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
   Warning: Documentation/devicetree/bindings/regulator/siliconmitus,sm5703-regulator.yaml references a file that doesn't exist: Documentation/devicetree/bindings/mfd/siliconmitus,sm5703.yaml
   Warning: Documentation/hwmon/g762.rst references a file that doesn't exist: Documentation/devicetree/bindings/hwmon/g762.txt
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/reserved-memory/qcom
>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/media/aspeed-video.txt
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/display/exynos/
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
   Using alabaster theme
Jammy Huang Sept. 2, 2024, 12:43 a.m. UTC | #4
Hi Rob,

I will do 'make dt_binding_check' before my next patch to avoid this kind of error.
Thanks for your reminder.

On 2024/8/29 下午 04:23, Rob Herring (Arm) wrote:
> On Thu, 29 Aug 2024 14:45:07 +0800, Jammy Huang wrote:
> > Convert the ASPEED SoCs video txt bindings to dt-schema.
> >
> > Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> > ---
> >  .../bindings/media/aspeed,video-engine.yaml   | 78
> +++++++++++++++++++
> >  .../bindings/media/aspeed-video.txt           | 33 --------
> >  2 files changed, 78 insertions(+), 33 deletions(-)  create mode
> > 100644
> > Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
> >  delete mode 100644
> > Documentation/devicetree/bindings/media/aspeed-video.txt
> >
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Error:
> Documentation/devicetree/bindings/media/aspeed,video-engine.example.dts:
> 27.29-30 syntax error FATAL ERROR: Unable to parse input tree
> make[2]: *** [scripts/Makefile.lib:442:
> Documentation/devicetree/bindings/media/aspeed,video-engine.example.dtb]
> Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1432:
> dt_binding_check] Error 2
> make: *** [Makefile:224: __sub-make] Error 2
> 
> doc reference errors (make refcheckdocs):
> Warning: MAINTAINERS references a file that doesn't exist:
> Documentation/devicetree/bindings/media/aspeed-video.txt
> MAINTAINERS: Documentation/devicetree/bindings/media/aspeed-video.txt
> 
> See
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/202408290645
> 08.3706672-2-jammy_huang@aspeedtech.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above error(s),
> then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
Jammy Huang Sept. 2, 2024, 1:44 a.m. UTC | #5
Hi Krzysztof,


On 2024/8/29 下午 03:56, Krzysztof Kozlowski wrote:
> 
> On 29/08/2024 08:45, Jammy Huang wrote:
> > Convert the ASPEED SoCs video txt bindings to dt-schema.
> >
> > Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
> > ---
> >  .../bindings/media/aspeed,video-engine.yaml   | 78
> +++++++++++++++++++
> >  .../bindings/media/aspeed-video.txt           | 33 --------
> >  2 files changed, 78 insertions(+), 33 deletions(-)  create mode
> > 100644
> > Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
> >  delete mode 100644
> > Documentation/devicetree/bindings/media/aspeed-video.txt
> 
> Fix the paths in kernel (git grep).
Sorry, I don't know exactly how to fix that.
Do you mean I need to change the file name from 'drivers/media/platform/aspeed/aspeed-video.c'
to ' drivers/media/platform/aspeed/aspeed-video-engine.c'???

> 
> ...
> 
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  memory-region:
> > +    description: |
> > +      Phandle to a memory region to allocate from, as defined in
> > +
> > +
> Documentation/devicetree/bindings/reserved-memory/reserved-memory.tx
> > + t
> 
> Useless description, completely redundant. Please say something useful about
> this particular memory region and its usage.
> 
> Missing maxItems.
OK, more information will be added for the usage of 'memory-region' in ASPEED VE.
> 
> > +
> > +  aspeed,scu:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: |
> > +      Specifies the scu node that is needed if video wants to capture
> > +      from sources other than Host VGA.
> > +
> > +  aspeed,gfx:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: |
> > +      Specifies the Soc Display(gfx) node that needs to be queried to get
> > +      related information if video wants to use gfx as capture source.
> 
> These two were not in the binding. Mention in the commit msg any changes
> from pure conversion with rationale WHY you are changing the binding.
OK, I will split the patch.

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - interrupts
> > +
> > +additionalProperties: false
> Best regards,
> Krzysztof

Best regards,
Jammy
Krzysztof Kozlowski Sept. 2, 2024, 6:57 a.m. UTC | #6
On 02/09/2024 03:44, Jammy Huang wrote:
> Hi Krzysztof,
> 
> 
> On 2024/8/29 下午 03:56, Krzysztof Kozlowski wrote:
>>
>> On 29/08/2024 08:45, Jammy Huang wrote:
>>> Convert the ASPEED SoCs video txt bindings to dt-schema.
>>>
>>> Signed-off-by: Jammy Huang <jammy_huang@aspeedtech.com>
>>> ---
>>>  .../bindings/media/aspeed,video-engine.yaml   | 78
>> +++++++++++++++++++
>>>  .../bindings/media/aspeed-video.txt           | 33 --------
>>>  2 files changed, 78 insertions(+), 33 deletions(-)  create mode
>>> 100644
>>> Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
>>>  delete mode 100644
>>> Documentation/devicetree/bindings/media/aspeed-video.txt
>>
>> Fix the paths in kernel (git grep).
> Sorry, I don't know exactly how to fix that.
> Do you mean I need to change the file name from 'drivers/media/platform/aspeed/aspeed-video.c'
> to ' drivers/media/platform/aspeed/aspeed-video-engine.c'???

No. You have warnings for stale paths.

Best regards,
Krzysztof
diff mbox series

Patch

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..837e15edb9b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/aspeed,video-engine.yaml
@@ -0,0 +1,78 @@ 
+# 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>
+  - Jammy Huang <jammy_huang@aspeedtech.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:
+    description: |
+      Phandle to a memory region to allocate from, as defined in
+      Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+
+  aspeed,scu:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      Specifies the scu node that is needed if video wants to capture
+      from sources other than Host VGA.
+
+  aspeed,gfx:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      Specifies the Soc Display(gfx) node that needs to be queried to get
+      related information if video wants to use gfx as capture source.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    video-engine@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>;
+      aspeed,scu = <&syscon>;
+      aspeed,gfx = <&gfx>;
+    };
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>;
-};