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 <[email protected]>
Signed-off-by: Aakarsh Jain <[email protected]>
---
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/
[email protected]/
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
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 <[email protected]>
Fixes: 752d3a23d1f6 ("ARM: dts: add MFC codec device node for exynos3250")
Signed-off-by: Aakarsh Jain <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Aakarsh Jain <[email protected]>
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/
> [email protected]/
> 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
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 <[email protected]>
> Fixes: 752d3a23d1f6 ("ARM: dts: add MFC codec device node for exynos3250")
> Signed-off-by: Aakarsh Jain <[email protected]>
> ---
> 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 <[email protected]>
Regards,
Tommaso
--
Tommaso Merciai
Embedded Linux Engineer
[email protected]
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com
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 <[email protected]>
> Signed-off-by: Aakarsh Jain <[email protected]>
> ---
> 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/
> [email protected]/
> 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 <[email protected]>
Regards,
Tommaso
--
Tommaso Merciai
Embedded Linux Engineer
[email protected]
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com
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 <[email protected]>
> Signed-off-by: Aakarsh Jain <[email protected]>
> ---
> 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/
> [email protected]/
> 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
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 <[email protected]>
> 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 <[email protected]>
> ---
> 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
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:[email protected]]
> Sent: 03 November 2022 18:04
> To: Aakarsh Jain <[email protected]>; linux-arm-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> 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 <[email protected]>
> > 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 <[email protected]>
> > ---
> > 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.
On 04/11/2022 06:49, Aakarsh Jain wrote:
>
>
>> -----Original Message-----
>> From: Krzysztof Kozlowski [mailto:[email protected]]
>> Sent: 03 November 2022 18:04
>> To: Aakarsh Jain <[email protected]>; linux-arm-
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]
>> Cc: [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]
>> 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 <[email protected]>
>>> 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 <[email protected]>
>>> ---
>>> 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