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

From: Krzysztof Kozlowski
Date: Tue Feb 04 2025 - 05:47:14 EST


On 04/02/2025 09:50, Jammy Huang wrote:
> Convert aspeed-video.txt to yaml format.
> Update aspeed-video.txt to aspeed,video-engine.yaml in MAINTAINER file.
>
> NOTE: The following checkpatch warning is generated since we do include
> the header in the example, but this is a false positive warning.
> WARNING: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst

This does not belong to commit message, it's irrelevant. You can mention
in the changelog, though, but IMO, that's really not important for this
patch.

>
> Signed-off-by: Jammy Huang <jammy_huang@xxxxxxxxxxxxxx>
> ---
> v2:
> - Update commit subject
> - Add NOTE for false positive warning
> ---

...

> +
> + 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
> +
> + aspeed,scu:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: |

Do not need '|' unless you need to preserve formatting.

> + 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

None of these two properties were in original binding with
*justification*. You must describe any changes done during conversion,
otherwise you could add here whatever property you wish, right?

> + 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:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/ast2600-clock.h>
> +
> + video@1e700000 {

Looks rather like: isp@

> + 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>;



Best regards,
Krzysztof