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

[Sunyoung Kang <sy0816.kang@samsung.com>]

Kukjin Kim wrote:
>
>Shaik Ameer Basha wrote:
>>
>> Add required clock support for Gscaler for exynos5
>>
>Hi,
>
>Cc'ed Sunyoung Kang who knows gscaler well in my team.
>
>> 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 |   79
>> ++++++++++++++++++++++++++++++++++
>>  1 files changed, 79 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-
>> exynos/clock-exynos5.c
>> index fefa336..c8293a3 100644
>> --- a/arch/arm/mach-exynos/clock-exynos5.c
>> +++ b/arch/arm/mach-exynos/clock-exynos5.c
>> @@ -741,6 +741,26 @@ static struct clk exynos5_init_clocks_off[] = {
>>  		.enable		= exynos5_clk_ip_peric_ctrl,
>>  		.ctrlbit	= (1 << 14),
>>  	}, {
>> +		.name		= "gscl",
>> +		.devname	= "exynos-gsc.0",
>> +		.enable		= exynos5_clk_ip_gscl_ctrl,
>> +		.ctrlbit	= (1 << 0),
>
>Sunyoung, I've seen (1 << 15) | (1 << 0) here instead, which one is right?
>

Actually, the ctrl bit, (1 << 0) is correct. The 15th bit Kukjin said is
used for another purpose.

>[snip]
>
>I think, following part should be moved between
>
>---
>static struct clksrc_clk exynos5_clk_aclk_66 = {
>[snip]
>};
>
><<<HERE>>>
>
>static struct clk exynos5_init_clocks_off[] = {
>---
>
>Please don't put your clock code without any checking the clock code.
>
>
>> +/* For ACLK_300_gscl_mid */
>> +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 */
>> +struct clk *exynos5_clkset_aclk_300_gscl_list[] = {
>> +	[0] = &exynos5_clk_mout_aclk_300_gscl_mid.clk,
>> +	[1] = &exynos5_clk_sclk_vpll.clk,
>
>As I know, this is wrong. Its [1] should be
>&exynos5_clk_mout_aclk_300_gscl_mid1.clk
>

Yes, right. It should be 'exynos5_clk_mout_aclk_300_gscl_mid1.clk' which is
generated by CPLL. The VPLL shouldn't be used here. It means you need to
implement CPLL for gscaler. As a note, CPLL can be used for fimd as well.

[snip]

Thanks.

BRs
Sunyoung