Re: [PATCH v2 01/12] dt-bindings: media: Convert MediaTek mt8173-mdp bindings to DT schema

[Krzysztof Kozlowski <krzk@kernel.org>]

On Thu, Sep 11, 2025 at 12:09:50PM -0300, Ariel D'Alessandro wrote:
> Convert the existing text-based DT bindings for MediaTek MT8173 Media Data
> Path to a DT schema.
> 
> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> ---
>  .../bindings/media/mediatek,mt8173-mdp.yaml   | 169 ++++++++++++++++++
>  .../bindings/media/mediatek-mdp.txt           |  95 ----------
>  2 files changed, 169 insertions(+), 95 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8173-mdp.yaml
>  delete mode 100644 Documentation/devicetree/bindings/media/mediatek-mdp.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8173-mdp.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8173-mdp.yaml
> new file mode 100644
> index 0000000000000..8ca33a733c478
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mt8173-mdp.yaml
> @@ -0,0 +1,169 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/mediatek,mt8173-mdp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek MT8173 Media Data Path
> +
> +maintainers:
> +  - Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> +
> +description:
> +  Media Data Path is used for scaling and color space conversion.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - mediatek,mt8173-mdp-rdma
> +          - mediatek,mt8173-mdp-rsz
> +          - mediatek,mt8173-mdp-wdma
> +          - mediatek,mt8173-mdp-wrot

Why there is no mediatek,mt8173-mdp here? What does this compatible
represent?

> +      - items:
> +          - const: mediatek,mt8173-mdp-rdma

Still suspicious. Device cannot be simulatanously: compatible and not
compatible. This is not a well known cat that has superposition of two
states, whenenver you look the other way.

Maybe the old binding was incorrect, maybe the in-tree DTS is incorrect.
Whichever the reason, this must be investigated and documented, because
by standard rules this is wrong. Each wrong code needs very clear
explanations (and "someone did it" is not a good enough explanation).

> +          - const: mediatek,mt8173-mdp
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 2
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  iommus:
> +    maxItems: 1
> +
> +  mediatek,vpu:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle to Mediatek Video Processor Unit for HW Codec encode/decode and
> +      image processing.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - power-domains
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt8173-mdp-rdma
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Main clock
> +            - description: Mutex clock
> +    else:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Main clock
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - mediatek,mt8173-mdp-rdma
> +              - mediatek,mt8173-mdp-wdma
> +              - mediatek,mt8173-mdp-wrot
> +    then:
> +      required:
> +        - iommus
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt8173-mdp
> +    then:
> +      required:
> +        - mediatek,vpu
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/mt8173-clk.h>
> +    #include <dt-bindings/memory/mt8173-larb-port.h>
> +    #include <dt-bindings/power/mt8173-power.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        mdp_rdma0: rdma@14001000 {
> +            compatible = "mediatek,mt8173-mdp-rdma",
> +                         "mediatek,mt8173-mdp";
> +            reg = <0 0x14001000 0 0x1000>;
> +            clocks = <&mmsys CLK_MM_MDP_RDMA0>,
> +                     <&mmsys CLK_MM_MUTEX_32K>;
> +            power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
> +            iommus = <&iommu M4U_PORT_MDP_RDMA0>;
> +            mediatek,vpu = <&vpu>;
> +        };
> +
> +        mdp_rdma1: rdma@14002000 {
> +            compatible = "mediatek,mt8173-mdp-rdma";
> +            reg = <0 0x14002000 0 0x1000>;
> +            clocks = <&mmsys CLK_MM_MDP_RDMA1>,
> +                     <&mmsys CLK_MM_MUTEX_32K>;
> +            power-domains = <&spm MT8173_POWER_DOMAIN_MM>;
> +            iommus = <&iommu M4U_PORT_MDP_RDMA1>;
> +        };

My previous comment applies.

Keep one or two examples.

Best regards,
Krzysztof