| Message ID | 20220517125548.14746-3-smitha.t@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add MFC v12 support. | expand |
On 17/05/2022 14:55, Smitha T Murthy wrote: > Adds DT schema for s5p-mfc in yaml format. > Thank you for your patch. There is something to discuss/improve. > Cc: linux-fsd@tesla.com > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> > --- > .../devicetree/bindings/media/s5p-mfc.txt | 77 +-------------- > .../devicetree/bindings/media/s5p-mfc.yaml | 98 +++++++++++++++++++ > 2 files changed, 99 insertions(+), 76 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/s5p-mfc.yaml > > diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt > index aa54c8159d9f..f00241ed407f 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 s5p-mfc.yaml Instead entirely remove the file. > diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.yaml b/Documentation/devicetree/bindings/media/s5p-mfc.yaml > new file mode 100644 > index 000000000000..fff7c7e0d575 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/s5p-mfc.yaml > @@ -0,0 +1,98 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/s5p-mfc.yaml# Let's convert the name as well, so "samsung,s5p-mfc.yaml" > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Samsung Exynos Multi Format Codec (MFC) > + > +maintainers: > + - Mauro Carvalho Chehab <mchehab@kernel.org> > + - Rob Herring <robh+dt@kernel.org> > + - Mark Rutland <mark.rutland@arm.com> > + - Smitha T Murthy <smitha.t@samsung.com> Only people with access to HW, so you can put here Marek and yourself. > + > +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 Ugh, how MFCv10 appeared here? Since 5433 we moved from versions to Soc compatibles as recommended... eh, please follow this convention, don't reverse it to other way. I propose to deprecated this in next patch and instead use SoC-based compatible. > + > + reg: > + maxItems: 1 > + > + clocks: > + description: > + Phandle to MFC IP clock. Here and other places: s/Phandle// Instead describe what is it, e.g. "MFC IP clock" > + maxItems: 1 > + > + clock-names: > + description: > + Must contain clock name (mfc) matching phandle in clocks > + property. Skip description, its obvious. Instead list the items. > + maxItems: 1 No need, list the items. > + > + interrupts: > + description: > + MFC interrupt number to the CPU. Skip description, it's obvious. > + maxItems: 1 > + > + memory-region: > + description: > + 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. > + minItems: 1 > + maxItems: 2 This needs allOf:if:then restricting two items to specific compatible. > + > + iommus: > + description: > + Include the IOMMU domain MFC belong to. Skip description, it's obvious. > + maxItems: 2 > + What happened to power domains? You also removed them from the example... Does this pass dtbs_check? > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - interrupts > + - iommus > + > +additionalProperties: false > + > +examples: > + - | > + /* Reserved memory specific DT entry for given board */ > + reserved-memory { Wrong indentation. Four spaces. See example schema. > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + mfc_left: region@84000000 { > + compatible = "shared-dma-pool"; > + no-map; > + reg = <0x84000000 0x800000>; > + }; > + > + mfc_right: region@A9000000 { lower case hex addresses, everywhere. > + compatible = "shared-dma-pool"; > + no-map; > + reg = <0xA9000000 0x800000>; > + }; > + }; > + > + mfc_0: mfc0@12880000 { Generic node names, so mfc. > + compatible = "samsung,mfc-v12"; Does not look like you tested the bindings. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Be sure to test your bindings before sending them. > + reg = <0x12880000 0x10000>; > + clock-names = "mfc"; > + interrupts = <0 137 4>; Use interrupt defines. > + clocks = <&clock_mfc 1>; > + memory-region = <&mfc_left>, <&mfc_right>; > + /* If IOMMU is present use below instead of memory-region property */ > + iommus = <&smmu_isp 0x1000 0x0>, <&smmu_isp 0x1400 0x0>; > + }; Best regards, Krzysztof
On Tue, 17 May 2022 18:25:30 +0530, Smitha T Murthy wrote: > Adds DT schema for s5p-mfc in yaml format. > > Cc: linux-fsd@tesla.com > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> > --- > .../devicetree/bindings/media/s5p-mfc.txt | 77 +-------------- > .../devicetree/bindings/media/s5p-mfc.yaml | 98 +++++++++++++++++++ > 2 files changed, 99 insertions(+), 76 deletions(-) > create mode 100644 Documentation/devicetree/bindings/media/s5p-mfc.yaml > Running 'make dtbs_check' with the schema in this patch gives the following warnings. Consider if they are expected or the schema is incorrect. These may not be new warnings. Note that it is not yet a requirement to have 0 warnings for dtbs_check. This will change in the future. Full log is available here: https://patchwork.ozlabs.org/patch/ codec@11000000: 'iommu-names', 'power-domains' do not match any of the regexes: 'pinctrl-[0-9]+' arch/arm/boot/dts/exynos5250-arndale.dtb arch/arm/boot/dts/exynos5250-smdk5250.dtb arch/arm/boot/dts/exynos5250-snow.dtb arch/arm/boot/dts/exynos5250-snow-rev5.dtb arch/arm/boot/dts/exynos5250-spring.dtb arch/arm/boot/dts/exynos5420-arndale-octa.dtb arch/arm/boot/dts/exynos5420-chagall-wifi.dtb arch/arm/boot/dts/exynos5420-klimt-wifi.dtb arch/arm/boot/dts/exynos5420-peach-pit.dtb arch/arm/boot/dts/exynos5420-smdk5420.dtb arch/arm/boot/dts/exynos5422-odroidhc1.dtb arch/arm/boot/dts/exynos5422-odroidxu3.dtb arch/arm/boot/dts/exynos5422-odroidxu3-lite.dtb arch/arm/boot/dts/exynos5422-odroidxu4.dtb arch/arm/boot/dts/exynos5800-peach-pi.dtb codec@13400000: clock-names: ['mfc', 'sclk_mfc'] is too long arch/arm/boot/dts/exynos3250-artik5-eval.dtb arch/arm/boot/dts/exynos3250-monk.dtb arch/arm/boot/dts/exynos3250-rinato.dtb arch/arm/boot/dts/exynos4210-i9100.dtb arch/arm/boot/dts/exynos4210-origen.dtb arch/arm/boot/dts/exynos4210-smdkv310.dtb arch/arm/boot/dts/exynos4210-trats.dtb arch/arm/boot/dts/exynos4210-universal_c210.dtb arch/arm/boot/dts/exynos4412-i9300.dtb arch/arm/boot/dts/exynos4412-i9305.dtb arch/arm/boot/dts/exynos4412-itop-elite.dtb arch/arm/boot/dts/exynos4412-n710x.dtb arch/arm/boot/dts/exynos4412-odroidu3.dtb arch/arm/boot/dts/exynos4412-odroidx2.dtb arch/arm/boot/dts/exynos4412-odroidx.dtb arch/arm/boot/dts/exynos4412-origen.dtb arch/arm/boot/dts/exynos4412-p4note-n8010.dtb arch/arm/boot/dts/exynos4412-smdk4412.dtb arch/arm/boot/dts/exynos4412-tiny4412.dtb arch/arm/boot/dts/exynos4412-trats2.dtb codec@13400000: clocks: [[5, 273], [5, 170]] is too long arch/arm/boot/dts/exynos4210-i9100.dtb arch/arm/boot/dts/exynos4210-origen.dtb arch/arm/boot/dts/exynos4210-smdkv310.dtb arch/arm/boot/dts/exynos4210-trats.dtb arch/arm/boot/dts/exynos4210-universal_c210.dtb codec@13400000: clocks: [[7, 178], [7, 228]] is too long arch/arm/boot/dts/exynos3250-artik5-eval.dtb arch/arm/boot/dts/exynos3250-monk.dtb arch/arm/boot/dts/exynos3250-rinato.dtb codec@13400000: clocks: [[7, 273], [7, 170]] is too long arch/arm/boot/dts/exynos4412-i9300.dtb arch/arm/boot/dts/exynos4412-i9305.dtb arch/arm/boot/dts/exynos4412-itop-elite.dtb arch/arm/boot/dts/exynos4412-n710x.dtb arch/arm/boot/dts/exynos4412-odroidu3.dtb arch/arm/boot/dts/exynos4412-odroidx2.dtb arch/arm/boot/dts/exynos4412-odroidx.dtb arch/arm/boot/dts/exynos4412-origen.dtb arch/arm/boot/dts/exynos4412-p4note-n8010.dtb arch/arm/boot/dts/exynos4412-smdk4412.dtb arch/arm/boot/dts/exynos4412-tiny4412.dtb arch/arm/boot/dts/exynos4412-trats2.dtb codec@13400000: 'iommu-names', 'power-domains' do not match any of the regexes: 'pinctrl-[0-9]+' arch/arm/boot/dts/exynos4210-i9100.dtb arch/arm/boot/dts/exynos4210-origen.dtb arch/arm/boot/dts/exynos4210-smdkv310.dtb arch/arm/boot/dts/exynos4210-trats.dtb arch/arm/boot/dts/exynos4210-universal_c210.dtb arch/arm/boot/dts/exynos4412-i9300.dtb arch/arm/boot/dts/exynos4412-i9305.dtb arch/arm/boot/dts/exynos4412-itop-elite.dtb arch/arm/boot/dts/exynos4412-n710x.dtb arch/arm/boot/dts/exynos4412-odroidu3.dtb arch/arm/boot/dts/exynos4412-odroidx2.dtb arch/arm/boot/dts/exynos4412-odroidx.dtb arch/arm/boot/dts/exynos4412-origen.dtb arch/arm/boot/dts/exynos4412-p4note-n8010.dtb arch/arm/boot/dts/exynos4412-smdk4412.dtb arch/arm/boot/dts/exynos4412-tiny4412.dtb arch/arm/boot/dts/exynos4412-trats2.dtb codec@13400000: iommus: [[36]] is too short arch/arm/boot/dts/exynos3250-monk.dtb codec@13400000: iommus: [[40]] is too short arch/arm/boot/dts/exynos3250-artik5-eval.dtb codec@13400000: iommus: [[47]] is too short arch/arm/boot/dts/exynos3250-rinato.dtb codec@13400000: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+' arch/arm/boot/dts/exynos3250-artik5-eval.dtb arch/arm/boot/dts/exynos3250-monk.dtb arch/arm/boot/dts/exynos3250-rinato.dtb codec@152e0000: clock-names: ['pclk', 'aclk', 'aclk_xiu'] is too long arch/arm64/boot/dts/exynos/exynos5433-tm2.dtb arch/arm64/boot/dts/exynos/exynos5433-tm2e.dtb codec@152e0000: clocks: [[34, 16], [34, 9], [34, 6]] is too long arch/arm64/boot/dts/exynos/exynos5433-tm2.dtb arch/arm64/boot/dts/exynos/exynos5433-tm2e.dtb codec@152e0000: 'iommu-names', 'power-domains' do not match any of the regexes: 'pinctrl-[0-9]+' arch/arm64/boot/dts/exynos/exynos5433-tm2.dtb arch/arm64/boot/dts/exynos/exynos5433-tm2e.dtb codec@f1700000: clock-names: ['sclk_mfc', 'mfc'] is too long arch/arm/boot/dts/s5pv210-aquila.dtb arch/arm/boot/dts/s5pv210-fascinate4g.dtb arch/arm/boot/dts/s5pv210-galaxys.dtb arch/arm/boot/dts/s5pv210-goni.dtb arch/arm/boot/dts/s5pv210-smdkc110.dtb arch/arm/boot/dts/s5pv210-smdkv210.dtb arch/arm/boot/dts/s5pv210-torbreck.dtb codec@f1700000: clocks: [[2, 60], [2, 92]] is too long arch/arm/boot/dts/s5pv210-aquila.dtb arch/arm/boot/dts/s5pv210-fascinate4g.dtb arch/arm/boot/dts/s5pv210-galaxys.dtb arch/arm/boot/dts/s5pv210-goni.dtb arch/arm/boot/dts/s5pv210-smdkc110.dtb arch/arm/boot/dts/s5pv210-smdkv210.dtb arch/arm/boot/dts/s5pv210-torbreck.dtb codec@f1700000: 'iommus' is a required property arch/arm/boot/dts/s5pv210-aquila.dtb arch/arm/boot/dts/s5pv210-fascinate4g.dtb arch/arm/boot/dts/s5pv210-galaxys.dtb arch/arm/boot/dts/s5pv210-goni.dtb arch/arm/boot/dts/s5pv210-smdkc110.dtb arch/arm/boot/dts/s5pv210-smdkv210.dtb arch/arm/boot/dts/s5pv210-torbreck.dtb
> -----Original Message----- > From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] > Sent: Tuesday, May 17, 2022 7:26 PM > 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 > Subject: Re: [PATCH 02/20] dt-bindings: media: s5p-mfc: Convert s5p-mfc.txt > to new DT schema > > On 17/05/2022 14:55, Smitha T Murthy wrote: > > Adds DT schema for s5p-mfc in yaml format. > > > > Thank you for your patch. There is something to discuss/improve. > Thank you for the review. > > Cc: linux-fsd@tesla.com > > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> > > --- > > .../devicetree/bindings/media/s5p-mfc.txt | 77 +-------------- > > .../devicetree/bindings/media/s5p-mfc.yaml | 98 > +++++++++++++++++++ > > 2 files changed, 99 insertions(+), 76 deletions(-) create mode > > 100644 Documentation/devicetree/bindings/media/s5p-mfc.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt > > b/Documentation/devicetree/bindings/media/s5p-mfc.txt > > index aa54c8159d9f..f00241ed407f 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 s5p-mfc.yaml > > Instead entirely remove the file. > Ok, I will remove this file. > > diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.yaml > > b/Documentation/devicetree/bindings/media/s5p-mfc.yaml > > new file mode 100644 > > index 000000000000..fff7c7e0d575 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/s5p-mfc.yaml > > @@ -0,0 +1,98 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: > > +https://protect2.fireeye.com/v1/url?k=8b32845f-ea492ed7-8b330f10- > 74fe > > +4860018a-302429095d2fce6e&q=1&e=73087855-3649-4be8-a878- > d487e0ae58f4& > > +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fmedia%2Fs5p- > mfc.yaml%23 > > Let's convert the name as well, so "samsung,s5p-mfc.yaml" > Ok, I will change the name in next series. > > +$schema: > > +https://protect2.fireeye.com/v1/url?k=a01c09cc-c167a344-a01d8283-74fe > > +4860018a-b8e3ca9e8a791d49&q=1&e=73087855-3649-4be8-a878- > d487e0ae58f4& > > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23 > > + > > +title: Samsung Exynos Multi Format Codec (MFC) > > + > > +maintainers: > > + - Mauro Carvalho Chehab <mchehab@kernel.org> > > + - Rob Herring <robh+dt@kernel.org> > > + - Mark Rutland <mark.rutland@arm.com> > > + - Smitha T Murthy <smitha.t@samsung.com> > > Only people with access to HW, so you can put here Marek and yourself. > Ok, I will change the authors list. > > + > > +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 > > Ugh, how MFCv10 appeared here? Since 5433 we moved from versions to > Soc compatibles as recommended... eh, please follow this convention, don't > reverse it to other way. > > I propose to deprecated this in next patch and instead use SoC-based > compatible. > MFCv10 was already mainlined as mfc-v10, maybe for v10 I will add it post this series. For MFCv12 I will add SoC-based in the next series. > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + description: > > + Phandle to MFC IP clock. > > Here and other places: s/Phandle// > Instead describe what is it, e.g. "MFC IP clock" > > Ok noted. > > + maxItems: 1 > > + > > + clock-names: > > + description: > > + Must contain clock name (mfc) matching phandle in clocks > > + property. > > Skip description, its obvious. Instead list the items. > Ok > > + maxItems: 1 > > No need, list the items. > > > + > > + interrupts: > > + description: > > + MFC interrupt number to the CPU. > > Skip description, it's obvious. > Ok > > + maxItems: 1 > > + > > + memory-region: > > + description: > > + 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. > > + minItems: 1 > > + maxItems: 2 > > This needs allOf:if:then restricting two items to specific compatible. > Ok, I will make this change. > > + > > + iommus: > > + description: > > + Include the IOMMU domain MFC belong to. > > Skip description, it's obvious. > Ok > > + maxItems: 2 > > + > > What happened to power domains? You also removed them from the > example... Does this pass dtbs_check? > This file passed the dtbs_check. For MFCv10 and v12 power domains are not required. I will add in the example. > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + - interrupts > > + - iommus > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + /* Reserved memory specific DT entry for given board */ > > + reserved-memory { > > Wrong indentation. Four spaces. See example schema. > Ok > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + mfc_left: region@84000000 { > > + compatible = "shared-dma-pool"; > > + no-map; > > + reg = <0x84000000 0x800000>; > > + }; > > + > > + mfc_right: region@A9000000 { > > lower case hex addresses, everywhere. > Ok > > + compatible = "shared-dma-pool"; > > + no-map; > > + reg = <0xA9000000 0x800000>; > > + }; > > + }; > > + > > + mfc_0: mfc0@12880000 { > > Generic node names, so mfc. > Ok > > + compatible = "samsung,mfc-v12"; > > Does not look like you tested the bindings. Please run `make > dt_binding_check` (see Documentation/devicetree/bindings/writing- > schema.rst for instructions). > Be sure to test your bindings before sending them. > I did do make dtbs and dt_binding_check using v2022.3, I will recheck post these changes. > > + reg = <0x12880000 0x10000>; > > + clock-names = "mfc"; > > + interrupts = <0 137 4>; > > Use interrupt defines. > When I use interrupt defines I get errors as "1. Syntax error: This was due to interrupts field has some macro reference and needed to give absolute value.", hence I gave absolute values. Regards, Smitha > > + clocks = <&clock_mfc 1>; > > + memory-region = <&mfc_left>, <&mfc_right>; > > + /* If IOMMU is present use below instead of memory-region > property */ > > + iommus = <&smmu_isp 0x1000 0x0>, <&smmu_isp 0x1400 0x0>; > > + }; > > > Best regards, > Krzysztof
On 05/07/2022 13:44, Smitha T Murthy wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] >> Sent: Tuesday, May 17, 2022 7:26 PM >> 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 >> Subject: Re: [PATCH 02/20] dt-bindings: media: s5p-mfc: Convert s5p-mfc.txt >> to new DT schema >> >> On 17/05/2022 14:55, Smitha T Murthy wrote: >>> Adds DT schema for s5p-mfc in yaml format. >>> >> >> Thank you for your patch. There is something to discuss/improve. >> > > Thank you for the review. > You responded after two months, I don't remember what I reviewed... Two months periods between resends do not really help to usptream. > >>> + compatible = "samsung,mfc-v12"; >> >> Does not look like you tested the bindings. Please run `make >> dt_binding_check` (see Documentation/devicetree/bindings/writing- >> schema.rst for instructions). >> Be sure to test your bindings before sending them. >> > > I did do make dtbs and dt_binding_check using v2022.3, I will recheck post these changes. > >>> + reg = <0x12880000 0x10000>; >>> + clock-names = "mfc"; >>> + interrupts = <0 137 4>; >> >> Use interrupt defines. >> > > When I use interrupt defines I get errors as "1. Syntax error: This was due to interrupts field has some macro reference and needed to give absolute value.", hence I gave absolute values. Look at other DT schema files... Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt index aa54c8159d9f..f00241ed407f 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 s5p-mfc.yaml diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.yaml b/Documentation/devicetree/bindings/media/s5p-mfc.yaml new file mode 100644 index 000000000000..fff7c7e0d575 --- /dev/null +++ b/Documentation/devicetree/bindings/media/s5p-mfc.yaml @@ -0,0 +1,98 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/s5p-mfc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Samsung Exynos Multi Format Codec (MFC) + +maintainers: + - Mauro Carvalho Chehab <mchehab@kernel.org> + - Rob Herring <robh+dt@kernel.org> + - Mark Rutland <mark.rutland@arm.com> + - Smitha T Murthy <smitha.t@samsung.com> + +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: + description: + Phandle to MFC IP clock. + maxItems: 1 + + clock-names: + description: + Must contain clock name (mfc) matching phandle in clocks + property. + maxItems: 1 + + interrupts: + description: + MFC interrupt number to the CPU. + maxItems: 1 + + memory-region: + description: + 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. + minItems: 1 + maxItems: 2 + + iommus: + description: + Include the IOMMU domain MFC belong to. + maxItems: 2 + +required: + - compatible + - reg + - clocks + - clock-names + - interrupts + - iommus + +additionalProperties: false + +examples: + - | + /* Reserved memory specific DT entry for given board */ + 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>; + }; + }; + + mfc_0: mfc0@12880000 { + compatible = "samsung,mfc-v12"; + reg = <0x12880000 0x10000>; + clock-names = "mfc"; + interrupts = <0 137 4>; + clocks = <&clock_mfc 1>; + memory-region = <&mfc_left>, <&mfc_right>; + /* If IOMMU is present use below instead of memory-region property */ + iommus = <&smmu_isp 0x1000 0x0>, <&smmu_isp 0x1400 0x0>; + };
Adds DT schema for s5p-mfc in yaml format. Cc: linux-fsd@tesla.com Signed-off-by: Smitha T Murthy <smitha.t@samsung.com> --- .../devicetree/bindings/media/s5p-mfc.txt | 77 +-------------- .../devicetree/bindings/media/s5p-mfc.yaml | 98 +++++++++++++++++++ 2 files changed, 99 insertions(+), 76 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/s5p-mfc.yaml