Skip to contentSkip to conversation

ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock

PATCH New Sylwester Nawrocki 2 participants 4 messages Aug 27, 2012 – Aug 28, 2012

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm Samsung Exynos
lore
Share & tools
lore

Sylwester Nawrocki

author
#0 lore raw

PATCH 1/1 ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock

Add missing get_rate callback for the "camif-upll" clock, so frequency of this clock is properly reported with clk_get_rate().

Signed-off-by Sylwester Nawrocki <[email protected]>
---
 arch/arm/mach-s3c24xx/clock-s3c2440.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
arch/arm/mach-s3c24xx/clock-s3c2440.c +14 -0
index cb2883d..749220f 100644
@@ -87,6 +87,19 @@ static int s3c2440_camif_upll_setrate(struct clk *clk, unsigned long rate)
 	return 0;
 }
 
+static unsigned long s3c2440_camif_upll_getrate(struct clk *clk)
+{
+	unsigned long parent_rate = clk_get_rate(clk->parent);
+	unsigned long camdivn =  __raw_readl(S3C2440_CAMDIVN);
+
+	if (!(camdivn & S3C2440_CAMDIVN_CAMCLK_SEL))
+		return parent_rate;
+
+	camdivn &= S3C2440_CAMDIVN_CAMCLK_MASK;
+
+	return parent_rate / (camdivn + 1) / 2;
+}
+
 /* Extra S3C2440 clocks */
 
 static struct clk s3c2440_clk_cam = {

@@ -99,6 +112,7 @@ static struct clk s3c2440_clk_cam_upll = {
 	.name		= "camif-upll",
 	.ops		= &(struct clk_ops) {
 		.set_rate	= s3c2440_camif_upll_setrate,
+		.get_rate	= s3c2440_camif_upll_getrate,
 		.round_rate	= s3c2440_camif_upll_round,
 	},
 };

--
1.7.4.1

#1 lore raw

PATCH 1/1 ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock

Quoted 17 lines · Sylwester Nawrocki

Add missing get_rate callback for the "camif-upll" clock, so frequency of this clock is properly reported with clk_get_rate().

Signed-off-by Sylwester Nawrocki<[email protected]>
---
  arch/arm/mach-s3c24xx/clock-s3c2440.c |   14 ++++++++++++++
  1 files changed, 14 insertions(+), 0 deletions(-)
arch/arm/mach-s3c24xx/clock-s3c2440.c +1 -0
index cb2883d..749220f 100644
@@ -87,6 +87,19 @@ static int s3c2440_camif_upll_setrate(struct clk *clk, unsigned long rate)
  	return 0;
  }
 
+static unsigned long s3c2440_camif_upll_getrate(struct clk *clk)

arch/arm/mach-s3c24xx/clock-s3c2440.c static int s3c2440_camif_upll_setrate(struct clk *clk, unsigned long rate) · line 100

