* [PATCH v2 0/5] clk: tegra: EMC/MC clock fixes and improvements
@ 2019-04-14 19:23 Dmitry Osipenko
2019-04-14 19:23 ` [PATCH v2 1/5] clk: tegra: emc: Don't enable EMC clock manually Dmitry Osipenko
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2019-04-14 19:23 UTC (permalink / raw)
To: Peter De Schrijver, Prashant Gaikwad, Michael Turquette,
Stephen Boyd, Thierry Reding, Jonathan Hunter, Joseph Lo
Cc: linux-clk, linux-tegra, linux-kernel
Hello,
I was helping with fixing EMC clock scaling on T124 Nyan Big and
in process found some weak points in the code. Primarily the ram code
parsing didn't work if device-tree defines memory timings for multiple
ram codes and after fixing that I spotted few other things that could be
improved.
Changelog:
v2: Corrected "Mark Memory Controller clock as read-only" patch that
had the "read-only divider" flag being set in a wrong place. Note
it also turned out that this patch is needed for a proper EMC freq
scaling on Tegra30 because some of memory timings may have
configuration where MC clock configuration should be changed
simultaneously with the EMC change and currently CLK framework
reconfigure the divider back to /2 mode after EMC clock changes
since it's the parent clock for MC. Although it's not really critical
since system remains fully functional if MC clock configuration
doesn't match the memory arbitration configuration.
Dmitry Osipenko (5):
clk: tegra: emc: Don't enable EMC clock manually
clk: tegra: emc: Support multiple RAM codes
clk: tegra: emc: Fix EMC max-rate clamping
clk: tegra: emc: Replace BUG() with WARN_ONCE()
clk: tegra: divider: Mark Memory Controller clock as read-only
drivers/clk/tegra/clk-divider.c | 3 +-
drivers/clk/tegra/clk-emc.c | 57 ++++++++++++++++++++-------------
2 files changed, 37 insertions(+), 23 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/5] clk: tegra: emc: Don't enable EMC clock manually
2019-04-14 19:23 [PATCH v2 0/5] clk: tegra: EMC/MC clock fixes and improvements Dmitry Osipenko
@ 2019-04-14 19:23 ` Dmitry Osipenko
2019-04-25 20:54 ` Stephen Boyd
2019-04-14 19:23 ` [PATCH v2 2/5] clk: tegra: emc: Support multiple RAM codes Dmitry Osipenko
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2019-04-14 19:23 UTC (permalink / raw)
To: Peter De Schrijver, Prashant Gaikwad, Michael Turquette,
Stephen Boyd, Thierry Reding, Jonathan Hunter, Joseph Lo
Cc: linux-clk, linux-tegra, linux-kernel
The EMC clock marked as critical, hence it is already enabled at the
registration time.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/clk/tegra/clk-emc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/clk/tegra/clk-emc.c b/drivers/clk/tegra/clk-emc.c
index 0621a3a82ea6..23416982e7c7 100644
--- a/drivers/clk/tegra/clk-emc.c
+++ b/drivers/clk/tegra/clk-emc.c
@@ -532,7 +532,5 @@ struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
/* Allow debugging tools to see the EMC clock */
clk_register_clkdev(clk, "emc", "tegra-clk-debug");
- clk_prepare_enable(clk);
-
return clk;
};
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] clk: tegra: emc: Support multiple RAM codes
2019-04-14 19:23 [PATCH v2 0/5] clk: tegra: EMC/MC clock fixes and improvements Dmitry Osipenko
2019-04-14 19:23 ` [PATCH v2 1/5] clk: tegra: emc: Don't enable EMC clock manually Dmitry Osipenko
@ 2019-04-14 19:23 ` Dmitry Osipenko
2019-04-25 20:54 ` Stephen Boyd
2019-04-14 19:23 ` [PATCH v2 3/5] clk: tegra: emc: Fix EMC max-rate clamping Dmitry Osipenko
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2019-04-14 19:23 UTC (permalink / raw)
To: Peter De Schrijver, Prashant Gaikwad, Michael Turquette,
Stephen Boyd, Thierry Reding, Jonathan Hunter, Joseph Lo
Cc: linux-clk, linux-tegra, linux-kernel
The timings parser doesn't append timings, but instead it parses only
the first timing and hence doesn't store all of the timings when
device-tree has timings for multiple RAM codes. In a result EMC scaling
doesn't work if timings are missing.
Tested-by: Steev Klimaszewski <steev@kali.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/clk/tegra/clk-emc.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/clk/tegra/clk-emc.c b/drivers/clk/tegra/clk-emc.c
index 23416982e7c7..28068584ff6e 100644
--- a/drivers/clk/tegra/clk-emc.c
+++ b/drivers/clk/tegra/clk-emc.c
@@ -121,18 +121,23 @@ static int emc_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
struct tegra_clk_emc *tegra;
u8 ram_code = tegra_read_ram_code();
struct emc_timing *timing = NULL;
- int i;
+ int i, k;
tegra = container_of(hw, struct tegra_clk_emc, hw);
- for (i = 0; i < tegra->num_timings; i++) {
+ for (k = 0; k < tegra->num_timings; k++) {
+ if (tegra->timings[k].ram_code == ram_code)
+ break;
+ }
+
+ for (i = k; i < tegra->num_timings; i++) {
if (tegra->timings[i].ram_code != ram_code)
- continue;
+ break;
timing = tegra->timings + i;
if (timing->rate > req->max_rate) {
- i = max(i, 1);
+ i = max(i, k + 1);
req->rate = tegra->timings[i - 1].rate;
return 0;
}
@@ -282,7 +287,7 @@ static struct emc_timing *get_backup_timing(struct tegra_clk_emc *tegra,
for (i = timing_index+1; i < tegra->num_timings; i++) {
timing = tegra->timings + i;
if (timing->ram_code != ram_code)
- continue;
+ break;
if (emc_parent_clk_sources[timing->parent_index] !=
emc_parent_clk_sources[
@@ -293,7 +298,7 @@ static struct emc_timing *get_backup_timing(struct tegra_clk_emc *tegra,
for (i = timing_index-1; i >= 0; --i) {
timing = tegra->timings + i;
if (timing->ram_code != ram_code)
- continue;
+ break;
if (emc_parent_clk_sources[timing->parent_index] !=
emc_parent_clk_sources[
@@ -433,19 +438,23 @@ static int load_timings_from_dt(struct tegra_clk_emc *tegra,
struct device_node *node,
u32 ram_code)
{
+ struct emc_timing *timings_ptr;
struct device_node *child;
int child_count = of_get_child_count(node);
int i = 0, err;
+ size_t size;
+
+ size = (tegra->num_timings + child_count) * sizeof(struct emc_timing);
- tegra->timings = kcalloc(child_count, sizeof(struct emc_timing),
- GFP_KERNEL);
+ tegra->timings = krealloc(tegra->timings, size, GFP_KERNEL);
if (!tegra->timings)
return -ENOMEM;
- tegra->num_timings = child_count;
+ timings_ptr = tegra->timings + tegra->num_timings;
+ tegra->num_timings += child_count;
for_each_child_of_node(node, child) {
- struct emc_timing *timing = tegra->timings + (i++);
+ struct emc_timing *timing = timings_ptr + (i++);
err = load_one_timing_from_dt(tegra, timing, child);
if (err) {
@@ -456,7 +465,7 @@ static int load_timings_from_dt(struct tegra_clk_emc *tegra,
timing->ram_code = ram_code;
}
- sort(tegra->timings, tegra->num_timings, sizeof(struct emc_timing),
+ sort(timings_ptr, child_count, sizeof(struct emc_timing),
cmp_timings, NULL);
return 0;
@@ -499,10 +508,10 @@ struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
* fuses until the apbmisc driver is loaded.
*/
err = load_timings_from_dt(tegra, node, node_ram_code);
- of_node_put(node);
- if (err)
+ if (err) {
+ of_node_put(node);
return ERR_PTR(err);
- break;
+ }
}
if (tegra->num_timings == 0)
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/5] clk: tegra: emc: Fix EMC max-rate clamping
2019-04-14 19:23 [PATCH v2 0/5] clk: tegra: EMC/MC clock fixes and improvements Dmitry Osipenko
2019-04-14 19:23 ` [PATCH v2 1/5] clk: tegra: emc: Don't enable EMC clock manually Dmitry Osipenko
2019-04-14 19:23 ` [PATCH v2 2/5] clk: tegra: emc: Support multiple RAM codes Dmitry Osipenko
@ 2019-04-14 19:23 ` Dmitry Osipenko
2019-04-25 20:55 ` Stephen Boyd
2019-04-14 19:23 ` [PATCH v2 4/5] clk: tegra: emc: Replace BUG() with WARN_ONCE() Dmitry Osipenko
2019-04-14 19:23 ` [PATCH v2 5/5] clk: tegra: divider: Mark Memory Controller clock as read-only Dmitry Osipenko
4 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2019-04-14 19:23 UTC (permalink / raw)
To: Peter De Schrijver, Prashant Gaikwad, Michael Turquette,
Stephen Boyd, Thierry Reding, Jonathan Hunter, Joseph Lo
Cc: linux-clk, linux-tegra, linux-kernel
When a clk user requests rate that is higher than the maximum possible,
the rate shall be clamped to the maximum and not to the current value.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/clk/tegra/clk-emc.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/tegra/clk-emc.c b/drivers/clk/tegra/clk-emc.c
index 28068584ff6e..9a0179235939 100644
--- a/drivers/clk/tegra/clk-emc.c
+++ b/drivers/clk/tegra/clk-emc.c
@@ -121,7 +121,7 @@ static int emc_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
struct tegra_clk_emc *tegra;
u8 ram_code = tegra_read_ram_code();
struct emc_timing *timing = NULL;
- int i, k;
+ int i, k, t;
tegra = container_of(hw, struct tegra_clk_emc, hw);
@@ -130,12 +130,17 @@ static int emc_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
break;
}
- for (i = k; i < tegra->num_timings; i++) {
- if (tegra->timings[i].ram_code != ram_code)
+ for (t = k; t < tegra->num_timings; t++) {
+ if (tegra->timings[t].ram_code != ram_code)
break;
+ }
+ for (i = k; i < t; i++) {
timing = tegra->timings + i;
+ if (timing->rate < req->rate && i != t - 1)
+ continue;
+
if (timing->rate > req->max_rate) {
i = max(i, k + 1);
req->rate = tegra->timings[i - 1].rate;
@@ -145,10 +150,8 @@ static int emc_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
if (timing->rate < req->min_rate)
continue;
- if (timing->rate >= req->rate) {
- req->rate = timing->rate;
- return 0;
- }
+ req->rate = timing->rate;
+ return 0;
}
if (timing) {
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/5] clk: tegra: emc: Replace BUG() with WARN_ONCE()
2019-04-14 19:23 [PATCH v2 0/5] clk: tegra: EMC/MC clock fixes and improvements Dmitry Osipenko
` (2 preceding siblings ...)
2019-04-14 19:23 ` [PATCH v2 3/5] clk: tegra: emc: Fix EMC max-rate clamping Dmitry Osipenko
@ 2019-04-14 19:23 ` Dmitry Osipenko
2019-04-25 20:55 ` Stephen Boyd
2019-04-14 19:23 ` [PATCH v2 5/5] clk: tegra: divider: Mark Memory Controller clock as read-only Dmitry Osipenko
4 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2019-04-14 19:23 UTC (permalink / raw)
To: Peter De Schrijver, Prashant Gaikwad, Michael Turquette,
Stephen Boyd, Thierry Reding, Jonathan Hunter, Joseph Lo
Cc: linux-clk, linux-tegra, linux-kernel
There is no justification for the BUG() in this code.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/clk/tegra/clk-emc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-emc.c b/drivers/clk/tegra/clk-emc.c
index 9a0179235939..93ecb538e59b 100644
--- a/drivers/clk/tegra/clk-emc.c
+++ b/drivers/clk/tegra/clk-emc.c
@@ -222,7 +222,10 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
if (emc_get_parent(&tegra->hw) == timing->parent_index &&
clk_get_rate(timing->parent) != timing->parent_rate) {
- BUG();
+ WARN_ONCE(1, "parent %s rate mismatch %lu %lu\n",
+ __clk_get_name(timing->parent),
+ clk_get_rate(timing->parent),
+ timing->parent_rate);
return -EINVAL;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] clk: tegra: divider: Mark Memory Controller clock as read-only
2019-04-14 19:23 [PATCH v2 0/5] clk: tegra: EMC/MC clock fixes and improvements Dmitry Osipenko
` (3 preceding siblings ...)
2019-04-14 19:23 ` [PATCH v2 4/5] clk: tegra: emc: Replace BUG() with WARN_ONCE() Dmitry Osipenko
@ 2019-04-14 19:23 ` Dmitry Osipenko
2019-04-25 20:55 ` Stephen Boyd
4 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2019-04-14 19:23 UTC (permalink / raw)
To: Peter De Schrijver, Prashant Gaikwad, Michael Turquette,
Stephen Boyd, Thierry Reding, Jonathan Hunter, Joseph Lo
Cc: linux-clk, linux-tegra, linux-kernel
The Memory Controller (MC) clock rate can't be simply changed and nothing
in kernel need to change the rate, hence let's make the clock read-only.
This id also needed for the EMC driver because timing configuration may
require the MC clock diver to be disabled, that is handled by the EMC
clock / EMC driver integration and CLK framework shall not touch the
MC divider configuration on the EMC clock rate change.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/clk/tegra/clk-divider.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c
index 205fe8ff63f0..2a1822a22740 100644
--- a/drivers/clk/tegra/clk-divider.c
+++ b/drivers/clk/tegra/clk-divider.c
@@ -175,6 +175,7 @@ struct clk *tegra_clk_register_mc(const char *name, const char *parent_name,
void __iomem *reg, spinlock_t *lock)
{
return clk_register_divider_table(NULL, name, parent_name,
- CLK_IS_CRITICAL, reg, 16, 1, 0,
+ CLK_IS_CRITICAL,
+ reg, 16, 1, CLK_DIVIDER_READ_ONLY,
mc_div_table, lock);
}
--
2.21.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/5] clk: tegra: emc: Don't enable EMC clock manually
2019-04-14 19:23 ` [PATCH v2 1/5] clk: tegra: emc: Don't enable EMC clock manually Dmitry Osipenko
@ 2019-04-25 20:54 ` Stephen Boyd
0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2019-04-25 20:54 UTC (permalink / raw)
To: Dmitry Osipenko, Jonathan Hunter, Joseph Lo, Michael Turquette,
Peter De Schrijver, Prashant Gaikwad, Thierry Reding
Cc: linux-clk, linux-tegra, linux-kernel
Quoting Dmitry Osipenko (2019-04-14 12:23:17)
> The EMC clock marked as critical, hence it is already enabled at the
> registration time.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
Applied to clk-next
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/5] clk: tegra: emc: Support multiple RAM codes
2019-04-14 19:23 ` [PATCH v2 2/5] clk: tegra: emc: Support multiple RAM codes Dmitry Osipenko
@ 2019-04-25 20:54 ` Stephen Boyd
0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2019-04-25 20:54 UTC (permalink / raw)
To: Dmitry Osipenko, Jonathan Hunter, Joseph Lo, Michael Turquette,
Peter De Schrijver, Prashant Gaikwad, Thierry Reding
Cc: linux-clk, linux-tegra, linux-kernel
Quoting Dmitry Osipenko (2019-04-14 12:23:18)
> The timings parser doesn't append timings, but instead it parses only
> the first timing and hence doesn't store all of the timings when
> device-tree has timings for multiple RAM codes. In a result EMC scaling
> doesn't work if timings are missing.
>
> Tested-by: Steev Klimaszewski <steev@kali.org>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
Applied to clk-next
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] clk: tegra: emc: Fix EMC max-rate clamping
2019-04-14 19:23 ` [PATCH v2 3/5] clk: tegra: emc: Fix EMC max-rate clamping Dmitry Osipenko
@ 2019-04-25 20:55 ` Stephen Boyd
0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2019-04-25 20:55 UTC (permalink / raw)
To: Dmitry Osipenko, Jonathan Hunter, Joseph Lo, Michael Turquette,
Peter De Schrijver, Prashant Gaikwad, Thierry Reding
Cc: linux-clk, linux-tegra, linux-kernel
Quoting Dmitry Osipenko (2019-04-14 12:23:19)
> When a clk user requests rate that is higher than the maximum possible,
> the rate shall be clamped to the maximum and not to the current value.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
Applied to clk-next
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/5] clk: tegra: emc: Replace BUG() with WARN_ONCE()
2019-04-14 19:23 ` [PATCH v2 4/5] clk: tegra: emc: Replace BUG() with WARN_ONCE() Dmitry Osipenko
@ 2019-04-25 20:55 ` Stephen Boyd
0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2019-04-25 20:55 UTC (permalink / raw)
To: Dmitry Osipenko, Jonathan Hunter, Joseph Lo, Michael Turquette,
Peter De Schrijver, Prashant Gaikwad, Thierry Reding
Cc: linux-clk, linux-tegra, linux-kernel
Quoting Dmitry Osipenko (2019-04-14 12:23:20)
> There is no justification for the BUG() in this code.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
Applied to clk-next
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 5/5] clk: tegra: divider: Mark Memory Controller clock as read-only
2019-04-14 19:23 ` [PATCH v2 5/5] clk: tegra: divider: Mark Memory Controller clock as read-only Dmitry Osipenko
@ 2019-04-25 20:55 ` Stephen Boyd
0 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2019-04-25 20:55 UTC (permalink / raw)
To: Dmitry Osipenko, Jonathan Hunter, Joseph Lo, Michael Turquette,
Peter De Schrijver, Prashant Gaikwad, Thierry Reding
Cc: linux-clk, linux-tegra, linux-kernel
Quoting Dmitry Osipenko (2019-04-14 12:23:21)
> The Memory Controller (MC) clock rate can't be simply changed and nothing
> in kernel need to change the rate, hence let's make the clock read-only.
> This id also needed for the EMC driver because timing configuration may
> require the MC clock diver to be disabled, that is handled by the EMC
> clock / EMC driver integration and CLK framework shall not touch the
> MC divider configuration on the EMC clock rate change.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
Applied to clk-next
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-04-25 20:55 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-14 19:23 [PATCH v2 0/5] clk: tegra: EMC/MC clock fixes and improvements Dmitry Osipenko
2019-04-14 19:23 ` [PATCH v2 1/5] clk: tegra: emc: Don't enable EMC clock manually Dmitry Osipenko
2019-04-25 20:54 ` Stephen Boyd
2019-04-14 19:23 ` [PATCH v2 2/5] clk: tegra: emc: Support multiple RAM codes Dmitry Osipenko
2019-04-25 20:54 ` Stephen Boyd
2019-04-14 19:23 ` [PATCH v2 3/5] clk: tegra: emc: Fix EMC max-rate clamping Dmitry Osipenko
2019-04-25 20:55 ` Stephen Boyd
2019-04-14 19:23 ` [PATCH v2 4/5] clk: tegra: emc: Replace BUG() with WARN_ONCE() Dmitry Osipenko
2019-04-25 20:55 ` Stephen Boyd
2019-04-14 19:23 ` [PATCH v2 5/5] clk: tegra: divider: Mark Memory Controller clock as read-only Dmitry Osipenko
2019-04-25 20:55 ` 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).