diff mbox series

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

Message ID 20221011122516.32135-2-aakarsh.jain@samsung.com
State Changes Requested, archived
Headers show
Series Add MFC v12 support. | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Aakarsh Jain Oct. 11, 2022, 12:25 p.m. UTC
From: Smitha T Murthy <smitha.t@samsung.com>

Convert DT schema for s5p-mfc in yaml format

Cc: linux-fsd@tesla.com
Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
---
 .../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

Comments

Krzysztof Kozlowski Oct. 12, 2022, 12:57 p.m. UTC | #1
On 11/10/2022 08:25, aakarsh jain wrote:
> From: Smitha T Murthy <smitha.t@samsung.com>
> 
> Convert DT schema for s5p-mfc in yaml format

Convert S5P-MFC bindings to DT schema.

You also missed full-stop.

My second comment was ignored here.


> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  .../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

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

Best regards,
Krzysztof
Rob Herring (Arm) Oct. 12, 2022, 1:19 p.m. UTC | #2
On Tue, 11 Oct 2022 17:55:02 +0530, aakarsh jain wrote:
> From: Smitha T Murthy <smitha.t@samsung.com>
> 
> Convert DT schema for s5p-mfc in yaml format
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  .../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
> 

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

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: iommus: [[36]] is too short
	arch/arm/boot/dts/exynos3250-monk.dtb
	arch/arm/boot/dts/exynos3250-monk.dtb

codec@13400000: iommus: [[40]] is too short
	arch/arm/boot/dts/exynos3250-artik5-eval.dtb
	arch/arm/boot/dts/exynos3250-artik5-eval.dtb

codec@13400000: iommus: [[47]] is too short
	arch/arm/boot/dts/exynos3250-rinato.dtb
	arch/arm/boot/dts/exynos3250-rinato.dtb

codec@13400000: memory-region: [[31], [32]] is too long
	arch/arm/boot/dts/exynos4210-smdkv310.dtb

codec@13400000: memory-region: [[37], [38]] is too long
	arch/arm/boot/dts/exynos4210-origen.dtb

codec@13400000: memory-region: [[41], [42]] is too long
	arch/arm/boot/dts/exynos4412-smdk4412.dtb

codec@13400000: memory-region: [[43], [44]] is too long
	arch/arm/boot/dts/exynos4412-origen.dtb

codec@13400000: memory-region: [[47], [48]] is too long
	arch/arm/boot/dts/exynos4412-itop-elite.dtb
	arch/arm/boot/dts/exynos4412-odroidx2.dtb
	arch/arm/boot/dts/exynos4412-odroidx.dtb

codec@13400000: memory-region: [[48], [49]] is too long
	arch/arm/boot/dts/exynos4412-odroidu3.dtb

codec@f1700000: clock-names:0: 'mfc' was expected
	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: clock-names:1: 'sclk_mfc' was expected
	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: memory-region: [[51], [52]] is too long
	arch/arm/boot/dts/s5pv210-fascinate4g.dtb

codec@f1700000: memory-region: [[55], [56]] is too long
	arch/arm/boot/dts/s5pv210-galaxys.dtb
Krzysztof Kozlowski Oct. 12, 2022, 1:41 p.m. UTC | #3
On 12/10/2022 09:19, Rob Herring wrote:
> On Tue, 11 Oct 2022 17:55:02 +0530, aakarsh jain wrote:
>> From: Smitha T Murthy <smitha.t@samsung.com>
>>
>> Convert DT schema for s5p-mfc in yaml format
>>
>> Cc: linux-fsd@tesla.com
>> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
>> ---
>>  .../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
>>
> 
> 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/
> 

All these must be fixed before schema can be applied.

Best regards,
Krzysztof
Aakarsh Jain Oct. 17, 2022, 2:16 p.m. UTC | #4
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzk@kernel.org]
> Sent: 12 October 2022 18:27
> To: aakarsh jain <aakarsh.jain@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; smitha.t@samsung.com
> Subject: Re: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT
> schema for MFC
> 
> On 11/10/2022 08:25, aakarsh jain wrote:
> > From: Smitha T Murthy <smitha.t@samsung.com>
> >
> > Convert DT schema for s5p-mfc in yaml format
> 
> Convert S5P-MFC bindings to DT schema.
> 
> You also missed full-stop.
> 
Ok will address it in next series.

