diff mbox series

[v2,1/3] arm: exynos: Add new compatible string for Exynos3250 SoC.

Message ID 20221109035507.69086-1-aakarsh.jain@samsung.com
State New
Headers
Series [v2,1/3] arm: exynos: Add new compatible string for Exynos3250 SoC. |

Commit Message

Aakarsh Jain Nov. 9, 2022, 3:55 a.m. UTC 
  Since,MFC v7 support was added for Exynos5420 and Exynos
3250 SoC with same compatible string "samsung,mfc-v7".As
both SoCs having different hardware properties and having
same compatible string for both SoCs doesn't seems to be correct.
New compatible is added for Exynos3250 SOC which will
differentiate the node properties for both SoCs which
support MFC v7.

Reviewed-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
---
 Documentation/devicetree/bindings/media/s5p-mfc.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

We are already in process of converting this txt file to yaml.
https://patchwork.kernel.org/project/linux-media/patch/20221011122516.32135-2-aakarsh.jain@samsung.com/
Modifying this txt binding for completeness.

Comments

Krzysztof Kozlowski Nov. 9, 2022, 9:01 a.m. UTC | #1 
On 09/11/2022 04:55, Aakarsh Jain wrote:
> Since,MFC v7 support was added for Exynos5420 and Exynos
> 3250 SoC with same compatible string "samsung,mfc-v7".As
> both SoCs having different hardware properties and having
> same compatible string for both SoCs doesn't seems to be correct.
> New compatible is added for Exynos3250 SOC which will
> differentiate the node properties for both SoCs which
> support MFC v7.
> 
> Reviewed-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  Documentation/devicetree/bindings/media/s5p-mfc.txt | 9 +++++----

Use subject prefixes matching the subsystem (git log --oneline -- ...).

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
Krzysztof Kozlowski Nov. 9, 2022, 9:13 a.m. UTC | #2 
On 09/11/2022 04:55, Aakarsh Jain wrote:
> Since,MFC v7 support was added for Exynos5420 and Exynos
> 3250 SoC with same compatible string "samsung,mfc-v7".As
> both SoCs having different hardware properties and having
> same compatible string for both SoCs doesn't seems to be correct.
> New compatible is added for Exynos3250 SOC which will
> differentiate the node properties for both SoCs which
> support MFC v7.
> 
> Reviewed-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  Documentation/devicetree/bindings/media/s5p-mfc.txt | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 

Beside my previous comment, please include changelog. This is v2, right?

You should consider joining something like:
https://www.linaro.org/events/member-training-upstream-kernel-development/

Best regards,
Krzysztof
Aakarsh Jain Nov. 9, 2022, 2:44 p.m. UTC | #3 
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 09 November 2022 14:31
> 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; krzysztof.kozlowski+dt@linaro.org;
> 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; smitha.t@samsung.com
> Subject: Re: [Patch v2 1/3] arm: exynos: Add new compatible string for
> Exynos3250 SoC.
> 
> On 09/11/2022 04:55, Aakarsh Jain wrote:
> > Since,MFC v7 support was added for Exynos5420 and Exynos
> > 3250 SoC with same compatible string "samsung,mfc-v7".As both SoCs
> > having different hardware properties and having same compatible string
> > for both SoCs doesn't seems to be correct.
> > New compatible is added for Exynos3250 SOC which will differentiate
> > the node properties for both SoCs which support MFC v7.
> >
> > Reviewed-by: Tommaso Merciai
> <tommaso.merciai@amarulasolutions.com>
> > Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  Documentation/devicetree/bindings/media/s5p-mfc.txt | 9 +++++----
> 
> Use subject prefixes matching the subsystem (git log --oneline -- ...).
> 
As with recent commits on Documentation/devicetree/bindings/media/s5p-mfc.txt with git log --oneline --  ,  subject prefix doesn't seems to be consistent.

