ARM: S3C24XX: Add clkdev entry for camif-upll clock

PATCH New Sylwester Nawrocki 3 participants 10 messages Nov 7, 2012 – Nov 22, 2012

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm Samsung Exynos

lore

Share & tools

lore

Sylwester Nawrocki

author

2012-11-07 22:00 UTC #0 lore raw

PATCH 1/2 ARM: S3C24XX: Add clkdev entry for camif-upll clock

The s3c-camif driver uses "camera" clock conn_id for the "camif-upll" (s3c244x) and "camera" (s3c64xx) platform clock. By adding this new clkdev entry the platform differences are isolated from the driver.

Signed-off-by Sylwester Nawrocki <[email protected]>

---
 arch/arm/mach-s3c24xx/clock-s3c2440.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/mach-s3c24xx/clock-s3c2440.c +1 -0

index 4407b17..04b87ec 100644
@@ -161,6 +161,7 @@ static struct clk_lookup s3c2440_clk_lookup[] = {
 	CLKDEV_INIT(NULL, "clk_uart_baud1", &s3c24xx_uclk),
 	CLKDEV_INIT(NULL, "clk_uart_baud2", &clk_p),
 	CLKDEV_INIT(NULL, "clk_uart_baud3", &s3c2440_clk_fclk_n),
+	CLKDEV_INIT("s3c2440-camif", "camera", &s3c2440_clk_cam_upll),
 };
 
 static int __init_refok s3c2440_clk_add(struct device *dev, struct subsys_interface *sif)

Signature · 1 line

1.7.4.1

Sylwester Nawrocki

author

2012-11-07 22:00 UTC #1 lore raw

PATCH 2/2 ARM: S3C64XX: Statically define parent clock of the "camera" clock

The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null clock source mux control register as it can have only one parent clock. In such cases there is a need to configure the parent clock statically, otherwise s3c_set_clksrc() bails out with an error message "no parent clock specified" leaving the parent clock not configured. Define statically the parent clock so it is possible to get or set rate of the "camera" clock.

Reported-by In-Bae Jeong <[email protected]>

Signed-off-by Sylwester Nawrocki <[email protected]>

---
 arch/arm/mach-s3c64xx/clock.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/mach-s3c64xx/clock.c +1 -0