> My second comment was ignored here.
> 
I missed one review comment which was related to change in commit message. Will address it in next series.
> >
> > Cc: linux-fsd@tesla.com
> > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  .../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
> 
> This is a friendly reminder during the review process.
> 
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all requested
> changes or keep discussing them.
> 
> Thank you.
> 
Apart from your second review comment  I guess we have addressed all your review comments in the current patch.
Please review other changes as well and let us know if any changes required.

Thanks for the review.

> Best regards,
> Krzysztof
Aakarsh Jain Oct. 18, 2022, 10:48 a.m. UTC | #5
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 12 October 2022 18:50
> To: aakarsh jain <aakarsh.jain@samsung.com>
> Cc: linux-fsd@tesla.com; linux-media@vger.kernel.org;
> pankaj.dubey@samsung.com; linux-arm-kernel@lists.infradead.org;
> dillon.minfei@gmail.com; devicetree@vger.kernel.org; krzk+dt@kernel.org;
> smitha.t@samsung.com; benjamin.gaignard@collabora.com;
> stanimir.varbanov@linaro.org; jernej.skrabec@gmail.com;
> robh+dt@kernel.org; aswani.reddy@samsung.com; mchehab@kernel.org;
> hverkuil-cisco@xs4all.nl; mark.rutland@arm.com;
> m.szyprowski@samsung.com; linux-kernel@vger.kernel.org;
> alim.akhtar@samsung.com; andi@etezian.org; andrzej.hajda@intel.com;
> ezequiel@vanguardiasur.com.ar; david.plowman@raspberrypi.com
> Subject: Re: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT
> schema for MFC
> 
> On Tue, 11 Oct 2022 17:55:02 +0530, aakarsh jain wrote:
> > From: Smitha T Murthy <smitha.t@samsung.com>
> >
> > Convert DT schema for s5p-mfc in yaml format
> >
> > Cc: linux-fsd@tesla.com
> > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  .../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
> >
> 
> 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://protect2.fireeye.com/v1/url?k=b651482e-
> e9ca7131-b650c361-000babdfecba-bb28230f82534754&q=1&e=25a7ee78-
> 697f-4371-880e-
> dcf4e3072c99&u=https%3A%2F%2Fpatchwork.ozlabs.org%2Fpatch%2F
> 
> 
> 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
> 
> 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: iommus: [[36]] is too short
> 	arch/arm/boot/dts/exynos3250-monk.dtb
> 	arch/arm/boot/dts/exynos3250-monk.dtb
> 
> codec@13400000: iommus: [[40]] is too short
> 	arch/arm/boot/dts/exynos3250-artik5-eval.dtb
> 	arch/arm/boot/dts/exynos3250-artik5-eval.dtb
> 
> codec@13400000: iommus: [[47]] is too short
> 	arch/arm/boot/dts/exynos3250-rinato.dtb
> 	arch/arm/boot/dts/exynos3250-rinato.dtb
> 
> codec@13400000: memory-region: [[31], [32]] is too long
> 	arch/arm/boot/dts/exynos4210-smdkv310.dtb
> 
> codec@13400000: memory-region: [[37], [38]] is too long
> 	arch/arm/boot/dts/exynos4210-origen.dtb
> 
> codec@13400000: memory-region: [[41], [42]] is too long
> 	arch/arm/boot/dts/exynos4412-smdk4412.dtb
> 
> codec@13400000: memory-region: [[43], [44]] is too long
> 	arch/arm/boot/dts/exynos4412-origen.dtb
> 
> codec@13400000: memory-region: [[47], [48]] is too long
> 	arch/arm/boot/dts/exynos4412-itop-elite.dtb
> 	arch/arm/boot/dts/exynos4412-odroidx2.dtb
> 	arch/arm/boot/dts/exynos4412-odroidx.dtb
> 
> codec@13400000: memory-region: [[48], [49]] is too long
> 	arch/arm/boot/dts/exynos4412-odroidu3.dtb
> 
> codec@f1700000: clock-names:0: 'mfc' was expected
> 	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: clock-names:1: 'sclk_mfc' was expected
> 	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: memory-region: [[51], [52]] is too long
> 	arch/arm/boot/dts/s5pv210-fascinate4g.dtb
> 
> codec@f1700000: memory-region: [[55], [56]] is too long
> 	arch/arm/boot/dts/s5pv210-galaxys.dtb



