* [PATCH] clk: at91: pll: fix input range validity check
@ 2015-03-29 1:53 ` Boris Brezillon
0 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2015-03-29 1:53 UTC (permalink / raw)
To: Mike Turquette, Nicolas Ferre, Jean-Christophe Plagniol-Villard,
Alexandre Belloni
Cc: Jonas Andersson, linux-kernel, linux-arm-kernel, Boris Brezillon, stable
The PLL impose a certain input range to work correctly, but it appears that
this input range does not apply on the input clock (or parent clock) but
on the input clock after it has passed the PLL divisor.
Fix the implementation accordingly.
Cc: <stable@vger.kernel.org> # v3.14+
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Reported-by: Jonas Andersson <jonas@microbit.se>
---
drivers/clk/at91/clk-pll.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/at91/clk-pll.c b/drivers/clk/at91/clk-pll.c
index 6ec79db..cbbe403 100644
--- a/drivers/clk/at91/clk-pll.c
+++ b/drivers/clk/at91/clk-pll.c
@@ -173,8 +173,7 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
int i = 0;
/* Check if parent_rate is a valid input rate */
- if (parent_rate < characteristics->input.min ||
- parent_rate > characteristics->input.max)
+ if (parent_rate < characteristics->input.min)
return -ERANGE;
/*
@@ -187,6 +186,15 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
if (!mindiv)
mindiv = 1;
+ if (parent_rate > characteristics->input.max) {
+ tmpdiv = DIV_ROUND_UP(parent_rate, characteristics->input.max);
+ if (tmpdiv > PLL_DIV_MAX)
+ return -ERANGE;
+
+ if (tmpdiv > mindiv)
+ mindiv = tmpdiv;
+ }
+
/*
* Calculate the maximum divider which is limited by PLL register
* layout (limited by the MUL or DIV field size).
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] clk: at91: pll: fix input range validity check
@ 2015-03-29 1:53 ` Boris Brezillon
0 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2015-03-29 1:53 UTC (permalink / raw)
To: linux-arm-kernel
The PLL impose a certain input range to work correctly, but it appears that
this input range does not apply on the input clock (or parent clock) but
on the input clock after it has passed the PLL divisor.
Fix the implementation accordingly.
Cc: <stable@vger.kernel.org> # v3.14+
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Reported-by: Jonas Andersson <jonas@microbit.se>
---
drivers/clk/at91/clk-pll.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/at91/clk-pll.c b/drivers/clk/at91/clk-pll.c
index 6ec79db..cbbe403 100644
--- a/drivers/clk/at91/clk-pll.c
+++ b/drivers/clk/at91/clk-pll.c
@@ -173,8 +173,7 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
int i = 0;
/* Check if parent_rate is a valid input rate */
- if (parent_rate < characteristics->input.min ||
- parent_rate > characteristics->input.max)
+ if (parent_rate < characteristics->input.min)
return -ERANGE;
/*
@@ -187,6 +186,15 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
if (!mindiv)
mindiv = 1;
+ if (parent_rate > characteristics->input.max) {
+ tmpdiv = DIV_ROUND_UP(parent_rate, characteristics->input.max);
+ if (tmpdiv > PLL_DIV_MAX)
+ return -ERANGE;
+
+ if (tmpdiv > mindiv)
+ mindiv = tmpdiv;
+ }
+
/*
* Calculate the maximum divider which is limited by PLL register
* layout (limited by the MUL or DIV field size).
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: at91: pll: fix input range validity check
2015-03-29 1:53 ` Boris Brezillon
@ 2015-04-13 4:37 ` Michael Turquette
-1 siblings, 0 replies; 8+ messages in thread
From: Michael Turquette @ 2015-04-13 4:37 UTC (permalink / raw)
To: Boris Brezillon, Nicolas Ferre, Jean-Christophe Plagniol-Villard,
Alexandre Belloni
Cc: Jonas Andersson, linux-kernel, linux-arm-kernel, Boris Brezillon, stable
Quoting Boris Brezillon (2015-03-28 18:53:43)
> The PLL impose a certain input range to work correctly, but it appears that
> this input range does not apply on the input clock (or parent clock) but
> on the input clock after it has passed the PLL divisor.
> Fix the implementation accordingly.
>
> Cc: <stable@vger.kernel.org> # v3.14+
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Reported-by: Jonas Andersson <jonas@microbit.se>
Hi Boris,
OK, so this patch along with your two previous submissions kind of
tackle some of items I mentioned earlier today[0].
Does this patch, combined with your two prior patches[1][2] resolve the
issue you brought up in your "Propagating clock rate constraints"
thread[3]?
[0] http://lkml.kernel.org/r/<20150412235021.19585.27431@quantum>
[1] http://lkml.kernel.org/r/<1427593728-9366-1-git-send-email-boris.brezillon@free-electrons.com>
[2] http://lkml.kernel.org/r/<1427593533-9019-1-git-send-email-boris.brezillon@free-electrons.com>
[3] http://lkml.kernel.org/r/<20150327004054.2f6f34ee@bbrezillon>
Regards,
Mike
> ---
> drivers/clk/at91/clk-pll.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/at91/clk-pll.c b/drivers/clk/at91/clk-pll.c
> index 6ec79db..cbbe403 100644
> --- a/drivers/clk/at91/clk-pll.c
> +++ b/drivers/clk/at91/clk-pll.c
> @@ -173,8 +173,7 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
> int i = 0;
>
> /* Check if parent_rate is a valid input rate */
> - if (parent_rate < characteristics->input.min ||
> - parent_rate > characteristics->input.max)
> + if (parent_rate < characteristics->input.min)
> return -ERANGE;
>
> /*
> @@ -187,6 +186,15 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
> if (!mindiv)
> mindiv = 1;
>
> + if (parent_rate > characteristics->input.max) {
> + tmpdiv = DIV_ROUND_UP(parent_rate, characteristics->input.max);
> + if (tmpdiv > PLL_DIV_MAX)
> + return -ERANGE;
> +
> + if (tmpdiv > mindiv)
> + mindiv = tmpdiv;
> + }
> +
> /*
> * Calculate the maximum divider which is limited by PLL register
> * layout (limited by the MUL or DIV field size).
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] clk: at91: pll: fix input range validity check
@ 2015-04-13 4:37 ` Michael Turquette
0 siblings, 0 replies; 8+ messages in thread
From: Michael Turquette @ 2015-04-13 4:37 UTC (permalink / raw)
To: linux-arm-kernel
Quoting Boris Brezillon (2015-03-28 18:53:43)
> The PLL impose a certain input range to work correctly, but it appears that
> this input range does not apply on the input clock (or parent clock) but
> on the input clock after it has passed the PLL divisor.
> Fix the implementation accordingly.
>
> Cc: <stable@vger.kernel.org> # v3.14+
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Reported-by: Jonas Andersson <jonas@microbit.se>
Hi Boris,
OK, so this patch along with your two previous submissions kind of
tackle some of items I mentioned earlier today[0].
Does this patch, combined with your two prior patches[1][2] resolve the
issue you brought up in your "Propagating clock rate constraints"
thread[3]?
[0] http://lkml.kernel.org/r/<20150412235021.19585.27431@quantum>
[1] http://lkml.kernel.org/r/<1427593728-9366-1-git-send-email-boris.brezillon@free-electrons.com>
[2] http://lkml.kernel.org/r/<1427593533-9019-1-git-send-email-boris.brezillon@free-electrons.com>
[3] http://lkml.kernel.org/r/<20150327004054.2f6f34ee@bbrezillon>
Regards,
Mike
> ---
> drivers/clk/at91/clk-pll.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/at91/clk-pll.c b/drivers/clk/at91/clk-pll.c
> index 6ec79db..cbbe403 100644
> --- a/drivers/clk/at91/clk-pll.c
> +++ b/drivers/clk/at91/clk-pll.c
> @@ -173,8 +173,7 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
> int i = 0;
>
> /* Check if parent_rate is a valid input rate */
> - if (parent_rate < characteristics->input.min ||
> - parent_rate > characteristics->input.max)
> + if (parent_rate < characteristics->input.min)
> return -ERANGE;
>
> /*
> @@ -187,6 +186,15 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
> if (!mindiv)
> mindiv = 1;
>
> + if (parent_rate > characteristics->input.max) {
> + tmpdiv = DIV_ROUND_UP(parent_rate, characteristics->input.max);
> + if (tmpdiv > PLL_DIV_MAX)
> + return -ERANGE;
> +
> + if (tmpdiv > mindiv)
> + mindiv = tmpdiv;
> + }
> +
> /*
> * Calculate the maximum divider which is limited by PLL register
> * layout (limited by the MUL or DIV field size).
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: at91: pll: fix input range validity check
2015-04-13 4:37 ` Michael Turquette
@ 2015-04-14 17:48 ` Boris Brezillon
-1 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2015-04-14 17:48 UTC (permalink / raw)
To: Michael Turquette
Cc: Nicolas Ferre, Jean-Christophe Plagniol-Villard,
Alexandre Belloni, Jonas Andersson, linux-kernel,
linux-arm-kernel, stable
Hi Mike,
On Sun, 12 Apr 2015 21:37:25 -0700
Michael Turquette <mturquette@linaro.org> wrote:
> Quoting Boris Brezillon (2015-03-28 18:53:43)
> > The PLL impose a certain input range to work correctly, but it appears that
> > this input range does not apply on the input clock (or parent clock) but
> > on the input clock after it has passed the PLL divisor.
> > Fix the implementation accordingly.
> >
> > Cc: <stable@vger.kernel.org> # v3.14+
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Reported-by: Jonas Andersson <jonas@microbit.se>
>
> Hi Boris,
>
> OK, so this patch along with your two previous submissions kind of
> tackle some of items I mentioned earlier today[0].
>
> Does this patch, combined with your two prior patches[1][2] resolve the
> issue you brought up in your "Propagating clock rate constraints"
> thread[3]?
Unfortunately it doesn't (though it does resolve one of my
issues, so I definitely need that patch :-)).
Take the following case:
1/ clock X takes clock Y as its parent (let's say clock X is a clock
divider)
2/ user U claims clock X and configure X's rate (X then propagates
rate change to Y) and assign a specific supported rate range to X
2/ user V claims clock Y and sets a specific rate
As of today, the constraint U has set on clock X is not propagated to
clock Y, which means user V might configure a rate that is not
fulfilling users V constraint, and the clk infrastructure won't
complain (actually it won't detect it).
Here's what I would expect: if a (MIN -> MAX) constraint is set on clock
X the (MIN * XDIV -> MAX * XDIV) constraint should be propagated to
clock Y.
Am I wrong ?
Best Regards,
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] clk: at91: pll: fix input range validity check
@ 2015-04-14 17:48 ` Boris Brezillon
0 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2015-04-14 17:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mike,
On Sun, 12 Apr 2015 21:37:25 -0700
Michael Turquette <mturquette@linaro.org> wrote:
> Quoting Boris Brezillon (2015-03-28 18:53:43)
> > The PLL impose a certain input range to work correctly, but it appears that
> > this input range does not apply on the input clock (or parent clock) but
> > on the input clock after it has passed the PLL divisor.
> > Fix the implementation accordingly.
> >
> > Cc: <stable@vger.kernel.org> # v3.14+
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Reported-by: Jonas Andersson <jonas@microbit.se>
>
> Hi Boris,
>
> OK, so this patch along with your two previous submissions kind of
> tackle some of items I mentioned earlier today[0].
>
> Does this patch, combined with your two prior patches[1][2] resolve the
> issue you brought up in your "Propagating clock rate constraints"
> thread[3]?
Unfortunately it doesn't (though it does resolve one of my
issues, so I definitely need that patch :-)).
Take the following case:
1/ clock X takes clock Y as its parent (let's say clock X is a clock
divider)
2/ user U claims clock X and configure X's rate (X then propagates
rate change to Y) and assign a specific supported rate range to X
2/ user V claims clock Y and sets a specific rate
As of today, the constraint U has set on clock X is not propagated to
clock Y, which means user V might configure a rate that is not
fulfilling users V constraint, and the clk infrastructure won't
complain (actually it won't detect it).
Here's what I would expect: if a (MIN -> MAX) constraint is set on clock
X the (MIN * XDIV -> MAX * XDIV) constraint should be propagated to
clock Y.
Am I wrong ?
Best Regards,
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] clk: at91: pll: fix input range validity check
2015-03-29 1:53 ` Boris Brezillon
@ 2015-06-18 10:59 ` Boris Brezillon
-1 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2015-06-18 10:59 UTC (permalink / raw)
To: Boris Brezillon
Cc: Mike Turquette, Nicolas Ferre, Jean-Christophe Plagniol-Villard,
Alexandre Belloni, Jonas Andersson, linux-kernel,
linux-arm-kernel, stable, Stephen Boyd
On Sun, 29 Mar 2015 03:53:43 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> The PLL impose a certain input range to work correctly, but it appears that
> this input range does not apply on the input clock (or parent clock) but
> on the input clock after it has passed the PLL divisor.
> Fix the implementation accordingly.
Ping (I was expecting this patch to be part of 4.1 :-/).
>
> Cc: <stable@vger.kernel.org> # v3.14+
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Reported-by: Jonas Andersson <jonas@microbit.se>
> ---
> drivers/clk/at91/clk-pll.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/at91/clk-pll.c b/drivers/clk/at91/clk-pll.c
> index 6ec79db..cbbe403 100644
> --- a/drivers/clk/at91/clk-pll.c
> +++ b/drivers/clk/at91/clk-pll.c
> @@ -173,8 +173,7 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
> int i = 0;
>
> /* Check if parent_rate is a valid input rate */
> - if (parent_rate < characteristics->input.min ||
> - parent_rate > characteristics->input.max)
> + if (parent_rate < characteristics->input.min)
> return -ERANGE;
>
> /*
> @@ -187,6 +186,15 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
> if (!mindiv)
> mindiv = 1;
>
> + if (parent_rate > characteristics->input.max) {
> + tmpdiv = DIV_ROUND_UP(parent_rate, characteristics->input.max);
> + if (tmpdiv > PLL_DIV_MAX)
> + return -ERANGE;
> +
> + if (tmpdiv > mindiv)
> + mindiv = tmpdiv;
> + }
> +
> /*
> * Calculate the maximum divider which is limited by PLL register
> * layout (limited by the MUL or DIV field size).
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] clk: at91: pll: fix input range validity check
@ 2015-06-18 10:59 ` Boris Brezillon
0 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2015-06-18 10:59 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, 29 Mar 2015 03:53:43 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> The PLL impose a certain input range to work correctly, but it appears that
> this input range does not apply on the input clock (or parent clock) but
> on the input clock after it has passed the PLL divisor.
> Fix the implementation accordingly.
Ping (I was expecting this patch to be part of 4.1 :-/).
>
> Cc: <stable@vger.kernel.org> # v3.14+
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Reported-by: Jonas Andersson <jonas@microbit.se>
> ---
> drivers/clk/at91/clk-pll.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/at91/clk-pll.c b/drivers/clk/at91/clk-pll.c
> index 6ec79db..cbbe403 100644
> --- a/drivers/clk/at91/clk-pll.c
> +++ b/drivers/clk/at91/clk-pll.c
> @@ -173,8 +173,7 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
> int i = 0;
>
> /* Check if parent_rate is a valid input rate */
> - if (parent_rate < characteristics->input.min ||
> - parent_rate > characteristics->input.max)
> + if (parent_rate < characteristics->input.min)
> return -ERANGE;
>
> /*
> @@ -187,6 +186,15 @@ static long clk_pll_get_best_div_mul(struct clk_pll *pll, unsigned long rate,
> if (!mindiv)
> mindiv = 1;
>
> + if (parent_rate > characteristics->input.max) {
> + tmpdiv = DIV_ROUND_UP(parent_rate, characteristics->input.max);
> + if (tmpdiv > PLL_DIV_MAX)
> + return -ERANGE;
> +
> + if (tmpdiv > mindiv)
> + mindiv = tmpdiv;
> + }
> +
> /*
> * Calculate the maximum divider which is limited by PLL register
> * layout (limited by the MUL or DIV field size).
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-06-18 10:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-29 1:53 [PATCH] clk: at91: pll: fix input range validity check Boris Brezillon
2015-03-29 1:53 ` Boris Brezillon
2015-04-13 4:37 ` Michael Turquette
2015-04-13 4:37 ` Michael Turquette
2015-04-14 17:48 ` Boris Brezillon
2015-04-14 17:48 ` Boris Brezillon
2015-06-18 10:59 ` Boris Brezillon
2015-06-18 10:59 ` Boris Brezillon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.