From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751275AbdGMHyS (ORCPT ); Thu, 13 Jul 2017 03:54:18 -0400 Received: from regular1.263xmail.com ([211.150.99.138]:57583 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750846AbdGMHyQ (ORCPT ); Thu, 13 Jul 2017 03:54:16 -0400 X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-RL-SENDER: zhangqing@rock-chips.com X-FST-TO: zhangqing@rock-chips.com X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: zhangqing@rock-chips.com X-UNIQUE-TAG: X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <596726E0.6010403@rock-chips.com> Date: Thu, 13 Jul 2017 15:53:04 +0800 From: Elaine Zhang User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Heiko Stuebner CC: mturquette@baylibre.com, sboyd@codeaurora.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org, huangtao@rock-chips.com, cl@rock-chips.com, xxx@rock-chips.com, xf@rock-chips.com, zhangqing@rock-chips.com Subject: Re: [PATCH v2] clk: fractional-divider: fix up the fractional clk's jitter References: <1499395943-19516-1-git-send-email-zhangqing@rock-chips.com> <1517147.8QBNcir4nx@phil> In-Reply-To: <1517147.8QBNcir4nx@phil> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> >> Signed-off-by: Elaine Zhang >> --- >> 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 > > > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: zhangqing@rock-chips.com (Elaine Zhang) Date: Thu, 13 Jul 2017 15:53:04 +0800 Subject: [PATCH v2] clk: fractional-divider: fix up the fractional clk's jitter In-Reply-To: <1517147.8QBNcir4nx@phil> References: <1499395943-19516-1-git-send-email-zhangqing@rock-chips.com> <1517147.8QBNcir4nx@phil> Message-ID: <596726E0.6010403@rock-chips.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 >> >> Signed-off-by: Elaine Zhang >> --- >> 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 > > > >