Re: [PATCH 2/2] clk: Add fractional scale clock support
Hi Geert,
On Thu, Jun 16, 2016 at 11:43 PM, Geert Uytterhoeven
wrote:
> On Fri, Jun 17, 2016 at 1:40 AM, Hoan Tran wrote:
>> This patch adds fractional scale clock support.
>> Fractional scale clock is implemented for a single register field.
>> Output rate = parent_rate * scale / denominator
>> For example, for 1 / 8 fractional scale, denominator will be 8 and scale
>> will be computed and programmed accordingly.
>>
>> Signed-off-by: Hoan Tran
>> Signed-off-by: Loc Ho
>
>> --- /dev/null
>> +++ b/drivers/clk/clk-fractional-scale.c
>
>> +static long clk_fs_round_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long *parent_rate)
>> +{
>> + struct clk_fractional_scale *fd = to_clk_sf(hw);
>> + u64 ret, scale;
>> +
>> + if (!rate || rate >= *parent_rate)
>> + return *parent_rate;
>> +
>> + /* freq = parent_rate * scaler / denom */
>> + ret = rate * fd->denom;
>
> Can this overflow? No, because fd->denom is u64.
> But if fd->denom is changed to u32, it needs a cast to u64 here.
>
>> + scale = ret / *parent_rate;
>
> As detected by the kbuild test robot, this should use do_div().
Yes, my mistake. I'll fix it.
>
>> +
>> + ret = (u64) *parent_rate * scale;
>> + do_div(ret, fd->denom);
>> +
>> + return ret;
>> +}
>> +
>> +static int clk_fs_set_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long parent_rate)
>> +{
>> + struct clk_fractional_scale *fd = to_clk_sf(hw);
>> + unsigned long flags = 0;
>> + u64 scale, ret;
>> + u32 val;
>> +
>> + /*
>> +* Compute the scaler:
>> +*
>> +* freq = parent_rate * scaler / denom, or
>> +* scaler = freq * denom / parent_rate
>> +*/
>> + ret = rate * fd->denom;
>
> Can this overflow? No, because fd->denom is u64.
> But if fd->denom is changed to u32, it needs a cast to u64 here.
>
>> + scale = ret / (u64)parent_rate;
>
> Don't cast the divider to u64, as this will turn a 64-by-32 division into a
> 64-by-64 division.
> As detected by the kbuild test robot, this should use do_div().
Will fix it.
Thanks
Hoan
>
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>
>> +struct clk_fractional_scale {
>> + struct clk_hw hw;
>> + void __iomem*reg;
>> + u8 shift;
>> + u32 mask;
>> + u64 denom;
>
> u64 sounds overkill to me, but it looks like that was done to avoid overflow
> in
> the multiplications?
>
>> + u32 flags;
>> + spinlock_t *lock;
>> +};
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
Re: [PATCH 2/2] clk: Add fractional scale clock support
On Fri, Jun 17, 2016 at 1:40 AM, Hoan Tran wrote:
> This patch adds fractional scale clock support.
> Fractional scale clock is implemented for a single register field.
> Output rate = parent_rate * scale / denominator
> For example, for 1 / 8 fractional scale, denominator will be 8 and scale
> will be computed and programmed accordingly.
>
> Signed-off-by: Hoan Tran
> Signed-off-by: Loc Ho
> --- /dev/null
> +++ b/drivers/clk/clk-fractional-scale.c
> +static long clk_fs_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + struct clk_fractional_scale *fd = to_clk_sf(hw);
> + u64 ret, scale;
> +
> + if (!rate || rate >= *parent_rate)
> + return *parent_rate;
> +
> + /* freq = parent_rate * scaler / denom */
> + ret = rate * fd->denom;
Can this overflow? No, because fd->denom is u64.
But if fd->denom is changed to u32, it needs a cast to u64 here.
> + scale = ret / *parent_rate;
As detected by the kbuild test robot, this should use do_div().
> +
> + ret = (u64) *parent_rate * scale;
> + do_div(ret, fd->denom);
> +
> + return ret;
> +}
> +
> +static int clk_fs_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_fractional_scale *fd = to_clk_sf(hw);
> + unsigned long flags = 0;
> + u64 scale, ret;
> + u32 val;
> +
> + /*
> +* Compute the scaler:
> +*
> +* freq = parent_rate * scaler / denom, or
> +* scaler = freq * denom / parent_rate
> +*/
> + ret = rate * fd->denom;
Can this overflow? No, because fd->denom is u64.
But if fd->denom is changed to u32, it needs a cast to u64 here.
> + scale = ret / (u64)parent_rate;
Don't cast the divider to u64, as this will turn a 64-by-32 division into a
64-by-64 division.
As detected by the kbuild test robot, this should use do_div().
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> +struct clk_fractional_scale {
> + struct clk_hw hw;
> + void __iomem*reg;
> + u8 shift;
> + u32 mask;
> + u64 denom;
u64 sounds overkill to me, but it looks like that was done to avoid overflow in
the multiplications?
> + u32 flags;
> + spinlock_t *lock;
> +};
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 2/2] clk: Add fractional scale clock support
Hi, [auto build test ERROR on clk/clk-next] [also build test ERROR on v4.7-rc3 next-20160616] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Hoan-Tran/clk-Add-fractional-scale-clock-support/20160617-074656 base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next config: arm-at91_dt_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): drivers/built-in.o: In function `clk_fs_set_rate': >> powercap_sys.c:(.text+0x17e7cc): undefined reference to `__aeabi_uldivmod' drivers/built-in.o: In function `clk_fs_round_rate': powercap_sys.c:(.text+0x17e884): undefined reference to `__aeabi_uldivmod' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH 2/2] clk: Add fractional scale clock support
Hi, [auto build test ERROR on clk/clk-next] [also build test ERROR on v4.7-rc3 next-20160616] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Hoan-Tran/clk-Add-fractional-scale-clock-support/20160617-074656 base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next config: arm-shmobile_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): drivers/built-in.o: In function `clk_fs_set_rate': >> core.c:(.text+0x1fbee8): undefined reference to `__aeabi_uldivmod' drivers/built-in.o: In function `clk_fs_round_rate': core.c:(.text+0x1fbfb4): undefined reference to `__aeabi_uldivmod' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH 2/2] clk: Add fractional scale clock support
Hi, [auto build test ERROR on clk/clk-next] [also build test ERROR on v4.7-rc3 next-20160616] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Hoan-Tran/clk-Add-fractional-scale-clock-support/20160617-074656 base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next config: microblaze-mmu_defconfig (attached as .config) compiler: microblaze-linux-gcc (GCC) 4.9.0 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=microblaze All errors (new ones prefixed by >>): drivers/built-in.o: In function `clk_fs_set_rate': >> drivers/clk/clk-fractional-scale.c:99: undefined reference to `__udivdi3' drivers/built-in.o: In function `clk_fs_round_rate': drivers/clk/clk-fractional-scale.c:76: undefined reference to `__udivdi3' vim +99 drivers/clk/clk-fractional-scale.c 93 * Compute the scaler: 94 * 95 * freq = parent_rate * scaler / denom, or 96 * scaler = freq * denom / parent_rate 97 */ 98 ret = rate * fd->denom; > 99 scale = ret / (u64)parent_rate; 100 101 /* Check if inverted */ 102 if (fd->flags & CLK_FRACTIONAL_SCALE_INVERTED) --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
[PATCH 2/2] clk: Add fractional scale clock support
This patch adds fractional scale clock support.
Fractional scale clock is implemented for a single register field.
Output rate = parent_rate * scale / denominator
For example, for 1 / 8 fractional scale, denominator will be 8 and scale
will be computed and programmed accordingly.
Signed-off-by: Hoan Tran
Signed-off-by: Loc Ho
---
drivers/clk/Makefile | 1 +
drivers/clk/clk-fractional-scale.c | 253 +
include/linux/clk-provider.h | 41 ++
3 files changed, 295 insertions(+)
create mode 100644 drivers/clk/clk-fractional-scale.c
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index dcc5e69..d7deddf 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-multiplier.o
obj-$(CONFIG_COMMON_CLK) += clk-mux.o
obj-$(CONFIG_COMMON_CLK) += clk-composite.o
obj-$(CONFIG_COMMON_CLK) += clk-fractional-divider.o
+obj-$(CONFIG_COMMON_CLK) += clk-fractional-scale.o
obj-$(CONFIG_COMMON_CLK) += clk-gpio.o
ifeq ($(CONFIG_OF), y)
obj-$(CONFIG_COMMON_CLK) += clk-conf.o
diff --git a/drivers/clk/clk-fractional-scale.c
b/drivers/clk/clk-fractional-scale.c
new file mode 100644
index 000..1cf6f57
--- /dev/null
+++ b/drivers/clk/clk-fractional-scale.c
@@ -0,0 +1,253 @@
+/*
+ * Copyright (C) 2016 Applied Micro Circuits Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Fractional scale clock is implemented for a single register field.
+ *
+ * Output rate = parent_rate * scale / denominator
+ *
+ * For example, for 1/8 fractional scale, denominator will be 8 and scale
+ * will be computed and programmed accordingly.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define to_clk_sf(_hw) container_of(_hw, struct clk_fractional_scale, hw)
+
+static DEFINE_SPINLOCK(clk_lock);
+
+static unsigned long clk_fs_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_fractional_scale *fd = to_clk_sf(hw);
+ unsigned long flags = 0;
+ u64 ret, scale;
+ u32 val;
+
+ if (fd->lock)
+ spin_lock_irqsave(fd->lock, flags);
+ else
+ __acquire(fd->lock);
+
+ val = clk_readl(fd->reg);
+
+ if (fd->lock)
+ spin_unlock_irqrestore(fd->lock, flags);
+ else
+ __release(fd->lock);
+
+ ret = (u64) parent_rate;
+
+ scale = (val & fd->mask) >> fd->shift;
+ if (fd->flags & CLK_FRACTIONAL_SCALE_INVERTED)
+ scale = fd->denom - scale;
+ else
+ scale++;
+
+ /* freq = parent_rate * scaler / denom */
+ do_div(ret, fd->denom);
+ ret *= scale;
+ if (ret == 0)
+ ret = (u64) parent_rate;
+
+ return ret;
+}
+
+static long clk_fs_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ struct clk_fractional_scale *fd = to_clk_sf(hw);
+ u64 ret, scale;
+
+ if (!rate || rate >= *parent_rate)
+ return *parent_rate;
+
+ /* freq = parent_rate * scaler / denom */
+ ret = rate * fd->denom;
+ scale = ret / *parent_rate;
+
+ ret = (u64) *parent_rate * scale;
+ do_div(ret, fd->denom);
+
+ return ret;
+}
+
+static int clk_fs_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_fractional_scale *fd = to_clk_sf(hw);
+ unsigned long flags = 0;
+ u64 scale, ret;
+ u32 val;
+
+ /*
+* Compute the scaler:
+*
+* freq = parent_rate * scaler / denom, or
+* scaler = freq * denom / parent_rate
+*/
+ ret = rate * fd->denom;
+ scale = ret / (u64)parent_rate;
+
+ /* Check if inverted */
+ if (fd->flags & CLK_FRACTIONAL_SCALE_INVERTED)
+ scale = fd->denom - scale;
+ else
+ scale--;
+
+ if (fd->lock)
+ spin_lock_irqsave(fd->lock, flags);
+ else
+ __acquire(fd->lock);
+
+ val = clk_readl(fd->reg);
+ val &= ~fd->mask;
+ val |= (scale << fd->shift);
+ clk_writel(val, fd->reg);
+
+ if (fd->lock)
+ spin_unlock_irqrestore(fd->lock, flags);
+ else
+ __release(fd->lock);
+
+ return 0;
+}
+
+const struct clk_ops clk_fractional_scale_ops = {
+ .recalc_rate = clk_fs_recalc_rate,
+ .round_rate = clk_fs_round_rate,
+ .set_rate = clk_fs_set_rate,
+};
+EXPORT_SYMBOL_GPL(clk_fractional_scale_ops);
+
+struct clk_hw *clk_hw_register_fractional_scale(struct device *dev,
+ const char *name, const char *parent_name, unsigned long flags,
+
