diff mbox series

[v2,01/15] dt-bindings: media: s5p-mfc: Add new DT schema for MFC

Message ID 20220907064715.55778-2-smitha.t@samsung.com (mailing list archive)
State New, archived
Headers show
Series Add MFC v12 support. | expand

Commit Message

Smitha T Murthy Sept. 7, 2022, 6:47 a.m. UTC
Adds DT schema for s5p-mfc in yaml format

Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
---
 .../devicetree/bindings/media/s5p-mfc.txt     |  77 +------------
 .../bindings/media/samsung,s5p-mfc.yaml       | 109 ++++++++++++++++++
 2 files changed, 110 insertions(+), 76 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml

Comments

Krzysztof Kozlowski Sept. 7, 2022, 11:22 a.m. UTC | #1
On 07/09/2022 08:47, Smitha T Murthy wrote:
> Adds DT schema for s5p-mfc in yaml format

s/Adds/Convert/
(as convert to DT schema)

Please mention here changes to original binding (I see at least adding
iommus and dropping some properties).

> 
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> ---
>  .../devicetree/bindings/media/s5p-mfc.txt     |  77 +------------
>  .../bindings/media/samsung,s5p-mfc.yaml       | 109 ++++++++++++++++++
>  2 files changed, 110 insertions(+), 76 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..0b7c4dd40095 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

When did they become obsolete? Is it enough of time to remove them?
> -
> -
> -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>;
> -};
> +This file has moved to samsung,s5p-mfc.yaml

Just drop the TXT completely. Nothing references it.

> 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..7cd26d4acbe4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
> @@ -0,0 +1,109 @@
> +# 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@samsung.com>
> +  - Aakarsh Jain <aakarsh.jain@samsung.com>

and maybe you as well?

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

You need to list the items. If this varies per compatible, do it in AllOf.

> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 3
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  iommus:
> +    maxItems: 2
> +
> +  iommu-names:
> +    maxItems: 2

You need to list the items.

> +
> +  power-domains:
> +    maxItems: 1
> +
> +  memory-region:
> +    maxItems: 1

This misses the description and old binding allowed it only for MFCv5,
not for others, right?

> +
> +allOf:
> +  - if:

allOf goes after required section.

> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - samsung,mfc-v5
> +    then:
> +      properties:
> +        memory-region:
> +          maxItems: 2

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

This won't work. Test it and you will see it.


> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    # SoC specific DT entry
> +    mfc: mfc@12880000 {
> +        compatible = "samsung,fsd-mfc";

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

> +        reg = <0x0 0x12880000 0x0 0x10000>;
> +        interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
> +        clock-names = "mfc";
> +        clocks = <&clock_mfc MFC_MFC_IPCLKPORT_ACLK>;
> +        iommus = <&smmu_isp 0x1000 0x0>, <&smmu_isp 0x1400 0x0>;
> +        iommu-names = "left", "right";
> +        power-domains = <&pd_mfc>;
> +        memory-region = <&mfc_left>, <&mfc_right>;
> +    };
> +
> +  - |
> +    # Reserved memory specific DT entry for given board
> +    # (see reserved memory binding for more information)
> +    reserved-memory {
> +        #address-cells = <1>;
> +        #size-cells = <1>;

Drop this example, not really related to MFC.

> +        ranges;


Best regards,
Krzysztof
Rob Herring (Arm) Sept. 7, 2022, 3:13 p.m. UTC | #2
On Wed, 07 Sep 2022 12:17:01 +0530, Smitha T Murthy wrote:
> Adds DT schema for s5p-mfc in yaml format
> 
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> ---
>  .../devicetree/bindings/media/s5p-mfc.txt     |  77 +------------
>  .../bindings/media/samsung,s5p-mfc.yaml       | 109 ++++++++++++++++++
>  2 files changed, 110 insertions(+), 76 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/media/samsung,s5p-mfc.example.dts:21.11-12 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:384: Documentation/devicetree/bindings/media/samsung,s5p-mfc.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1420: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Aakarsh Jain Sept. 8, 2022, 11:54 a.m. UTC | #3
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 07 September 2022 20:43
> To: Smitha T Murthy <smitha.t@samsung.com>
> Cc: linux-kernel@vger.kernel.org; aakarsh.jain@samsung.com;
> andi@etezian.org; david.plowman@raspberrypi.com;
> dillon.minfei@gmail.com; stanimir.varbanov@linaro.org;
> jernej.skrabec@gmail.com; andrzej.hajda@intel.com; hverkuil-
> cisco@xs4all.nl; mark.rutland@arm.com; linux-arm-
> kernel@lists.infradead.org; robh+dt@kernel.org;
> aswani.reddy@samsung.com; benjamin.gaignard@collabora.com;
> m.szyprowski@samsung.com; ezequiel@vanguardiasur.com.ar;
> pankaj.dubey@samsung.com; krzk+dt@kernel.org;
> devicetree@vger.kernel.org; linux-media@vger.kernel.org;
> mchehab@kernel.org; linux-fsd@tesla.com; alim.akhtar@samsung.com
> Subject: Re: [Patch v2 01/15] dt-bindings: media: s5p-mfc: Add new DT
> schema for MFC
> 
> On Wed, 07 Sep 2022 12:17:01 +0530, Smitha T Murthy wrote:
> > Adds DT schema for s5p-mfc in yaml format
> >
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > ---
> >  .../devicetree/bindings/media/s5p-mfc.txt     |  77 +------------
> >  .../bindings/media/samsung,s5p-mfc.yaml       | 109
> ++++++++++++++++++
> >  2 files changed, 110 insertions(+), 76 deletions(-)  create mode
> > 100644 Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
> >
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m
> dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Error: Documentation/devicetree/bindings/media/samsung,s5p-
> mfc.example.dts:21.11-12 syntax error FATAL ERROR: Unable to parse input
> tree
> make[1]: *** [scripts/Makefile.lib:384:
> Documentation/devicetree/bindings/media/samsung,s5p-mfc.example.dtb]
> Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1420: dt_binding_check] Error 2
> 
> doc reference errors (make refcheckdocs):
> 
> See https://protect2.fireeye.com/v1/url?k=88c9803a-e9429571-88c80b75-
> 74fe485fb305-6955573d9da6ae9f&q=1&e=4823af8c-cd6a-4171-9c03-
> cf0e37ae544b&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fpatch%2F
> 
> This check can fail if there are any dependencies. The base for a patch
series
> is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
error(s),
> then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.

