* [PATCH] ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock
@ 2012-08-27 20:12 Sylwester Nawrocki
2012-08-27 22:58 ` Kukjin Kim
0 siblings, 1 reply; 4+ messages in thread
From: Sylwester Nawrocki @ 2012-08-27 20:12 UTC (permalink / raw)
To: linux-arm-kernel
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 <sylvester.nawrocki@gmail.com>
---
arch/arm/mach-s3c24xx/clock-s3c2440.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-s3c24xx/clock-s3c2440.c b/arch/arm/mach-s3c24xx/clock-s3c2440.c
index cb2883d..749220f 100644
--- a/arch/arm/mach-s3c24xx/clock-s3c2440.c
+++ b/arch/arm/mach-s3c24xx/clock-s3c2440.c
@@ -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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock
2012-08-27 20:12 [PATCH] ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock Sylwester Nawrocki
@ 2012-08-27 22:58 ` Kukjin Kim
2012-08-28 20:58 ` Sylwester Nawrocki
0 siblings, 1 reply; 4+ messages in thread
From: Kukjin Kim @ 2012-08-27 22:58 UTC (permalink / raw)
To: linux-arm-kernel
On 08/27/12 13:12, Sylwester Nawrocki wrote:
> 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<sylvester.nawrocki@gmail.com>
> ---
> arch/arm/mach-s3c24xx/clock-s3c2440.c | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-s3c24xx/clock-s3c2440.c b/arch/arm/mach-s3c24xx/clock-s3c2440.c
> index cb2883d..749220f 100644
> --- a/arch/arm/mach-s3c24xx/clock-s3c2440.c
> +++ b/arch/arm/mach-s3c24xx/clock-s3c2440.c
> @@ -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;
Well, why do we need '/ 2' here?
> +}
> +
> /* 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 <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock
2012-08-27 22:58 ` Kukjin Kim
@ 2012-08-28 20:58 ` Sylwester Nawrocki
2012-08-28 23:10 ` Kukjin Kim
0 siblings, 1 reply; 4+ messages in thread
From: Sylwester Nawrocki @ 2012-08-28 20:58 UTC (permalink / raw)
To: linux-arm-kernel
On 08/28/2012 12:58 AM, Kukjin Kim wrote:
> On 08/27/12 13:12, Sylwester Nawrocki wrote:
>> 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<sylvester.nawrocki@gmail.com>
>> ---
>> arch/arm/mach-s3c24xx/clock-s3c2440.c | 14 ++++++++++++++
>> 1 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-s3c24xx/clock-s3c2440.c
>> b/arch/arm/mach-s3c24xx/clock-s3c2440.c
>> index cb2883d..749220f 100644
>> --- a/arch/arm/mach-s3c24xx/clock-s3c2440.c
>> +++ b/arch/arm/mach-s3c24xx/clock-s3c2440.c
>> @@ -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;
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock
2012-08-28 20:58 ` Sylwester Nawrocki
@ 2012-08-28 23:10 ` Kukjin Kim
0 siblings, 0 replies; 4+ messages in thread
From: Kukjin Kim @ 2012-08-28 23:10 UTC (permalink / raw)
To: linux-arm-kernel
On 08/28/12 13:58, Sylwester Nawrocki wrote:
> On 08/28/2012 12:58 AM, Kukjin Kim wrote:
>> On 08/27/12 13:12, Sylwester Nawrocki wrote:
>>> 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<sylvester.nawrocki@gmail.com>
>>> ---
>>> arch/arm/mach-s3c24xx/clock-s3c2440.c | 14 ++++++++++++++
>>> 1 files changed, 14 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-s3c24xx/clock-s3c2440.c
>>> b/arch/arm/mach-s3c24xx/clock-s3c2440.c
>>> index cb2883d..749220f 100644
>>> --- a/arch/arm/mach-s3c24xx/clock-s3c2440.c
>>> +++ b/arch/arm/mach-s3c24xx/clock-s3c2440.c
>>> @@ -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;
>>
>> 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);
>
> --
Looks OK, thanks for your reply :-)
Applied.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-08-28 23:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-27 20:12 [PATCH] ARM: S3C24XX: Add .get_rate callback for "camif-upll" clock Sylwester Nawrocki
2012-08-27 22:58 ` Kukjin Kim
2012-08-28 20:58 ` Sylwester Nawrocki
2012-08-28 23:10 ` Kukjin Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).