On 08/09/2025 19:52, Ariel D'Alessandro wrote: > 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: >>> 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. > >> >>> >>> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@xxxxxxxxxxxxx> >>> --- >>> .../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@xxxxxxxxxxxxx> >>> + >>> +description: >>> + Media Data Path is used for scaling and color space conversion. >>> + >>> +properties: >>> + compatible: >>> + oneOf: >>> + - items: >> >> Just enum, no items here > > See below. > >> >> >>> + - 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 > >> >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clocks: true >> >> No, there's no such syntax. Look at other bindings. > > Ack. > >> >> >>> + >>> + power-domains: >>> + maxItems: 1 >>> + >>> + iommus: >>> + description: | >> >> Drop | > > Ack. > >> >>> + 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. > >> >>> + 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. > >> >>> + >>> +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
