[RFC] i.MX clock support

[Richard Zhao]

2010/12/13 Sascha Hauer <s.hauer at pengutronix.de>:
> On Mon, Dec 13, 2010 at 04:01:20PM +0100, Uwe Kleine-König wrote:
>> Hi Sascha,
>>
>> On Mon, Dec 13, 2010 at 11:25:38AM +0100, Sascha Hauer wrote:
>> > I am not willing to accept patches for adding i.MX50 support in the mess
>> > we currently have. These patches offer a way to cleanup the clock support
>> > and the i.MX50 may be a good test bed for an implementation without
>> > old cruft to worry about. That said the following patch is not set in
>> > stone, it's a request for comments and I'm of course open to other
>> > suggestions, but it's clear that we have to do something.
>> Full ack.
>>
>> > +#define to_clk_divider(clk) (container_of(clk, struct clk_divider, clk))
>> > +
>> > +static unsigned long clk_divider_get_rate(struct clk *clk)
>> > +{
>> > +   struct clk_divider *divider = to_clk_divider(clk);
>> > +
>> > +   unsigned long rate = clk_get_rate(divider->parent);
>> > +   unsigned int div = 1;
>> > +
>> > +   if (divider->reg) {
>> > +           div = readl(divider->reg) >> divider->shift;
>> > +           div &= (1 << divider->width) - 1;
>> > +           div++;
>> > +   }
>> > +
>> > +   return rate / div / divider->div * divider->mult;
>> Maybe you need to spend more effort to exactness e.g. by using
>> DIV_ROUND_CLOSEST and/or reordering?
>> (You didn't describe div and mult in struct clk_divider (below), so this
>> is a bit guess work for me here.)
>
> Ok, this needs some work. My original idea was to have seperate fixed
> dividers and configurable dividers. Then I decided to combine these into
> one divider. The end result was a mixure of both. We have a struct
> clk_divider_fixed, which is described but unused.
>
>>
>> > +}
>> > +
>> > +static long clk_divider_round_rate(struct clk *clk, unsigned long rate)
>> > +{
>> > +   struct clk_divider *divider = to_clk_divider(clk);
>> > +   unsigned long parent_rate = clk_get_rate(divider->parent);
>> > +   unsigned int max_div, div;
>> > +
>> > +   if (rate > parent_rate)
>> > +           return parent_rate;
>> > +
>> > +   max_div = 1 << divider->width;
>> > +
>> > +   div = parent_rate / rate;
>> > +   div = max(div, max_div);
>> > +
>> > +   return parent_rate / div / divider->div * divider->mult;
>> ditto
>>
>> > +}
>> > +
>> > +static int clk_divider_set_rate(struct clk *clk, unsigned long rate)
>> > +{
>> > +   struct clk_divider *divider = to_clk_divider(clk);
>> > +   unsigned long parent_rate = clk_get_rate(divider->parent);
>> > +   unsigned int max_div, div;
>> > +   u32 val;
>> > +
>> > +   parent_rate /= divider->div;
>> > +   parent_rate *= divider->mult;
>> > +
>> > +   if (rate > parent_rate)
>> > +           rate = parent_rate;
>> > +
>> > +   max_div = 1 << divider->width;
>> > +
>> > +   div = parent_rate / rate;
>> > +
>> > +   div = max(div, max_div);
>> > +   div--;
>> > +
>> > +   val = readl(divider->reg);
>> > +   val &= ~(((1 << divider->width) - 1) << divider->shift);
>> > +   val |= div << divider->shift;
>> > +   writel(val, divider->reg);
>> > +
>> > +   return 0;
>> You could spend more efforts here, but I think this is OK for now.
>>
>> > [...]
>> > +struct clk_ops clk_multiplexer_ops = {
>> > +   .enable = clk_parent_enable,
>> > +   .disable = clk_parent_disable,
>> > +   .get_rate = clk_parent_get_rate,
>> > +   .round_rate = clk_parent_round_rate,
>> > +   .set_rate = clk_parent_set_rate,
>> Oh, this might have surprising effects if the parent is "public".
>> Is this intended?
>
> I have no idea what the best way is here. We could remove it and wait
> if somebody comes up with a good reason to add it again.
How about adding a child_count. If child_count >1, we stop its child
calling its set_rate/set_parent. In such way, we have to register
every clock, which is easier to debug. child_count maybe none zero
intend, in case there're some clocks in physical we don't set up in
software.

Thanks
Richard
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>