b1394dc151cb media: s5p-mfc: Adding initial support for MFC v10.10
60641e22599a [media] s5p-mfc: Use preallocated block allocator always for MFC v6+
003611334d55 [media] s5p-mfc: Add support for MFC v8 available in Exynos 5433 SoCs
0da658704136 ARM: dts: convert to generic power domain bindings for exynos DT
77634289286a ARM: dts: Update clocks entry in MFC binding documentation
2eae613b95a7 ARM: EXYNOS: Add MFC device tree support

Closest is ARM: dts.
so what is your suggestion on this?

Anyway we are in a process of converting this txt file to yaml . 


> 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

Thanks for the review.
Krzysztof Kozlowski Nov. 10, 2022, 8:25 a.m. UTC | #4 
On 09/11/2022 15:44, Aakarsh Jain wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
>> Sent: 09 November 2022 14:31
>> 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; krzysztof.kozlowski+dt@linaro.org;
>> 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; smitha.t@samsung.com
>> Subject: Re: [Patch v2 1/3] arm: exynos: Add new compatible string for
>> Exynos3250 SoC.
>>
>> On 09/11/2022 04:55, Aakarsh Jain wrote:
>>> Since,MFC v7 support was added for Exynos5420 and Exynos
>>> 3250 SoC with same compatible string "samsung,mfc-v7".As both SoCs
>>> having different hardware properties and having same compatible string
>>> for both SoCs doesn't seems to be correct.
>>> New compatible is added for Exynos3250 SOC which will differentiate
>>> the node properties for both SoCs which support MFC v7.
>>>
>>> Reviewed-by: Tommaso Merciai
>> <tommaso.merciai@amarulasolutions.com>
>>> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
>>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
>>> ---
>>>  Documentation/devicetree/bindings/media/s5p-mfc.txt | 9 +++++----
>>
>> Use subject prefixes matching the subsystem (git log --oneline -- ...).
>>
> As with recent commits on Documentation/devicetree/bindings/media/s5p-mfc.txt with git log --oneline --  ,  subject prefix doesn't seems to be consistent.
> 
> b1394dc151cb media: s5p-mfc: Adding initial support for MFC v10.10
> 60641e22599a [media] s5p-mfc: Use preallocated block allocator always for MFC v6+
> 003611334d55 [media] s5p-mfc: Add support for MFC v8 available in Exynos 5433 SoCs
> 0da658704136 ARM: dts: convert to generic power domain bindings for exynos DT
> 77634289286a ARM: dts: Update clocks entry in MFC binding documentation
> 2eae613b95a7 ARM: EXYNOS: Add MFC device tree support

s5p-mfc is not a subsystem.

git log --oneline  -- Documentation/devicetree/bindings/media/

media: dt-bindings: NAME_OF_FILE:


> 
> Closest is ARM: dts.

This is not ARM subsystem and not a DTS file.

> so what is your suggestion on this?
> 
> Anyway we are in a process of converting this txt file to yaml . 
> 

