* [PATCH v2] clk: at91: fix programmable clock for sama5d2
@ 2019-03-18 10:50 Nicolas Ferre
2019-03-18 19:54 ` Stephen Boyd
2019-03-19 19:44 ` Stephen Boyd
0 siblings, 2 replies; 5+ messages in thread
From: Nicolas Ferre @ 2019-03-18 10:50 UTC (permalink / raw)
To: sboyd, linux-clk
Cc: Alexandre Belloni, matthias.wieloch, mturquette, linux-kernel,
stable, Ludovic Desroches, Claudiu Beznea, linux-arm-kernel
From: Matthias Wieloch <matthias.wieloch@few-bauer.de>
The prescaler formula of the programmable clock has changed for sama5d2. Update
the driver accordingly.
Fixes: a2038077de9a ("clk: at91: add sama5d2 PMC driver")
Cc: <stable@vger.kernel.org> # v4.20+
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
[nicolas.ferre@microchip.com: adapt the prescaler range,
fix clk_programmable_recalc_rate, split patch]
Signed-off-by: Matthias Wieloch <matthias.wieloch@few-bauer.de>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
v2: adapt to v5.1-rc1
remove unneeded sentence about DT in commit message
Stephen,
I think it would be good to see this fix going upstream during v5.1-rc phase.
Best regards,
Nicolas
drivers/clk/at91/clk-programmable.c | 57 ++++++++++++++++++++++-------
drivers/clk/at91/pmc.h | 2 +
drivers/clk/at91/sama5d2.c | 10 ++++-
3 files changed, 54 insertions(+), 15 deletions(-)
diff --git a/drivers/clk/at91/clk-programmable.c b/drivers/clk/at91/clk-programmable.c
index 89d6f3736dbf..f8edbb65eda3 100644
--- a/drivers/clk/at91/clk-programmable.c
+++ b/drivers/clk/at91/clk-programmable.c
@@ -20,8 +20,7 @@
#define PROG_ID_MAX 7
#define PROG_STATUS_MASK(id) (1 << ((id) + 8))
-#define PROG_PRES_MASK 0x7
-#define PROG_PRES(layout, pckr) ((pckr >> layout->pres_shift) & PROG_PRES_MASK)
+#define PROG_PRES(layout, pckr) ((pckr >> layout->pres_shift) & layout->pres_mask)
#define PROG_MAX_RM9200_CSS 3
struct clk_programmable {
@@ -37,20 +36,29 @@ static unsigned long clk_programmable_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
struct clk_programmable *prog = to_clk_programmable(hw);
+ const struct clk_programmable_layout *layout = prog->layout;
unsigned int pckr;
+ unsigned long rate;
regmap_read(prog->regmap, AT91_PMC_PCKR(prog->id), &pckr);
- return parent_rate >> PROG_PRES(prog->layout, pckr);
+ if (layout->is_pres_direct)
+ rate = parent_rate / (PROG_PRES(layout, pckr) + 1);
+ else
+ rate = parent_rate >> PROG_PRES(layout, pckr);
+
+ return rate;
}
static int clk_programmable_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
+ struct clk_programmable *prog = to_clk_programmable(hw);
+ const struct clk_programmable_layout *layout = prog->layout;
struct clk_hw *parent;
long best_rate = -EINVAL;
unsigned long parent_rate;
- unsigned long tmp_rate;
+ unsigned long tmp_rate = 0;
int shift;
int i;
@@ -60,10 +68,18 @@ static int clk_programmable_determine_rate(struct clk_hw *hw,
continue;
parent_rate = clk_hw_get_rate(parent);
- for (shift = 0; shift < PROG_PRES_MASK; shift++) {
- tmp_rate = parent_rate >> shift;
- if (tmp_rate <= req->rate)
- break;
+ if (layout->is_pres_direct) {
+ for (shift = 0; shift <= layout->pres_mask; shift++) {
+ tmp_rate = parent_rate / (shift + 1);
+ if (tmp_rate <= req->rate)
+ break;
+ }
+ } else {
+ for (shift = 0; shift < layout->pres_mask; shift++) {
+ tmp_rate = parent_rate >> shift;
+ if (tmp_rate <= req->rate)
+ break;
+ }
}
if (tmp_rate > req->rate)
@@ -137,16 +153,23 @@ static int clk_programmable_set_rate(struct clk_hw *hw, unsigned long rate,
if (!div)
return -EINVAL;
- shift = fls(div) - 1;
+ if (layout->is_pres_direct) {
+ shift = div - 1;
- if (div != (1 << shift))
- return -EINVAL;
+ if (shift > layout->pres_mask)
+ return -EINVAL;
+ } else {
+ shift = fls(div) - 1;
- if (shift >= PROG_PRES_MASK)
- return -EINVAL;
+ if (div != (1 << shift))
+ return -EINVAL;
+
+ if (shift >= layout->pres_mask)
+ return -EINVAL;
+ }
regmap_update_bits(prog->regmap, AT91_PMC_PCKR(prog->id),
- PROG_PRES_MASK << layout->pres_shift,
+ layout->pres_mask << layout->pres_shift,
shift << layout->pres_shift);
return 0;
@@ -202,19 +225,25 @@ at91_clk_register_programmable(struct regmap *regmap,
}
const struct clk_programmable_layout at91rm9200_programmable_layout = {
+ .pres_mask = 0x7,
.pres_shift = 2,
.css_mask = 0x3,
.have_slck_mck = 0,
+ .is_pres_direct = 0,
};
const struct clk_programmable_layout at91sam9g45_programmable_layout = {
+ .pres_mask = 0x7,
.pres_shift = 2,
.css_mask = 0x3,
.have_slck_mck = 1,
+ .is_pres_direct = 0,
};
const struct clk_programmable_layout at91sam9x5_programmable_layout = {
+ .pres_mask = 0x7,
.pres_shift = 4,
.css_mask = 0x7,
.have_slck_mck = 0,
+ .is_pres_direct = 0,
};
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 672a79bda88c..a0e5ce9c9b9e 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -71,9 +71,11 @@ struct clk_pll_characteristics {
};
struct clk_programmable_layout {
+ u8 pres_mask;
u8 pres_shift;
u8 css_mask;
u8 have_slck_mck;
+ u8 is_pres_direct;
};
extern const struct clk_programmable_layout at91rm9200_programmable_layout;
diff --git a/drivers/clk/at91/sama5d2.c b/drivers/clk/at91/sama5d2.c
index 1f70cb164b06..81943fac4537 100644
--- a/drivers/clk/at91/sama5d2.c
+++ b/drivers/clk/at91/sama5d2.c
@@ -125,6 +125,14 @@ static const struct {
.pll = true },
};
+static const struct clk_programmable_layout sama5d2_programmable_layout = {
+ .pres_mask = 0xff,
+ .pres_shift = 4,
+ .css_mask = 0x7,
+ .have_slck_mck = 0,
+ .is_pres_direct = 1,
+};
+
static void __init sama5d2_pmc_setup(struct device_node *np)
{
struct clk_range range = CLK_RANGE(0, 0);
@@ -249,7 +257,7 @@ static void __init sama5d2_pmc_setup(struct device_node *np)
hw = at91_clk_register_programmable(regmap, name,
parent_names, 6, i,
- &at91sam9x5_programmable_layout);
+ &sama5d2_programmable_layout);
if (IS_ERR(hw))
goto err_free;
}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] clk: at91: fix programmable clock for sama5d2
2019-03-18 10:50 [PATCH v2] clk: at91: fix programmable clock for sama5d2 Nicolas Ferre
@ 2019-03-18 19:54 ` Stephen Boyd
2019-03-19 8:28 ` Nicolas.Ferre
2019-03-19 19:44 ` Stephen Boyd
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Boyd @ 2019-03-18 19:54 UTC (permalink / raw)
To: Nicolas Ferre, linux-clk
Cc: Alexandre Belloni, matthias.wieloch, mturquette, linux-kernel,
stable, Ludovic Desroches, Claudiu Beznea, linux-arm-kernel
Quoting Nicolas Ferre (2019-03-18 03:50:45)
> From: Matthias Wieloch <matthias.wieloch@few-bauer.de>
>
> The prescaler formula of the programmable clock has changed for sama5d2. Update
> the driver accordingly.
>
> Fixes: a2038077de9a ("clk: at91: add sama5d2 PMC driver")
> Cc: <stable@vger.kernel.org> # v4.20+
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> [nicolas.ferre@microchip.com: adapt the prescaler range,
> fix clk_programmable_recalc_rate, split patch]
> Signed-off-by: Matthias Wieloch <matthias.wieloch@few-bauer.de>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> v2: adapt to v5.1-rc1
> remove unneeded sentence about DT in commit message
>
> Stephen,
>
> I think it would be good to see this fix going upstream during v5.1-rc phase.
Ok. I can apply this clk-fixes. I presume that things are real bad and
it can't wait until v5.2?
> @@ -60,10 +68,18 @@ static int clk_programmable_determine_rate(struct clk_hw *hw,
> continue;
>
> parent_rate = clk_hw_get_rate(parent);
> - for (shift = 0; shift < PROG_PRES_MASK; shift++) {
> - tmp_rate = parent_rate >> shift;
> - if (tmp_rate <= req->rate)
> - break;
> + if (layout->is_pres_direct) {
> + for (shift = 0; shift <= layout->pres_mask; shift++) {
> + tmp_rate = parent_rate / (shift + 1);
> + if (tmp_rate <= req->rate)
> + break;
> + }
> + } else {
> + for (shift = 0; shift < layout->pres_mask; shift++) {
> + tmp_rate = parent_rate >> shift;
> + if (tmp_rate <= req->rate)
> + break;
> + }
This looks like a lot of copy paste when the if statement could have been
pulled into the for loop instead of duplicating the loops and
surrounding if condition check for tmp_rate.
> }
>
> if (tmp_rate > req->rate)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] clk: at91: fix programmable clock for sama5d2
2019-03-18 19:54 ` Stephen Boyd
@ 2019-03-19 8:28 ` Nicolas.Ferre
2019-03-19 9:24 ` Alexandre Belloni
0 siblings, 1 reply; 5+ messages in thread
From: Nicolas.Ferre @ 2019-03-19 8:28 UTC (permalink / raw)
To: sboyd, linux-clk
Cc: alexandre.belloni, matthias.wieloch, mturquette, linux-kernel,
stable, Ludovic.Desroches, Claudiu.Beznea, linux-arm-kernel
Stephen,
Thanks for the review
On 18/03/2019 at 20:54, Stephen Boyd wrote:
> Quoting Nicolas Ferre (2019-03-18 03:50:45)
>> From: Matthias Wieloch <matthias.wieloch@few-bauer.de>
>>
>> The prescaler formula of the programmable clock has changed for sama5d2. Update
>> the driver accordingly.
>>
>> Fixes: a2038077de9a ("clk: at91: add sama5d2 PMC driver")
>> Cc: <stable@vger.kernel.org> # v4.20+
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
>> [nicolas.ferre@microchip.com: adapt the prescaler range,
>> fix clk_programmable_recalc_rate, split patch]
>> Signed-off-by: Matthias Wieloch <matthias.wieloch@few-bauer.de>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> ---
>> v2: adapt to v5.1-rc1
>> remove unneeded sentence about DT in commit message
>>
>> Stephen,
>>
>> I think it would be good to see this fix going upstream during v5.1-rc phase.
>
> Ok. I can apply this clk-fixes. I presume that things are real bad and
> it can't wait until v5.2?
To be perfectly clear, it's not a regression.
But as we're at the very beginning of the '-rc' phase and as it's a bug,
I was thinking about adding it now. But you to choose, no problem either
way.
>> @@ -60,10 +68,18 @@ static int clk_programmable_determine_rate(struct clk_hw *hw,
>> continue;
>>
>> parent_rate = clk_hw_get_rate(parent);
>> - for (shift = 0; shift < PROG_PRES_MASK; shift++) {
>> - tmp_rate = parent_rate >> shift;
>> - if (tmp_rate <= req->rate)
>> - break;
>> + if (layout->is_pres_direct) {
>> + for (shift = 0; shift <= layout->pres_mask; shift++) {
>> + tmp_rate = parent_rate / (shift + 1);
>> + if (tmp_rate <= req->rate)
>> + break;
>> + }
>> + } else {
>> + for (shift = 0; shift < layout->pres_mask; shift++) {
>> + tmp_rate = parent_rate >> shift;
>> + if (tmp_rate <= req->rate)
>> + break;
>> + }
>
> This looks like a lot of copy paste when the if statement could have been
> pulled into the for loop instead of duplicating the loops and
> surrounding if condition check for tmp_rate.
Stop condition of loops not being the same made me separate them instead
of adding artificial test conditions for shift == layout->pres_mask. I'm
not sure the other way around is more obvious then...
>
>> }
>>
>> if (tmp_rate > req->rate)
--
Nicolas Ferre
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] clk: at91: fix programmable clock for sama5d2
2019-03-19 8:28 ` Nicolas.Ferre
@ 2019-03-19 9:24 ` Alexandre Belloni
0 siblings, 0 replies; 5+ messages in thread
From: Alexandre Belloni @ 2019-03-19 9:24 UTC (permalink / raw)
To: Nicolas.Ferre
Cc: sboyd, mturquette, linux-kernel, stable, Claudiu.Beznea,
matthias.wieloch, Ludovic.Desroches, linux-clk, linux-arm-kernel
On 19/03/2019 08:28:40+0000, Nicolas Ferre wrote:
> > Ok. I can apply this clk-fixes. I presume that things are real bad and
> > it can't wait until v5.2?
>
> To be perfectly clear, it's not a regression.
> But as we're at the very beginning of the '-rc' phase and as it's a bug,
> I was thinking about adding it now. But you to choose, no problem either
> way.
>
> >> @@ -60,10 +68,18 @@ static int clk_programmable_determine_rate(struct clk_hw *hw,
> >> continue;
> >>
> >> parent_rate = clk_hw_get_rate(parent);
> >> - for (shift = 0; shift < PROG_PRES_MASK; shift++) {
> >> - tmp_rate = parent_rate >> shift;
> >> - if (tmp_rate <= req->rate)
> >> - break;
> >> + if (layout->is_pres_direct) {
> >> + for (shift = 0; shift <= layout->pres_mask; shift++) {
> >> + tmp_rate = parent_rate / (shift + 1);
> >> + if (tmp_rate <= req->rate)
> >> + break;
> >> + }
> >> + } else {
> >> + for (shift = 0; shift < layout->pres_mask; shift++) {
> >> + tmp_rate = parent_rate >> shift;
> >> + if (tmp_rate <= req->rate)
> >> + break;
> >> + }
> >
> > This looks like a lot of copy paste when the if statement could have been
> > pulled into the for loop instead of duplicating the loops and
> > surrounding if condition check for tmp_rate.
>
> Stop condition of loops not being the same made me separate them instead
> of adding artificial test conditions for shift == layout->pres_mask. I'm
> not sure the other way around is more obvious then...
>
I also tried different ways (e.g. setting up a different determine_rate
for the sama5d2) but this ended up being the more concise one.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] clk: at91: fix programmable clock for sama5d2
2019-03-18 10:50 [PATCH v2] clk: at91: fix programmable clock for sama5d2 Nicolas Ferre
2019-03-18 19:54 ` Stephen Boyd
@ 2019-03-19 19:44 ` Stephen Boyd
1 sibling, 0 replies; 5+ messages in thread
From: Stephen Boyd @ 2019-03-19 19:44 UTC (permalink / raw)
To: Nicolas Ferre, linux-clk
Cc: Alexandre Belloni, matthias.wieloch, mturquette, linux-kernel,
stable, Ludovic Desroches, Claudiu Beznea, linux-arm-kernel
Quoting Nicolas Ferre (2019-03-18 03:50:45)
> From: Matthias Wieloch <matthias.wieloch@few-bauer.de>
>
> The prescaler formula of the programmable clock has changed for sama5d2. Update
> the driver accordingly.
>
> Fixes: a2038077de9a ("clk: at91: add sama5d2 PMC driver")
> Cc: <stable@vger.kernel.org> # v4.20+
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> [nicolas.ferre@microchip.com: adapt the prescaler range,
> fix clk_programmable_recalc_rate, split patch]
> Signed-off-by: Matthias Wieloch <matthias.wieloch@few-bauer.de>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
Applied to clk-fixes
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-19 19:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-18 10:50 [PATCH v2] clk: at91: fix programmable clock for sama5d2 Nicolas Ferre
2019-03-18 19:54 ` Stephen Boyd
2019-03-19 8:28 ` Nicolas.Ferre
2019-03-19 9:24 ` Alexandre Belloni
2019-03-19 19:44 ` Stephen Boyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).