linux-kernel - Re: [PATCH 3/7] dt-bindings: media: Document STM32MP25 VENC video encoder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <866fd143-a290-63ec-103c-b49368d9dc03@foss.st.com>
Date:   Thu, 5 Oct 2023 09:44:47 +0200
From:   Hugues FRUCHET <hugues.fruchet@...s.st.com>
To:     Adam Ford <aford173@...il.com>
CC:     Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Hans Verkuil <hverkuil@...all.nl>,
        <linux-media@...r.kernel.org>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        <linux-stm32@...md-mailman.stormreply.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-rockchip@...ts.infradead.org>,
        Andrzej Pietrasiewicz <andrzej.p@...labora.com>
Subject: Re: [PATCH 3/7] dt-bindings: media: Document STM32MP25 VENC video
 encoder

Hi Adam,

Thanks for review,

On 10/5/23 01:41, Adam Ford wrote:
> On Wed, Oct 4, 2023 at 4:16 AM Hugues Fruchet
> <hugues.fruchet@...s.st.com> wrote:
>>
>> Add STM32MP25 VENC video encoder bindings.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@...s.st.com>
>> ---
>>   .../bindings/media/st,stm32mp25-venc.yaml     | 56 +++++++++++++++++++
>>   1 file changed, 56 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/media/st,stm32mp25-venc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/media/st,stm32mp25-venc.yaml b/Documentation/devicetree/bindings/media/st,stm32mp25-venc.yaml
>> new file mode 100644
>> index 000000000000..c69e0a34f675
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/st,stm32mp25-venc.yaml
>> @@ -0,0 +1,56 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/st,stm32mp25-venc.yaml#
> 
> Can this dt-binding be made more generic, like something like
> hantro-h1 or VC8000NanoE?
> 
> I think there will be more boards that may incorporate the Hantro-H1
> or a VC8000 in the future, because I don't think this IP is unique to
> the STM32MP25.

This is already the case, check variants in hantro_drv.c.
Several SoCs are sharing this IP but each IP slightly differs because of
supported resolution, codec, preprocessing features, ...
There are also some differences on how clock, interrupt, reset are 
hardware mapped: shared or not by decoder and encoder for ex.

> 
> adam
> 
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: STMicroelectronics STM32MP25 VENC video encoder
>> +
>> +maintainers:
>> +  - Hugues Fruchet <hugues.fruchet@...s.st.com>
>> +
>> +description:
>> +  The STMicroelectronics STM32MP25 SOCs embeds a VENC video hardware encoder
>> +  peripheral based on Verisilicon VC8000NanoE IP (former Hantro H1).
>> +
>> +properties:
>> +  compatible:
>> +    const: st,stm32mp25-venc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  interrupt-names:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  clock-names:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - interrupt-names
>> +  - clocks
>> +  - clock-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    venc: venc@...e0000 {
>> +        compatible = "st,stm32mp25-venc";
>> +        reg = <0x580e0000 0x800>;
>> +        interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
>> +        interrupt-names = "venc";
> 
> 
> Is the interrupt-names needed if there is only one?
> 

Not really, could be dropped.

>> +        clocks = <&ck_icn_p_venc>;
>> +        clock-names = "venc-clk";
> 
> Same thing for the clock.  if there is only one clock, doe they need names?
> 
Not really, could be dropped.

> adam
>> +    };
>> --
>> 2.25.1
>>

BR,
Hugues.