Hi Rob,

We tried reproducing warnings as reported above, but I am not able to see
these warnings after  running make dtbs_check & make DT_CHECKER_FLAGS=-m
dt_binding_check.
Packages used-
yamllint 1.10.0
$ dt-mk-schema --version
2022.9

Am I missing anything? Do let me know.


Thanks,
Aakarsh
Krzysztof Kozlowski Oct. 18, 2022, 1:09 p.m. UTC | #6
On 17/10/2022 10:16, Aakarsh Jain wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzk@kernel.org]
>> Sent: 12 October 2022 18:27
>> To: aakarsh jain <aakarsh.jain@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; smitha.t@samsung.com
>> Subject: Re: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT
>> schema for MFC
>>
>> On 11/10/2022 08:25, aakarsh jain wrote:
>>> From: Smitha T Murthy <smitha.t@samsung.com>
>>>
>>> Convert DT schema for s5p-mfc in yaml format
>>
>> Convert S5P-MFC bindings to DT schema.
>>
>> You also missed full-stop.
>>
> Ok will address it in next series.
> 
>> My second comment was ignored here.
>>
> I missed one review comment which was related to change in commit message. Will address it in next series.
>>>
>>> Cc: linux-fsd@tesla.com
>>> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
>>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
>>> ---
>>>  .../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
>>
>> This is a friendly reminder during the review process.
>>
>> It seems my previous comments were not fully addressed. Maybe my
>> feedback got lost between the quotes, maybe you just forgot to apply it.
>> Please go back to the previous discussion and either implement all requested
>> changes or keep discussing them.
>>
>> Thank you.
>>
> Apart from your second review comment  I guess we have addressed all your review comments in the current patch.
> Please review other changes as well and let us know if any changes required.

No, as I said you did not. Instead of bouncing back immediately, go
through previous emails and through reviews.

I already reviewed this patch and it did not apply all my comments.
Asking me to review second time is not the solution here. The solution
here is for you to go through the comments and implement them, or answer
and keep discussion going.

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 18, 2022, 1:10 p.m. UTC | #7
On 18/10/2022 06:48, Aakarsh Jain wrote:
>>
>> codec@f1700000: clock-names:1: 'sclk_mfc' was expected
>> 	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: memory-region: [[51], [52]] is too long
>> 	arch/arm/boot/dts/s5pv210-fascinate4g.dtb
>>
>> codec@f1700000: memory-region: [[55], [56]] is too long
>> 	arch/arm/boot/dts/s5pv210-galaxys.dtb
> 
> 
> 
> Hi Rob,
> 
> We tried reproducing warnings as reported above, but I am not able to see
> these warnings after  running make dtbs_check & make DT_CHECKER_FLAGS=-m
> dt_binding_check.
> Packages used-
> yamllint 1.10.0
> $ dt-mk-schema --version
> 2022.9


Are you sure you are running these commands on proper config and arch?