Quoted 10 lines
arch/arm/mach-s3c24xx/clock-s3c2440.c +10 -0
+{
+	unsigned long parent_rate = clk_get_rate(clk->parent);
+	unsigned long camdivn =  __raw_readl(S3C2440_CAMDIVN);
+
+	if (!(camdivn&  S3C2440_CAMDIVN_CAMCLK_SEL))
+		return parent_rate;
+
+	camdivn&= S3C2440_CAMDIVN_CAMCLK_MASK;
+
+	return parent_rate / (camdivn + 1) / 2;

Well, why do we need '/ 2' here?

arch/arm/mach-s3c24xx/clock-s3c2440.c static struct clk s3c2440_clk_cam_upll = { · line 118

Quoted 13 lines
arch/arm/mach-s3c24xx/clock-s3c2440.c +3 -0
+}
+
  /* Extra S3C2440 clocks */
 
  static struct clk s3c2440_clk_cam = {

@@ -99,6 +112,7 @@ static struct clk s3c2440_clk_cam_upll = {
  	.name		= "camif-upll",
  	.ops		=&(struct clk_ops) {
  		.set_rate	= s3c2440_camif_upll_setrate,
+		.get_rate	= s3c2440_camif_upll_getrate,
  		.round_rate	= s3c2440_camif_upll_round,
  	},
  };

Thanks.

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

author
#2 lore raw

PATCH 1/1 ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock

Quoted 19 lines · Kukjin Kim
Quoted 18 lines · Sylwester Nawrocki

Add missing get_rate callback for the "camif-upll" clock, so frequency of this clock is properly reported with clk_get_rate().

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

---
arch/arm/mach-s3c24xx/clock-s3c2440.c | 14 ++++++++++++++

1 files changed, 14 insertions(+), 0 deletions(-)
arch/arm/mach-s3c24xx/clock-s3c2440.c +0 -0
index cb2883d..749220f 100644
@@ -87,6 +87,19 @@ static int s3c2440_camif_upll_setrate(struct clk 
 *clk, unsigned long rate)
 return 0;
 }
Quoted 13 lines

arch/arm/mach-s3c24xx/clock-s3c2440.c static int s3c2440_camif_upll_setrate(struct clk *clk, unsigned long rate) · line 100

Quoted 11 lines
arch/arm/mach-s3c24xx/clock-s3c2440.c +11 -0
+static unsigned long s3c2440_camif_upll_getrate(struct clk *clk)
+{
+ unsigned long parent_rate = clk_get_rate(clk->parent);
+ unsigned long camdivn = __raw_readl(S3C2440_CAMDIVN);
+
+ if (!(camdivn& S3C2440_CAMDIVN_CAMCLK_SEL))
+ return parent_rate;
+
+ camdivn&= S3C2440_CAMDIVN_CAMCLK_MASK;
+
+ return parent_rate / (camdivn + 1) / 2;

Well, why do we need '/ 2' here?

I simply followed documentation of the camera clock divider register (CAMDIVN). This is how CAMCLK_SEL and CAMCLK_DIV bit fields are described there:

CAMDIVN | Bit | Description
--------------------------------------------------------------------------

CAMCLK_SEL   | [4]   | 0 : Use CAMCLK with UPLL output(CAMCLK=UPLL output)
             |       | 1 : CAMCLK is divided by CAMCLK_DIV value
-------------+-------+----------------------------------------------------
CAMCLK_DIV   | [3:0] | CAMCLK divide factor setting register(0 ? 15).
             |       | Camera clock = UPLL / [(CAMCLK_DIV +1)x2].
             |       | This bit is valid when CAMCLK_SEL=1.

So, camera clock frequency = parent_rate / ((camdivn + 1) * 2). This is exactly same as 'parent_rate / ( camdivn + 1) / 2'.

To be 1000% sure I compared frequency values reported from clk_get_rate() in the driver with values measured with an oscilloscope at the CAMCLKOUT pin. Everything was as expected (UPLL - 48 MHz, CAMCLKOUT - 12 MHz).

Also, it's easy to confirm correctness of this code by inversing equations found in function s3c2440_camif_upll_setrate(), i.e.

camdivn &= ~(S3C2440_CAMDIVN_CAMCLK_SEL | S3C2440_CAMDIVN_CAMCLK_MASK); if (rate != parent_rate) {
camdivn |= S3C2440_CAMDIVN_CAMCLK_SEL;
camdivn |= (((parent_rate / rate) / 2) - 1);
}

__raw_writel(camdivn, S3C2440_CAMDIVN);

--
Regards,
Sylwester

Applied
#3 lore raw

PATCH 1/1 ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock

Quoted 65 lines · Sylwester Nawrocki
Quoted 19 lines · Kukjin Kim
Quoted 18 lines · Sylwester Nawrocki

Add missing get_rate callback for the "camif-upll" clock, so frequency of this clock is properly reported with clk_get_rate().

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

---
arch/arm/mach-s3c24xx/clock-s3c2440.c | 14 ++++++++++++++

1 files changed, 14 insertions(+), 0 deletions(-)
arch/arm/mach-s3c24xx/clock-s3c2440.c +0 -0
index cb2883d..749220f 100644
@@ -87,6 +87,19 @@ static int s3c2440_camif_upll_setrate(struct clk
 *clk, unsigned long rate)
 return 0;
 }
Quoted 13 lines

arch/arm/mach-s3c24xx/clock-s3c2440.c static int s3c2440_camif_upll_setrate(struct clk *clk, unsigned long rate) · line 100

Quoted 11 lines
arch/arm/mach-s3c24xx/clock-s3c2440.c +11 -0
+static unsigned long s3c2440_camif_upll_getrate(struct clk *clk)
+{
+ unsigned long parent_rate = clk_get_rate(clk->parent);
+ unsigned long camdivn = __raw_readl(S3C2440_CAMDIVN);
+
+ if (!(camdivn&  S3C2440_CAMDIVN_CAMCLK_SEL))
+ return parent_rate;
+
+ camdivn&= S3C2440_CAMDIVN_CAMCLK_MASK;
+
+ return parent_rate / (camdivn + 1) / 2;

Well, why do we need '/ 2' here?

I simply followed documentation of the camera clock divider register (CAMDIVN). This is how CAMCLK_SEL and CAMCLK_DIV bit fields are described there:

CAMDIVN | Bit | Description
--------------------------------------------------------------------------
CAMCLK_SEL | [4] | 0 : Use CAMCLK with UPLL output(CAMCLK=UPLL output)
| | 1 : CAMCLK is divided by CAMCLK_DIV value
-------------+-------+----------------------------------------------------
CAMCLK_DIV | [3:0] | CAMCLK divide factor setting register(0 ? 15).

              |       | Camera clock = UPLL / [(CAMCLK_DIV +1)x2].
              |       | This bit is valid when CAMCLK_SEL=1.

So, camera clock frequency = parent_rate / ((camdivn + 1) * 2). This is exactly same as 'parent_rate / ( camdivn + 1) / 2'.

To be 1000% sure I compared frequency values reported from clk_get_rate() in the driver with values measured with an oscilloscope at the CAMCLKOUT pin. Everything was as expected (UPLL - 48 MHz, CAMCLKOUT - 12 MHz).

Also, it's easy to confirm correctness of this code by inversing equations found in function s3c2440_camif_upll_setrate(), i.e.

  camdivn&= ~(S3C2440_CAMDIVN_CAMCLK_SEL | S3C2440_CAMDIVN_CAMCLK_MASK);
  if (rate != parent_rate) {
  	camdivn |= S3C2440_CAMDIVN_CAMCLK_SEL;
  	camdivn |= (((parent_rate / rate) / 2) - 1);
  }

  __raw_writel(camdivn, S3C2440_CAMDIVN);
Quoted 1 line

--

Looks OK, thanks for your reply :-)

Applied.

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