clk: tegra: EMC/MC clock fixes and improvements
Subsystems (3)
v2 cover clk: tegra: EMC/MC clock fixes and improvements
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(-)
Signature · 1 line
2.21.0
PATCH v2 1/5 clk: tegra: emc: Don't enable EMC clock manually
The EMC clock marked as critical, hence it is already enabled at the registration time.
--- drivers/clk/tegra/clk-emc.c | 2 -- 1 file changed, 2 deletions(-)
drivers/clk/tegra/clk-emc.c +0 -2
@@ -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; };
Signature · 1 line
2.21.0
Re: [PATCH v2 1/5] clk: tegra: emc: Don't enable EMC clock manually
Quoting Dmitry Osipenko (2019-04-14 12:23:17)
Quoted 5 lines
The EMC clock marked as critical, hence it is already enabled at the registration time.
Signed-off-by Dmitry Osipenko <[email protected]>---
Applied to clk-next
PATCH v2 2/5 clk: tegra: emc: Support multiple RAM codes
In reply to Dmitry OsipenkoThe 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.
--- drivers/clk/tegra/clk-emc.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-)
drivers/clk/tegra/clk-emc.c +23 -14
@@ -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 )
Signature · 1 line
2.21.0
Re: [PATCH v2 2/5] clk: tegra: emc: Support multiple RAM codes
Quoting Dmitry Osipenko (2019-04-14 12:23:18)
Quoted 8 lines
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 <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>---
Applied to clk-next
PATCH v2 3/5 clk: tegra: emc: Fix EMC max-rate clamping
In reply to Dmitry OsipenkoWhen 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.
--- drivers/clk/tegra/clk-emc.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/clk/tegra/clk-emc.c +10 -7
@@ -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) {
Signature · 1 line
2.21.0
Re: [PATCH v2 3/5] clk: tegra: emc: Fix EMC max-rate clamping
Quoting Dmitry Osipenko (2019-04-14 12:23:19)
Quoted 5 lines
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 <[email protected]>---
Applied to clk-next
PATCH v2 4/5 clk: tegra: emc: Replace BUG() with WARN_ONCE()
In reply to Dmitry OsipenkoThere is no justification for the BUG() in this code.
--- drivers/clk/tegra/clk-emc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/clk/tegra/clk-emc.c +4 -1
@@ -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 ; }
Signature · 1 line
2.21.0
Re: [PATCH v2 4/5] clk: tegra: emc: Replace BUG() with WARN_ONCE()
Quoting Dmitry Osipenko (2019-04-14 12:23:20)
Quoted 4 lines
There is no justification for the
BUG()in this code.Signed-off-by Dmitry Osipenko <[email protected]>---
Applied to clk-next
PATCH v2 5/5 clk: tegra: divider: Mark Memory Controller clock as read-only
In reply to Dmitry OsipenkoThe 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.
--- drivers/clk/tegra/clk-divider.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/clk/tegra/clk-divider.c +2 -1
@@ -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 ); }
Signature · 1 line
2.21.0
Re: [PATCH v2 5/5] clk: tegra: divider: Mark Memory Controller clock as read-only
Quoting Dmitry Osipenko (2019-04-14 12:23:21)
Quoted 9 lines
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 <[email protected]>---
Applied to clk-next