| Message ID | 20221102130602.48969-2-aakarsh.jain@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| 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: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
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
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
> -----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/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,
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(-)