RE: [PATCH v3 1/2] ARM: EXYNOS: Add clock support for Gscaler

[Kukjin Kim <kgene.kim@samsung.com>]

Shaik Ameer Basha wrote:
> 
> Add required clock support for Gscaler for exynos5
> 
> Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Leela Krishna Amudala <l.krishna@samsung.com>
> Signed-off-by: Prathyush K <prathyush.k@samsung.com>
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> ---
>  arch/arm/mach-exynos/clock-exynos5.c |  100
> ++++++++++++++++++++++++++++++++++
>  1 files changed, 100 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-
> exynos/clock-exynos5.c
> index 774533c..49a76b1 100644
> --- a/arch/arm/mach-exynos/clock-exynos5.c
> +++ b/arch/arm/mach-exynos/clock-exynos5.c
> @@ -552,6 +552,81 @@ static struct clksrc_clk exynos5_clk_aclk_66 = {
>  	.reg_div = { .reg = EXYNOS5_CLKDIV_TOP0, .shift = 0, .size = 3 },
>  };
> 
> +/* for aclk_300_gscl_mid */

No need above comment which is certain.

> +static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl_mid = {
> +	.clk = {
> +		.name		= "mout_aclk_300_gscl_mid",
> +	},
> +	.sources = &exynos5_clkset_aclk,
> +	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 24, .size = 1 },
> +};
> +
> +/* for aclk_300_gscl_mid1 */

Same as above.

> +static struct clk *exynos5_clkset_aclk_300_gscl_mid1_list[] = {
> +	[0] = &exynos5_clk_sclk_vpll.clk,
> +	[1] = &exynos5_clk_mout_cpll.clk,
> +};

In this case, the above sources can be used for gscl_mid1 and disp1_mid as
well. So how about exynos5_clkset_mid1_list?

> +
> +static struct clksrc_sources exynos5_clkset_aclk_300_gscl_mid1 = {
> +	.sources	= exynos5_clkset_aclk_300_gscl_mid1_list,
> +	.nr_sources	=
ARRAY_SIZE(exynos5_clkset_aclk_300_gscl_mid1_list),
> +};

If so, need to update this.

> +
> +

no need double empty lines.

> +static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl_mid1 = {
> +	.clk    = {
> +		.name		= "mout_aclk_300_gscl_mid1",
> +	},
> +	.sources = &exynos5_clkset_aclk_300_gscl_mid1,
> +	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP1, .shift = 12, .size = 1 },
> +};
> +
> +/* for aclk_300_gscl */

no need useless comment.

> +static struct clk *exynos5_clkset_aclk_300_gscl_list[] = {
> +	[0] = &exynos5_clk_mout_aclk_300_gscl_mid.clk,
> +	[1] = &exynos5_clk_mout_aclk_300_gscl_mid1.clk,
> +};
> +
> +static struct clksrc_sources exynos5_clkset_aclk_300_gscl = {
> +	.sources	= exynos5_clkset_aclk_300_gscl_list,
> +	.nr_sources	= ARRAY_SIZE(exynos5_clkset_aclk_300_gscl_list),
> +};
> +
> +static struct clksrc_clk exynos5_clk_mout_aclk_300_gscl = {
> +	.clk    = {
            ^^^^
Tap please.

> +		.name		= "mout_aclk_300_gscl",
> +	},
> +	.sources = &exynos5_clkset_aclk_300_gscl,
> +	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 25, .size = 1 },
> +};
> +
> +static struct clksrc_clk exynos5_clk_dout_aclk_300_gscl = {
> +	.clk    = {
            ^^^^
Same as above.

> +		.name		= "dout_aclk_300_gscl",
> +		.parent		= &exynos5_clk_mout_aclk_300_gscl.clk,
> +	},
> +	.reg_div = { .reg = EXYNOS5_CLKDIV_TOP1, .shift = 12, .size = 3 },
> +};

And I think, we don't need to define above 'clksrc_clk's?

+static struct clksrc_clk exynos5_clk_mdout_aclk_300_gscl = {
+	.clk    = {
+		.name		= "mdout_aclk_300_gscl",
+	},
+	.sources = &exynos5_clkset_aclk_300_gscl,
+	.reg_src = { .reg = EXYNOS5_CLKSRC_TOP0, .shift = 25, .size = 1 },
> +	.reg_div = { .reg = EXYNOS5_CLKDIV_TOP1, .shift = 12, .size = 3 },
+};

[...]

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.