Best regards,
Krzysztof
Aakarsh Jain Oct. 18, 2022, 1:21 p.m. UTC | #8
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzk@kernel.org]
> Sent: 18 October 2022 18:40
> To: Aakarsh Jain <aakarsh.jain@samsung.com>; 'Rob Herring'
> <robh@kernel.org>
> Cc: linux-fsd@tesla.com; linux-media@vger.kernel.org;
> pankaj.dubey@samsung.com; linux-arm-kernel@lists.infradead.org;
> dillon.minfei@gmail.com; devicetree@vger.kernel.org; krzk+dt@kernel.org;
> smitha.t@samsung.com; benjamin.gaignard@collabora.com;
> stanimir.varbanov@linaro.org; jernej.skrabec@gmail.com;
> robh+dt@kernel.org; aswani.reddy@samsung.com; mchehab@kernel.org;
> hverkuil-cisco@xs4all.nl; mark.rutland@arm.com;
> m.szyprowski@samsung.com; linux-kernel@vger.kernel.org;
> alim.akhtar@samsung.com; andi@etezian.org; andrzej.hajda@intel.com;
> ezequiel@vanguardiasur.com.ar; david.plowman@raspberrypi.com
> Subject: Re: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT
> schema for MFC
> 
> On 18/10/2022 06:48, Aakarsh Jain wrote:
> >>
> >> codec@f1700000: clock-names:1: 'sclk_mfc' was expected
> >> 	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: memory-region: [[51], [52]] is too long
> >> 	arch/arm/boot/dts/s5pv210-fascinate4g.dtb
> >>
> >> codec@f1700000: memory-region: [[55], [56]] is too long
> >> 	arch/arm/boot/dts/s5pv210-galaxys.dtb
> >
> >
> >
> > Hi Rob,
> >
> > We tried reproducing warnings as reported above, but I am not able to
> > see these warnings after  running make dtbs_check & make
> > DT_CHECKER_FLAGS=-m dt_binding_check.
> > Packages used-
> > yamllint 1.10.0
> > $ dt-mk-schema --version
> > 2022.9
> 
> 
> Are you sure you are running these commands on proper config and arch?
> 
yes. arm64/defconfig .

> Best regards,
> Krzysztof
Krzysztof Kozlowski Oct. 18, 2022, 2:07 p.m. UTC | #9
On 18/10/2022 09:21, Aakarsh Jain wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzk@kernel.org]
>> Sent: 18 October 2022 18:40
>> To: Aakarsh Jain <aakarsh.jain@samsung.com>; 'Rob Herring'
>> <robh@kernel.org>
>> Cc: linux-fsd@tesla.com; linux-media@vger.kernel.org;
>> pankaj.dubey@samsung.com; linux-arm-kernel@lists.infradead.org;
>> dillon.minfei@gmail.com; devicetree@vger.kernel.org; krzk+dt@kernel.org;
>> smitha.t@samsung.com; benjamin.gaignard@collabora.com;
>> stanimir.varbanov@linaro.org; jernej.skrabec@gmail.com;
>> robh+dt@kernel.org; aswani.reddy@samsung.com; mchehab@kernel.org;
>> hverkuil-cisco@xs4all.nl; mark.rutland@arm.com;
>> m.szyprowski@samsung.com; linux-kernel@vger.kernel.org;
>> alim.akhtar@samsung.com; andi@etezian.org; andrzej.hajda@intel.com;
>> ezequiel@vanguardiasur.com.ar; david.plowman@raspberrypi.com
>> Subject: Re: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT
>> schema for MFC
>>
>> On 18/10/2022 06:48, Aakarsh Jain wrote:
>>>>
>>>> codec@f1700000: clock-names:1: 'sclk_mfc' was expected
>>>> 	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: memory-region: [[51], [52]] is too long
>>>> 	arch/arm/boot/dts/s5pv210-fascinate4g.dtb
>>>>
>>>> codec@f1700000: memory-region: [[55], [56]] is too long
>>>> 	arch/arm/boot/dts/s5pv210-galaxys.dtb
>>>
>>>
>>>
>>> Hi Rob,
>>>
>>> We tried reproducing warnings as reported above, but I am not able to
>>> see these warnings after  running make dtbs_check & make
>>> DT_CHECKER_FLAGS=-m dt_binding_check.
>>> Packages used-
>>> yamllint 1.10.0
>>> $ dt-mk-schema --version
>>> 2022.9
>>
>>
>> Are you sure you are running these commands on proper config and arch?
>>
> yes. arm64/defconfig .

Which does not look like proper arch. Look at the warnings Rob robot
provided.


Best regards,
Krzysztof
Alim Akhtar Oct. 27, 2022, 10:39 a.m. UTC | #10
Hi Aakarsh
Thanks for the patch.

