diff mbox

[v3,01/13] clk: sunxi: register factors clocks behind composite

Message ID 1387769564-12894-2-git-send-email-emilio@elopez.com.ar
State New
Headers show

Commit Message

Emilio López Dec. 23, 2013, 3:32 a.m. UTC
This commit reworks factors clock registration to be done behind a
composite clock. This allows us to additionally add a gate, mux or
divisors, as it will be needed by some future PLLs.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/clk-factors.c | 63 +------------------------------------
 drivers/clk/sunxi/clk-factors.h | 16 +++++-----
 drivers/clk/sunxi/clk-sunxi.c   | 70 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 76 insertions(+), 73 deletions(-)

Comments

Boris Brezillon Jan. 7, 2014, 4:56 p.m. UTC | #1
Hello Emilio, Mike,

On 23/12/2013 04:32, Emilio López wrote:
> This commit reworks factors clock registration to be done behind a
> composite clock. This allows us to additionally add a gate, mux or
> divisors, as it will be needed by some future PLLs.
>


> Signed-off-by: Emilio López <emilio@elopez.com.ar>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>   drivers/clk/sunxi/clk-factors.c | 63 +------------------------------------
>   drivers/clk/sunxi/clk-factors.h | 16 +++++-----
>   drivers/clk/sunxi/clk-sunxi.c   | 70 ++++++++++++++++++++++++++++++++++++++---
>   3 files changed, 76 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
> index f05207a..9e23264 100644
> --- a/drivers/clk/sunxi/clk-factors.c
> +++ b/drivers/clk/sunxi/clk-factors.c
> @@ -30,14 +30,6 @@
>    * parent - fixed parent.  No clk_set_parent support
>    */
>
> -struct clk_factors {
> -	struct clk_hw hw;
> -	void __iomem *reg;
> -	struct clk_factors_config *config;
> -	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
> -	spinlock_t *lock;
> -};
> -
>   #define to_clk_factors(_hw) container_of(_hw, struct clk_factors, hw)
>
>   #define SETMASK(len, pos)		(((1U << (len)) - 1) << (pos))
> @@ -120,61 +112,8 @@ static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
>   	return 0;
>   }
>
> -static const struct clk_ops clk_factors_ops = {
> +const struct clk_ops clk_factors_ops = {
>   	.recalc_rate = clk_factors_recalc_rate,
>   	.round_rate = clk_factors_round_rate,
>   	.set_rate = clk_factors_set_rate,
>   };
> -
> -/**
> - * clk_register_factors - register a factors clock with
> - * the clock framework
> - * @dev: device registering this clock
> - * @name: name of this clock
> - * @parent_name: name of clock's parent
> - * @flags: framework-specific flags
> - * @reg: register address to adjust factors
> - * @config: shift and width of factors n, k, m and p
> - * @get_factors: function to calculate the factors for a given frequency
> - * @lock: shared register lock for this clock
> - */
> -struct clk *clk_register_factors(struct device *dev, const char *name,
> -				 const char *parent_name,
> -				 unsigned long flags, void __iomem *reg,
> -				 struct clk_factors_config *config,
> -				 void (*get_factors)(u32 *rate, u32 parent,
> -						     u8 *n, u8 *k, u8 *m, u8 *p),
> -				 spinlock_t *lock)
> -{
> -	struct clk_factors *factors;
> -	struct clk *clk;
> -	struct clk_init_data init;
> -
> -	/* allocate the factors */
> -	factors = kzalloc(sizeof(struct clk_factors), GFP_KERNEL);
> -	if (!factors) {
> -		pr_err("%s: could not allocate factors clk\n", __func__);
> -		return ERR_PTR(-ENOMEM);
> -	}
> -
> -	init.name = name;
> -	init.ops = &clk_factors_ops;
> -	init.flags = flags;
> -	init.parent_names = (parent_name ? &parent_name : NULL);
> -	init.num_parents = (parent_name ? 1 : 0);
> -
> -	/* struct clk_factors assignments */
> -	factors->reg = reg;
> -	factors->config = config;
> -	factors->lock = lock;
> -	factors->hw.init = &init;
> -	factors->get_factors = get_factors;
> -
> -	/* register the clock */
> -	clk = clk_register(dev, &factors->hw);
> -
> -	if (IS_ERR(clk))
> -		kfree(factors);
> -
> -	return clk;
> -}
> diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
> index f49851c..02e1a43 100644
> --- a/drivers/clk/sunxi/clk-factors.h
> +++ b/drivers/clk/sunxi/clk-factors.h
> @@ -17,11 +17,13 @@ struct clk_factors_config {
>   	u8 pwidth;
>   };
>
> -struct clk *clk_register_factors(struct device *dev, const char *name,
> -				 const char *parent_name,
> -				 unsigned long flags, void __iomem *reg,
> -				 struct clk_factors_config *config,
> -				 void (*get_factors) (u32 *rate, u32 parent_rate,
> -						      u8 *n, u8 *k, u8 *m, u8 *p),
> -				 spinlock_t *lock);
> +struct clk_factors {
> +	struct clk_hw hw;
> +	void __iomem *reg;
> +	struct clk_factors_config *config;
> +	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
> +	spinlock_t *lock;
> +};
> +
> +extern const struct clk_ops clk_factors_ops;
>   #endif
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index 8fc1375..b823613 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -23,6 +23,9 @@
>
>   static DEFINE_SPINLOCK(clk_lock);
>
> +/* Maximum number of parents our clocks have */
> +#define SUNXI_MAX_PARENTS	5
> +
>   /**
>    * sun4i_osc_clk_setup() - Setup function for gatable oscillator
>    */
> @@ -255,7 +258,11 @@ static void sun4i_get_apb1_factors(u32 *freq, u32 parent_rate,
>    * sunxi_factors_clk_setup() - Setup function for factor clocks
>    */
>
> +#define SUNXI_FACTORS_MUX_MASK 0x3
> +
>   struct factors_data {
> +	int enable;
> +	int mux;
>   	struct clk_factors_config *table;
>   	void (*getter) (u32 *rate, u32 parent_rate, u8 *n, u8 *k, u8 *m, u8 *p);
>   };
> @@ -306,16 +313,71 @@ static void __init sunxi_factors_clk_setup(struct device_node *node,
>   					   struct factors_data *data)
>   {
>   	struct clk *clk;
> +	struct clk_factors *factors;
> +	struct clk_gate *gate = NULL;
> +	struct clk_mux *mux = NULL;
> +	struct clk_hw *gate_hw = NULL;
> +	struct clk_hw *mux_hw = NULL;
>   	const char *clk_name = node->name;
> -	const char *parent;
> +	const char *parents[SUNXI_MAX_PARENTS];
>   	void *reg;
> +	int i = 0;
>
>   	reg = of_iomap(node, 0);
>
> -	parent = of_clk_get_parent_name(node, 0);
> +	/* if we have a mux, we will have >1 parents */
> +	while (i < SUNXI_MAX_PARENTS &&
> +	       (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
> +		i++;
> +
> +	factors = kzalloc(sizeof(struct clk_factors), GFP_KERNEL);
> +	if (!factors)
> +		return;
> +
> +	/* Add a gate if this factor clock can be gated */
> +	if (data->enable) {
> +		gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
> +		if (!gate) {
> +			kfree(factors);
> +			return;
> +		}
> +
> +		/* set up gate properties */
> +		gate->reg = reg;
> +		gate->bit_idx = data->enable;
> +		gate->lock = &clk_lock;
> +		gate_hw = &gate->hw;
> +	}
> +
> +	/* Add a mux if this factor clock can be muxed */
> +	if (data->mux) {
> +		mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
> +		if (!mux) {
> +			kfree(factors);
> +			kfree(gate);
> +			return;
> +		}
> +
> +		/* set up gate properties */
> +		mux->reg = reg;
> +		mux->shift = data->mux;
> +		mux->mask = SUNXI_FACTORS_MUX_MASK;
> +		mux->lock = &clk_lock;
> +		mux_hw = &mux->hw;
> +	}
> +
> +	/* set up factors properties */
> +	factors->reg = reg;
> +	factors->config = data->table;
> +	factors->get_factors = data->getter;
> +	factors->lock = &clk_lock;
>
> -	clk = clk_register_factors(NULL, clk_name, parent, 0, reg,
> -				   data->table, data->getter, &clk_lock);
> +	clk = clk_register_composite(NULL, clk_name,
> +			parents, i,
> +			mux_hw, &clk_mux_ops,
> +			&factors->hw, &clk_factors_ops,
> +			gate_hw, &clk_gate_ops,
> +			i ? 0 : CLK_IS_ROOT);
>

I'm currently working on the sunxi NFC (Nand Flash Controller) driver
and I need to set the NAND clk (which is a mod0 clk type) rate.

It seems that the composite clk fallbacks to the mux_hw's determine_rate
instead of calling the rate_hw's (or factors hw) round_rate, if rate_hw
does not implement determine_rate.

In the sunxi specific case, this leads to a DIV by 0 error when later
calling set_rate on this composite clk, because the requested freq is
set to 0 (result from mux_hw determine_rate):


[    0.632125] Division by zero in kernel.
[    0.635970] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
3.13.0-rc1-00005-ge2b2be1-dirty #203
[    0.644230] [<c00139bc>] (unwind_backtrace+0x0/0xf8) from 
[<c0010fb8>] (show_stack+0x10/0x14)
[    0.652765] [<c0010fb8>] (show_stack+0x10/0x14) from [<c029276c>] 
(dump_stack+0x68/0x84)
[    0.660861] [<c029276c>] (dump_stack+0x68/0x84) from [<c015e8b0>] 
(Ldiv0+0x8/0x10)
[    0.668438] [<c015e8b0>] (Ldiv0+0x8/0x10) from [<c020d63c>] 
(sun4i_get_mod0_factors+0x6c/0xa8)
[    0.677057] [<c020d63c>] (sun4i_get_mod0_factors+0x6c/0xa8) from 
[<c020d87c>] (clk_factors_set_rate+0x50/0x138)
[    0.687149] [<c020d87c>] (clk_factors_set_rate+0x50/0x138) from 
[<c020cf18>] (clk_composite_set_rate+0x20/0x24)
[    0.697241] [<c020cf18>] (clk_composite_set_rate+0x20/0x24) from 
[<c020b538>] (clk_change_rate+0x5c/0xf8)
[    0.706812] [<c020b538>] (clk_change_rate+0x5c/0xf8) from 
[<c020b64c>] (clk_set_rate+0x78/0xb0)
[    0.715518] [<c020b64c>] (clk_set_rate+0x78/0xb0) from [<c01cc370>] 
(sunxi_nfc_probe+0x1ec/0x5e0)
[    0.724383] [<c01cc370>] (sunxi_nfc_probe+0x1ec/0x5e0) from 
[<c01b50d4>] (platform_drv_probe+0x18/0x48)
[    0.733779] [<c01b50d4>] (platform_drv_probe+0x18/0x48) from 
[<c01b37f0>] (driver_probe_device+0x100/0x210)
[    0.743521] [<c01b37f0>] (driver_probe_device+0x100/0x210) from 
[<c01b398c>] (__driver_attach+0x8c/0x90)
[    0.753005] [<c01b398c>] (__driver_attach+0x8c/0x90) from 
[<c01b2178>] (bus_for_each_dev+0x54/0x88)
[    0.762058] [<c01b2178>] (bus_for_each_dev+0x54/0x88) from 
[<c01b2fd4>] (bus_add_driver+0xd8/0x1cc)
[    0.771108] [<c01b2fd4>] (bus_add_driver+0xd8/0x1cc) from 
[<c01b3fc8>] (driver_register+0x78/0xf4)
[    0.780069] [<c01b3fc8>] (driver_register+0x78/0xf4) from 
[<c00087c8>] (do_one_initcall+0x38/0x160)
[    0.789118] [<c00087c8>] (do_one_initcall+0x38/0x160) from 
[<c0372bc0>] (kernel_init_freeable+0xfc/0x1c8)
[    0.798688] [<c0372bc0>] (kernel_init_freeable+0xfc/0x1c8) from 
[<c028fd00>] (kernel_init+0x8/0x110)
[    0.807825] [<c028fd00>] (kernel_init+0x8/0x110) from [<c000e2b8>] 
(ret_from_fork+0x14/0x3c)


We should definitely check the requested freq and gracefully handle the
0 case.

But anyway, I think there is a drawback in the way composite clk
implement the determine_rate handler.
Mike, shouldn't we choose the best parent (using mux) fullfilling the
rate_hw needs ?


I'll post a proposal...


Best Regards,

Boris

>   	if (!IS_ERR(clk)) {
>   		of_clk_add_provider(node, of_clk_src_simple_get, clk);
>
Emilio López Jan. 7, 2014, 5:47 p.m. UTC | #2
Hi Boris,

[ Apologies if the email looks weird and for using a link; I'm not
using my typical email client ]

> I'm currently working on the sunxi NFC (Nand Flash Controller) driver
> and I need to set the NAND clk (which is a mod0 clk type) rate.
>
> It seems that the composite clk fallbacks to the mux_hw's determine_rate
> instead of calling the rate_hw's (or factors hw) round_rate, if rate_hw
> does not implement determine_rate.

That is to be expected; if you have a composite clock that has all
fixed parents you'd not implement the determine_rate on the rate
component but on the mux one.

(...)

> But anyway, I think there is a drawback in the way composite clk
> implement the determine_rate handler.
> Mike, shouldn't we choose the best parent (using mux) fullfilling the
> rate_hw needs ?

Someone using the composite clock may not want their clocks to start
changing parents though. I believe it's easier and better to just
implement determine_rate as on this patch on my tree (which I haven't
sent yet, but it's literally the next in queue.)

http://git.elopez.com.ar/linux/commits/5b4eb3ac406b9c98965714d40e8dd6da943d1ab0

Let me know if that resolves your issue.

Cheers,

Emilio
Boris Brezillon Jan. 7, 2014, 8:46 p.m. UTC | #3
"Emilio López" <emilio@elopez.com.ar> a écrit :
>Hi Boris,
>
>[ Apologies if the email looks weird and for using a link; I'm not
>using my typical email client ]
>
>> I'm currently working on the sunxi NFC (Nand Flash Controller) driver
>> and I need to set the NAND clk (which is a mod0 clk type) rate.
>>
>> It seems that the composite clk fallbacks to the mux_hw's
>determine_rate
>> instead of calling the rate_hw's (or factors hw) round_rate, if
>rate_hw
>> does not implement determine_rate.
>
>That is to be expected; if you have a composite clock that has all
>fixed parents you'd not implement the determine_rate on the rate
>component but on the mux one.
>

Right, but this is not the case for mod0 
clks, is it ?

>(...)
>
>> But anyway, I think there is a drawback in the way composite clk
>> implement the determine_rate handler.
>> Mike, shouldn't we choose the best parent (using mux) fullfilling the
>> rate_hw needs ?
>
>Someone using the composite clock may not want their clocks to start
>changing parents though. I believe it's easier and better to just
>implement determine_rate as on this patch on my tree (which I haven't
>sent yet, but it's literally the next in queue.)
>

Fair enough (I did pretty much the same thing in my first attempt to fix this bug ;-))


>http://git.elopez.com.ar/linux/commits/5b4eb3ac406b9c98965714d40e8dd6da943d1ab0
>
>Let me know if that resolves your issue.

I'll try it tomorrow.

BTW, I managed to get a working NAND driver (without DMA and hw ECC).

I'll post it on the ML soon (just need to clean it up a little bit).

Best regards,

Boris

>
>Cheers,
>
>Emilio
Boris Brezillon Jan. 8, 2014, 9:30 a.m. UTC | #4
On 07/01/2014 18:47, Emilio López wrote:
> Hi Boris,
>
> [ Apologies if the email looks weird and for using a link; I'm not
> using my typical email client ]
>
>> I'm currently working on the sunxi NFC (Nand Flash Controller) driver
>> and I need to set the NAND clk (which is a mod0 clk type) rate.
>>
>> It seems that the composite clk fallbacks to the mux_hw's determine_rate
>> instead of calling the rate_hw's (or factors hw) round_rate, if rate_hw
>> does not implement determine_rate.
> That is to be expected; if you have a composite clock that has all
> fixed parents you'd not implement the determine_rate on the rate
> component but on the mux one.
>
> (...)
>
>> But anyway, I think there is a drawback in the way composite clk
>> implement the determine_rate handler.
>> Mike, shouldn't we choose the best parent (using mux) fullfilling the
>> rate_hw needs ?
> Someone using the composite clock may not want their clocks to start
> changing parents though. I believe it's easier and better to just
> implement determine_rate as on this patch on my tree (which I haven't
> sent yet, but it's literally the next in queue.)
>
> http://git.elopez.com.ar/linux/commits/5b4eb3ac406b9c98965714d40e8dd6da943d1ab0
>
> Let me know if that resolves your issue.

Yes, it works.
Thanks.

Can you submit it on LAKML so that I can state the dependency in my
NFC driver series ?

Best Regards,

Boris

>
> Cheers,
>
> Emilio
Mike Turquette Jan. 14, 2014, 8:41 p.m. UTC | #5
Quoting Boris BREZILLON (2014-01-07 09:03:52)
> In case the rate_hw does not implement determine_rate, but only round_rate
> we fallback to best_parent selection if mux_hw is present and support
> reparenting.
> 
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>

Hi Boris,

Since this change affects users of the composite clock type I will hold
off reviewing/applying this patch until after the merge window.

Thanks,
Mike

> ---
>  drivers/clk/clk-composite.c |   49 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> index 753d0b7..d3cf49a 100644
> --- a/drivers/clk/clk-composite.c
> +++ b/drivers/clk/clk-composite.c
> @@ -64,11 +64,57 @@ static long clk_composite_determine_rate(struct clk_hw *hw, unsigned long rate,
>         const struct clk_ops *mux_ops = composite->mux_ops;
>         struct clk_hw *rate_hw = composite->rate_hw;
>         struct clk_hw *mux_hw = composite->mux_hw;
> +       struct clk *parent;
> +       unsigned long parent_rate;
> +       long tmp_rate;
> +       unsigned long rate_diff;
> +       unsigned long best_rate_diff = ULONG_MAX;
> +       int i;
>  
>         if (rate_hw && rate_ops && rate_ops->determine_rate) {
>                 rate_hw->clk = hw->clk;
>                 return rate_ops->determine_rate(rate_hw, rate, best_parent_rate,
>                                                 best_parent_p);
> +       } else if (rate_hw && rate_ops && rate_ops->round_rate &&
> +                  mux_hw && mux_ops && mux_ops->set_parent) {
> +               *best_parent_p = NULL;
> +
> +               if (__clk_get_flags(hw->clk) & CLK_SET_RATE_NO_REPARENT) {
> +                       *best_parent_p = clk_get_parent(mux_hw->clk);
> +                       *best_parent_rate = __clk_get_rate(*best_parent_p);
> +
> +                       return rate_ops->round_rate(rate_hw, rate,
> +                                                   best_parent_rate);
> +               }
> +
> +               for (i = 0; i < __clk_get_num_parents(mux_hw->clk); i++) {
> +                       parent = clk_get_parent_by_index(mux_hw->clk, i);
> +                       if (!parent)
> +                               continue;
> +
> +                       parent_rate = __clk_get_rate(parent);
> +
> +                       tmp_rate = rate_ops->round_rate(rate_hw, rate,
> +                                                       &parent_rate);
> +                       if (tmp_rate < 0)
> +                               continue;
> +
> +                       if (tmp_rate < rate)
> +                               rate_diff = rate - tmp_rate;
> +                       else
> +                               rate_diff = tmp_rate - rate;
> +
> +                       if (!rate_diff || !*best_parent_p || best_rate_diff > rate_diff) {
> +                               *best_parent_p = parent;
> +                               *best_parent_rate = parent_rate;
> +                               best_rate_diff = rate_diff;
> +                       }
> +
> +                       if (!rate_diff)
> +                               return rate;
> +               }
> +
> +               return best_rate_diff;
>         } else if (mux_hw && mux_ops && mux_ops->determine_rate) {
>                 mux_hw->clk = hw->clk;
>                 return mux_ops->determine_rate(rate_hw, rate, best_parent_rate,
> @@ -196,7 +242,8 @@ struct clk *clk_register_composite(struct device *dev, const char *name,
>                 composite->rate_hw = rate_hw;
>                 composite->rate_ops = rate_ops;
>                 clk_composite_ops->recalc_rate = clk_composite_recalc_rate;
> -               if (rate_ops->determine_rate)
> +               if (rate_ops->determine_rate ||
> +                   (rate_ops->round_rate && clk_composite_ops->set_parent))
>                         clk_composite_ops->determine_rate = clk_composite_determine_rate;
>         }
>  
> -- 
> 1.7.9.5
>
Heiko Stuebner May 18, 2014, 10:23 p.m. UTC | #6
Am Dienstag, 14. Januar 2014, 12:41:46 schrieb Mike Turquette:
> Quoting Boris BREZILLON (2014-01-07 09:03:52)
> 
> > In case the rate_hw does not implement determine_rate, but only round_rate
> > we fallback to best_parent selection if mux_hw is present and support
> > reparenting.
> > 
> > Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
> 
> Since this change affects users of the composite clock type I will hold
> off reviewing/applying this patch until after the merge window.

sorry for resurrecting this old code, but it looks like it was forgotten 
somehow and I'm currently hit by a problem that this patch fixes.

When using a composite clk containing both a divider and mux using the 
standard clock ops, all rate changes ignore the divider completely and only 
handle the mux. In my case the parents provide 800 and 600MHz while I'm 
requesting 300MHz. As only mux_ops->determine_rate runs, I get 600MHz instead 
of anything <= 300 when using the included divider.

Boris' patch fixes this issue for me, so

Tested-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>


The patch applies nicely after adapting it according to 
5d2043fbe4ddc6cc16ba71b49c2c13f4cb2fe932 ("clk: composite: pass mux_hw into 
determine_rate")


Heiko

> > ---
> > 
> >  drivers/clk/clk-composite.c |   49
> >  ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48
> >  insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> > index 753d0b7..d3cf49a 100644
> > --- a/drivers/clk/clk-composite.c
> > +++ b/drivers/clk/clk-composite.c
> > @@ -64,11 +64,57 @@ static long clk_composite_determine_rate(struct clk_hw
> > *hw, unsigned long rate,> 
> >         const struct clk_ops *mux_ops = composite->mux_ops;
> >         struct clk_hw *rate_hw = composite->rate_hw;
> >         struct clk_hw *mux_hw = composite->mux_hw;
> > 
> > +       struct clk *parent;
> > +       unsigned long parent_rate;
> > +       long tmp_rate;
> > +       unsigned long rate_diff;
> > +       unsigned long best_rate_diff = ULONG_MAX;
> > +       int i;
> > 
> >         if (rate_hw && rate_ops && rate_ops->determine_rate) {
> >         
> >                 rate_hw->clk = hw->clk;
> >                 return rate_ops->determine_rate(rate_hw, rate,
> >                 best_parent_rate,
> >                 
> >                                                 best_parent_p);
> > 
> > +       } else if (rate_hw && rate_ops && rate_ops->round_rate &&
> > +                  mux_hw && mux_ops && mux_ops->set_parent) {
> > +               *best_parent_p = NULL;
> > +
> > +               if (__clk_get_flags(hw->clk) & CLK_SET_RATE_NO_REPARENT) {
> > +                       *best_parent_p = clk_get_parent(mux_hw->clk);
> > +                       *best_parent_rate =
> > __clk_get_rate(*best_parent_p);
> > +
> > +                       return rate_ops->round_rate(rate_hw, rate,
> > +                                                   best_parent_rate);
> > +               }
> > +
> > +               for (i = 0; i < __clk_get_num_parents(mux_hw->clk); i++) {
> > +                       parent = clk_get_parent_by_index(mux_hw->clk, i);
> > +                       if (!parent)
> > +                               continue;
> > +
> > +                       parent_rate = __clk_get_rate(parent);
> > +
> > +                       tmp_rate = rate_ops->round_rate(rate_hw, rate,
> > +                                                       &parent_rate);
> > +                       if (tmp_rate < 0)
> > +                               continue;
> > +
> > +                       if (tmp_rate < rate)
> > +                               rate_diff = rate - tmp_rate;
> > +                       else
> > +                               rate_diff = tmp_rate - rate;
> > +
> > +                       if (!rate_diff || !*best_parent_p ||
> > best_rate_diff > rate_diff) { +                              
> > *best_parent_p = parent;
> > +                               *best_parent_rate = parent_rate;
> > +                               best_rate_diff = rate_diff;
> > +                       }
> > +
> > +                       if (!rate_diff)
> > +                               return rate;
> > +               }
> > +
> > +               return best_rate_diff;
> > 
> >         } else if (mux_hw && mux_ops && mux_ops->determine_rate) {
> >         
> >                 mux_hw->clk = hw->clk;
> >                 return mux_ops->determine_rate(rate_hw, rate,
> >                 best_parent_rate,
> > 
> > @@ -196,7 +242,8 @@ struct clk *clk_register_composite(struct device *dev,
> > const char *name,> 
> >                 composite->rate_hw = rate_hw;
> >                 composite->rate_ops = rate_ops;
> >                 clk_composite_ops->recalc_rate =
> >                 clk_composite_recalc_rate;
> > 
> > -               if (rate_ops->determine_rate)
> > +               if (rate_ops->determine_rate ||
> > +                   (rate_ops->round_rate &&
> > clk_composite_ops->set_parent))> 
> >                         clk_composite_ops->determine_rate =
> >                         clk_composite_determine_rate;
> >         
> >         }
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox

Patch

diff --git a/drivers/clk/sunxi/clk-factors.c b/drivers/clk/sunxi/clk-factors.c
index f05207a..9e23264 100644
--- a/drivers/clk/sunxi/clk-factors.c
+++ b/drivers/clk/sunxi/clk-factors.c
@@ -30,14 +30,6 @@ 
  * parent - fixed parent.  No clk_set_parent support
  */
 
-struct clk_factors {
-	struct clk_hw hw;
-	void __iomem *reg;
-	struct clk_factors_config *config;
-	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
-	spinlock_t *lock;
-};
-
 #define to_clk_factors(_hw) container_of(_hw, struct clk_factors, hw)
 
 #define SETMASK(len, pos)		(((1U << (len)) - 1) << (pos))
@@ -120,61 +112,8 @@  static int clk_factors_set_rate(struct clk_hw *hw, unsigned long rate,
 	return 0;
 }
 
-static const struct clk_ops clk_factors_ops = {
+const struct clk_ops clk_factors_ops = {
 	.recalc_rate = clk_factors_recalc_rate,
 	.round_rate = clk_factors_round_rate,
 	.set_rate = clk_factors_set_rate,
 };
-
-/**
- * clk_register_factors - register a factors clock with
- * the clock framework
- * @dev: device registering this clock
- * @name: name of this clock
- * @parent_name: name of clock's parent
- * @flags: framework-specific flags
- * @reg: register address to adjust factors
- * @config: shift and width of factors n, k, m and p
- * @get_factors: function to calculate the factors for a given frequency
- * @lock: shared register lock for this clock
- */
-struct clk *clk_register_factors(struct device *dev, const char *name,
-				 const char *parent_name,
-				 unsigned long flags, void __iomem *reg,
-				 struct clk_factors_config *config,
-				 void (*get_factors)(u32 *rate, u32 parent,
-						     u8 *n, u8 *k, u8 *m, u8 *p),
-				 spinlock_t *lock)
-{
-	struct clk_factors *factors;
-	struct clk *clk;
-	struct clk_init_data init;
-
-	/* allocate the factors */
-	factors = kzalloc(sizeof(struct clk_factors), GFP_KERNEL);
-	if (!factors) {
-		pr_err("%s: could not allocate factors clk\n", __func__);
-		return ERR_PTR(-ENOMEM);
-	}
-
-	init.name = name;
-	init.ops = &clk_factors_ops;
-	init.flags = flags;
-	init.parent_names = (parent_name ? &parent_name : NULL);
-	init.num_parents = (parent_name ? 1 : 0);
-
-	/* struct clk_factors assignments */
-	factors->reg = reg;
-	factors->config = config;
-	factors->lock = lock;
-	factors->hw.init = &init;
-	factors->get_factors = get_factors;
-
-	/* register the clock */
-	clk = clk_register(dev, &factors->hw);
-
-	if (IS_ERR(clk))
-		kfree(factors);
-
-	return clk;
-}
diff --git a/drivers/clk/sunxi/clk-factors.h b/drivers/clk/sunxi/clk-factors.h
index f49851c..02e1a43 100644
--- a/drivers/clk/sunxi/clk-factors.h
+++ b/drivers/clk/sunxi/clk-factors.h
@@ -17,11 +17,13 @@  struct clk_factors_config {
 	u8 pwidth;
 };
 
-struct clk *clk_register_factors(struct device *dev, const char *name,
-				 const char *parent_name,
-				 unsigned long flags, void __iomem *reg,
-				 struct clk_factors_config *config,
-				 void (*get_factors) (u32 *rate, u32 parent_rate,
-						      u8 *n, u8 *k, u8 *m, u8 *p),
-				 spinlock_t *lock);
+struct clk_factors {
+	struct clk_hw hw;
+	void __iomem *reg;
+	struct clk_factors_config *config;
+	void (*get_factors) (u32 *rate, u32 parent, u8 *n, u8 *k, u8 *m, u8 *p);
+	spinlock_t *lock;
+};
+
+extern const struct clk_ops clk_factors_ops;
 #endif
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 8fc1375..b823613 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -23,6 +23,9 @@ 
 
 static DEFINE_SPINLOCK(clk_lock);
 
+/* Maximum number of parents our clocks have */
+#define SUNXI_MAX_PARENTS	5
+
 /**
  * sun4i_osc_clk_setup() - Setup function for gatable oscillator
  */
@@ -255,7 +258,11 @@  static void sun4i_get_apb1_factors(u32 *freq, u32 parent_rate,
  * sunxi_factors_clk_setup() - Setup function for factor clocks
  */
 
+#define SUNXI_FACTORS_MUX_MASK 0x3
+
 struct factors_data {
+	int enable;
+	int mux;
 	struct clk_factors_config *table;
 	void (*getter) (u32 *rate, u32 parent_rate, u8 *n, u8 *k, u8 *m, u8 *p);
 };
@@ -306,16 +313,71 @@  static void __init sunxi_factors_clk_setup(struct device_node *node,
 					   struct factors_data *data)
 {
 	struct clk *clk;
+	struct clk_factors *factors;
+	struct clk_gate *gate = NULL;
+	struct clk_mux *mux = NULL;
+	struct clk_hw *gate_hw = NULL;
+	struct clk_hw *mux_hw = NULL;
 	const char *clk_name = node->name;
-	const char *parent;
+	const char *parents[SUNXI_MAX_PARENTS];
 	void *reg;
+	int i = 0;
 
 	reg = of_iomap(node, 0);
 
-	parent = of_clk_get_parent_name(node, 0);
+	/* if we have a mux, we will have >1 parents */
+	while (i < SUNXI_MAX_PARENTS &&
+	       (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
+		i++;
+
+	factors = kzalloc(sizeof(struct clk_factors), GFP_KERNEL);
+	if (!factors)
+		return;
+
+	/* Add a gate if this factor clock can be gated */
+	if (data->enable) {
+		gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
+		if (!gate) {
+			kfree(factors);
+			return;
+		}
+
+		/* set up gate properties */
+		gate->reg = reg;
+		gate->bit_idx = data->enable;
+		gate->lock = &clk_lock;
+		gate_hw = &gate->hw;
+	}
+
+	/* Add a mux if this factor clock can be muxed */
+	if (data->mux) {
+		mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
+		if (!mux) {
+			kfree(factors);
+			kfree(gate);
+			return;
+		}
+
+		/* set up gate properties */
+		mux->reg = reg;
+		mux->shift = data->mux;
+		mux->mask = SUNXI_FACTORS_MUX_MASK;
+		mux->lock = &clk_lock;
+		mux_hw = &mux->hw;
+	}
+
+	/* set up factors properties */
+	factors->reg = reg;
+	factors->config = data->table;
+	factors->get_factors = data->getter;
+	factors->lock = &clk_lock;
 
-	clk = clk_register_factors(NULL, clk_name, parent, 0, reg,
-				   data->table, data->getter, &clk_lock);
+	clk = clk_register_composite(NULL, clk_name,
+			parents, i,
+			mux_hw, &clk_mux_ops,
+			&factors->hw, &clk_factors_ops,
+			gate_hw, &clk_gate_ops,
+			i ? 0 : CLK_IS_ROOT);
 
 	if (!IS_ERR(clk)) {
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);