[PATCH v1] clk: fractional-divider: fix up the fractional clk's jitter
Elaine Zhang
zhangqing at rock-chips.com
Tue Jul 11 20:19:51 PDT 2017
On 07/06/2017 06:01 PM, Heiko Stübner wrote:
> Hi Elaine,
>
> Am Donnerstag, 6. Juli 2017, 16:28:34 CEST schrieb Elaine Zhang:
>> 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.
>>
>> Signed-off-by: Elaine Zhang <zhangqing at rock-chips.com>
>> ---
>> drivers/clk/clk-fractional-divider.c | 13 +++++++++++++
>
> As I said in our previous private mails, having soc-specific quirks in
> the generic driver won't work.
>
> I just realized that while I mentioned a different approach in one my
> mails, I seem to have forgotten to attach the patch showcasing my idea.
> [Just realized that while looking at the old mails] Sorry about that.
>
> So the idea would be allowing clk-fractional users to just provide their
> own approximation function, which you could then provide one
> for the rockchip-specific requirements. That change was only compile-
> tested at the time, so you might want to check if it actually works,
>
>
> For the below you would "just" need to create an approximation
> function and adapt the rockchip_clk_register_frac_branch
> to set div->approx to your approximation function.
>
>
> Heiko
>
>
> ---------
> From: Heiko Stuebner <heiko at sntech.de>
> Date: Fri, 2 Jun 2017 13:42:51 +0200
> Subject: [PATCH] clk: fractional-divider: allow clk-specific approximation
> functions
>
> Fractional dividers may have special requirements concerning numerator
> and denominator selection that differ from just getting the best
> approximation.
>
> For example on Rockchip socs the denominator must be at least 20 times
> larger than the numerator to generate precise clock frequencies.
>
> Therefore add the ability to provide custom approximation functions
> but fall back to rational_best_approximation in the default case.
The custom approximation functions is not need.
The rational_best_approximation is work well, I modified is parent_rate,
I just need to make sure the parent_rate > frac_rate * 20.
According to your patch, I can't correction the parent_rate.
In clk_fd_round_rate() function:
fd->approx(rate, *parent_rate,
GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
&m, &n);
I just need to set the *parent_rate >= 20* rate.
>
> Signed-off-by: Heiko Stuebner <heiko at sntech.de>
> ---
> drivers/clk/clk-fractional-divider.c | 42 ++++++++++++++++++++++++++++++++----
> include/linux/clk-provider.h | 17 +++++++++++++++
> 2 files changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
> index aab904618eb6..2a066d8182e1 100644
> --- a/drivers/clk/clk-fractional-divider.c
> +++ b/drivers/clk/clk-fractional-divider.c
> @@ -69,7 +69,7 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate,
> if (scale > fd->nwidth)
> rate <<= scale - fd->nwidth;
>
> - rational_best_approximation(rate, *parent_rate,
> + fd->approx(rate, *parent_rate,
> GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
> &m, &n);
>
> @@ -87,7 +87,7 @@ static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate,
> unsigned long m, n;
> u32 val;
>
> - rational_best_approximation(rate, parent_rate,
> + fd->approx(rate, parent_rate,
> GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
> &m, &n);
>
> @@ -116,10 +116,13 @@ const struct clk_ops clk_fractional_divider_ops = {
> };
> EXPORT_SYMBOL_GPL(clk_fractional_divider_ops);
>
> -struct clk_hw *clk_hw_register_fractional_divider(struct device *dev,
> +struct clk_hw *clk_hw_register_fractional_divider_custom(struct device *dev,
> const char *name, const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
> - u8 clk_divider_flags, spinlock_t *lock)
> + u8 clk_divider_flags, spinlock_t *lock, void (*approx)
> + (unsigned long gnum, unsigned long gdenom,
> + unsigned long mnum, unsigned long mdenom,
> + unsigned long *bnum, unsigned long *bdenom))
> {
> struct clk_fractional_divider *fd;
> struct clk_init_data init;
> @@ -145,6 +148,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 = approx;
> fd->hw.init = &init;
>
> hw = &fd->hw;
> @@ -156,8 +160,38 @@ struct clk_hw *clk_hw_register_fractional_divider(struct device *dev,
>
> return hw;
> }
> +EXPORT_SYMBOL_GPL(clk_hw_register_fractional_divider_custom);
> +
> +struct clk_hw *clk_hw_register_fractional_divider(struct device *dev,
> + const char *name, const char *parent_name, unsigned long flags,
> + void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
> + u8 clk_divider_flags, spinlock_t *lock)
> +{
> + return clk_hw_register_fractional_divider_custom(dev, name, parent_name,
> + flags, reg, mshift, mwidth, nshift, nwidth, clk_divider_flags,
> + lock, rational_best_approximation);
> +}
> EXPORT_SYMBOL_GPL(clk_hw_register_fractional_divider);
>
> +struct clk *clk_register_fractional_divider_custom(struct device *dev,
> + const char *name, const char *parent_name, unsigned long flags,
> + void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
> + u8 clk_divider_flags, spinlock_t *lock, void (*approx)
> + (unsigned long gnum, unsigned long gdenom,
> + unsigned long mnum, unsigned long mdenom,
> + unsigned long *bnum, unsigned long *bdenom))
> +{
> + struct clk_hw *hw;
> +
> + hw = clk_hw_register_fractional_divider_custom(dev, name, parent_name,
> + flags, reg, mshift, mwidth, nshift, nwidth,
> + clk_divider_flags, lock, approx);
> + if (IS_ERR(hw))
> + return ERR_CAST(hw);
> + return hw->clk;
> +}
> +EXPORT_SYMBOL_GPL(clk_register_fractional_divider_custom);
> +
> struct clk *clk_register_fractional_divider(struct device *dev,
> const char *name, const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index a428aec36ace..5bf2bc49ccb7 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -564,16 +564,33 @@ struct clk_fractional_divider {
> u8 nwidth;
> u32 nmask;
> u8 flags;
> + void (*approx)(unsigned long gnum, unsigned long gdenom,
> + unsigned long mnum, unsigned long mdenom,
> + unsigned long *bnum, unsigned long *bdenom);
> spinlock_t *lock;
> };
>
> #define to_clk_fd(_hw) container_of(_hw, struct clk_fractional_divider, hw)
>
> extern const struct clk_ops clk_fractional_divider_ops;
> +struct clk *clk_register_fractional_divider_custom(struct device *dev,
> + const char *name, const char *parent_name, unsigned long flags,
> + void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
> + u8 clk_divider_flags, spinlock_t *lock, void (*approx)
> + (unsigned long gnum, unsigned long gdenom,
> + unsigned long mnum, unsigned long mdenom,
> + unsigned long *bnum, unsigned long *bdenom));
> struct clk *clk_register_fractional_divider(struct device *dev,
> const char *name, const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
> u8 clk_divider_flags, spinlock_t *lock);
> +struct clk_hw *clk_hw_register_fractional_divider_custom(struct device *dev,
> + const char *name, const char *parent_name, unsigned long flags,
> + void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
> + u8 clk_divider_flags, spinlock_t *lock, void (*approx)
> + (unsigned long gnum, unsigned long gdenom,
> + unsigned long mnum, unsigned long mdenom,
> + unsigned long *bnum, unsigned long *bdenom));
> struct clk_hw *clk_hw_register_fractional_divider(struct device *dev,
> const char *name, const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
>
More information about the Linux-rockchip
mailing list