[PATCH 0/3] clk: at91: various fixes and improvements

[PATCH 0/3] clk: at91: various fixes and improvements

[Boris BREZILLON]

Hello Mike,

This series fixes a bug in the prog clk prepare function (the platform hangs
when preparing a prog clk).

It also implements the determine_rate callback for these prog clks and allow
system clk to propagate the rate change to its parent.

These modifications are needed to get the atmel_wm8904 driver working (this
driver make use of prog clks), and if possible, should be merged in the next
3.14 release (at least the first patch of this series).

Let me know if this is not possible.

Thanks.

Best Regards,

Boris

Boris BREZILLON (3):
  clk: at91: fix programmable clk irq handling
  clk: at91: replace prog clk round_rate with determine_rate
  clk: at91: propagate rate change on system clks

 drivers/clk/at91/clk-programmable.c |   61 ++++++++++++++++++-----------------
 drivers/clk/at91/clk-system.c       |    3 +-
 2 files changed, 34 insertions(+), 30 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] clk: at91: fix programmable clk irq handling

[Boris BREZILLON]

The prog irq is a level irq reflecting the prog clk status. As a result the
irq line will stay high when the prog clk is ready and the system will
hang.
Disable the irq when it is handled to avoid this problem.

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
---
 drivers/clk/at91/clk-programmable.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/at91/clk-programmable.c b/drivers/clk/at91/clk-programmable.c
index fd792b2..02f62a0 100644
--- a/drivers/clk/at91/clk-programmable.c
+++ b/drivers/clk/at91/clk-programmable.c
@@ -55,6 +55,7 @@ static irqreturn_t clk_programmable_irq_handler(int irq, void *dev_id)
 	struct clk_programmable *prog = (struct clk_programmable *)dev_id;
 
 	wake_up(&prog->wait);
+	disable_irq_nosync(prog->irq);
 
 	return IRQ_HANDLED;
 }
@@ -74,8 +75,10 @@ static int clk_programmable_prepare(struct clk_hw *hw)
 
 	pmc_write(pmc, AT91_PMC_PCKR(id), tmp);
 
-	while (!(pmc_read(pmc, AT91_PMC_SR) & mask))
+	while (!(pmc_read(pmc, AT91_PMC_SR) & mask)) {
+		enable_irq(prog->irq);
 		wait_event(prog->wait, pmc_read(pmc, AT91_PMC_SR) & mask);
+	}
 
 	return 0;
 }
-- 
1.7.9.5

[PATCH 2/3] clk: at91: replace prog clk round_rate with determine_rate

[Boris BREZILLON]

Implement the determine_rate callback to choose the best parent clk that
fulfills the requested rate.

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
---
 drivers/clk/at91/clk-programmable.c |   56 +++++++++++++++++------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/clk/at91/clk-programmable.c b/drivers/clk/at91/clk-programmable.c
index 02f62a0..8e242c7 100644
--- a/drivers/clk/at91/clk-programmable.c
+++ b/drivers/clk/at91/clk-programmable.c
@@ -105,40 +105,40 @@ static unsigned long clk_programmable_recalc_rate(struct clk_hw *hw,
 	return parent_rate >> prog->pres;
 }
 
