02.08.2019 23:32, Sowjanya Komatineni пишет: > > On 8/2/19 1:17 PM, Dmitry Osipenko wrote: >> 02.08.2019 23:13, Dmitry Osipenko пишет: >>> 02.08.2019 21:33, Sowjanya Komatineni пишет: >>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote: >>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет: >>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote: >>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote: >>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote: >>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет: >>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote: >>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет: >>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote: >>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет: >>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote: >>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote: >>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет: >>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет: >>>>>>>>>>>>>>>>>> This patch implements save and restore context for >>>>>>>>>>>>>>>>>> peripheral >>>>>>>>>>>>>>>>>> fixed >>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock >>>>>>>>>>>>>>>>>> ops, and >>>>>>>>>>>>>>>>>> peripheral clock ops. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> During system suspend, core power goes off and looses the >>>>>>>>>>>>>>>>>> settings >>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of >>>>>>>>>>>>>>>>>> peripherals is >>>>>>>>>>>>>>>>>> saved >>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks back to >>>>>>>>>>>>>>>>>> same >>>>>>>>>>>>>>>>>> rate and >>>>>>>>>>>>>>>>>> state as before suspend. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@xxxxxxxxxx> >>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni >>>>>>>>>>>>>>>>>> <skomatineni@xxxxxxxxxx> >>>>>>>>>>>>>>>>>> --- >>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33 >>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++ >>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34 >>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++ >>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37 >>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++ >>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28 >>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++ >>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++ >>>>>>>>>>>>>>>>>> 5 files changed, 138 insertions(+) >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c >>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c >>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644 >>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c >>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c >>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@ >>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct >>>>>>>>>>>>>>>>>> clk_hw *hw, >>>>>>>>>>>>>>>>>> return (unsigned long)rate; >>>>>>>>>>>>>>>>>> } >>>>>>>>>>>>>>>>>> +static int >>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct >>>>>>>>>>>>>>>>>> clk_hw >>>>>>>>>>>>>>>>>> *hw) >>>>>>>>>>>>>>>>>> +{ >>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed = >>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw); >>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base + >>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) & >>>>>>>>>>>>>>>>>> + mask; >>>>>>>>>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base + >>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) & >>>>>>>>>>>>>>>>>> + mask; >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + return 0; >>>>>>>>>>>>>>>>>> +} >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> +static void >>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct >>>>>>>>>>>>>>>>>> clk_hw >>>>>>>>>>>>>>>>>> *hw) >>>>>>>>>>>>>>>>>> +{ >>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed = >>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw); >>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + if (fixed->enb_ctx) >>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base + >>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg); >>>>>>>>>>>>>>>>>> + else >>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base + >>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + udelay(2); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + if (!fixed->rst_ctx) { >>>>>>>>>>>>>>>>>> + udelay(5); /* reset propogation delay */ >>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base + >>>>>>>>>>>>>>>>>> fixed->regs->rst_reg); >>>>>>>>>>>>>>>>>> + } >>>>>>>>>>>>>>>>>> +} >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> static const struct clk_ops >>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops >>>>>>>>>>>>>>>>>> = { >>>>>>>>>>>>>>>>>> .is_enabled = >>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled, >>>>>>>>>>>>>>>>>> .enable = tegra_clk_periph_fixed_enable, >>>>>>>>>>>>>>>>>> .disable = tegra_clk_periph_fixed_disable, >>>>>>>>>>>>>>>>>> .recalc_rate = >>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate, >>>>>>>>>>>>>>>>>> + .save_context = tegra_clk_periph_fixed_save_context, >>>>>>>>>>>>>>>>>> + .restore_context = >>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context, >>>>>>>>>>>>>>>>>> }; >>>>>>>>>>>>>>>>>> struct clk >>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const >>>>>>>>>>>>>>>>>> char >>>>>>>>>>>>>>>>>> *name, >>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c >>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c >>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644 >>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c >>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c >>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static >>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock); >>>>>>>>>>>>>>>>>> #define read_rst(gate) \ >>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base + >>>>>>>>>>>>>>>>>> (gate->regs->rst_reg)) >>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \ >>>>>>>>>>>>>>>>>> + writel_relaxed(val, gate->clk_base + >>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg)) >>>>>>>>>>>>>>>>>> #define write_rst_clr(val, gate) \ >>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base + >>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg)) >>>>>>>>>>>>>>>>>> @@ -110,10 +112,42 @@ static void >>>>>>>>>>>>>>>>>> clk_periph_disable(struct >>>>>>>>>>>>>>>>>> clk_hw *hw) >>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags); >>>>>>>>>>>>>>>>>> } >>>>>>>>>>>>>>>>>> +static int clk_periph_gate_save_context(struct >>>>>>>>>>>>>>>>>> clk_hw >>>>>>>>>>>>>>>>>> *hw) >>>>>>>>>>>>>>>>>> +{ >>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate = >>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) & >>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate); >>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) & >>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + return 0; >>>>>>>>>>>>>>>>>> +} >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct >>>>>>>>>>>>>>>>>> clk_hw >>>>>>>>>>>>>>>>>> *hw) >>>>>>>>>>>>>>>>>> +{ >>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate = >>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + if (gate->clk_state_ctx) >>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate); >>>>>>>>>>>>>>>>>> + else >>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + udelay(5); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) && >>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) { >>>>>>>>>>>>>>>>>> + if (gate->rst_state_ctx) >>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate); >>>>>>>>>>>>>>>>>> + else >>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate); >>>>>>>>>>>>>>>>>> + } >>>>>>>>>>>>>>>>>> +} >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> const struct clk_ops tegra_clk_periph_gate_ops = { >>>>>>>>>>>>>>>>>> .is_enabled = clk_periph_is_enabled, >>>>>>>>>>>>>>>>>> .enable = clk_periph_enable, >>>>>>>>>>>>>>>>>> .disable = clk_periph_disable, >>>>>>>>>>>>>>>>>> + .save_context = clk_periph_gate_save_context, >>>>>>>>>>>>>>>>>> + .restore_context = clk_periph_gate_restore_context, >>>>>>>>>>>>>>>>>> }; >>>>>>>>>>>>>>>>>> struct clk *tegra_clk_register_periph_gate(const >>>>>>>>>>>>>>>>>> char *name, >>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c >>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c >>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644 >>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c >>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c >>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct >>>>>>>>>>>>>>>>>> clk_hw >>>>>>>>>>>>>>>>>> *hw) >>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw); >>>>>>>>>>>>>>>>>> } >>>>>>>>>>>>>>>>>> +static int clk_periph_save_context(struct >>>>>>>>>>>>>>>>>> clk_hw *hw) >>>>>>>>>>>>>>>>>> +{ >>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw); >>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops; >>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw; >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE)) >>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + return 0; >>>>>>>>>>>>>>>>>> +} >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw >>>>>>>>>>>>>>>>>> *hw) >>>>>>>>>>>>>>>>>> +{ >>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw); >>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops; >>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw; >>>>>>>>>>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops; >>>>>>>>>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw; >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx); >>>>>>>>>>>>>>>>>> + >>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV)) >>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw); >>>>>>>>>>>>>>>>> Could you please point to where the divider's >>>>>>>>>>>>>>>>> save_context() >>>>>>>>>>>>>>>>> happens? >>>>>>>>>>>>>>>>> Because I can't see it. >>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the dividers >>>>>>>>>>>>>>>> context >>>>>>>>>>>>>>>> because >>>>>>>>>>>>>>>> clk itself has enough info that is needed for the context's >>>>>>>>>>>>>>>> restoring >>>>>>>>>>>>>>>> (like I pointed in the review to v6). >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Looks like you could also implement a new >>>>>>>>>>>>>>>> clk_hw_get_parent_index() >>>>>>>>>>>>>>>> generic helper to get the index instead of storing it >>>>>>>>>>>>>>>> manually. >>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing clk_mux_ops >>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> All existing drivers are using directly get_parent() from >>>>>>>>>>>>>>> clk_mux >>>>>>>>>>>>>>> which actually gets index from the register read. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context >>>>>>>>>>>>>>> point of >>>>>>>>>>>>>>> view, >>>>>>>>>>>>>>> probably instead of implementing new get_parent_index >>>>>>>>>>>>>>> helper, >>>>>>>>>>>>>>> I think >>>>>>>>>>>>>>> its better to implement save_context and restore_context to >>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into >>>>>>>>>>>>>>> clk_mux to >>>>>>>>>>>>>>> cache index during set_parent. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and >>>>>>>>>>>>>>> restore_context. >>>>>>>>>>>>>>> >>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops >>>>>>>>>>>>>> to be >>>>>>>>>>>>>> more >>>>>>>>>>>>>> generic w.r.t save/restore context rather than >>>>>>>>>>>>>> get_parent_index >>>>>>>>>>>>>> API. >>>>>>>>>>>>>> Please confirm if you agree. >>>>>>>>>>>>> Sounds like a good idea. I see that there is a 'restoring' >>>>>>>>>>>>> helper for >>>>>>>>>>>>> the generic clk_gate, seems something similar could be done >>>>>>>>>>>>> for the >>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to associate the >>>>>>>>>>>>> parent >>>>>>>>>>>>> clock >>>>>>>>>>>>> with the hw index in order to restore the muxing. >>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you >>>>>>>>>>>> referring to >>>>>>>>>>>> clk_gate_restore_context API? >>>>>>>>>>> Yes. >>>>>>>>>>> >>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can >>>>>>>>>>>> use for >>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops. >>>>>>>>>>>> >>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from >>>>>>>>>>>> clk_mux >>>>>>>>>>>> so I >>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and then >>>>>>>>>>>> during >>>>>>>>>>>> clk-periph restore need to invoke mux_ops->restore_context. >>>>>>>>>>> I'm not sure whether it will be good for every driver that uses >>>>>>>>>>> generic >>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic helper >>>>>>>>>>> function >>>>>>>>>>> that any driver could use in order to restore the clock's >>>>>>>>>>> parent. >>>>>>>>>>> >>>>>>>>>>> The clk-periph restoring also includes case of combining divider >>>>>>>>>>> and >>>>>>>>>>> parent restoring, so generic helper could be useful in that case >>>>>>>>>>> as well. >>>>>>>>>>> >>>>>>>>>>> It also looks like you could actually use the >>>>>>>>>>> clk_gate_restore_context() >>>>>>>>>>> instead of manually saving the clock's enable-state, couldn't >>>>>>>>>>> you? >>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API >>>>>>>>>> rather >>>>>>>>>> than >>>>>>>>>> using restore_context in clk_ops and will invoke that during >>>>>>>>>> clk_periph >>>>>>>>>> restore. >>>>>>>>>> >>>>>> digging thru looks like for clk_periph source restore instead of >>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and >>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent >>>>>> anyway. >>>>>> Will do this for periph clk mux >>>>>>>>>> Reg clk_gate, looks like we cant use generic >>>>>>>>>> clk_gate_restore_context >>>>>>>>>> for clk-periph as it calls enable/disable callbacks and >>>>>>>>>> clk_periph_enable/disable in clk-periph-gate also updated >>>>>>>>>> refcnt and >>>>>>>>>> depending on that actual enable/disable is set. >>>>>>>>>> >>>>>>>>>> During suspend, peripherals that are already enabled have their >>>>>>>>>> refcnt > >>>>>>>>>> 1, so they dont go thru enable/disable on restore if we use same >>>>>>>>>> enable/disable callback. >>>>>>>>> Looks like you could just decrement the gate's enable_refcnt on >>>>>>>>> save_context, wouldn't that work? >>>>>>>>> >>>>>>> gate->enable_refcnt is within clk-periph-gate which gets updated >>>>>>> when >>>>>>> enable/disable callbacks get execute thru clk_core_enable/disable. >>>>>>> But actual enable_count used in clk_gate_restore_context is the one >>>>>>> which gets updated with in the clk core enable/disable functions >>>>>>> which >>>>>>> invokes these callbacks. Depending on this enable_count in clk >>>>>>> core it >>>>>>> invokes enable/disable. >>>>>>> >>>>>>> So, this will cause mismatch if we handle refcnt during save/restore >>>>>>> of tegra_clk_periph_gate_ops and also enable/disable thru this >>>>>>> clk_gate_restore_context is based on enable_count from clk core. >>>>>>> >>>>>>>>>> Also to align exact reset state along with CLK (like for case >>>>>>>>>> where >>>>>>>>>> CLK >>>>>>>>>> is enabled but peripheral might be in reset state), implemented >>>>>>>>>> save/restore in tegra specific tegra_clk_periph_gate_ops >>>>>>>>> I'm wondering whether instead of saving/restoring reset-state of >>>>>>>>> every >>>>>>>>> clock, you could simply save/restore the whole RST_DEV_x_SET >>>>>>>>> register. >>>>>>>>> Couldn't you? >>>>>>>> Thats what I was doing in first version of patch. But later as we >>>>>>>> moved to use clk_save_context and clk_restore_context, peripheral >>>>>>>> clk_hw RST & CLK enables happen thru its corresponding save/restore >>>>>>>> after source restore >>>>>>> Also, to align both CLK & RST to the exact state of register, doing >>>>>>> save/restore in tegra_clk_periph_gate_ops and invoking this after >>>>>>> source restore for peripheral clock, seems cleaner to avoid any >>>>>>> misconfiguration b/w rst & clk settings. >>>>>>> >>>>> It looks to me that it is very wasteful to store/restore each >>>>> individual >>>>> gate and reset state, also given that some of them are shared. I think >>>>> that the gates and resets should be restored separately for the >>>>> peripherals by a custom tegra_clk_save/restore_periph_gates/resets(). >>>> clk_periph_fixed_disable just disables clock only without deasserting >>>> the corresponding peripheral. >>>> >>>> corresponding peripheral drivers can also issue reset assert/deassert >>>> thru reset_control_assert/deassert. >>>> >>>> So, we will not get the actual state of clk and rst unless we read and >>>> save state of reset and clock separately during save_context. >>>> >>>> Currently patch is already using custom >>>> tegra_clk_periph_fixed_save/restore_context for corresponding clk_ops. >>>> Are you suggesting to do save and restore of complete CLK_ENB/RST_DEV >>>> register settings instead of individual peripheral bits? >>> Yes, I'm suggesting to do a complete ungate/reset handling of the >>> devices in a separate function. All enabling/deassertion will be done in >>> a single hop, hence using 7us delay and four u32 words, which is much >>> nicer IMHO. >> Actually six words, three for CLKs and three for RSTs. > > OK, So with separate function doing complete register save/restore for > clk & rst, we can't do this thru clk_ops save/restore as clk_ops > save_restore happens per peripheral wise. So if we decide to do this, > then this should be invoked in clk-tegra210 driver suspend/resume. Yes, per-clock save/restore should be used for setting rate and parent. The ungating and resetting could be done separately to keep things cleaner.