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

Hi Kukjin Kim & Sunyoung,

thanks for the review comments...

On Wed, Jul 18, 2012 at 2:14 PM, Sunyoung Kang [off-list ref] wrote:

Kukjin Kim wrote:

quoted

Shaik Ameer Basha wrote:

quoted

Add required clock support for Gscaler for exynos5

Hi,

Cc'ed Sunyoung Kang who knows gscaler well in my team.

quoted

Signed-off-by: Abhilash Kesavan <redacted>
Signed-off-by: Leela Krishna Amudala <redacted>
Signed-off-by: Prathyush K <redacted>
Signed-off-by: Shaik Ameer Basha <redacted>
---
 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.

As Sunyoung pointed out the '.ctrlbit' value is correct...

quoted

[snip]

I think, following part should be moved between

---
static struct clksrc_clk exynos5_clk_aclk_66 = {
[snip]
};

<<<HERE>>>

ok.. i will do that.

quoted

static struct clk exynos5_init_clocks_off[] = {
---

Please don't put your clock code without any checking the clock code.

quoted

+/* 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.

Sorry, I was referring to the old manual. 
I will add the 'exynos5_clk_mout_aclk_300_gscl_mid1' clk... in v3 release...

[snip]

Thanks.

BRs
Sunyoung

Thanks,
Shaik Ameer Basha