| Message ID | 20190205130346.77175-11-anup.patel@wdc.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | SiFive FU540 Support | expand |
Hi Anup, On Tue, 5 Feb 2019 at 06:05, Anup Patel <Anup.Patel@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? > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > Signed-off-by: Anup Patel <anup.patel@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? > + }; > }; > > 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, wd@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. > +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.patel@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? > + 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? > 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)); > -- > 2.17.1 > Regards, Simon
On Tue, Feb 19, 2019 at 8:47 PM Simon Glass <sjg@chromium.org> wrote: > > Hi Anup, > > On Tue, 5 Feb 2019 at 06:05, Anup Patel <Anup.Patel@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.patra@wdc.com> > > Signed-off-by: Anup Patel <anup.patel@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, wd@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.patel@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
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>; + }; }; 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, wd@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 +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.patel@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; + 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)); + 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));