>-----Original Message-----
>From: aakarsh jain [mailto:aakarsh.jain@samsung.com]
>Sent: Tuesday, October 11, 2022 5:55 PM
>To: 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; smitha.t@samsung.com;
>aakarsh.jain@samsung.com
>Subject: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT schema for
>MFC
>
>From: Smitha T Murthy <smitha.t@samsung.com>
>
>Convert DT schema for s5p-mfc in yaml format
>
>Cc: linux-fsd@tesla.com
>Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
>Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
>---
> .../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@samsung.com>
>+  - Smitha T Murthy <smitha.t@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
>+
>+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
Aakarsh Jain Oct. 28, 2022, 4:37 a.m. UTC | #11
> -----Original Message-----
> From: Alim Akhtar [mailto:alim.akhtar@samsung.com]
> Sent: 27 October 2022 16:10
> To: 'aakarsh jain' <aakarsh.jain@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; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; linux-fsd@tesla.com; smitha.t@samsung.com
> 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@samsung.com]
> >Sent: Tuesday, October 11, 2022 5:55 PM
> >To: 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;
> smitha.t@samsung.com;
> >aakarsh.jain@samsung.com
> >Subject: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT
> >schema for MFC
> >
> >From: Smitha T Murthy <smitha.t@samsung.com>
> >
> >Convert DT schema for s5p-mfc in yaml format
> >
> >Cc: linux-fsd@tesla.com
> >Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> >Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> >---
> > .../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@samsung.com>
> >+  - Smitha T Murthy <smitha.t@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
> >+
> >+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.
Krzysztof Kozlowski Feb. 21, 2023, 4:25 p.m. UTC | #12
On 11/10/2022 14:25, aakarsh jain wrote:
> From: Smitha T Murthy <smitha.t@samsung.com>
> 
> Convert DT schema for s5p-mfc in yaml format
> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  .../devicetree/bindings/media/s5p-mfc.txt     |  75 --------
>  .../bindings/media/samsung,s5p-mfc.yaml       | 163 ++++++++++++++++++

Hi Aakarsh,

Any plans for new version of this DT schema conversion? The Samsung MFC
is one of last remaining bigger pieces for conversion. We already have
100% bindings compliance for ARM64 but the MFC is missing for ARMv7:

https://pastebin.com/D8jXnjEe
(that's on my pending branch with all patches from LKML collected).

Best regards,
Krzysztof
Aakarsh Jain Feb. 22, 2023, 9:12 a.m. UTC | #13
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 21 February 2023 21:56
> To: aakarsh jain <aakarsh.jain@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; smitha.t@samsung.com
> Subject: Re: [Patch v3 01/15] dt-bindings: media: s5p-mfc: Add new DT
> schema for MFC
> 
> On 11/10/2022 14:25, aakarsh jain wrote:
> > From: Smitha T Murthy <smitha.t@samsung.com>
> >
> > Convert DT schema for s5p-mfc in yaml format
> >
> > Cc: linux-fsd@tesla.com
> > Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  .../devicetree/bindings/media/s5p-mfc.txt     |  75 --------
> >  .../bindings/media/samsung,s5p-mfc.yaml       | 163
> ++++++++++++++++++
> 
> Hi Aakarsh,
> 
> Any plans for new version of this DT schema conversion? The Samsung MFC
> is one of last remaining bigger pieces for conversion. We already have 100%
> bindings compliance for ARM64 but the MFC is missing for ARMv7:
> 
> https://protect2.fireeye.com/v1/url?k=9661b377-f7eaa63c-96603838-
> 74fe485fb305-bc4c89dbd0204557&q=1&e=1485ea1d-43e8-4541-8078-
> 4d13ff6cc42f&u=https%3A%2F%2Fpastebin.com%2FD8jXnjEe
> (that's on my pending branch with all patches from LKML collected).
> 
> Best regards,
> Krzysztof

Hi Krzysztof,

I was little occupied with other stuff. I will fix all warnings and errors. Will post the patch soon.

Thanks,
Aakarsh
diff mbox series

Patch

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@samsung.com>
+  - Smitha T Murthy <smitha.t@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
+
+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
+    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
+    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";
+
+    };