Hi Krzysztof,
Thanks for your feedback.
> 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.
OK.
>
> > + 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?
Sure, I will add information for these properties in later patch.
>
> > + 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@
Do you prefer other node-name??
>
> > + 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
Best regards,
Jammy
