[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <uld753xxm3ryp2iav6zgb47m6parp6vqz7mpflmdiftpgg6iim@7menlew7pycg>
Date: Tue, 17 Jun 2025 06:33:49 +0000
From: Yassine OUAISSA <yassine.ouaissa@...egrodvt.com>
To: Michael Tretter <m.tretter@...gutronix.de>,
Mauro Carvalho Chehab <mchehab@...nel.org>, Pengutronix Kernel Team <kernel@...gutronix.de>,
Michal Simek <michal.simek@....com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Nicolas Dufresne <nicolas@...fresne.ca>, linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 2/4] dt-bindings: media: allegro-dvt: add decoder
dt-bindings for Gen3 IP
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@...egrodvt.com>
>>
>> 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@allegrodvt.com/
>>
>> Signed-off-by: Yassine Ouaissa <yassine.ouaissa@...egrodvt.com>
>> ---
>> .../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@...egrodvt.com>
>> +
>> +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@...20000 {
>> + 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@...gutronix.de>
>> R: Pengutronix Kernel Team <kernel@...gutronix.de>
>> L: linux-media@...r.kernel.org
>> 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
Powered by blists - more mailing lists