Re: [PATCH v8 08/21] clk: tegra: periph: Add restore_context support

From: Sowjanya Komatineni
Date: Fri Aug 09 2019 - 12:56:05 EST


On 8/9/19 5:20 AM, Dmitry Osipenko wrote:
09.08.2019 14:55, Dmitry Osipenko ÐÐÑÐÑ:
09.08.2019 2:46, Sowjanya Komatineni ÐÐÑÐÑ:
This patch implements restore_context support for clk-periph and
clk-sdmmc-mux clock operations to restore clock parent and rates
on system resume.

During system suspend, core power goes off and looses the context
of the Tegra clock controller registers.

So on system resume, clocks parent and rate are restored back to
the context before suspend based on cached data.

Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
---
drivers/clk/tegra/clk-periph.c | 18 ++++++++++++++++++
drivers/clk/tegra/clk-sdmmc-mux.c | 12 ++++++++++++
2 files changed, 30 insertions(+)

diff --git a/drivers/clk/tegra/clk-periph.c b/drivers/clk/tegra/clk-periph.c
index 58437da25156..c9d28cbadccc 100644
--- a/drivers/clk/tegra/clk-periph.c
+++ b/drivers/clk/tegra/clk-periph.c
@@ -3,6 +3,7 @@
* Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved.
*/
+#include <linux/clk.h>
#include <linux/clk-provider.h>
#include <linux/export.h>
#include <linux/slab.h>
@@ -99,6 +100,20 @@ static void clk_periph_disable(struct clk_hw *hw)
gate_ops->disable(gate_hw);
}
+static void clk_periph_restore_context(struct clk_hw *hw)
+{
+ struct tegra_clk_periph *periph = to_clk_periph(hw);
+ const struct clk_ops *div_ops = periph->div_ops;
+ struct clk_hw *div_hw = &periph->divider.hw;
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+ int parent_id = clk_hw_get_parent_index(hw, parent);
+
+ if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
+ div_ops->restore_context(div_hw);
+
+ clk_periph_set_parent(hw, parent_id);
+}
+
const struct clk_ops tegra_clk_periph_ops = {
.get_parent = clk_periph_get_parent,
.set_parent = clk_periph_set_parent,
@@ -108,6 +123,7 @@ const struct clk_ops tegra_clk_periph_ops = {
.is_enabled = clk_periph_is_enabled,
.enable = clk_periph_enable,
.disable = clk_periph_disable,
+ .restore_context = clk_periph_restore_context,
};
static const struct clk_ops tegra_clk_periph_nodiv_ops = {
@@ -116,6 +132,7 @@ static const struct clk_ops tegra_clk_periph_nodiv_ops = {
.is_enabled = clk_periph_is_enabled,
.enable = clk_periph_enable,
.disable = clk_periph_disable,
+ .restore_context = clk_periph_restore_context,
};
static const struct clk_ops tegra_clk_periph_no_gate_ops = {
@@ -124,6 +141,7 @@ static const struct clk_ops tegra_clk_periph_no_gate_ops = {
.recalc_rate = clk_periph_recalc_rate,
.round_rate = clk_periph_round_rate,
.set_rate = clk_periph_set_rate,
+ .restore_context = clk_periph_restore_context,
};
static struct clk *_tegra_clk_register_periph(const char *name,
diff --git a/drivers/clk/tegra/clk-sdmmc-mux.c b/drivers/clk/tegra/clk-sdmmc-mux.c
index a5cd3e31dbae..8db48966b100 100644
--- a/drivers/clk/tegra/clk-sdmmc-mux.c
+++ b/drivers/clk/tegra/clk-sdmmc-mux.c
@@ -194,6 +194,17 @@ static void clk_sdmmc_mux_disable(struct clk_hw *hw)
gate_ops->disable(gate_hw);
}
+static void clk_sdmmc_mux_restore_context(struct clk_hw *hw)
+{
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+ unsigned long parent_rate = clk_hw_get_rate(parent);
+ unsigned long rate = clk_hw_get_rate(hw);
+ int parent_id = clk_hw_get_parent_index(hw, parent);
+
+ clk_sdmmc_mux_set_parent(hw, parent_id);
+ clk_sdmmc_mux_set_rate(hw, rate, parent_rate);
For the periph clocks you're restoring rate at first and then the
parent, while here it's the other way around. I'm wondering if there is
any difference in practice and thus whether rate's divider need to be
set to a some safe value before switching to a new parent that has a
higher clock rate than the old parent.
Although, I guess that all peripheral clocks should be disabled at this
point of resume. Correct?

by the time restore happens, peripheral clocks are enabled but held in reset state.

For non-boot clocks, doing divider programming before parent source is preferred.

For sdmmc clock, programming parent before divisor is allowed.

Also, clk_sdmmc_mux_set_rate gets parent MUX selection thru register read so restoring parent prior to this will have right divisor based on rate and parent.

+}
+
static const struct clk_ops tegra_clk_sdmmc_mux_ops = {
.get_parent = clk_sdmmc_mux_get_parent,
.set_parent = clk_sdmmc_mux_set_parent,
@@ -203,6 +214,7 @@ static const struct clk_ops tegra_clk_sdmmc_mux_ops = {
.is_enabled = clk_sdmmc_mux_is_enabled,
.enable = clk_sdmmc_mux_enable,
.disable = clk_sdmmc_mux_disable,
+ .restore_context = clk_sdmmc_mux_restore_context,
};
struct clk *tegra_clk_register_sdmmc_mux_div(const char *name,