lkml.org 
[lkml]   [2017]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v2] clk: fractional-divider: fix up the fractional clk's jitter


On 07/10/2017 07:05 PM, Heiko Stuebner wrote:
> Hi Elaine,
>
> Am Freitag, 7. Juli 2017, 10:52:23 CEST schrieb Elaine Zhang:
>> add clk_fractional_divider_special_ops for rockchip specific requirements,
>> fractional divider must set that denominator is 20 times larger than
>> numerator to generate precise clock frequency.
>> Otherwise the CLK jitter is very big, poor quality of the clock signal.
>>
>> RK document description:
>> 3.1.9 Fractional divider usage
>> To get specific frequency, clocks of I2S, SPDIF, UARTcan be generated by
>> fractional divider. Generally you must set that denominator is 20 times
>> larger than numerator to generate precise clock frequency. So the
>> fractional divider applies only to generate low frequency clock like
>> I2S, UART.igned-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>>
>> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
>> ---
>> drivers/clk/clk-fractional-divider.c | 32 ++++++++++++++++++++++++++++++++
>> drivers/clk/rockchip/clk.c | 2 +-
>> include/linux/clk-provider.h | 1 +
>> 3 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
>> index aab904618eb6..3107b33327f9 100644
>> --- a/drivers/clk/clk-fractional-divider.c
>> +++ b/drivers/clk/clk-fractional-divider.c
>> @@ -158,6 +158,38 @@ struct clk_hw *clk_hw_register_fractional_divider(struct device *dev,
>> }
>> EXPORT_SYMBOL_GPL(clk_hw_register_fractional_divider);
>>
>> +static long clk_fd_round_rate_special(struct clk_hw *hw, unsigned long rate,
>> + unsigned long *parent_rate)
>> +{
>
> this obviously still encodes Rockchip-specific things into the generic
> fractional-divider driver. And it's of course only special for Rockchip
> fractional dividers and will end it chaos if every implementation wants
> to add a "special" function there.
>
> Did you have a look at the patch I added to the last mail (for real this time)?
>

I think your patch is not most appropriate.
Because I think the rational_best_approximation is work well, I modified
is parent_rate,
I just need to make sure the parent_rate > frac_rate * 20.
And I modify this like: (It is more appropriate?)

(I tested in RK SoCs. It's work well.)
diff --git a/drivers/clk/clk-fractional-divider.c
b/drivers/clk/clk-fractional-divider.c
index e7a315e840e6..ccc9a98e53ce 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -62,6 +62,9 @@ static long clk_fd_round_rate(struct clk_hw *hw,
unsigned long rate,
if (!rate || rate >= *parent_rate)
return *parent_rate;

+ if (fd->approx)
+ fd->approx(hw, rate, parent_rate);
+
/*
* Get rate closer to *parent_rate to guarantee there is no overflow
* for m and n. In the result it will be the nearest rate left shifted
@@ -147,6 +150,7 @@ struct clk_hw
*clk_hw_register_fractional_divider(struct device *dev,
fd->nmask = GENMASK(nwidth - 1, 0) << nshift;
fd->flags = clk_divider_flags;
fd->lock = lock;
+ fd->approx = NULL;
fd->hw.init = &init;

hw = &fd->hw;
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index e96a2e187862..a8ed48ffd531 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -160,6 +160,21 @@ static int rockchip_clk_frac_notifier_cb(struct
notifier_block *nb,
return notifier_from_errno(ret);
}

+void rockchip_fractional_special_approx(struct clk_hw *hw,
+ unsigned long rate,
+ unsigned long *parent_rate)
+{
+ struct clk_hw *p_parent;
+ unsigned long p_rate, p_parent_rate;
+
+ p_rate = clk_hw_get_rate(clk_hw_get_parent(hw));
+ if ((rate * 20 > p_rate) && (p_rate % rate != 0)) {
+ p_parent = clk_hw_get_parent(clk_hw_get_parent(hw));
+ p_parent_rate = clk_hw_get_rate(p_parent);
+ *parent_rate = p_parent_rate;
+ }
+}
+
static struct clk *rockchip_clk_register_frac_branch(
struct rockchip_clk_provider *ctx, const char *name,
const char *const *parent_names, u8 num_parents,
@@ -206,6 +221,7 @@ static struct clk *rockchip_clk_register_frac_branch(
div->nwidth = 16;
div->nmask = GENMASK(div->nwidth - 1, 0) << div->nshift;
div->lock = lock;
+ div->approx = rockchip_fractional_special_approx;
div_ops = &clk_fractional_divider_ops;

clk = clk_register_composite(NULL, name, parent_names, num_parents,
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index f70440a4edd7..6e4f6940e39e 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -513,6 +513,9 @@ struct clk_fractional_divider {
u8 nwidth;
u32 nmask;
u8 flags;
+ void (*approx)(struct clk_hw *hw,
+ unsigned long rate,
+ unsigned long *parent_rate);
spinlock_t *lock;
};


>
> Heiko
>
>
>
>

\
 
 \ /
  Last update: 2017-07-13 09:55    [W:0.060 / U:0.412 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site