[PATCH 1/3] arm64: exynos: Add new compatible string for Exynos3250 SoC.

[PATCH 1/3] arm64: exynos: Add new compatible string for Exynos3250 SoC.

[Aakarsh Jain]

Exynos3250 uses the same compatible as Exynos5420, but both
the MFC IPs found in these SoC are different interms of clock
property. So using same compatible for both SoC is not correct.
Lets have a separate compatible for Exynos3250 and Exynos5420
to differentiate these SoCs.

Suggested-by: Alim Akhtar <alim.akhtar@samsung.com> 
Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
---
 Documentation/devicetree/bindings/media/s5p-mfc.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 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.

diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
index aa54c8159d9f..4ff1898e5a51 100644
--- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
+++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
@@ -11,9 +11,10 @@ Required properties:
 	(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
+	(d) "samsung,exynos3250-mfc" for MFC v7 present in Exynos3250 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.
-- 
2.17.1

[PATCH 2/3] media: s5p-mfc:Add variant data for MFC v7 hardware for Exynos 3250 SOC

[Aakarsh Jain]

commit "5441e9dafdfc6dc40fa" which adds mfc v7 support for
Exynos3250 and used the same compatible string as used by
Exynos5240 but both the IPs are a bit different in terms of
IP clock.
Lets add variant driver data based on the new compatible string
"samsung,exynos3250-mfc" for Exynos3250 SoC.

Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
Fixes: 5441e9dafdfc ("[media] s5p-mfc: Core support for MFC v7")
Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
---
 .../media/platform/samsung/s5p-mfc/s5p_mfc.c    | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c
index fca5c6405eec..007c7dbee037 100644
--- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c
@@ -1576,8 +1576,18 @@ static struct s5p_mfc_variant mfc_drvdata_v7 = {
 	.port_num	= MFC_NUM_PORTS_V7,
 	.buf_size	= &buf_size_v7,
 	.fw_name[0]     = "s5p-mfc-v7.fw",
-	.clk_names	= {"mfc", "sclk_mfc"},
-	.num_clocks	= 2,
+	.clk_names	= {"mfc"},
+	.num_clocks	= 1,
+};
+
+static struct s5p_mfc_variant mfc_drvdata_v7_3250 = {
+	.version        = MFC_VERSION_V7,
+	.version_bit    = MFC_V7_BIT,
+	.port_num       = MFC_NUM_PORTS_V7,
+	.buf_size       = &buf_size_v7,
+	.fw_name[0]     = "s5p-mfc-v7.fw",
+	.clk_names      = {"mfc", "sclk_mfc"},
+	.num_clocks     = 2,
 };
 
 static struct s5p_mfc_buf_size_v6 mfc_buf_size_v8 = {
@@ -1647,6 +1657,9 @@ static const struct of_device_id exynos_mfc_match[] = {
 	}, {
 		.compatible = "samsung,mfc-v7",
 		.data = &mfc_drvdata_v7,
+	}, {
+		.compatible = "samsung,exynos3250-mfc",
+		.data = &mfc_drvdata_v7_3250,
 	}, {
 		.compatible = "samsung,mfc-v8",
 		.data = &mfc_drvdata_v8,
-- 
2.17.1

Re: [PATCH 2/3] media: s5p-mfc:Add variant data for MFC v7 hardware for Exynos 3250 SOC

[Tommaso Merciai]

Hi Aakarsh,

On Wed, Nov 02, 2022 at 06:36:01PM +0530, Aakarsh Jain wrote:
> commit "5441e9dafdfc6dc40fa" which adds mfc v7 support for
> Exynos3250 and used the same compatible string as used by
> Exynos5240 but both the IPs are a bit different in terms of
> IP clock.
> Lets add variant driver data based on the new compatible string
> "samsung,exynos3250-mfc" for Exynos3250 SoC.
> 
> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> Fixes: 5441e9dafdfc ("[media] s5p-mfc: Core support for MFC v7")
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  .../media/platform/samsung/s5p-mfc/s5p_mfc.c    | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c
> index fca5c6405eec..007c7dbee037 100644
> --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c
> @@ -1576,8 +1576,18 @@ static struct s5p_mfc_variant mfc_drvdata_v7 = {
>  	.port_num	= MFC_NUM_PORTS_V7,
>  	.buf_size	= &buf_size_v7,
>  	.fw_name[0]     = "s5p-mfc-v7.fw",
> -	.clk_names	= {"mfc", "sclk_mfc"},
> -	.num_clocks	= 2,
> +	.clk_names	= {"mfc"},
> +	.num_clocks	= 1,
> +};
> +
> +static struct s5p_mfc_variant mfc_drvdata_v7_3250 = {
> +	.version        = MFC_VERSION_V7,
> +	.version_bit    = MFC_V7_BIT,
> +	.port_num       = MFC_NUM_PORTS_V7,
> +	.buf_size       = &buf_size_v7,
> +	.fw_name[0]     = "s5p-mfc-v7.fw",
> +	.clk_names      = {"mfc", "sclk_mfc"},
> +	.num_clocks     = 2,
>  };
>  
>  static struct s5p_mfc_buf_size_v6 mfc_buf_size_v8 = {
> @@ -1647,6 +1657,9 @@ static const struct of_device_id exynos_mfc_match[] = {
>  	}, {
>  		.compatible = "samsung,mfc-v7",
>  		.data = &mfc_drvdata_v7,
> +	}, {
> +		.compatible = "samsung,exynos3250-mfc",
> +		.data = &mfc_drvdata_v7_3250,
>  	}, {
>  		.compatible = "samsung,mfc-v8",
>  		.data = &mfc_drvdata_v8,
> -- 
> 2.17.1
> 

Patch looks good to me, only one fix in commit body:

"... Exynos3250 and used the same compatible string..."

with:

"... Exynos3250 and use the same compatible string...

But this is a nitpicking :)

Regards,
Tommaso


-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH 2/3] media: s5p-mfc:Add variant data for MFC v7 hardware for Exynos 3250 SOC

[Krzysztof Kozlowski]

On 02/11/2022 09:06, Aakarsh Jain wrote:
> commit "5441e9dafdfc6dc40fa" which adds mfc v7 support for

Please run scripts/checkpatch.pl and fix reported warnings.

> Exynos3250 and used the same compatible string as used by
> Exynos5240 but both the IPs are a bit different in terms of
> IP clock.
> Lets add variant driver data based on the new compatible string
> "samsung,exynos3250-mfc" for Exynos3250 SoC.

Aren't you just missing the clock on Exynos3250?

Best regards,
Krzysztof

Re: [PATCH 2/3] media: s5p-mfc:Add variant data for MFC v7 hardware for Exynos 3250 SOC

[Marek Szyprowski]

On 03.11.2022 13:35, Krzysztof Kozlowski wrote:
> On 02/11/2022 09:06, Aakarsh Jain wrote:
>> commit "5441e9dafdfc6dc40fa" which adds mfc v7 support for
> Please run scripts/checkpatch.pl and fix reported warnings.
>
>> Exynos3250 and used the same compatible string as used by
>> Exynos5240 but both the IPs are a bit different in terms of
>> IP clock.
>> Lets add variant driver data based on the new compatible string
>> "samsung,exynos3250-mfc" for Exynos3250 SoC.
> Aren't you just missing the clock on Exynos3250?

Nope, the Exynos3250 variant indeed has only one clock and the driver 
code simply ignored the -ENOENT error while getting the clocks, see the 
code in drivers/media/platform/samsung/s5p-mfc/s5p_mfc_pm.c, so it 
worked fine even without it.

IMHO it is a good idea to clean this up.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 2/3] media: s5p-mfc:Add variant data for MFC v7 hardware for Exynos 3250 SOC

[Krzysztof Kozlowski]

On 03/11/2022 08:44, Marek Szyprowski wrote:
> On 03.11.2022 13:35, Krzysztof Kozlowski wrote:
>> On 02/11/2022 09:06, Aakarsh Jain wrote:
>>> commit "5441e9dafdfc6dc40fa" which adds mfc v7 support for
>> Please run scripts/checkpatch.pl and fix reported warnings.
>>
>>> Exynos3250 and used the same compatible string as used by
>>> Exynos5240 but both the IPs are a bit different in terms of
>>> IP clock.
>>> Lets add variant driver data based on the new compatible string
>>> "samsung,exynos3250-mfc" for Exynos3250 SoC.
>> Aren't you just missing the clock on Exynos3250?
> 
> Nope, the Exynos3250 variant indeed has only one clock and the driver 
> code simply ignored the -ENOENT error while getting the clocks, see the 
> code in drivers/media/platform/samsung/s5p-mfc/s5p_mfc_pm.c, so it 
> worked fine even without it.
> 
> IMHO it is a good idea to clean this up.

OK, then please make the new compatible followed by old.

Best regards,
Krzysztof

RE: [PATCH 2/3] media: s5p-mfc:Add variant data for MFC v7 hardware for Exynos 3250 SOC

[Aakarsh Jain]

> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 03 November 2022 18:16
> To: Marek Szyprowski <m.szyprowski@samsung.com>; 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: 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 2/3] media: s5p-mfc:Add variant data for MFC v7
> hardware for Exynos 3250 SOC
> 
> On 03/11/2022 08:44, Marek Szyprowski wrote:
> > On 03.11.2022 13:35, Krzysztof Kozlowski wrote:
> >> On 02/11/2022 09:06, Aakarsh Jain wrote:
> >>> commit "5441e9dafdfc6dc40fa" which adds mfc v7 support for
> >> Please run scripts/checkpatch.pl and fix reported warnings.
> >>
As I didn't see any checkpatch warnings from the above commit message.
Anyway I fixed all checkpatch warnings from drivers/media/platform/samsung/s5p-mfc/s5p_mfc.c and posted 3 patches for the same.
https://patchwork.kernel.org/project/linux-media/patch/20221109035348.69026-1-aakarsh.jain@samsung.com/

> >>> Exynos3250 and used the same compatible string as used by
> >>> Exynos5240 but both the IPs are a bit different in terms of IP
> >>> clock.
> >>> Lets add variant driver data based on the new compatible string
> >>> "samsung,exynos3250-mfc" for Exynos3250 SoC.
> >> Aren't you just missing the clock on Exynos3250?
> >
> > Nope, the Exynos3250 variant indeed has only one clock and the driver
> > code simply ignored the -ENOENT error while getting the clocks, see
> > the code in drivers/media/platform/samsung/s5p-mfc/s5p_mfc_pm.c, so it
> > worked fine even without it.
> >
> > IMHO it is a good idea to clean this up.
> 
> OK, then please make the new compatible followed by old.
> 
> Best regards,
> Krzysztof


Thanks for the review.

[PATCH 3/3] arm64: dts: exynos: Rename compatible string property from version to SOC specific

[Aakarsh Jain]

commit "752d3a23d1f68de87e3c" which adds MFC codec device node
for exynos3250 SOC. Since exynos3250.dtsi and exynos5420.dtsi are
using same compatible string as "samsung,mfc-v7" but their
node properties are different.As both SoCs have MFC v7 hardware
module but with different clock hierarchy and complexity.
So renaming compatible string from version specific to SOC based.

Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
Fixes: 752d3a23d1f6 ("ARM: dts: add MFC codec device node for exynos3250")
Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
---
 arch/arm/boot/dts/exynos3250.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index 326b9e0ed8d3..98105c64f7d9 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -485,7 +485,7 @@
 		};
 
 		mfc: codec@13400000 {
-			compatible = "samsung,mfc-v7";
+			compatible = "samsung,exynos3250-mfc";
 			reg = <0x13400000 0x10000>;
 			interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
 			clock-names = "mfc", "sclk_mfc";
-- 
2.17.1

Re: [PATCH 3/3] arm64: dts: exynos: Rename compatible string property from version to SOC specific

[Tommaso Merciai]

Hi Aakarsh,

On Wed, Nov 02, 2022 at 06:36:02PM +0530, Aakarsh Jain wrote:
> commit "752d3a23d1f68de87e3c" which adds MFC codec device node
> for exynos3250 SOC. Since exynos3250.dtsi and exynos5420.dtsi are
> using same compatible string as "samsung,mfc-v7" but their
> node properties are different.As both SoCs have MFC v7 hardware
> module but with different clock hierarchy and complexity.
> So renaming compatible string from version specific to SOC based.
> 
> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> Fixes: 752d3a23d1f6 ("ARM: dts: add MFC codec device node for exynos3250")
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  arch/arm/boot/dts/exynos3250.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
> index 326b9e0ed8d3..98105c64f7d9 100644
> --- a/arch/arm/boot/dts/exynos3250.dtsi
> +++ b/arch/arm/boot/dts/exynos3250.dtsi
> @@ -485,7 +485,7 @@
>  		};
>  
>  		mfc: codec@13400000 {
> -			compatible = "samsung,mfc-v7";
> +			compatible = "samsung,exynos3250-mfc";
>  			reg = <0x13400000 0x10000>;
>  			interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
>  			clock-names = "mfc", "sclk_mfc";
> -- 
> 2.17.1
> 

Looks good to me.
Reviewed-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>


Regards,
Tommaso

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH 3/3] arm64: dts: exynos: Rename compatible string property from version to SOC specific

