Messages in this thread | | | Subject | Re: [PATCH v8 08/21] clk: tegra: periph: Add restore_context support | From | Sowjanya Komatineni <> | Date | Fri, 9 Aug 2019 09:55:59 -0700 |
| |
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@nvidia.com> >>> --- >>> 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, >>>
| |