| Message ID | 20180317100952.28538-1-manivannan.sadhasivam@linaro.org |
|---|---|
| Headers | show |
| Series | Add clock driver for Actions S900 SoC | expand |
Quoting Manivannan Sadhasivam (2018-03-17 03:09:49) > Add support for Actions Semi fixed factor clock together with > helper functions to be used in composite clock. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/clk/actions/Makefile | 1 + > drivers/clk/actions/owl-fixed-factor.c | 81 ++++++++++++++++++++++++++++++++++ > drivers/clk/actions/owl-fixed-factor.h | 62 ++++++++++++++++++++++++++ > 3 files changed, 144 insertions(+) > create mode 100644 drivers/clk/actions/owl-fixed-factor.c > create mode 100644 drivers/clk/actions/owl-fixed-factor.h > > diff --git a/drivers/clk/actions/Makefile b/drivers/clk/actions/Makefile > index 994357fa560b..b618696ba54e 100644 > --- a/drivers/clk/actions/Makefile > +++ b/drivers/clk/actions/Makefile > @@ -5,3 +5,4 @@ clk-owl-y += owl-gate.o > clk-owl-y += owl-mux.o > clk-owl-y += owl-divider.o > clk-owl-y += owl-factor.o > +clk-owl-y += owl-fixed-factor.o > diff --git a/drivers/clk/actions/owl-fixed-factor.c b/drivers/clk/actions/owl-fixed-factor.c > new file mode 100644 > index 000000000000..f1281565129c > --- /dev/null > +++ b/drivers/clk/actions/owl-fixed-factor.c > @@ -0,0 +1,81 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +// > +// OWL fixed factor clock driver > +// > +// Copyright (c) 2014 Actions Semi Inc. > +// Author: David Liu <liuwei@actions-semi.com> > +// > +// Copyright (c) 2018 Linaro Ltd. > +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > + > +#include <linux/clk-provider.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +#include "owl-fixed-factor.h" > + > +long owl_fix_fact_helper_round_rate(struct owl_clk_common *common, > + const struct owl_fix_fact_hw *fix_fact_hw, > + unsigned long rate, > + unsigned long *parent_rate) > +{ > + if (clk_hw_get_flags(&common->hw) & CLK_SET_RATE_PARENT) { > + unsigned long best_parent; > + > + best_parent = (rate / fix_fact_hw->mul) * fix_fact_hw->div; > + *parent_rate = clk_hw_round_rate(clk_hw_get_parent(&common->hw), > + best_parent); > + } > + > + return (*parent_rate / fix_fact_hw->div) * fix_fact_hw->mul; > +} > + > +static long owl_fix_fact_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct owl_fix_fact *fix_fact = hw_to_owl_fix_fact(hw); > + struct owl_fix_fact_hw *fix_fact_hw = &fix_fact->fix_fact_hw; > + > + return owl_fix_fact_helper_round_rate(&fix_fact->common, fix_fact_hw, > + rate, parent_rate); > +} > + > +unsigned long owl_fix_fact_helper_recalc_rate(struct owl_clk_common *common, > + const struct owl_fix_fact_hw *fix_fact_hw, > + unsigned long parent_rate) > +{ > + unsigned long long int rate; > + > + rate = (unsigned long long int)parent_rate * fix_fact_hw->mul; > + do_div(rate, fix_fact_hw->div); > + > + return (unsigned long)rate; You can drop the cast. > +} > + > +static unsigned long owl_fix_fact_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct owl_fix_fact *fix_fact = hw_to_owl_fix_fact(hw); > + struct owl_fix_fact_hw *fix_fact_hw = &fix_fact->fix_fact_hw; > + > + return owl_fix_fact_helper_recalc_rate(&fix_fact->common, fix_fact_hw, > + parent_rate); > +} > + > +static int owl_fix_fact_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + /* > + * We must report success but we can do so unconditionally because > + * clk_fix_fact_round_rate returns values that ensure this call is a What function is that? > + * nop. > + */ > + > + return 0; > +} > + > +const struct clk_ops owl_fix_fact_ops = { > + .round_rate = owl_fix_fact_round_rate, > + .recalc_rate = owl_fix_fact_recalc_rate, > + .set_rate = owl_fix_fact_set_rate, > +}; Why can't you use the regular fixed factor clk code and ops?
Hi Stephen, On Mon, Mar 19, 2018 at 06:10:03PM -0700, Stephen Boyd wrote: > Quoting Manivannan Sadhasivam (2018-03-17 03:09:49) > > Add support for Actions Semi fixed factor clock together with > > helper functions to be used in composite clock. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/clk/actions/Makefile | 1 + > > drivers/clk/actions/owl-fixed-factor.c | 81 ++++++++++++++++++++++++++++++++++ > > drivers/clk/actions/owl-fixed-factor.h | 62 ++++++++++++++++++++++++++ > > 3 files changed, 144 insertions(+) > > create mode 100644 drivers/clk/actions/owl-fixed-factor.c > > create mode 100644 drivers/clk/actions/owl-fixed-factor.h > > > > diff --git a/drivers/clk/actions/Makefile b/drivers/clk/actions/Makefile > > index 994357fa560b..b618696ba54e 100644 > > --- a/drivers/clk/actions/Makefile > > +++ b/drivers/clk/actions/Makefile > > @@ -5,3 +5,4 @@ clk-owl-y += owl-gate.o > > clk-owl-y += owl-mux.o > > clk-owl-y += owl-divider.o > > clk-owl-y += owl-factor.o > > +clk-owl-y += owl-fixed-factor.o > > diff --git a/drivers/clk/actions/owl-fixed-factor.c b/drivers/clk/actions/owl-fixed-factor.c > > new file mode 100644 > > index 000000000000..f1281565129c > > --- /dev/null > > +++ b/drivers/clk/actions/owl-fixed-factor.c > > @@ -0,0 +1,81 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +// > > +// OWL fixed factor clock driver > > +// > > +// Copyright (c) 2014 Actions Semi Inc. > > +// Author: David Liu <liuwei@actions-semi.com> > > +// > > +// Copyright (c) 2018 Linaro Ltd. > > +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > + > > +#include <linux/clk-provider.h> > > +#include <linux/regmap.h> > > +#include <linux/slab.h> > > + > > +#include "owl-fixed-factor.h" > > + > > +long owl_fix_fact_helper_round_rate(struct owl_clk_common *common, > > + const struct owl_fix_fact_hw *fix_fact_hw, > > + unsigned long rate, > > + unsigned long *parent_rate) > > +{ > > + if (clk_hw_get_flags(&common->hw) & CLK_SET_RATE_PARENT) { > > + unsigned long best_parent; > > + > > + best_parent = (rate / fix_fact_hw->mul) * fix_fact_hw->div; > > + *parent_rate = clk_hw_round_rate(clk_hw_get_parent(&common->hw), > > + best_parent); > > + } > > + > > + return (*parent_rate / fix_fact_hw->div) * fix_fact_hw->mul; > > +} > > + > > +static long owl_fix_fact_round_rate(struct clk_hw *hw, unsigned long rate, > > + unsigned long *parent_rate) > > +{ > > + struct owl_fix_fact *fix_fact = hw_to_owl_fix_fact(hw); > > + struct owl_fix_fact_hw *fix_fact_hw = &fix_fact->fix_fact_hw; > > + > > + return owl_fix_fact_helper_round_rate(&fix_fact->common, fix_fact_hw, > > + rate, parent_rate); > > +} > > + > > +unsigned long owl_fix_fact_helper_recalc_rate(struct owl_clk_common *common, > > + const struct owl_fix_fact_hw *fix_fact_hw, > > + unsigned long parent_rate) > > +{ > > + unsigned long long int rate; > > + > > + rate = (unsigned long long int)parent_rate * fix_fact_hw->mul; > > + do_div(rate, fix_fact_hw->div); > > + > > + return (unsigned long)rate; > > You can drop the cast. > Okay. > > +} > > + > > +static unsigned long owl_fix_fact_recalc_rate(struct clk_hw *hw, > > + unsigned long parent_rate) > > +{ > > + struct owl_fix_fact *fix_fact = hw_to_owl_fix_fact(hw); > > + struct owl_fix_fact_hw *fix_fact_hw = &fix_fact->fix_fact_hw; > > + > > + return owl_fix_fact_helper_recalc_rate(&fix_fact->common, fix_fact_hw, > > + parent_rate); > > +} > > + > > +static int owl_fix_fact_set_rate(struct clk_hw *hw, unsigned long rate, > > + unsigned long parent_rate) > > +{ > > + /* > > + * We must report success but we can do so unconditionally because > > + * clk_fix_fact_round_rate returns values that ensure this call is a > > What function is that? > It should be owl_fix_fact_round_rate. > > + * nop. > > + */ > > + > > + return 0; > > +} > > + > > +const struct clk_ops owl_fix_fact_ops = { > > + .round_rate = owl_fix_fact_round_rate, > > + .recalc_rate = owl_fix_fact_recalc_rate, > > + .set_rate = owl_fix_fact_set_rate, > > +}; > > Why can't you use the regular fixed factor clk code and ops? > That's going to be really messy. Since I'm having the clk_hw embedded inside owl_clk_common and using it for registering all the clocks, using generic fixed factor functions will be a different approach _only_ for this clock and it won't look good I guess. Also, it may become complicated with composite clocks. If you still want to use the generic fixed factor code, I can do that in next revision. Your views? Thanks, Mani