we didn't got any errors while running dt_binding_check with path to the
yaml file but we are seeing errors while running  dt_binding_check without
path.
we will fix it in next series.

Thanks for the review.


Thanks,
Aakarsh
Aakarsh Jain Sept. 8, 2022, 12:56 p.m. UTC | #4
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 07 September 2022 16:52
> To: Smitha T Murthy <smitha.t@samsung.com>; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
> ezequiel@vanguardiasur.com.ar; jernej.skrabec@gmail.com;
> benjamin.gaignard@collabora.com; stanimir.varbanov@linaro.org;
> dillon.minfei@gmail.com; david.plowman@raspberrypi.com;
> mark.rutland@arm.com; robh+dt@kernel.org; krzk+dt@kernel.org;
> andi@etezian.org; alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; linux-fsd@tesla.com;
> aakarsh.jain@samsung.com
> Subject: Re: [Patch v2 01/15] dt-bindings: media: s5p-mfc: Add new DT
> schema for MFC
> 
> On 07/09/2022 08:47, Smitha T Murthy wrote:
> > Adds DT schema for s5p-mfc in yaml format
> 
> s/Adds/Convert/
> (as convert to DT schema)
> 
ok, I will change.

> Please mention here changes to original binding (I see at least adding
> iommus and dropping some properties).
> 
ok. I will make this changes. 
> >
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > ---
> >  .../devicetree/bindings/media/s5p-mfc.txt     |  77 +------------
> >  .../bindings/media/samsung,s5p-mfc.yaml       | 109
> ++++++++++++++++++
> >  2 files changed, 110 insertions(+), 76 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..0b7c4dd40095 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
> 
> When did they become obsolete? Is it enough of time to remove them?

these properties were there earlier, we didn't added them in obsolete properties.
> > -
> > -
> > -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>;
> > -};
> > +This file has moved to samsung,s5p-mfc.yaml
> 
> Just drop the TXT completely. Nothing references it.

