2014-02-03 11:25:43

by Boris BREZILLON

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

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


2014-02-03 11:25:45

by Boris BREZILLON

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

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 <[email protected]>
---
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

2014-02-03 11:25:42

by Boris BREZILLON

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

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 <[email protected]>
---
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

2014-02-03 11:26:26

by Boris BREZILLON

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

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

Signed-off-by: Boris BREZILLON <[email protected]>
---
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

2014-02-04 09:04:30

by Nicolas Ferre

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

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 <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>

> ---
> 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

2014-02-04 09:05:17

by Nicolas Ferre

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

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 <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>

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

2014-02-04 09:05:41

by Nicolas Ferre

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

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 <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>

> ---
> 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

2014-02-04 13:58:58

by Boris BREZILLON

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

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(-)
>