-static long clk_programmable_round_rate(struct clk_hw *hw, unsigned long rate,
-					unsigned long *parent_rate)
+static long clk_programmable_determine_rate(struct clk_hw *hw,
+					    unsigned long rate,
+					    unsigned long *best_parent_rate,
+					    struct clk **best_parent_clk)
 {
-	unsigned long best_rate = *parent_rate;
-	unsigned long best_diff;
-	unsigned long new_diff;
-	unsigned long cur_rate;
-	int shift = shift;
-
-	if (rate > *parent_rate)
-		return *parent_rate;
-	else
-		best_diff = *parent_rate - rate;
-
-	if (!best_diff)
-		return best_rate;
+	struct clk *parent = NULL;
+	long best_rate = -EINVAL;
+	unsigned long parent_rate;
+	unsigned long tmp_rate;
+	int shift;
+	int i;
 
-	for (shift = 1; shift < PROG_PRES_MASK; shift++) {
-		cur_rate = *parent_rate >> shift;
+	for (i = 0; i < __clk_get_num_parents(hw->clk); i++) {
+		parent = clk_get_parent_by_index(hw->clk, i);
+		if (!parent)
+			continue;
 
-		if (cur_rate > rate)
-			new_diff = cur_rate - rate;
-		else
-			new_diff = rate - cur_rate;
+		parent_rate = __clk_get_rate(parent);
+		for (shift = 0; shift < PROG_PRES_MASK; shift++) {
+			tmp_rate = parent_rate >> shift;
+			if (tmp_rate <= rate)
+				break;
+		}
 
-		if (!new_diff)
-			return cur_rate;
+		if (tmp_rate > rate)
+			continue;
 
-		if (new_diff < best_diff) {
-			best_diff = new_diff;
-			best_rate = cur_rate;
+		if (best_rate < 0 || (rate - tmp_rate) < (rate - best_rate)) {
+			best_rate = tmp_rate;
+			*best_parent_rate = parent_rate;
+			*best_parent_clk = parent;
 		}
 
-		if (rate > cur_rate)
+		if (!best_rate)
 			break;
 	}
 
@@ -231,7 +231,7 @@ static const struct clk_ops programmable_ops = {
 	.prepare = clk_programmable_prepare,
 	.is_prepared = clk_programmable_is_ready,
 	.recalc_rate = clk_programmable_recalc_rate,
-	.round_rate = clk_programmable_round_rate,
+	.determine_rate = clk_programmable_determine_rate,
 	.get_parent = clk_programmable_get_parent,
 	.set_parent = clk_programmable_set_parent,
 	.set_rate = clk_programmable_set_rate,
-- 
1.7.9.5

[PATCH 3/3] clk: at91: propagate rate change on system clks

[Boris BREZILLON]

System clks are just gates, and thus do not provide any rate operations.
Authorize clk rate change to be propagated to system clk parents.

Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>
---
 drivers/clk/at91/clk-system.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/at91/clk-system.c b/drivers/clk/at91/clk-system.c
index 8f7c043..a98557b 100644
--- a/drivers/clk/at91/clk-system.c
+++ b/drivers/clk/at91/clk-system.c
@@ -84,7 +84,8 @@ at91_clk_register_system(struct at91_pmc *pmc, const char *name,
 	 * (see drivers/memory) which would request and enable the ddrck clock.
 	 * When this is done we will be able to remove CLK_IGNORE_UNUSED flag.
 	 */
-	init.flags = CLK_IGNORE_UNUSED;
+	init.flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT |
+		     CLK_IGNORE_UNUSED;
 
 	sys->id = id;
 	sys->hw.init = &init;
-- 
1.7.9.5

Re: [PATCH 1/3] clk: at91: fix programmable clk irq handling

[Nicolas Ferre]

On 03/02/2014 12:25, Boris BREZILLON :
> The prog irq is a level irq reflecting the prog clk status. As a result the
> irq line will stay high when the prog clk is ready and the system will
> hang.
> Disable the irq when it is handled to avoid this problem.
> 
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  drivers/clk/at91/clk-programmable.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/at91/clk-programmable.c b/drivers/clk/at91/clk-programmable.c
> index fd792b2..02f62a0 100644
> --- a/drivers/clk/at91/clk-programmable.c
> +++ b/drivers/clk/at91/clk-programmable.c
> @@ -55,6 +55,7 @@ static irqreturn_t clk_programmable_irq_handler(int irq, void *dev_id)
>  	struct clk_programmable *prog = (struct clk_programmable *)dev_id;
>  
>  	wake_up(&prog->wait);
> +	disable_irq_nosync(prog->irq);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -74,8 +75,10 @@ static int clk_programmable_prepare(struct clk_hw *hw)
>  
>  	pmc_write(pmc, AT91_PMC_PCKR(id), tmp);
>  
> -	while (!(pmc_read(pmc, AT91_PMC_SR) & mask))
> +	while (!(pmc_read(pmc, AT91_PMC_SR) & mask)) {
> +		enable_irq(prog->irq);
>  		wait_event(prog->wait, pmc_read(pmc, AT91_PMC_SR) & mask);
> +	}
>  
>  	return 0;
>  }
> 


-- 
Nicolas Ferre

Re: [PATCH 2/3] clk: at91: replace prog clk round_rate with determine_rate

[Nicolas Ferre]

On 03/02/2014 12:25, Boris BREZILLON :
> Implement the determine_rate callback to choose the best parent clk that
> fulfills the requested rate.
> 
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Nice feature, thanks!

> ---
>  drivers/clk/at91/clk-programmable.c |   56 +++++++++++++++++------------------
>  1 file changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/clk/at91/clk-programmable.c b/drivers/clk/at91/clk-programmable.c
> index 02f62a0..8e242c7 100644
> --- a/drivers/clk/at91/clk-programmable.c
> +++ b/drivers/clk/at91/clk-programmable.c
> @@ -105,40 +105,40 @@ static unsigned long clk_programmable_recalc_rate(struct clk_hw *hw,
>  	return parent_rate >> prog->pres;
>  }
>  
> -static long clk_programmable_round_rate(struct clk_hw *hw, unsigned long rate,
> -					unsigned long *parent_rate)
> +static long clk_programmable_determine_rate(struct clk_hw *hw,
> +					    unsigned long rate,
> +					    unsigned long *best_parent_rate,
> +					    struct clk **best_parent_clk)
>  {
> -	unsigned long best_rate = *parent_rate;
> -	unsigned long best_diff;
> -	unsigned long new_diff;
> -	unsigned long cur_rate;
> -	int shift = shift;
> -
> -	if (rate > *parent_rate)
> -		return *parent_rate;
> -	else
> -		best_diff = *parent_rate - rate;
> -
> -	if (!best_diff)
> -		return best_rate;
> +	struct clk *parent = NULL;
> +	long best_rate = -EINVAL;
> +	unsigned long parent_rate;
> +	unsigned long tmp_rate;
> +	int shift;
> +	int i;
>  
> -	for (shift = 1; shift < PROG_PRES_MASK; shift++) {
> -		cur_rate = *parent_rate >> shift;
> +	for (i = 0; i < __clk_get_num_parents(hw->clk); i++) {
> +		parent = clk_get_parent_by_index(hw->clk, i);
> +		if (!parent)
> +			continue;
>  
> -		if (cur_rate > rate)
> -			new_diff = cur_rate - rate;
> -		else
> -			new_diff = rate - cur_rate;
> +		parent_rate = __clk_get_rate(parent);
> +		for (shift = 0; shift < PROG_PRES_MASK; shift++) {
> +			tmp_rate = parent_rate >> shift;
> +			if (tmp_rate <= rate)
> +				break;
> +		}
>  
> -		if (!new_diff)
> -			return cur_rate;
> +		if (tmp_rate > rate)
> +			continue;
>  
> -		if (new_diff < best_diff) {
> -			best_diff = new_diff;
> -			best_rate = cur_rate;
> +		if (best_rate < 0 || (rate - tmp_rate) < (rate - best_rate)) {
> +			best_rate = tmp_rate;
> +			*best_parent_rate = parent_rate;
> +			*best_parent_clk = parent;
>  		}
>  
> -		if (rate > cur_rate)
> +		if (!best_rate)
>  			break;
>  	}
>  
> @@ -231,7 +231,7 @@ static const struct clk_ops programmable_ops = {
>  	.prepare = clk_programmable_prepare,
>  	.is_prepared = clk_programmable_is_ready,
>  	.recalc_rate = clk_programmable_recalc_rate,
> -	.round_rate = clk_programmable_round_rate,
> +	.determine_rate = clk_programmable_determine_rate,
>  	.get_parent = clk_programmable_get_parent,
>  	.set_parent = clk_programmable_set_parent,
>  	.set_rate = clk_programmable_set_rate,
> 


-- 
Nicolas Ferre

Re: [PATCH 3/3] clk: at91: propagate rate change on system clks

[Nicolas Ferre]

On 03/02/2014 12:25, Boris BREZILLON :
> System clks are just gates, and thus do not provide any rate operations.
> Authorize clk rate change to be propagated to system clk parents.
> 
> Signed-off-by: Boris BREZILLON <b.brezillon@overkiz.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  drivers/clk/at91/clk-system.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/at91/clk-system.c b/drivers/clk/at91/clk-system.c
> index 8f7c043..a98557b 100644
> --- a/drivers/clk/at91/clk-system.c
> +++ b/drivers/clk/at91/clk-system.c
> @@ -84,7 +84,8 @@ at91_clk_register_system(struct at91_pmc *pmc, const char *name,
>  	 * (see drivers/memory) which would request and enable the ddrck clock.
>  	 * When this is done we will be able to remove CLK_IGNORE_UNUSED flag.
>  	 */
> -	init.flags = CLK_IGNORE_UNUSED;
> +	init.flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT |
> +		     CLK_IGNORE_UNUSED;
>  
>  	sys->id = id;
>  	sys->hw.init = &init;
> 


-- 
Nicolas Ferre

Re: [PATCH 0/3] clk: at91: various fixes and improvements

[Boris BREZILLON]

Hello, Mike,

Please do not take this series: the prog clk driver is still buggy (this was
reported by Jean-Jacques Hiblot).

Jean-Jacques is preparing a new series to fix these bugs, and I'll rebase
patch 2 and 3 of this series on top of his work.

Sorry for the inconvenience.

Best Regards,

Boris

On 03/02/2014 12:25, Boris BREZILLON wrote:
> Hello Mike,
>
> This series fixes a bug in the prog clk prepare function (the platform hangs
> when preparing a prog clk).
>
> It also implements the determine_rate callback for these prog clks and allow
> system clk to propagate the rate change to its parent.
>
> These modifications are needed to get the atmel_wm8904 driver working (this
> driver make use of prog clks), and if possible, should be merged in the next
> 3.14 release (at least the first patch of this series).
>
> Let me know if this is not possible.
>
> Thanks.
>
> Best Regards,
>
> Boris
>
> Boris BREZILLON (3):
>    clk: at91: fix programmable clk irq handling
>    clk: at91: replace prog clk round_rate with determine_rate
>    clk: at91: propagate rate change on system clks
>
>   drivers/clk/at91/clk-programmable.c |   61 ++++++++++++++++++-----------------
>   drivers/clk/at91/clk-system.c       |    3 +-
>   2 files changed, 34 insertions(+), 30 deletions(-)
>