index 28041e8..85b9cf1 100644
@@ -744,6 +744,7 @@ static struct clksrc_clk clksrcs[] = {
 			.name		= "camera",
 			.ctrlbit        = S3C_CLKCON_SCLK_CAM,
 			.enable		= s3c64xx_sclk_ctrl,
+			.parent		= &clk_h2,
 		},
 		.reg_div	= { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
 		.reg_src	= { .reg = NULL, .shift = 0, .size = 0  },

Signature · 1 line

1.7.4.1

Andrey Gusakov

2012-11-09 07:42 UTC #2 lore raw

PATCH 2/2 ARM: S3C64XX: Statically define parent clock of the "camera" clock

Hi.

I think .reg_src can be removed? This clock have only one source.

Quoted 33 lines · Sylwester Nawrocki

The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null clock source mux control register as it can have only one parent clock. In such cases there is a need to configure the parent clock statically, otherwise s3c_set_clksrc() bails out with an error message "no parent clock specified" leaving the parent clock not configured. Define statically the parent clock so it is possible to get or set rate of the "camera" clock.
Reported-by In-Bae Jeong <[email protected]>
Signed-off-by Sylwester Nawrocki <[email protected]>
---
 arch/arm/mach-s3c64xx/clock.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/mach-s3c64xx/clock.c +1 -1
index 28041e8..85b9cf1 100644
@@ -744,6 +744,7 @@ static struct clksrc_clk clksrcs[] = {
                        .name           = "camera",
                        .ctrlbit        = S3C_CLKCON_SCLK_CAM,
                        .enable         = s3c64xx_sclk_ctrl,
+                       .parent         = &clk_h2,
                },
                .reg_div        = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
                .reg_src        = { .reg = NULL, .shift = 0, .size = 0  },
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Sylwester Nawrocki

author

2012-11-09 08:30 UTC #3 lore raw

PATCH 2/2 ARM: S3C64XX: Statically define parent clock of the "camera" clock

Quoted 3 lines · Andrey Gusakov

Hi.
I think .reg_src can be removed? This clock have only one source.

Yes, good point. I'll repost with reg_src removed. Can you test that patch then ?

Thanks,
Sylwester

Quoted 30 lines

Quoted 28 lines · Sylwester Nawrocki
The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null clock source mux control register as it can have only one parent clock. In such cases there is a need to configure the parent clock statically, otherwise s3c_set_clksrc() bails out with an error message "no parent clock specified" leaving the parent clock not configured. Define statically the parent clock so it is possible to get or set rate of the "camera" clock.
Reported-by In-Bae Jeong<[email protected]>
Signed-off-by Sylwester Nawrocki<[email protected]>
---
  arch/arm/mach-s3c64xx/clock.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/mach-s3c64xx/clock.c +1 -1
index 28041e8..85b9cf1 100644
@@ -744,6 +744,7 @@ static struct clksrc_clk clksrcs[] = {
                         .name           = "camera",
                         .ctrlbit        = S3C_CLKCON_SCLK_CAM,
                         .enable         = s3c64xx_sclk_ctrl,
+                       .parent         =&clk_h2,
                 },
                 .reg_div        = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
                 .reg_src        = { .reg = NULL, .shift = 0, .size = 0  },
--
1.7.4.1

Sylwester Nawrocki

author

2012-11-09 08:32 UTC #4 lore raw

PATCH 1/1 ARM: S3C64XX: Statically define parent clock of the "camera" clock

In reply to Andrey Gusakov

Reported-by In-Bae Jeong <[email protected]>

Signed-off-by Sylwester Nawrocki <[email protected]>

---
 arch/arm/mach-s3c64xx/clock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/arm/mach-s3c64xx/clock.c +1 -1

index 28041e8..2e6d7f9 100644
@@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
 			.name		= "camera",
 			.ctrlbit        = S3C_CLKCON_SCLK_CAM,
 			.enable		= s3c64xx_sclk_ctrl,
+			.parent		= &clk_h2,
 		},
 		.reg_div	= { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
-		.reg_src	= { .reg = NULL, .shift = 0, .size = 0  },
 		.sources	= &clkset_camif,
 	},
 };

Signature · 1 line

1.7.4.1

Andrey Gusakov

2012-11-09 09:31 UTC #5 lore raw

PATCH 1/1 ARM: S3C64XX: Statically define parent clock of the "camera" clock

Hi.

Quoted 20 lines · Sylwester Nawrocki

The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null clock source mux control register as it can have only one parent clock. In such cases there is a need to configure the parent clock statically, otherwise s3c_set_clksrc() bails out with an error message "no parent clock specified" leaving the parent clock not configured. Define statically the parent clock so it is possible to get or set rate of the "camera" clock.
While at it remove the unneded null reg_src definition.
Reported-by In-Bae Jeong <[email protected]>
Signed-off-by Sylwester Nawrocki <[email protected]>
---
 arch/arm/mach-s3c64xx/clock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-s3c64xx/clock.c +0 -0
index 28041e8..2e6d7f9 100644

arch/arm/mach-s3c64xx/clock.c static struct clksrc_clk clksrcs[] = { · line 750

Quoted 8 lines

arch/arm/mach-s3c64xx/clock.c +1 -1

@@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
                        .name           = "camera",
                        .ctrlbit        = S3C_CLKCON_SCLK_CAM,
                        .enable         = s3c64xx_sclk_ctrl,
+                       .parent         = &clk_h2,
                },
                .reg_div        = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
-               .reg_src        = { .reg = NULL, .shift = 0, .size = 0  },
                .sources        = &clkset_camif,

Just figure out that .sources can be removed to. And seems "clkset_camif" and "clkset_camif_list" can be removed as unused.

I'll try to test it tiday.

Sylwester Nawrocki

author

2012-11-09 10:05 UTC #6 lore raw

PATCH 1/1 ARM: S3C64XX: Statically define parent clock of the "camera" clock

Hi,

Quoted 17 lines · Andrey Gusakov

