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

[Stephen Boyd]

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/