| Message ID | 20221102130602.48969-1-aakarsh.jain@samsung.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] arm64: exynos: Add new compatible string for Exynos3250 SoC. | expand |
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
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
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
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
> -----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.
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.
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.