Quoted 15 lines · Sylwester Nawrocki
The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null clock source mux control register as it can have only one parent clock. In such cases there is a need to configure the parent clock statically, otherwise s3c_set_clksrc() bails out with an error message "no parent clock specified" leaving the parent clock not configured. Define statically the parent clock so it is possible to get or set rate of the "camera" clock.
While at it remove the unneded null reg_src definition.
Reported-by In-Bae Jeong<[email protected]>
Signed-off-by Sylwester Nawrocki<[email protected]>
---
  arch/arm/mach-s3c64xx/clock.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

Quoted 15 lines

arch/arm/mach-s3c64xx/clock.c static struct clksrc_clk clksrcs[] = { · line 750

Quoted 13 lines

arch/arm/mach-s3c64xx/clock.c +1 -1

index 28041e8..2e6d7f9 100644
@@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
                         .name           = "camera",
                         .ctrlbit        = S3C_CLKCON_SCLK_CAM,
                         .enable         = s3c64xx_sclk_ctrl,
+                       .parent         =&clk_h2,
                 },
                 .reg_div        = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4  },
-               .reg_src        = { .reg = NULL, .shift = 0, .size = 0  },
                 .sources        =&clkset_camif,

Just figure out that .sources can be removed to. And seems "clkset_camif" and "clkset_camif_list" can be removed as unused.

OK, you're right. I think it could be done as a separate patch, depending on this one to avoid conflicts.

Quoted 1 line

I'll try to test it tiday.

Thanks.

Sylwester Nawrocki

author

2012-11-09 10:18 UTC #7 lore raw

PATCH 1/1 ARM: S3C64XX: Statically define parent clock of the "camera" clock

Quoted 37 lines · Sylwester Nawrocki

Hi,
Quoted 17 lines · Andrey Gusakov
Quoted 15 lines · Sylwester Nawrocki
The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null clock source mux control register as it can have only one parent clock. In such cases there is a need to configure the parent clock statically, otherwise s3c_set_clksrc() bails out with an error message "no parent clock specified" leaving the parent clock not configured. Define statically the parent clock so it is possible to get or set rate of the "camera" clock.
While at it remove the unneded null reg_src definition.
Reported-by In-Bae Jeong<[email protected]>
Signed-off-by Sylwester Nawrocki<[email protected]>
---
arch/arm/mach-s3c64xx/clock.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 16 lines
Quoted 6 lines
arch/arm/mach-s3c64xx/clock.c +0 -0
index 28041e8..2e6d7f9 100644
arch/arm/mach-s3c64xx/clock.c static struct clksrc_clk clksrcs[] = { · line 750
Quoted 8 lines
arch/arm/mach-s3c64xx/clock.c +1 -1
@@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
 .name = "camera",
 .ctrlbit = S3C_CLKCON_SCLK_CAM,
 .enable = s3c64xx_sclk_ctrl,
+ .parent =&clk_h2,
 },
 .reg_div = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4 },
- .reg_src = { .reg = NULL, .shift = 0, .size = 0 },
 .sources =&clkset_camif,
Just figure out that .sources can be removed to. And seems "clkset_camif" and "clkset_camif_list" can be removed as unused.

Quoted 2 lines

OK, you're right. I think it could be done as a separate patch, depending on this one to avoid conflicts.

Or feel free to make a new patch, ignoring this one.

--
Regards,
Sylwester

Kukjin Kim

2012-11-09 10:08 UTC #8 lore raw

PATCH 1/1 ARM: S3C64XX: Statically define parent clock of the "camera" clock

In reply to Andrey Gusakov

Quoted 22 lines · Andrey Gusakov

Hi.
arch/arm/mach-
Quoted 16 lines · Sylwester Nawrocki
The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null clock source mux control register as it can have only one parent clock. In such cases there is a need to configure the parent clock statically, otherwise s3c_set_clksrc() bails out with an error message "no parent clock specified" leaving the parent clock not configured. Define statically the parent clock so it is possible to get or set rate of the "camera" clock.
While at it remove the unneded null reg_src definition.
Reported-by In-Bae Jeong <[email protected]>
Signed-off-by Sylwester Nawrocki <[email protected]>
---
 arch/arm/mach-s3c64xx/clock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-s3c64xx/clock.c +0 -0
s3c64xx/clock.c

