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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxx>
> > +//
> > +// Copyright (c) 2018 Linaro Ltd.
> > +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> > +
> > +#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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