[Krzysztof Kozlowski]

On 02/11/2022 09:06, Aakarsh Jain wrote:
> commit "752d3a23d1f68de87e3c" which adds MFC codec device node
> for exynos3250 SOC. Since exynos3250.dtsi and exynos5420.dtsi are
> using same compatible string as "samsung,mfc-v7" but their
> node properties are different.As both SoCs have MFC v7 hardware
> module but with different clock hierarchy and complexity.
> So renaming compatible string from version specific to SOC based.
> 
> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> Fixes: 752d3a23d1f6 ("ARM: dts: add MFC codec device node for exynos3250")

There is no bug to fix and backporting is forbidden as it breaks the
usage of DTS in older kernel.

> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  arch/arm/boot/dts/exynos3250.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
> index 326b9e0ed8d3..98105c64f7d9 100644
> --- a/arch/arm/boot/dts/exynos3250.dtsi
> +++ b/arch/arm/boot/dts/exynos3250.dtsi
> @@ -485,7 +485,7 @@
>  		};
>  
>  		mfc: codec@13400000 {
> -			compatible = "samsung,mfc-v7";
> +			compatible = "samsung,exynos3250-mfc";

The change is non-bisectable and breaks using DTS in older kernel.

Best regards,
Krzysztof

RE: [PATCH 3/3] arm64: dts: exynos: Rename compatible string property from version to SOC specific

[Aakarsh Jain]

> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
> Sent: 03 November 2022 18:04
> 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 3/3] arm64: dts: exynos: Rename compatible string
> property from version to SOC specific
> 
> On 02/11/2022 09:06, Aakarsh Jain wrote:
> > commit "752d3a23d1f68de87e3c" which adds MFC codec device node for
> > exynos3250 SOC. Since exynos3250.dtsi and exynos5420.dtsi are using
> > same compatible string as "samsung,mfc-v7" but their node properties
> > are different.As both SoCs have MFC v7 hardware module but with
> > different clock hierarchy and complexity.
> > So renaming compatible string from version specific to SOC based.
> >
> > Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> > Fixes: 752d3a23d1f6 ("ARM: dts: add MFC codec device node for
> > exynos3250")
> 
> There is no bug to fix and backporting is forbidden as it breaks the usage of
> DTS in older kernel.
> 
Okay will remove this Fix tag in next series.
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  arch/arm/boot/dts/exynos3250.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/exynos3250.dtsi
> > b/arch/arm/boot/dts/exynos3250.dtsi
> > index 326b9e0ed8d3..98105c64f7d9 100644
> > --- a/arch/arm/boot/dts/exynos3250.dtsi
> > +++ b/arch/arm/boot/dts/exynos3250.dtsi
> > @@ -485,7 +485,7 @@
> >  		};
> >
> >  		mfc: codec@13400000 {
> > -			compatible = "samsung,mfc-v7";
> > +			compatible = "samsung,exynos3250-mfc";
> 
> The change is non-bisectable and breaks using DTS in older kernel.
> 
Right, so what is your suggestion on this?
I can see two ways here:
1> To squash patch2 and patch3 in one?
2> Have a warning about this breakage in the patch-3 commit message?

> Best regards,
> Krzysztof


Thanks for the review.

Re: [PATCH 3/3] arm64: dts: exynos: Rename compatible string property from version to SOC specific

[Krzysztof Kozlowski]

On 04/11/2022 06:49, Aakarsh Jain wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
>> Sent: 03 November 2022 18:04
>> 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 3/3] arm64: dts: exynos: Rename compatible string
>> property from version to SOC specific
>>
>> On 02/11/2022 09:06, Aakarsh Jain wrote:
>>> commit "752d3a23d1f68de87e3c" which adds MFC codec device node for
>>> exynos3250 SOC. Since exynos3250.dtsi and exynos5420.dtsi are using
>>> same compatible string as "samsung,mfc-v7" but their node properties
>>> are different.As both SoCs have MFC v7 hardware module but with
>>> different clock hierarchy and complexity.
>>> So renaming compatible string from version specific to SOC based.
>>>
>>> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
>>> Fixes: 752d3a23d1f6 ("ARM: dts: add MFC codec device node for
>>> exynos3250")
>>
>> There is no bug to fix and backporting is forbidden as it breaks the usage of
>> DTS in older kernel.
>>
> Okay will remove this Fix tag in next series.
>>> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
>>> ---
>>>  arch/arm/boot/dts/exynos3250.dtsi | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos3250.dtsi
>>> b/arch/arm/boot/dts/exynos3250.dtsi
>>> index 326b9e0ed8d3..98105c64f7d9 100644
>>> --- a/arch/arm/boot/dts/exynos3250.dtsi
>>> +++ b/arch/arm/boot/dts/exynos3250.dtsi
>>> @@ -485,7 +485,7 @@
>>>  		};
>>>
>>>  		mfc: codec@13400000 {
>>> -			compatible = "samsung,mfc-v7";
>>> +			compatible = "samsung,exynos3250-mfc";
>>
>> The change is non-bisectable and breaks using DTS in older kernel.
>>
> Right, so what is your suggestion on this?
> I can see two ways here:
> 1> To squash patch2 and patch3 in one?
> 2> Have a warning about this breakage in the patch-3 commit message?

Here - nothing except implementing bindings. The suggestion was given to
the bindings patch - use fallback.

Best regards,
Krzysztof

Re: [PATCH 1/3] arm64: exynos: Add new compatible string for Exynos3250 SoC.

[Marek Szyprowski]

Hi

On 02.11.2022 14:06, Aakarsh Jain wrote:
> Exynos3250 uses the same compatible as Exynos5420, but both
> the MFC IPs found in these SoC are different interms of clock
> property. So using same compatible for both SoC is not correct.
> Lets have a separate compatible for Exynos3250 and Exynos5420
> to differentiate these SoCs.
>
> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com>
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>

Minor issue, Exynos3250 is based on ARM 32bit not ARM 64bit (see the 
patch subject).

> ---
>   Documentation/devicetree/bindings/media/s5p-mfc.txt | 7 ++++---
>   1 file changed, 4 insertions(+), 3 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.
>
> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> index aa54c8159d9f..4ff1898e5a51 100644
> --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> @@ -11,9 +11,10 @@ Required properties:
>   	(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
> +	(d) "samsung,exynos3250-mfc" for MFC v7 present in Exynos3250 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.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 1/3] arm64: exynos: Add new compatible string for Exynos3250 SoC.

[Tommaso Merciai]

Hi Aakarsh,

On Wed, Nov 02, 2022 at 06:36:00PM +0530, Aakarsh Jain wrote:
> Exynos3250 uses the same compatible as Exynos5420, but both
> the MFC IPs found in these SoC are different interms of clock
> property. So using same compatible for both SoC is not correct.
> Lets have a separate compatible for Exynos3250 and Exynos5420
> to differentiate these SoCs.
> 
> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com> 
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  Documentation/devicetree/bindings/media/s5p-mfc.txt | 7 ++++---
>  1 file changed, 4 insertions(+), 3 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.
> 
> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> index aa54c8159d9f..4ff1898e5a51 100644
> --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> @@ -11,9 +11,10 @@ Required properties:
>  	(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
> +	(d) "samsung,exynos3250-mfc" for MFC v7 present in Exynos3250 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.
> -- 
> 2.17.1
> 

Looks good to me.

Reviewed-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>

Regards,
Tommaso

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH 1/3] arm64: exynos: Add new compatible string for Exynos3250 SoC.

[Krzysztof Kozlowski]

On 02/11/2022 09:06, Aakarsh Jain wrote:
> Exynos3250 uses the same compatible as Exynos5420, but both
> the MFC IPs found in these SoC are different interms of clock
> property. So using same compatible for both SoC is not correct.
> Lets have a separate compatible for Exynos3250 and Exynos5420
> to differentiate these SoCs.
> 
> Suggested-by: Alim Akhtar <alim.akhtar@samsung.com> 
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  Documentation/devicetree/bindings/media/s5p-mfc.txt | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

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

> 
> 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.
> 
> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> index aa54c8159d9f..4ff1898e5a51 100644
> --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> @@ -11,9 +11,10 @@ Required properties:
>  	(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
> +	(d) "samsung,exynos3250-mfc" for MFC v7 present in Exynos3250 SoC

This should be followed by mfc-v7 fallback.

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

Best regards,
Krzysztof