> -----Original Message----- > From: Alim Akhtar [mailto:alim.akhtar@xxxxxxxxxxx] > Sent: 27 October 2022 16:10 > To: 'aakarsh jain' <aakarsh.jain@xxxxxxxxxxx>; 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; aswani.reddy@xxxxxxxxxxx; > pankaj.dubey@xxxxxxxxxxx; linux-fsd@xxxxxxxxx; smitha.t@xxxxxxxxxxx > Subject: RE: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT > schema for MFC > > 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 > ok. > >+ 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. > > ok will fix this issue . > >+ 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 > > Thanks for the review.
