Re: [PATCH 2/2] clk: divider: fix to set parent rate from CLK_DIVIDER_READ_ONLY flag

From: Stephen Boyd
Date: Tue May 12 2015 - 19:57:33 EST


On 05/12, Michael Turquette wrote:
> Quoting Joonyoung Shim (2015-04-07 00:46:46)
> > The round_rate callback function will returns alway same parent clk rate
> > of divider with CLK_DIVIDER_READ_ONLY flag. If be used
> > CLK_SET_RATE_PARENT flag with CLK_DIVIDER_READ_ONLY flag, then never
> > change parent clk rate anymore.
> >
> > From this case, this patch allows to change parent clk rate.
> >
> > Signed-off-by: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
> > ---
> > drivers/clk/clk-divider.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> > index ce34d29a..37e285e 100644
> > --- a/drivers/clk/clk-divider.c
> > +++ b/drivers/clk/clk-divider.c
> > @@ -352,6 +352,11 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
> > bestdiv = readl(divider->reg) >> divider->shift;
> > bestdiv &= div_mask(divider->width);
> > bestdiv = _get_div(divider->table, bestdiv, divider->flags);
> > +
> > + if ((__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
> > + *prate = __clk_round_rate(__clk_get_parent(hw->clk),
> > + rate);
> > +
> > return DIV_ROUND_UP(*prate, bestdiv);
> > }
> >
> > --
> > 1.9.1
> >
>
> Hello Joonyoung Shim,
>
> Thanks for reporting the bug and providing a fix!
>
> I've come up with an alternative solution to this. This patch should
> replace both of your patches. Can you test this and see if it fixes the
> problem for you?
>
> Thanks,
> Mike
>
>
>
> From 655dddad2700a30aaa397cd804422e0d9195efad Mon Sep 17 00:00:00 2001
> From: Michael Turquette <mturquette@xxxxxxxxxx>
> Date: Tue, 12 May 2015 16:13:46 -0700
> Subject: [PATCH] clk: divider: support read-only dividers
>
> An arbitrary clock rate divider may be set out of reset, or perhaps by
> the bootloader or something other than Linux. In these cases we may want
> to know the frequency of the clock signal, but we do not want to allow
> Linux to change it.
>
> The CLK_DIVIDER_READ_ONLY flag was intended to express this, but the
> functionality was missing in the code. Add read-only clk_ops for divider
> clocks to handle this case.
>
> For hardware with fixed dividers it is still best to use the
> fixed-factor clock type.
>
> Reported-by: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
> Signed-off-by: Michael Turquette <mturquette@xxxxxxxxxx>
> ---
> drivers/clk/clk-divider.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 25006a8..5d2de26 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -412,6 +412,11 @@ const struct clk_ops clk_divider_ops = {
> };
> EXPORT_SYMBOL_GPL(clk_divider_ops);
>
> +const struct clk_ops clk_divider_ro_ops = {
> + .recalc_rate = clk_divider_recalc_rate,
> +};
> +EXPORT_SYMBOL_GPL(clk_divider_ro_ops);
> +
> static struct clk *_register_divider(struct device *dev, const char *name,
> const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 shift, u8 width,
> @@ -437,7 +442,10 @@ static struct clk *_register_divider(struct device *dev, const char *name,
> }
>
> init.name = name;
> - init.ops = &clk_divider_ops;
> + if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
> + init.ops = &clk_divider_ro_ops;
> + else
> + init.ops = &clk_divider_ops;
> init.flags = flags | CLK_IS_BASIC;
> init.parent_names = (parent_name ? &parent_name: NULL);
> init.num_parents = (parent_name ? 1 : 0);
> --

Isn't this sort of reverting commit e6d5e7d90be9 (clk-divider:
Fix READ_ONLY when divider > 1, 2014-11-14)?

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/