Thread (51 messages) 51 messages, 6 authors, 2025-09-11

Re: [PATCH v1 01/14] media: dt-bindings: Convert MediaTek mt8173-mdp bindings to YAML

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-09-09 06:32:29
Also in: dri-devel, linux-arm-kernel, linux-clk, linux-devicetree, linux-gpio, linux-input, linux-media, linux-mediatek, linux-sound, lkml

On 08/09/2025 19:52, Ariel D'Alessandro wrote:
Krzysztof,

On 8/21/25 3:46 AM, Krzysztof Kozlowski wrote:
quoted
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
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
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 
But you did not define "mediatek,mt8173-mdp" here, so what are you
talking about?

I talk here about "wrot" and others, I thought it is obvious from the
mistake in the schema.

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
quoted
+
+  reg:
+    maxItems: 1
+
+  clocks: true
No, there's no such syntax. Look at other bindings.
Ack.
quoted
quoted
+
+  power-domains:
+    maxItems: 1
+
+  iommus:
+    description: |
Drop |
Ack.
quoted
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
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
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.
How same question? Do you understand this code? It is nothing the same -
you have here contains!


Best regards,
Krzysztof
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help