On Tue, Feb 19, 2019 at 8:47 PM Simon Glass <s...@chromium.org> wrote: > > Hi Anup, > > On Tue, 5 Feb 2019 at 06:05, Anup Patel <anup.pa...@wdc.com> wrote: > > > > This patch adds fixed-factor clock driver which derives clock > > rate by dividing (div) and multiplying (mult) fixed factors > > to a parent clock. > > Did someone else send a similar patch recently?
The patch numbering got changed after v5 because we dropped few patches. The v7 patch is "[PATCH v7 09/15] clk: Add fixed-factor clock driver" https://patchwork.ozlabs.org/patch/1039638/ > > > > > Signed-off-by: Atish Patra <atish.pa...@wdc.com> > > Signed-off-by: Anup Patel <anup.pa...@wdc.com> > > --- > > arch/sandbox/dts/test.dts | 8 ++++ > > drivers/clk/Makefile | 4 +- > > drivers/clk/clk_fixed_factor.c | 72 ++++++++++++++++++++++++++++++++++ > > test/dm/clk.c | 5 ++- > > 4 files changed, 87 insertions(+), 2 deletions(-) > > create mode 100644 drivers/clk/clk_fixed_factor.c > > > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts > > index 1d011ded7c..cb8d686e46 100644 > > --- a/arch/sandbox/dts/test.dts > > +++ b/arch/sandbox/dts/test.dts > > @@ -203,6 +203,14 @@ > > #clock-cells = <0>; > > clock-frequency = <1234>; > > }; > > + > > + clk_fixed_factor: clk-fixed-factor { > > + compatible = "fixed-factor-clock"; > > + #clock-cells = <0>; > > + clock-div = <3>; > > + clock-mult = <2>; > > + clocks = <&clk_fixed>; > > Is there a binding file for this somewhere? Please can you include it > in doc/device-tree-bindings? Fixed factor clock is standard Linux style clock. We are following Linux DT bindings for fixed factor clock (just like fixed rate clock). For Linux DT bindings refer, <linux_source>/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt > > > + }; > > }; > > > > clk_sandbox: clk-sbox { > > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > > index 2f4446568c..fa59259ea3 100644 > > --- a/drivers/clk/Makefile > > +++ b/drivers/clk/Makefile > > @@ -4,7 +4,9 @@ > > # Wolfgang Denk, DENX Software Engineering, w...@denx.de. > > # > > > > -obj-$(CONFIG_$(SPL_TPL_)CLK) += clk-uclass.o clk_fixed_rate.o > > +obj-$(CONFIG_$(SPL_TPL_)CLK) += clk-uclass.o > > +obj-$(CONFIG_$(SPL_TPL_)CLK) += clk_fixed_rate.o > > Why change that? It is not related to your patch. Well, this was crossing 80 characters per-line. > > > +obj-$(CONFIG_$(SPL_TPL_)CLK) += clk_fixed_factor.o > > > > obj-y += imx/ > > obj-y += tegra/ > > diff --git a/drivers/clk/clk_fixed_factor.c b/drivers/clk/clk_fixed_factor.c > > new file mode 100644 > > index 0000000000..3487c11729 > > --- /dev/null > > +++ b/drivers/clk/clk_fixed_factor.c > > @@ -0,0 +1,72 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2019 Western Digital Corporation or its affiliates. > > + * > > + * Author: Anup Patel <anup.pa...@wdc.com> > > + */ > > + > > +#include <common.h> > > +#include <clk-uclass.h> > > +#include <div64.h> > > +#include <dm.h> > > + > > +struct clk_fixed_factor { > > + struct clk parent; > > + unsigned int div; > > + unsigned int mult; > > +}; > > + > > +#define to_clk_fixed_factor(dev) \ > > + ((struct clk_fixed_factor *)dev_get_platdata(dev)) > > + > > +static ulong clk_fixed_factor_get_rate(struct clk *clk) > > +{ > > + uint64_t ret; > > How about 'rate' instead? Okay, will update. > > > + struct clk_fixed_factor *ff = to_clk_fixed_factor(clk->dev); > > + > > + if (clk->id != 0) > > + return -EINVAL; > > + > > + ret = clk_get_rate(&ff->parent); > > + if (IS_ERR_VALUE(ret)) > > + return ret; > > + > > + do_div(ret, ff->div); > > + > > + return ret * ff->mult; > > +} > > + > > +const struct clk_ops clk_fixed_factor_ops = { > > + .get_rate = clk_fixed_factor_get_rate, > > +}; > > + > > +static int clk_fixed_factor_ofdata_to_platdata(struct udevice *dev) > > +{ > > + int err; > > + struct clk_fixed_factor *ff = to_clk_fixed_factor(dev); > > + > > + err = clk_get_by_index(dev, 0, &ff->parent); > > + if (err) > > + return err; > > + > > + ff->div = dev_read_u32_default(dev, "clock-div", 1); > > + ff->mult = dev_read_u32_default(dev, "clock-mult", 1); > > + > > + return 0; > > +} > > + > > +static const struct udevice_id clk_fixed_factor_match[] = { > > + { > > + .compatible = "fixed-factor-clock", > > + }, > > + { /* sentinel */ } > > +}; > > + > > +U_BOOT_DRIVER(clk_fixed_factor) = { > > + .name = "fixed_factor_clock", > > + .id = UCLASS_CLK, > > + .of_match = clk_fixed_factor_match, > > + .ofdata_to_platdata = clk_fixed_factor_ofdata_to_platdata, > > + .platdata_auto_alloc_size = sizeof(struct clk_fixed_factor), > > + .ops = &clk_fixed_factor_ops, > > +}; > > diff --git a/test/dm/clk.c b/test/dm/clk.c > > index 898c034e27..112d5cbbc9 100644 > > --- a/test/dm/clk.c > > +++ b/test/dm/clk.c > > @@ -12,12 +12,15 @@ > > > > static int dm_test_clk(struct unit_test_state *uts) > > { > > - struct udevice *dev_fixed, *dev_clk, *dev_test; > > + struct udevice *dev_fixed, *dev_fixed_factor, *dev_clk, *dev_test; > > ulong rate; > > > > ut_assertok(uclass_get_device_by_name(UCLASS_CLK, "clk-fixed", > > &dev_fixed)); > > > > + ut_assertok(uclass_get_device_by_name(UCLASS_CLK, > > "clk-fixed-factor", > > + &dev_fixed_factor)); > > + > > Could you test a little more - e.g. set the clock rate and check it? Fixed factor clock (just like Fixed rate clock) does not provide set_rate() so testing set clock rate won't be applicable here. > > > ut_assertok(uclass_get_device_by_name(UCLASS_CLK, "clk-sbox", > > &dev_clk)); > > ut_asserteq(0, sandbox_clk_query_enable(dev_clk, > > SANDBOX_CLK_ID_SPI)); Regards, Anup _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
