Re: [PATCH v2 2/4] dt-bindings: media: allegro-dvt: add decoder dt-bindings for Gen3 IP

[Yassine OUAISSA]

On 12.06.2025 14:42, Michael Tretter wrote:
> On Thu, 05 Jun 2025 12:26:57 +0000, Yassine Ouaissa via B4 Relay wrote:
> > From: Yassine Ouaissa <yassine.ouaissa@xxxxxxxxxxxxxx>
> >
> > Add compatible for video decoder on allegrodvt Gen 3 IP.
> >
> > v2:
> > - Change the YAML file name, use the existing vendor-prefix.
> > - Improuve the dt-bindings description.
> > - Change the device compatible identifier, from "allegrodvt, al300-vdec",
> >   to "allegro, al300-vdec"
> > - Simplify the register property specification,
> >   by using the simple min/max items constraint (Krzysztof Kozlowski)
> > - Remove the clock-names property. And remove it from the required
> >   properties list (Krzysztof Kozlowski) (Conor Dooley)
> > - Use the simple maxItems constraint for the memory-region property.
> >   Also for the firmware-name (Krzysztof Kozlowski)
> > - Example changes:
> >   - Use header provides definitions for the interrupts (Conor Dooley)
> >   - Improuve Interrupt specification using GIC constants (Conor Dooley)
> >   - Use generic node name "video-decoder" (Krzysztof Kozlowski) (Conor Dooley)
> >   - Remove unused label (Krzysztof Kozlowski)
> >   - Change clock reference from <&mcu_clock_dec> to <&mcu_core_clk>
> >   - Use hex format for reg property (Krzysztof Kozlowski) (Conor Dooley)
> >   - Reduce memory region size (Krzysztof Kozlowski) (Conor Dooley)
> >
> >   - Link v1: https://patchwork.linuxtv.org/project/linux-media/patch/20250511144752.504162-4-yassine.ouaissa@xxxxxxxxxxxxxx/
> >
> > Signed-off-by: Yassine Ouaissa <yassine.ouaissa@xxxxxxxxxxxxxx>
> > ---
> >  .../bindings/media/allegro,al300-vdec.yaml         | 75 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  2 +
> >  2 files changed, 77 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/allegro,al300-vdec.yaml b/Documentation/devicetree/bindings/media/allegro,al300-vdec.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..26f9ac39682431b1d4828aed5d1ed43ef099e204
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/allegro,al300-vdec.yaml
> > @@ -0,0 +1,75 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/allegro,al300-vdec.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allegro DVT Video IP Decoder Gen 3
> > +
> > +maintainers:
> > +  - Yassine OUAISSA <yassine.ouaissa@xxxxxxxxxxxxxx>
> > +
> > +description: The al300-vdec represents the gen 3 of Allegro DVT IP video
> > +  decoding technology, offering significant advancements over its
> > +  predecessors. This new decoder features enhanced processing capabilities
> > +  with improved throughput and reduced latency.
> > +
> > +  Communication between the host driver software and the MCU is implemented
> > +  through a specialized mailbox interface mechanism. This mailbox system
> > +  provides a structured channel for exchanging commands, parameters, and
> > +  status information between the host CPU and the MCU controlling the codec
> > +  engines.
> > +
> > +properties:
> > +  compatible:
> > +    const: allegro,al300-vdec
> > +
> > +  reg:
> > +    maxItems: 2
> > +    minItems: 2
> > +
> > +  reg-names:
> > +    items:
> > +      - const: regs
> > +      - const: apb
>
> If I understand correctly, "regs" are the registers to control the MCU
> and "apb" are the registers of the actual codec engines, which is
> controlled by the MCU. The driver never accesses the apb registers, but
> uses the apb address only to configure the firmware and tell it, where
> the registers of the codec engines are found.
>
> Maybe a separate node for the actual codec that is referred via a
> phandle could be a better description of the hardware?
the regs is actually is used to configure the decoder and also the mcu.
the APB address is used to map the mcu peripheral, which includes the
codec engine.

I think it's sampler to let it as reg.

the naming has changed in the v3.
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: MCU core clock
> > +
> > +  memory-region:
> > +    maxItems: 1
> > +
> > +  firmware-name:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - interrupts
> > +  - clocks
> > +
> > +additionalProperties: False
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    axi {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        video-decoder@a0120000 {
> > +            compatible = "allegro,al300-vdec";
> > +            reg = <0x00 0xa0120000 0x00 0x10000>,
> > +                  <0x01 0x80000000 0x00 0x8000>;
> > +            reg-names = "regs", "apb";
> > +            interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
> > +            clocks = <&mcu_core_clk>;
> > +            firmware-name = "al300_vdec.fw";
> > +        };
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index abc6ba61048771303bc219102f2db602266b7c30..1ff78b9a76cb8cdf32263fcd9b4579b4a2bb6b2a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -816,7 +816,9 @@ M:	Michael Tretter <m.tretter@xxxxxxxxxxxxxx>
> >  R:	Pengutronix Kernel Team <kernel@xxxxxxxxxxxxxx>
> >  L:	linux-media@xxxxxxxxxxxxxxx
> >  S:	Maintained
> > +F:	Documentation/devicetree/bindings/media/allegro,al300-vdec.yaml
> >  F:	Documentation/devicetree/bindings/media/allegro,al5e.yaml
> > +F:	drivers/media/platform/allegro-dvt/al300
> >  F:	drivers/media/platform/allegro-dvt/zynqmp
> >
> >  ALLIED VISION ALVIUM CAMERA DRIVER
> >
> > --
> > 2.30.2

Best regards,
Yassine OUAISSA