Okay. will remove this text .
> 
> > 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..7cd26d4acbe4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > +---
> > +$id:
> > +https://protect2.fireeye.com/v1/url?k=a9dc6a19-c8577f3c-a9dde156-74fe
> > +485cbff6-58df42a60c876b34&q=1&e=edb5d5a1-11b4-42cd-9005-
> acaa7257669a&
> >
> +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fmedia%2Fsamsung%2Cs
> 5p-mfc.y
> > +aml%23
> > +$schema:
> > +https://protect2.fireeye.com/v1/url?k=1ad7b405-7b5ca120-1ad63f4a-
> 74fe
> > +485cbff6-5f46417fdb588b6c&q=1&e=edb5d5a1-11b4-42cd-9005-
> acaa7257669a&
> > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> > +
> > +title: Samsung Exynos Multi Format Codec (MFC)
> > +
> > +maintainers:
> > +  - Marek Szyprowski <m.szyprowski@samsung.com>
> > +  - Aakarsh Jain <aakarsh.jain@samsung.com>
> 
> and maybe you as well?
> 
okay. will update the list.
> > +
> > +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
> 
> You need to list the items. If this varies per compatible, do it in AllOf.
> 
okay. we will do the changes.
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    maxItems: 3
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  iommus:
> > +    maxItems: 2
> > +
> > +  iommu-names:
> > +    maxItems: 2
> 
> You need to list the items.
> 
Okay. We will do the changes.
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  memory-region:
> > +    maxItems: 1
> 
> This misses the description and old binding allowed it only for MFCv5, not for
> others, right?
> 
Okay. will add the description. Yes it is for MFCv5 only.
> > +
> > +allOf:
> > +  - if:
> 
> allOf goes after required section.
> 
ok. we will change.
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - samsung,mfc-v5
> > +    then:
> > +      properties:
> > +        memory-region:
> > +          maxItems: 2
> 
> Does not look like you tested the bindings. Please run `make
> dt_binding_check` (see Documentation/devicetree/bindings/writing-
> schema.rst for instructions).
> 
> This won't work. Test it and you will see it.
> 
Okay .we will check.
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - interrupts
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    # SoC specific DT entry
> > +    mfc: mfc@12880000 {
> > +        compatible = "samsung,fsd-mfc";
> 
> Does not look like you tested the bindings. Please run `make
> dt_binding_check` (see Documentation/devicetree/bindings/writing-
> schema.rst for instructions).

we didn't got any errors while running dt_binding_check with path to the yaml file but we are seeing errors while running  dt_binding_check without path.
we will fix it in next series.
> 
> > +        reg = <0x0 0x12880000 0x0 0x10000>;
> > +        interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
> > +        clock-names = "mfc";
> > +        clocks = <&clock_mfc MFC_MFC_IPCLKPORT_ACLK>;
> > +        iommus = <&smmu_isp 0x1000 0x0>, <&smmu_isp 0x1400 0x0>;
> > +        iommu-names = "left", "right";
> > +        power-domains = <&pd_mfc>;
> > +        memory-region = <&mfc_left>, <&mfc_right>;
> > +    };
> > +
> > +  - |
> > +    # Reserved memory specific DT entry for given board
> > +    # (see reserved memory binding for more information)
> > +    reserved-memory {
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> 
> Drop this example, not really related to MFC.
> 
> > +        ranges;
>
Okay. will remove this.

Thanks for the review. 

> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Sept. 8, 2022, 3:11 p.m. UTC | #5
On 08/09/2022 14:56, Aakarsh Jain wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
>> Sent: 07 September 2022 16:52
>> To: Smitha T Murthy <smitha.t@samsung.com>; linux-arm-
>> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
>> kernel@vger.kernel.org; devicetree@vger.kernel.org
>> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
>> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
>> ezequiel@vanguardiasur.com.ar; jernej.skrabec@gmail.com;
>> benjamin.gaignard@collabora.com; stanimir.varbanov@linaro.org;
>> dillon.minfei@gmail.com; david.plowman@raspberrypi.com;
>> mark.rutland@arm.com; robh+dt@kernel.org; krzk+dt@kernel.org;
>> andi@etezian.org; alim.akhtar@samsung.com; aswani.reddy@samsung.com;
>> pankaj.dubey@samsung.com; linux-fsd@tesla.com;
>> aakarsh.jain@samsung.com
>> Subject: Re: [Patch v2 01/15] dt-bindings: media: s5p-mfc: Add new DT
>> schema for MFC
>>
>> On 07/09/2022 08:47, Smitha T Murthy wrote:
>>> Adds DT schema for s5p-mfc in yaml format
>>
>> s/Adds/Convert/
>> (as convert to DT schema)
>>
> ok, I will change.
> 
>> Please mention here changes to original binding (I see at least adding
>> iommus and dropping some properties).
>>
> ok. I will make this changes. 
>>>
>>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
>>> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
>>> ---
>>>  .../devicetree/bindings/media/s5p-mfc.txt     |  77 +------------
>>>  .../bindings/media/samsung,s5p-mfc.yaml       | 109
>> ++++++++++++++++++
>>>  2 files changed, 110 insertions(+), 76 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..0b7c4dd40095 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
>>
>> When did they become obsolete? Is it enough of time to remove them?
> 
> these properties were there earlier, we didn't added them in obsolete properties.

This is not the answer to my question. Is it enough of time to remove
deprecated properties?



Best regards,
Krzysztof
Aakarsh Jain Sept. 14, 2022, 12:48 p.m. UTC | #6
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 08 September 2022 20:41
> To: Aakarsh Jain <aakarsh.jain@samsung.com>; 'Krzysztof Kozlowski'
> <krzysztof.kozlowski@linaro.org>; 'Smitha T Murthy'
> <smitha.t@samsung.com>; linux-arm-kernel@lists.infradead.org; linux-
> media@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
> ezequiel@vanguardiasur.com.ar; jernej.skrabec@gmail.com;
> benjamin.gaignard@collabora.com; stanimir.varbanov@linaro.org;
> dillon.minfei@gmail.com; david.plowman@raspberrypi.com;
> mark.rutland@arm.com; robh+dt@kernel.org; krzk+dt@kernel.org;
> andi@etezian.org; alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; linux-fsd@tesla.com
> Subject: Re: [Patch v2 01/15] dt-bindings: media: s5p-mfc: Add new DT
> schema for MFC
> 
> On 08/09/2022 14:56, Aakarsh Jain wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> >> Sent: 07 September 2022 16:52
> >> To: Smitha T Murthy <smitha.t@samsung.com>; linux-arm-
> >> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> >> kernel@vger.kernel.org; devicetree@vger.kernel.org
> >> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> >> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
> >> ezequiel@vanguardiasur.com.ar; jernej.skrabec@gmail.com;
> >> benjamin.gaignard@collabora.com; stanimir.varbanov@linaro.org;
> >> dillon.minfei@gmail.com; david.plowman@raspberrypi.com;
> >> mark.rutland@arm.com; robh+dt@kernel.org; krzk+dt@kernel.org;
> >> andi@etezian.org; alim.akhtar@samsung.com;
> aswani.reddy@samsung.com;
> >> pankaj.dubey@samsung.com; linux-fsd@tesla.com;
> >> aakarsh.jain@samsung.com
> >> Subject: Re: [Patch v2 01/15] dt-bindings: media: s5p-mfc: Add new DT
> >> schema for MFC
> >>
> >> On 07/09/2022 08:47, Smitha T Murthy wrote:
> >>> Adds DT schema for s5p-mfc in yaml format
> >>
> >> s/Adds/Convert/
> >> (as convert to DT schema)
> >>
> > ok, I will change.
> >
> >> Please mention here changes to original binding (I see at least
> >> adding iommus and dropping some properties).
> >>
> > ok. I will make this changes.
> >>>
> >>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> >>> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> >>> ---
> >>>  .../devicetree/bindings/media/s5p-mfc.txt     |  77 +------------
> >>>  .../bindings/media/samsung,s5p-mfc.yaml       | 109
> >> ++++++++++++++++++
> >>>  2 files changed, 110 insertions(+), 76 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..0b7c4dd40095 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
> >>
> >> When did they become obsolete? Is it enough of time to remove them?
> >
> > these properties were there earlier, we didn't added them in obsolete
> properties.
> 
> This is not the answer to my question. Is it enough of time to remove
> deprecated properties?
> 
sorry I misunderstood. These two mfc-l and mfc-r are still used in mfcv5 version as they use MFC_NUM_PORTS as 2, so we cannot make them obsolete yet. In next patch series we will add allOf: if: else: conditions for all the properties of each compatible string.
Thanks for the review.
> 
> 
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
index aa54c8159d9f..0b7c4dd40095 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>;
-};
+This file has moved to samsung,s5p-mfc.yaml
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..7cd26d4acbe4
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/samsung,s5p-mfc.yaml
@@ -0,0 +1,109 @@ 
+# 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@samsung.com>
+  - Aakarsh Jain <aakarsh.jain@samsung.com>
+
+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
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - samsung,mfc-v5
+    then:
+      properties:
+        memory-region:
+          maxItems: 2
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    # SoC specific DT entry
+    mfc: mfc@12880000 {
+        compatible = "samsung,fsd-mfc";
+        reg = <0x0 0x12880000 0x0 0x10000>;
+        interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
+        clock-names = "mfc";
+        clocks = <&clock_mfc MFC_MFC_IPCLKPORT_ACLK>;
+        iommus = <&smmu_isp 0x1000 0x0>, <&smmu_isp 0x1400 0x0>;
+        iommu-names = "left", "right";
+        power-domains = <&pd_mfc>;
+        memory-region = <&mfc_left>, <&mfc_right>;
+    };
+
+  - |
+    # 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@84000000 {
+                compatible = "shared-dma-pool";
+                no-map;
+                reg = <0x84000000 0x800000>;
+        };
+
+        mfc_right: region@a9000000 {
+                compatible = "shared-dma-pool";
+                no-map;
+                reg = <0xa9000000 0x800000>;
+        };
+    };
+