Re: [PATCH v5 1/3] media: dt-bindings: media: mediatek: Rename child node names for decoder

From: "Nícolas F. R. A. Prado" <nfraprado@collabora.com>
To: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Project_Global_Chrome_Upstream_Group@mediatek.com,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	yunfei.dong@mediatek.com
Subject: Re: [PATCH v5 1/3] media: dt-bindings: media: mediatek: Rename child node names for decoder
Date: Mon, 5 Dec 2022 18:32:04 -0300	[thread overview]
Message-ID: <20221205213204.ftnarhtsk33pprq3@notapiano> (raw)
In-Reply-To: <20221128143832.25584-2-allen-kh.cheng@mediatek.com>

On Mon, Nov 28, 2022 at 10:38:30PM +0800, Allen-KH Cheng wrote:
> In order to make the names of the child nodes more generic, we rename
> "vcodec-lat" and "vcodec-core" to "video-codec" for decoder in
> patternProperties and example.
> 
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
> ---
>  .../media/mediatek,vcodec-subdev-decoder.yaml | 60 ++-----------------
>  1 file changed, 4 insertions(+), 56 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> index c4f20acdc1f8..695402041e04 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml
> @@ -91,12 +91,13 @@ properties:
>  
>  # Required child node:
>  patternProperties:
> -  '^vcodec-lat@[0-9a-f]+$':
> +  '^video-codec@[0-9a-f]+$':
>      type: object
>  
>      properties:
>        compatible:
>          enum:
> +          - mediatek,mtk-vcodec-core
>            - mediatek,mtk-vcodec-lat
>            - mediatek,mtk-vcodec-lat-soc
>  
> @@ -145,59 +146,6 @@ patternProperties:
>  
>      additionalProperties: false
>  
> -  '^vcodec-core@[0-9a-f]+$':
> -    type: object
> -
> -    properties:
> -      compatible:
> -        const: mediatek,mtk-vcodec-core
> -
> -      reg:
> -        maxItems: 1
> -
> -      interrupts:
> -        maxItems: 1
> -
> -      iommus:
> -        minItems: 1
> -        maxItems: 32
> -        description: |
> -          List of the hardware port in respective IOMMU block for current Socs.
> -          Refer to bindings/iommu/mediatek,iommu.yaml.
> -
> -      clocks:
> -        maxItems: 5
> -
> -      clock-names:
> -        items:
> -          - const: sel
> -          - const: soc-vdec
> -          - const: soc-lat
> -          - const: vdec
> -          - const: top
> -
> -      assigned-clocks:
> -        maxItems: 1
> -
> -      assigned-clock-parents:
> -        maxItems: 1
> -
> -      power-domains:
> -        maxItems: 1
> -
> -    required:
> -      - compatible
> -      - reg
> -      - interrupts

Looks like interrupts was required for vcodec-core, but it isn't for the generic
video-codec node. Which seems correct, given that the vcodec-lat-soc doesn't
have an interrupt [1]. So I guess this is just the generic video-codec node in
the binding being too generic for some cases. Ideally we would override
interrupts to be required based on which subnode we're dealing with (for lat and
core, but not lat-soc), but given these are subnodes matched through
patternProperties, I'm not sure that would be possible.

[1] https://lore.kernel.org/all/20221202034450.3808-3-yunfei.dong@mediatek.com/

Thanks,
Nícolas

> -      - iommus
> -      - clocks
> -      - clock-names
> -      - assigned-clocks
> -      - assigned-clock-parents
> -      - power-domains
> -
> -    additionalProperties: false
> -