Krzysztof,

On 8/21/25 3:46 AM, Krzysztof Kozlowski wrote:

On Wed, Aug 20, 2025 at 02:12:49PM -0300, Ariel D'Alessandro wrote:

quoted

Convert the existing text-based DT bindings for MediaTek MT8173 Media Data Path
to a YAML schema.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

Thanks. Looks like my editor was misconfigured, sorry. Will fix in v2.

quoted

Signed-off-by: Ariel D'Alessandro <redacted>
---
  .../bindings/media/mediatek,mt8173-mdp.yaml   | 174 ++++++++++++++++++
  .../bindings/media/mediatek-mdp.txt           |  95 ----------
  2 files changed, 174 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..f3a08afc305b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/mediatek,mt8173-mdp.yaml

@@ -0,0 +1,174 @@
+# 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:
+      - items:

Just enum, no items here

See below.

quoted

+          - enum:
+              - mediatek,mt8173-mdp-rdma
+              - mediatek,mt8173-mdp-rsz
+              - mediatek,mt8173-mdp-wdma
+              - mediatek,mt8173-mdp-wrot
+      - items:
+          - enum:
+              - mediatek,mt8173-mdp-rdma
+              - mediatek,mt8173-mdp-rsz
+              - mediatek,mt8173-mdp-wdma
+              - mediatek,mt8173-mdp-wrot
+          - const: mediatek,mt8173-mdp

This makes no sense. How devices can be compatible and can not be
compatible.

According to the driver source code (and the previous txt mt8173-mdp 
bindings), there must be a "controller node" with compatible 
`mediatek,mt8173-mdp`. Then its sibling nodes (including itself) should 
be one of the component node ids, listed in `struct of_device_id 
mtk_mdp_comp_dt_ids[]`.

Is there a proper/different way to describe this compatible binding in 
the yaml? Or you're saying the driver doesn't make sense here?

[0] drivers/media/platform/mediatek/mdp/mtk_mdp_core.c

quoted

+
+  reg:
+    maxItems: 1
+
+  clocks: true

No, there's no such syntax. Look at other bindings.

Ack.

quoted

+
+  power-domains:
+    maxItems: 1
+
+  iommus:
+    description: |

Drop |

Ack.

quoted

+      This property should point to the respective IOMMU block with master port as argument,
+      see Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details.

Drop entire description, completely redundant. I don't know why my patch
fixing this was not applied, so you keep repeating same mistakes...

Ack.

quoted

+    maxItems: 1
+
+  mediatek,vpu:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Describes point to vpu.

Useless description. We see that from the property name. Explain the
purpose in the hardware.

Ack.

quoted

+
+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

This makes no sense either.

Same question above about compatibles.

quoted

+    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 {

One example is enough. Two could be fine if they differ significantly.

Sounds good. Will keep just a single example, including a node for the 
controller node and one for each of the components.

Thanks a lot for the feedback!

-- 
Ariel D'Alessandro
Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK 
Registered in England & Wales, no. 5513718