Quoted 10 lines

arch/arm/mach-s3c64xx/clock.c static struct clksrc_clk clksrcs[] = { · line 749

Quoted 10 lines

index 28041e8..2e6d7f9 100644

arch/arm/mach-s3c64xx/clock.c +1 -0

@@ -744,9 +744,9 @@ static struct clksrc_clk clksrcs[] = {
                        .name           = "camera",
                        .ctrlbit        = S3C_CLKCON_SCLK_CAM,
                        .enable         = s3c64xx_sclk_ctrl,
+                       .parent         = &clk_h2,
                },
                .reg_div        = { .reg = S3C_CLK_DIV0, .shift = 20,

.size =

Quoted 2 lines

4 },
Quoted 1 line
arch/arm/mach-s3c64xx/clock.c +0 -1
-               .reg_src        = { .reg = NULL, .shift = 0, .size = 0

Quoted 4 lines

arch/arm/mach-s3c64xx/clock.c static struct clksrc_clk clksrcs[] = { · line 750
Quoted 1 line
arch/arm/mach-s3c64xx/clock.c +0 -0
                .sources        = &clkset_camif,
Just figure out that .sources can be removed to. And seems "clkset_camif" and "clkset_camif_list" can be removed as unused.

Yeah, in this case, right. And we don't need source field as well because its source is fixed.

See below.

8<----

arch/arm/mach-s3c64xx/clock.c +1 -9

index 28041e8..e963711 100644
@@ -681,15 +681,6 @@ static struct clksrc_sources clkset_audio2 = {
 	.nr_sources	= ARRAY_SIZE(clkset_audio2_list),
 };
 
-static struct clk *clkset_camif_list[] = {
-	&clk_h2,
-};
-
-static struct clksrc_sources clkset_camif = {
-	.sources	= clkset_camif_list,
-	.nr_sources	= ARRAY_SIZE(clkset_camif_list),
-};
-
 static struct clksrc_clk clksrcs[] = {
 	{
 		.clk	= {

@@ -742,12 +733,11 @@ static struct clksrc_clk clksrcs[] = {
 	}, {
 		.clk	= {
 			.name		= "camera",
+			.parent		= &clk_h2,
 			.ctrlbit        = S3C_CLKCON_SCLK_CAM,
 			.enable		= s3c64xx_sclk_ctrl,
 		},
 		.reg_div	= { .reg = S3C_CLK_DIV0, .shift = 20, .size

= 4 },
- .reg_src = { .reg = NULL, .shift = 0, .size = 0 },
- .sources = &clkset_camif,
},
};
8<----

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

Kukjin Kim

Applied

2012-11-22 06:51 UTC #9 lore raw

PATCH 1/2 ARM: S3C24XX: Add clkdev entry for camif-upll clock

In reply to Sylwester Nawrocki

Quoted 23 lines · Sylwester Nawrocki

The s3c-camif driver uses "camera" clock conn_id for the "camif-upll" (s3c244x) and "camera" (s3c64xx) platform clock. By adding this new clkdev entry the platform differences are isolated from the driver.
Signed-off-by Sylwester Nawrocki <[email protected]>
---
 arch/arm/mach-s3c24xx/clock-s3c2440.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/mach- renamed from arch/arm/mach-s3c24xx/clock-s3c2440.c +0 -0
s3c24xx/clock-s3c2440.c
index 4407b17..04b87ec 100644
arch/arm/mach-s3c24xx/clock-s3c2440.c +1 -0
@@ -161,6 +161,7 @@ static struct clk_lookup s3c2440_clk_lookup[] = {
 	CLKDEV_INIT(NULL, "clk_uart_baud1", &s3c24xx_uclk),
 	CLKDEV_INIT(NULL, "clk_uart_baud2", &clk_p),
 	CLKDEV_INIT(NULL, "clk_uart_baud3", &s3c2440_clk_fclk_n),
+	CLKDEV_INIT("s3c2440-camif", "camera", &s3c2440_clk_cam_upll),
 };
 
 static int __init_refok s3c2440_clk_add(struct device *dev, struct

Quoted 3 lines

subsys_interface *sif)
--
1.7.4.1

Applied, thanks.

K-Gene <[email protected]>