Re: [PATCH 1/7] dt-bindings: media: Document STM32MP25 VDEC video decoder

[Hugues FRUCHET]

Hi Krzysztof,

On 10/5/23 21:45, Krzysztof Kozlowski wrote:
> On 04/10/2023 11:15, Hugues Fruchet wrote:
> > Add STM32MP25 VDEC video decoder bindings.
> >
> > Signed-off-by: Hugues Fruchet <hugues.fruchet@xxxxxxxxxxx>
> > ---
> >   .../bindings/media/st,stm32mp25-vdec.yaml     | 56 +++++++++++++++++++
> >   1 file changed, 56 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/media/st,stm32mp25-vdec.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/st,stm32mp25-vdec.yaml b/Documentation/devicetree/bindings/media/st,stm32mp25-vdec.yaml
> > new file mode 100644
> > index 000000000000..cf41f704113f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/st,stm32mp25-vdec.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-vdec.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: STMicroelectronics STM32MP25 VDEC video decoder
> > +
> > +maintainers:
> > +  - Hugues Fruchet <hugues.fruchet@xxxxxxxxxxx>
> > +
> > +description:
> > +  The STMicroelectronics STM32MP25 SOCs embeds a VDEC video hardware decoder
> > +  peripheral based on Verisilicon VC8000NanoD IP (former Hantro G1).
> > +
> > +properties:
> > +  compatible:
> > +    const: st,stm32mp25-vdec
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-names:
> > +    maxItems: 1
>
> This must be specific or just drop (little use of names for one entry)
I'll drop in v2.

> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    maxItems: 1
>
> Same problem.
I'll drop in v2.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-names
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    vdec: vdec@580d0000 {
>
> Drop label. Node name: video-codec
> (assuming this is video coder/decoder)
>
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
I'll do in v2, thanks for pointing-out the "video-codec" generic name.

> Best regards,
> Krzysztof

BR,
Hugues.