Best regards,
Krzysztof
Aakarsh Jain Nov. 11, 2022, 3:19 a.m. UTC | #5 
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzk@kernel.org]
> Sent: 10 November 2022 13:56
> To: Aakarsh Jain <aakarsh.jain@samsung.com>; 'Krzysztof Kozlowski'
> <krzysztof.kozlowski@linaro.org>; 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; krzysztof.kozlowski+dt@linaro.org;
> 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; smitha.t@samsung.com
> Subject: Re: [Patch v2 1/3] arm: exynos: Add new compatible string for
> Exynos3250 SoC.
> 
> On 09/11/2022 15:44, Aakarsh Jain wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> >> Sent: 09 November 2022 14:31
> >> 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; krzysztof.kozlowski+dt@linaro.org;
> >> 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; smitha.t@samsung.com
> >> Subject: Re: [Patch v2 1/3] arm: exynos: Add new compatible string
> >> for
> >> Exynos3250 SoC.
> >>
> >> On 09/11/2022 04:55, Aakarsh Jain wrote:
> >>> Since,MFC v7 support was added for Exynos5420 and Exynos
> >>> 3250 SoC with same compatible string "samsung,mfc-v7".As both SoCs
> >>> having different hardware properties and having same compatible
> >>> string for both SoCs doesn't seems to be correct.
> >>> New compatible is added for Exynos3250 SOC which will differentiate
> >>> the node properties for both SoCs which support MFC v7.
> >>>
> >>> Reviewed-by: Tommaso Merciai
> >> <tommaso.merciai@amarulasolutions.com>
> >>> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> >>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> >>> ---
> >>>  Documentation/devicetree/bindings/media/s5p-mfc.txt | 9 +++++----
> >>
> >> Use subject prefixes matching the subsystem (git log --oneline -- ...).
> >>
> > As with recent commits on
> Documentation/devicetree/bindings/media/s5p-mfc.txt with git log --
> oneline --  ,  subject prefix doesn't seems to be consistent.
> >
> > b1394dc151cb media: s5p-mfc: Adding initial support for MFC v10.10
> > 60641e22599a [media] s5p-mfc: Use preallocated block allocator always
> > for MFC v6+
> > 003611334d55 [media] s5p-mfc: Add support for MFC v8 available in
> > Exynos 5433 SoCs
> > 0da658704136 ARM: dts: convert to generic power domain bindings for
> > exynos DT 77634289286a ARM: dts: Update clocks entry in MFC binding
> > documentation
> > 2eae613b95a7 ARM: EXYNOS: Add MFC device tree support
> 
> s5p-mfc is not a subsystem.
> 
> git log --oneline  -- Documentation/devicetree/bindings/media/
> 
> media: dt-bindings: NAME_OF_FILE:
> 
Thanks for clarification.
> 
> >
> > Closest is ARM: dts.
> 
> This is not ARM subsystem and not a DTS file.
> 
> > so what is your suggestion on this?
> >
> > Anyway we are in a process of converting this txt file to yaml .
> >
> 
> Best regards,
> Krzysztof
Aakarsh Jain Nov. 11, 2022, 3:20 a.m. UTC | #6 
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 09 November 2022 14:43
> 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; krzysztof.kozlowski+dt@linaro.org;
> 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; smitha.t@samsung.com
> Subject: Re: [Patch v2 1/3] arm: exynos: Add new compatible string for
> Exynos3250 SoC.
> 
> On 09/11/2022 04:55, Aakarsh Jain wrote:
> > Since,MFC v7 support was added for Exynos5420 and Exynos
> > 3250 SoC with same compatible string "samsung,mfc-v7".As both SoCs
> > having different hardware properties and having same compatible string
> > for both SoCs doesn't seems to be correct.
> > New compatible is added for Exynos3250 SOC which will differentiate
> > the node properties for both SoCs which support MFC v7.
> >
> > Reviewed-by: Tommaso Merciai
> <tommaso.merciai@amarulasolutions.com>
> > Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  Documentation/devicetree/bindings/media/s5p-mfc.txt | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> 
> Beside my previous comment, please include changelog. This is v2, right?
> 
Yes this is v2.
I have sent v3 along with cover letter.

> You should consider joining something like:
> https://protect2.fireeye.com/v1/url?k=bf47b121-dfa52c7c-bf463a6e-
> 000babd9f1ba-202e284cd3057a5d&q=1&e=009abfcf-7d3c-433c-8eea-
> 0f9d92caf6cc&u=https%3A%2F%2Fwww.linaro.org%2Fevents%2Fmember-
> training-upstream-kernel-development%2F
> 
> Best regards,
> Krzysztof

Thanks for review.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
index aa54c8159d9f..cb166654fa81 100644
--- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
+++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
@@ -10,10 +10,11 @@  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
+	(c) "samsung,exynos3250-mfc" for MFC v7 present in Exynos3250 SoC
+	(d) "samsung,mfc-v7" for MFC v7 present in Exynos5420 SoC
+	(e) "samsung,mfc-v8" for MFC v8 present in Exynos5800 SoC
+	(f) "samsung,exynos5433-mfc" for MFC v8 present in Exynos5433 SoC
+	(g) "samsung,mfc-v10" for MFC v10 present in Exynos7880 SoC
 
   - reg : Physical base address of the IP registers and length of memory
 	  mapped region.