Hi Aakarsh Thanks for the patch. >-----Original Message----- >From: aakarsh jain [mailto:aakarsh.jain@xxxxxxxxxxx] >Sent: Tuesday, October 11, 2022 5:55 PM >To: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; linux- >kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx >Cc: m.szyprowski@xxxxxxxxxxx; andrzej.hajda@xxxxxxxxx; >mchehab@xxxxxxxxxx; hverkuil-cisco@xxxxxxxxx; >ezequiel@xxxxxxxxxxxxxxxxxxxx; jernej.skrabec@xxxxxxxxx; >benjamin.gaignard@xxxxxxxxxxxxx; stanimir.varbanov@xxxxxxxxxx; >dillon.minfei@xxxxxxxxx; david.plowman@xxxxxxxxxxxxxxx; >mark.rutland@xxxxxxx; robh+dt@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; >andi@xxxxxxxxxxx; alim.akhtar@xxxxxxxxxxx; aswani.reddy@xxxxxxxxxxx; >pankaj.dubey@xxxxxxxxxxx; linux-fsd@xxxxxxxxx; smitha.t@xxxxxxxxxxx; >aakarsh.jain@xxxxxxxxxxx >Subject: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT schema for >MFC > >From: Smitha T Murthy <smitha.t@xxxxxxxxxxx> > >Convert DT schema for s5p-mfc in yaml format > >Cc: linux-fsd@xxxxxxxxx >Signed-off-by: Smitha T Murthy <smitha.t@xxxxxxxxxxx> >Signed-off-by: Aakarsh Jain <aakarsh.jain@xxxxxxxxxxx> >--- > .../devicetree/bindings/media/s5p-mfc.txt | 75 -------- > .../bindings/media/samsung,s5p-mfc.yaml | 163 ++++++++++++++++++ > 2 files changed, 163 insertions(+), 75 deletions(-) create mode 100644 >Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml > >diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt >b/Documentation/devicetree/bindings/media/s5p-mfc.txt >index aa54c8159d9f..8b137891791f 100644 >--- a/Documentation/devicetree/bindings/media/s5p-mfc.txt >+++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt >@@ -1,76 +1 @@ >-* Samsung Multi Format Codec (MFC) > >-Multi Format Codec (MFC) is the IP present in Samsung SoCs which -supports >high resolution decoding and encoding functionalities. >-The MFC device driver is a v4l2 driver which can encode/decode -video >raw/elementary streams and has support for all popular -video codecs. >- >-Required properties: >- - compatible : value should be either one among the following >- (a) "samsung,mfc-v5" for MFC v5 present in Exynos4 SoCs >- (b) "samsung,mfc-v6" for MFC v6 present in Exynos5 SoCs >- (c) "samsung,mfc-v7" for MFC v7 present in Exynos5420 SoC >- (d) "samsung,mfc-v8" for MFC v8 present in Exynos5800 SoC >- (e) "samsung,exynos5433-mfc" for MFC v8 present in Exynos5433 SoC >- (f) "samsung,mfc-v10" for MFC v10 present in Exynos7880 SoC >- >- - reg : Physical base address of the IP registers and length of memory >- mapped region. >- >- - interrupts : MFC interrupt number to the CPU. >- - clocks : from common clock binding: handle to mfc clock. >- - clock-names : from common clock binding: must contain "mfc", >- corresponding to entry in the clocks property. >- >-Optional properties: >- - power-domains : power-domain property defined with a phandle >- to respective power domain. >- - memory-region : from reserved memory binding: phandles to two >reserved >- memory regions, first is for "left" mfc memory bus interfaces, >- second if for the "right" mfc memory bus, used when no SYSMMU >- support is available; used only by MFC v5 present in Exynos4 SoCs >- >-Obsolete properties: >- - samsung,mfc-r, samsung,mfc-l : support removed, please use memory- >region >- property instead >- >- >-Example: >-SoC specific DT entry: >- >-mfc: codec@13400000 { >- compatible = "samsung,mfc-v5"; >- reg = <0x13400000 0x10000>; >- interrupts = <0 94 0>; >- power-domains = <&pd_mfc>; >- clocks = <&clock 273>; >- clock-names = "mfc"; >-}; >- >-Reserved memory specific DT entry for given board (see reserved memory >binding -for more information): >- >-reserved-memory { >- #address-cells = <1>; >- #size-cells = <1>; >- ranges; >- >- mfc_left: region@51000000 { >- compatible = "shared-dma-pool"; >- no-map; >- reg = <0x51000000 0x800000>; >- }; >- >- mfc_right: region@43000000 { >- compatible = "shared-dma-pool"; >- no-map; >- reg = <0x43000000 0x800000>; >- }; >-}; >- >-Board specific DT entry: >- >-codec@13400000 { >- memory-region = <&mfc_left>, <&mfc_right>; >-}; >diff --git a/Documentation/devicetree/bindings/media/samsung,s5p- >mfc.yaml b/Documentation/devicetree/bindings/media/samsung,s5p- >mfc.yaml >new file mode 100644 >index 000000000000..ad61b509846f >--- /dev/null >+++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml >@@ -0,0 +1,163 @@ >+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2 >+--- >+$id: http://devicetree.org/schemas/media/samsung,s5p-mfc.yaml# >+$schema: http://devicetree.org/meta-schemas/core.yaml# >+ >+title: Samsung Exynos Multi Format Codec (MFC) >+ >+maintainers: >+ - Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> >+ - Smitha T Murthy <smitha.t@xxxxxxxxxxx> >+ - Aakarsh Jain <aakarsh.jain@xxxxxxxxxxx> >+ >+description: >+ Multi Format Codec (MFC) is the IP present in Samsung SoCs which >+ supports high resolution decoding and encoding functionalities. >+ >+properties: >+ compatible: >+ enum: >+ - samsung,mfc-v5 # Exynos4 >+ - samsung,mfc-v6 # Exynos5 >+ - samsung,mfc-v7 # Exynos5420 >+ - samsung,mfc-v8 # Exynos5800 >+ - samsung,exynos5433-mfc # Exynos5433 >+ - samsung,mfc-v10 # Exynos7880 >+ >+ reg: >+ maxItems: 1 >+ >+ clocks: >+ minItems: 1 >+ maxItems: 3 >+ >+ clock-names: >+ minItems: 1 >+ maxItems: 3 >+ >+ interrupts: >+ maxItems: 1 >+ >+ iommus: >+ maxItems: 2 >+ >+ iommu-names: >+ maxItems: 2 >+ >+ power-domains: >+ maxItems: 1 >+ >+ memory-region: >+ maxItems: 1 >+ >+required: >+ - compatible >+ - reg >+ - clocks >+ - clock-names >+ - interrupts >+ >+allOf: >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - samsung,mfc-v6 >+ - samsung,mfc-v7 >+ - samsung,mfc-v8 >+ - tesla,fsd-mfc You have not introduce "tesla,fsd-mfc" compatible yet, so this should be part of the patch which adds fsd-mfc compatible >+ then: >+ properties: >+ clocks: >+ maxItems: 1 >+ clock-names: >+ items: >+ - const: mfc >+ >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - samsung,mfc-v5 >+ then: >+ properties: >+ clocks: >+ minItems: 2 >+ maxItems: 2 >+ clock-names: >+ items: >+ - const: mfc >+ - const: sclk_mfc >+ >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - samsung,exynos5433-mfc >+ then: >+ properties: >+ clocks: >+ minItems: 3 >+ maxItems: 3 >+ clock-names: >+ items: >+ - const: pclk >+ - const: aclk >+ - const: aclk_xiu >+ >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - samsung,mfc-v5 >+ - samsung,mfc-v6 >+ - samsung,mfc-v7 >+ - samsung,mfc-v8 >+ - samsung,exynos5433-mfc >+ >+ then: >+ properties: >+ iommus: >+ minItems: 2 >+ maxItems: 2 >+ iommu-names: >+ items: >+ - const: left >+ - const: right >+ >+ - if: >+ properties: >+ compatible: >+ contains: >+ enum: >+ - tesla,fsd-mfc Same comments as above. My suggestion, first just convert the current s5p-mfc.txt to yaml format and then add fsd-mfc support. That way it will be easy for reviewer to understand the changes. While reviewing and code walk through, I also noticed that exynos3250.dtsi and exynos5420.dtsi are using same compatible as "samsung,mfc-v7" but there "clocks" property is different. That doesn't look correct. This will cause issues when you convert s5p-mfc.txt to schema format. I don’t know why that was done that way, but surely this need to be _fixed_ first as h/w itself are different. So my suggestion is to fix exynos3250.dtsi and exynos5420.dtsi compatible and mfc driver for it first (as a separate patch series). You can use the same approach what was done for mfc-v8 and mfc-v8-5433. >+ then: >+ properties: >+ memory-region: >+ maxItems: 1 >+ >+additionalProperties: false >+ >+examples: >+ - | >+ #include <dt-bindings/clock/exynos4.h> >+ #include <dt-bindings/clock/exynos-audss-clk.h> >+ #include <dt-bindings/interrupt-controller/arm-gic.h> >+ #include <dt-bindings/interrupt-controller/irq.h> >+ >+ codec@13400000 { >+ compatible = "samsung,mfc-v5"; >+ reg = <0x13400000 0x10000>; >+ interrupts = <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH>; >+ power-domains = <&pd_mfc>; >+ clocks = <&clock CLK_MFC>, <&clock CLK_SCLK_MFC>; >+ clock-names = "mfc", "sclk_mfc"; >+ iommus = <&sysmmu_mfc_l>, <&sysmmu_mfc_r>; >+ iommu-names = "left", "right"; >+ >+ }; >-- >2.17.1