[v5,1/2] clk: qcom: Configure the RCGs to a safe source as needed

Commit Message

Amit Nischal April 18, 2018, 1:34 p.m. UTC

For some root clock generators, there could be child branches which are
controlled by an entity other than application processor subsystem. For
such RCGs, as per application processor subsystem clock driver, all of
its downstream clocks are disabled and RCG is in disabled state but in
reality downstream clocks can be left enabled before.

So in this scenario, when RCG is disabled as per clock driver's point of
view and when rate scaling request comes before downstream clock enable
request, then RCG fails to update its configuration because in reality
RCG is on and it expects its new source to alredy be in enable state but
in reality new source is off. In order to avoid having the RCG to go into
an invalid state, add support to cache the rate of RCG during set_rate(),
defer actual RCG configuration update to be done during clk_enable() as
at this point of time, both its new parent and safe source will be
already enabled and RCG can safely switch to new parent.

During clk_disable() request, configure it to safe source as both its
parents, safe source and current parent will be enabled and RCG can
safely execute a switch. Also add support to have safe configuration
frequency table structure for each shared RCG.

Signed-off-by: Taniya Das <[e-mail]>
Signed-off-by: Amit Nischal <[e-mail]>
---
 drivers/clk/qcom/clk-rcg.h  |   7 +-
 drivers/clk/qcom/clk-rcg2.c | 173 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 178 insertions(+), 2 deletions(-)

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Stephen Boyd April 19, 2018, 7:37 a.m. UTC | #1

Quoting Amit Nischal (2018-04-18 06:34:32)
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index 2a7489a..9d9d59d 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -153,8 +155,10 @@ struct clk_rcg2 {
>         u32                     cmd_rcgr;
>         u8                      mnd_width;
>         u8                      hid_width;
> +       const u8                safe_src_index;

Please drop const.

>         const struct parent_map *parent_map;
>         const struct freq_tbl   *freq_tbl;
> +       unsigned long           current_freq;
>         struct clk_regmap       clkr;
>  };
> 
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 984de9c..4d971bf 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> +
> +static int clk_rcg2_shared_set_rate(struct clk_hw *hw, unsigned long rate,
> +                                   unsigned long parent_rate)
> +{
> +       struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> +       int ret;
> +
> +       /*
> +        * Return if the RCG is currently disabled. This configuration
> +        * update will happen as part of the RCG enable sequence.
> +        */
> +       if (!__clk_is_enabled(hw->clk)) {
> +               rcg->current_freq = rate;

We should probably check that 'rate' can be achieved during
clk_set_rate() though. So I imagine we would need to do all the
rounding, etc. and then cache the frequency at that point. Or cross
fingers that this set rate op is never called without the round_rate op
being called on it too.

> +               return 0;
> +       }
> +
> +       ret = clk_rcg2_shared_force_enable_clear(hw, rate);
> +       if (ret)
> +               return ret;
> +
> +       /* Update current frequency with the requested frequency. */
> +       rcg->current_freq = rate;

Is this even needed? recalc_rate() would be called shortly after and do
this too.

> +
> +       return ret;
> +}
> +
> +static int clk_rcg2_shared_set_rate_and_parent(struct clk_hw *hw,
> +               unsigned long rate, unsigned long parent_rate, u8 index)
> +{
> +       return clk_rcg2_shared_set_rate(hw, rate, parent_rate);

We lost index. I guess that's OK...

> +}
> +
[...]
> +
> +static unsigned long clk_rcg2_get_safe_src_rate(struct clk_hw *hw)
> +{
> +       struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> +       int index;
> +
> +       index = qcom_find_src_index(hw, rcg->parent_map, rcg->safe_src_index);

Why are we finding it though? Can't we just set the bit to be what it's
supposed to be all the time at compile time instead of going through the
parent map to find the register value?

> +       if (index < 0)
> +               index = 0;
> +
> +       return clk_hw_get_rate(clk_hw_get_parent_by_index(hw, index));

I'm still failing to see why the 'rate' is so important for the safe
source.

> +}
> +
> +static int clk_rcg2_shared_enable(struct clk_hw *hw)
> +{
> +       struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> +       struct freq_tbl safe_src_freq_tbl = { 0 };
> +
> +       safe_src_freq_tbl.freq = clk_rcg2_get_safe_src_rate(hw);
> +
> +       if (rcg->current_freq == safe_src_freq_tbl.freq) {

I'd prefer this became a bool condition like rcg->on_safe_src or
something like that, instead of calculating the safe src frequency and
comparing that to whatever recalc_rate saw.

> +               safe_src_freq_tbl.src = rcg->safe_src_index;
> +               /*
> +                * Reconfigure the RCG - Incase if any other sub system updates
> +                * the div or src without the knowledge of application processor
> +                * subsystem and RCG could run at different rate other than
> +                * software cached rate.

This happens? But then if we force enable RCG and the src was changed by
another subsystem and that src isn't enabled anymore we're going to get
a stuck rcg switch. So they really shouldn't be switching it over to a
non-safe src before handing it off to us. I can believe div, so ok we
may need to update div, but then either the 'current_freq' is in the
table that matches the safe src speed, and then we can call
clk_rcg2_shared_force_enable_clear() or the frequency doesn't match safe
src speed and we can still call clk_rcg2_shared_force_enable_clear()
because it's in the freq table. I thought these three lines below in
this if condition were purely there because the frequency table didn't
hold the entry for safe src speed, sometimes.

> +                */
> +               clk_rcg2_set_force_enable(hw);
> +               clk_rcg2_configure(rcg, &safe_src_freq_tbl);

This could be a few more lines to regmap_write() the cfg reg with the
proper index then leave the mnd registers empty, and then call
update_config()? Instead of doing index gymnastics.

> +               clk_rcg2_clear_force_enable(hw);
> +
> +               return 0;
> +       }
> +
> +       /*
> +        * Switch from safe source to the stashed mux selection. The current
> +        * parent has already been prepared and enabled at this point, and
> +        * the safe source is always on while application processor subsystem
> +        * is online. Therefore, the RCG can safely switch its source.
> +        */
> +
> +       return clk_rcg2_shared_force_enable_clear(hw, rcg->current_freq);

Ok maybe it's because we may boot up, rate is safe src speed in
hardware, that entry is not in the freq table, recalc_rate() is called
and that finds safe src speed and assigns it into current_freq. I can
believe that. So in this case, the above if condition is going to catch
it and allow enable to be called before clk_set_rate().

How about changing 'current_freq' to literally a struct freq_table
member, that stores a copy of the frequency table entry to use or
constructs the entry on the fly when the frequency doesn't exist in the
plan? At boot time, that could be populated with the value we read from
hardware, and then otherwise when we call disable() it would just copy
it over into the member and on enable() it would slam it into the
hardware and do update_config().

Or we could even just read the M, N, D, and CFG registers and save them
off at driver probe time or during disable and then restore them on
enable. Make a pointer in clk_rcg2 to some regcache structure and then
assign those statically in the SoC driver to avoid wasting space for
each RCG out there. The recalc_rate() function could fill in the cache
too if there's some special bit set indicating it needs to load it
initially, and then a new recalc_rate() function could be written to
operate on the mini-cache of register values for these clks. We could
even go a step further and have the clk_set_rate() path write into the
cache when the clk is disabled to update the mnd and cfg and bypass the
cache when the clk is enabled.

It would be super nice if regmap could handle all of this too, so we
could enable caching for a few registers, regmap write into them and
skip the update_config() step when clk disabled, and then flush the
cache when we enable, update_config(), and disable caching for the
registers. recalc_rate() would work unchanged then because it would
always hit in the cache when caching is on, or read the hardware when
caching was off. We would need something that lets us bypass the cache
though and write directly when we force the safe src. There's already
some regmap caching code, so it might work with some more APIs added. Or
we could go the custom cache route and see how bad it is.

> +}
> +
> +static void clk_rcg2_shared_disable(struct clk_hw *hw)
> +{
> +       struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> +       struct freq_tbl safe_src_freq_tbl = { 0 };
> +
> +       safe_src_freq_tbl.src = rcg->safe_src_index;
> +       safe_src_freq_tbl.freq = clk_rcg2_get_safe_src_rate(hw);
> +
> +       /*
> +        * Park the RCG at a safe configuration - sourced off from safe source.
> +        * Force enable and disable the RCG while configuring it to safeguard
> +        * against any update signal coming from the downstream clock.
> +        * The current parent is still prepared and enabled at this point, and
> +        * the safe source is always on while application processor subsystem
> +        * is online. Therefore, the RCG can safely switch its parent.
> +        */
> +       clk_rcg2_set_force_enable(hw);
> +       clk_rcg2_configure(rcg, &safe_src_freq_tbl);

Again, slam in index and call update_config() directly.

> +       clk_rcg2_clear_force_enable(hw);
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Amit Nischal April 26, 2018, 1:52 p.m. UTC | #2

On 2018-04-19 13:07, Stephen Boyd wrote:
> Quoting Amit Nischal (2018-04-18 06:34:32)
>> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
>> index 2a7489a..9d9d59d 100644
>> --- a/drivers/clk/qcom/clk-rcg.h
>> +++ b/drivers/clk/qcom/clk-rcg.h
>> @@ -153,8 +155,10 @@ struct clk_rcg2 {
>>         u32                     cmd_rcgr;
>>         u8                      mnd_width;
>>         u8                      hid_width;
>> +       const u8                safe_src_index;
> 
> Please drop const.

Thanks for the review. Will remove the const.

> 
>>         const struct parent_map *parent_map;
>>         const struct freq_tbl   *freq_tbl;
>> +       unsigned long           current_freq;
>>         struct clk_regmap       clkr;
>>  };
>> 
>> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
>> index 984de9c..4d971bf 100644
>> --- a/drivers/clk/qcom/clk-rcg2.c
>> +++ b/drivers/clk/qcom/clk-rcg2.c
>> +
>> +static int clk_rcg2_shared_set_rate(struct clk_hw *hw, unsigned long 
>> rate,
>> +                                   unsigned long parent_rate)
>> +{
>> +       struct clk_rcg2 *rcg = to_clk_rcg2(hw);
>> +       int ret;
>> +
>> +       /*
>> +        * Return if the RCG is currently disabled. This configuration
>> +        * update will happen as part of the RCG enable sequence.
>> +        */
>> +       if (!__clk_is_enabled(hw->clk)) {
>> +               rcg->current_freq = rate;
> 
> We should probably check that 'rate' can be achieved during
> clk_set_rate() though. So I imagine we would need to do all the
> rounding, etc. and then cache the frequency at that point. Or cross
> fingers that this set rate op is never called without the round_rate op
> being called on it too.

Yes set_rate() op would not be called without 
round_rate/determine_rate().

> 
>> +               return 0;
>> +       }
>> +
>> +       ret = clk_rcg2_shared_force_enable_clear(hw, rate);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* Update current frequency with the requested frequency. */
>> +       rcg->current_freq = rate;
> 
> Is this even needed? recalc_rate() would be called shortly after and do
> this too.

Here updating of current_freq with the new requested rate is required as
recalc_rate() op is using the current_freq and returning the cached rate
(current_freq) when the clock is in disabled state.

> 
>> +
>> +       return ret;
>> +}
>> +
>> +static int clk_rcg2_shared_set_rate_and_parent(struct clk_hw *hw,
>> +               unsigned long rate, unsigned long parent_rate, u8 
>> index)
>> +{
>> +       return clk_rcg2_shared_set_rate(hw, rate, parent_rate);
> 
> We lost index. I guess that's OK...
> 
>> +}
>> +
> [...]
>> +
>> +static unsigned long clk_rcg2_get_safe_src_rate(struct clk_hw *hw)
>> +{
>> +       struct clk_rcg2 *rcg = to_clk_rcg2(hw);
>> +       int index;
>> +
>> +       index = qcom_find_src_index(hw, rcg->parent_map, 
>> rcg->safe_src_index);
> 
> Why are we finding it though? Can't we just set the bit to be what it's
> supposed to be all the time at compile time instead of going through 
> the
> parent map to find the register value?

This is to find the safe source rate from the safe_src_index value. As 
suggested
in v2 series, we are constructing the safe source frequency table on the 
fly
from the safe_src_index value.

> 
>> +       if (index < 0)
>> +               index = 0;
>> +
>> +       return clk_hw_get_rate(clk_hw_get_parent_by_index(hw, index));
> 
> I'm still failing to see why the 'rate' is so important for the safe
> source.

Safe source's rate is getting used in the clk_rcg2_shared_enable() op.
Please have a look at next comment which explains about the need of
safe source's rate.

> 
>> +}
>> +
>> +static int clk_rcg2_shared_enable(struct clk_hw *hw)
>> +{
>> +       struct clk_rcg2 *rcg = to_clk_rcg2(hw);
>> +       struct freq_tbl safe_src_freq_tbl = { 0 };
>> +
>> +       safe_src_freq_tbl.freq = clk_rcg2_get_safe_src_rate(hw);
>> +
>> +       if (rcg->current_freq == safe_src_freq_tbl.freq) {
> 
> I'd prefer this became a bool condition like rcg->on_safe_src or
> something like that, instead of calculating the safe src frequency and
> comparing that to whatever recalc_rate saw.
> 
>> +               safe_src_freq_tbl.src = rcg->safe_src_index;
>> +               /*
>> +                * Reconfigure the RCG - Incase if any other sub 
>> system updates
>> +                * the div or src without the knowledge of application 
>> processor
>> +                * subsystem and RCG could run at different rate other 
>> than
>> +                * software cached rate.
> 
> This happens? But then if we force enable RCG and the src was changed 
> by
> another subsystem and that src isn't enabled anymore we're going to get
> a stuck rcg switch. So they really shouldn't be switching it over to a
> non-safe src before handing it off to us. I can believe div, so ok we
> may need to update div, but then either the 'current_freq' is in the
> table that matches the safe src speed, and then we can call
> clk_rcg2_shared_force_enable_clear() or the frequency doesn't match 
> safe
> src speed and we can still call clk_rcg2_shared_force_enable_clear()
> because it's in the freq table. I thought these three lines below in
> this if condition were purely there because the frequency table didn't
> hold the entry for safe src speed, sometimes.

Yes, your understanding is right. Above lines are there to catch the 
below:
1. When pre-boot entity configures the clock to run from safe_src.
2. RCG frequency table does not hold the entry for safe_src speed.
3. The clock_enable() gets call without the set_rate().

If all the above three conditions will be true then, enable op would 
configure
the RCG at safe_src speed otherwise, enable() would configure the RCG
with the first frequency entry in the frequency table and RCG will get 
enable
at a rate which BOOT has not configured and the old configuration will 
be
lost.

> 
>> +                */
>> +               clk_rcg2_set_force_enable(hw);
>> +               clk_rcg2_configure(rcg, &safe_src_freq_tbl);
> 
> This could be a few more lines to regmap_write() the cfg reg with the
> proper index then leave the mnd registers empty, and then call
> update_config()? Instead of doing index gymnastics.
> 
>> +               clk_rcg2_clear_force_enable(hw);
>> +
>> +               return 0;
>> +       }
>> +
>> +       /*
>> +        * Switch from safe source to the stashed mux selection. The 
>> current
>> +        * parent has already been prepared and enabled at this point, 
>> and
>> +        * the safe source is always on while application processor 
>> subsystem
>> +        * is online. Therefore, the RCG can safely switch its source.
>> +        */
>> +
>> +       return clk_rcg2_shared_force_enable_clear(hw, 
>> rcg->current_freq);
> 
> Ok maybe it's because we may boot up, rate is safe src speed in
> hardware, that entry is not in the freq table, recalc_rate() is called
> and that finds safe src speed and assigns it into current_freq. I can
> believe that. So in this case, the above if condition is going to catch
> it and allow enable to be called before clk_set_rate().
> 
> How about changing 'current_freq' to literally a struct freq_table
> member, that stores a copy of the frequency table entry to use or
> constructs the entry on the fly when the frequency doesn't exist in the
> plan? At boot time, that could be populated with the value we read from
> hardware, and then otherwise when we call disable() it would just copy
> it over into the member and on enable() it would slam it into the
> hardware and do update_config().
> 
> Or we could even just read the M, N, D, and CFG registers and save them
> off at driver probe time or during disable and then restore them on
> enable. Make a pointer in clk_rcg2 to some regcache structure and then
> assign those statically in the SoC driver to avoid wasting space for
> each RCG out there. The recalc_rate() function could fill in the cache
> too if there's some special bit set indicating it needs to load it
> initially, and then a new recalc_rate() function could be written to
> operate on the mini-cache of register values for these clks. We could
> even go a step further and have the clk_set_rate() path write into the
> cache when the clk is disabled to update the mnd and cfg and bypass the
> cache when the clk is enabled.
> 
> It would be super nice if regmap could handle all of this too, so we
> could enable caching for a few registers, regmap write into them and
> skip the update_config() step when clk disabled, and then flush the
> cache when we enable, update_config(), and disable caching for the
> registers. recalc_rate() would work unchanged then because it would
> always hit in the cache when caching is on, or read the hardware when
> caching was off. We would need something that lets us bypass the cache
> though and write directly when we force the safe src. There's already
> some regmap caching code, so it might work with some more APIs added. 
> Or
> we could go the custom cache route and see how bad it is.

Thanks for your valuable suggestions. We would like to revisit this 
feature
to construct a frequency table entry on the fly when boot configures the 
RCG
to a different frequency which is not present in the freq_tbl at a later 
point
of time.

As of now, it's expected that any pre-boot entity should follow the 
frequency
plan and configure the RCG accordingly.

The main intention of this patch is to switch the RCG to the safe 
source(XO)
in disable path and switch back to stashed mux source from the safe 
source(XO)
in enable path.

> 
>> +}
>> +
>> +static void clk_rcg2_shared_disable(struct clk_hw *hw)
>> +{
>> +       struct clk_rcg2 *rcg = to_clk_rcg2(hw);
>> +       struct freq_tbl safe_src_freq_tbl = { 0 };
>> +
>> +       safe_src_freq_tbl.src = rcg->safe_src_index;
>> +       safe_src_freq_tbl.freq = clk_rcg2_get_safe_src_rate(hw);
>> +
>> +       /*
>> +        * Park the RCG at a safe configuration - sourced off from 
>> safe source.
>> +        * Force enable and disable the RCG while configuring it to 
>> safeguard
>> +        * against any update signal coming from the downstream clock.
>> +        * The current parent is still prepared and enabled at this 
>> point, and
>> +        * the safe source is always on while application processor 
>> subsystem
>> +        * is online. Therefore, the RCG can safely switch its parent.
>> +        */
>> +       clk_rcg2_set_force_enable(hw);
>> +       clk_rcg2_configure(rcg, &safe_src_freq_tbl);
> 
> Again, slam in index and call update_config() directly.
> 
>> +       clk_rcg2_clear_force_enable(hw);
>> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index 2a7489a..9d9d59d 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013, 2018, The Linux Foundation. All rights reserved.
  *
  * This software is licensed under the terms of the GNU General Public
  * License version 2, as published by the Free Software Foundation, and
@@ -144,8 +144,10 @@  struct clk_dyn_rcg {
  * @cmd_rcgr: corresponds to *_CMD_RCGR
  * @mnd_width: number of bits in m/n/d values
  * @hid_width: number of bits in half integer divider
+ * @safe_src_index: safe src index value
  * @parent_map: map from software's parent index to hardware's src_sel field
  * @freq_tbl: frequency table
+ * @current_freq: last cached frequency when using branches with shared RCGs
  * @clkr: regmap clock handle
  *
  */
@@ -153,8 +155,10 @@  struct clk_rcg2 {
 	u32			cmd_rcgr;
 	u8			mnd_width;
 	u8			hid_width;
+	const u8		safe_src_index;
 	const struct parent_map	*parent_map;
 	const struct freq_tbl	*freq_tbl;
+	unsigned long		current_freq;
 	struct clk_regmap	clkr;
 };

@@ -167,5 +171,6 @@  struct clk_rcg2 {
 extern const struct clk_ops clk_byte2_ops;
 extern const struct clk_ops clk_pixel_ops;
 extern const struct clk_ops clk_gfx3d_ops;
+extern const struct clk_ops clk_rcg2_shared_ops;

 #endif
diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index 984de9c..4d971bf 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013, 2018 The Linux Foundation. All rights reserved.
  *
  * This software is licensed under the terms of the GNU General Public
  * License version 2, as published by the Free Software Foundation, and
@@ -790,3 +790,174 @@  static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate,
 	.determine_rate = clk_gfx3d_determine_rate,
 };
 EXPORT_SYMBOL_GPL(clk_gfx3d_ops);
+
+static int clk_rcg2_set_force_enable(struct clk_hw *hw)
+{
+	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+	const char *name = clk_hw_get_name(hw);
+	int ret, count;
+
+	/* Force enable bit */
+	ret = regmap_update_bits(rcg->clkr.regmap, rcg->cmd_rcgr + CMD_REG,
+				 CMD_ROOT_EN, CMD_ROOT_EN);
+	if (ret)
+		return ret;
+
+	/* wait for RCG to turn ON */
+	for (count = 500; count > 0; count--) {
+		if (clk_rcg2_is_enabled(hw))
+			return 0;
+
+		/* Delay for 1usec and retry polling the status bit */
+		udelay(1);
+	}
+	if (!count)
+		pr_err("%s: RCG did not turn on\n", name);
+
+	return -ETIMEDOUT;
+}
+
+static int clk_rcg2_clear_force_enable(struct clk_hw *hw)
+{
+	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+
+	/* Clear force enable bit */
+	return regmap_update_bits(rcg->clkr.regmap, rcg->cmd_rcgr + CMD_REG,
+					CMD_ROOT_EN, 0);
+}
+
+static int
+clk_rcg2_shared_force_enable_clear(struct clk_hw *hw, unsigned long rate)
+{
+	int ret;
+
+	ret = clk_rcg2_set_force_enable(hw);
+	if (ret)
+		return ret;
+
+	/* set clock rate */
+	ret = __clk_rcg2_set_rate(hw, rate, CEIL);
+	if (ret)
+		return ret;
+
+	return clk_rcg2_clear_force_enable(hw);
+}
+
+static int clk_rcg2_shared_set_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long parent_rate)
+{
+	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+	int ret;
+
+	/*
+	 * Return if the RCG is currently disabled. This configuration
+	 * update will happen as part of the RCG enable sequence.
+	 */
+	if (!__clk_is_enabled(hw->clk)) {
+		rcg->current_freq = rate;
+		return 0;
+	}
+
+	ret = clk_rcg2_shared_force_enable_clear(hw, rate);
+	if (ret)
+		return ret;
+
+	/* Update current frequency with the requested frequency. */
+	rcg->current_freq = rate;
+
+	return ret;
+}
+
+static int clk_rcg2_shared_set_rate_and_parent(struct clk_hw *hw,
+		unsigned long rate, unsigned long parent_rate, u8 index)
+{
+	return clk_rcg2_shared_set_rate(hw, rate, parent_rate);
+}
+
+static unsigned long
+clk_rcg2_shared_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+
+	if (!__clk_is_enabled(hw->clk) && rcg->current_freq)
+		return rcg->current_freq;
+
+	return rcg->current_freq = clk_rcg2_recalc_rate(hw, parent_rate);
+}
+
+static unsigned long clk_rcg2_get_safe_src_rate(struct clk_hw *hw)
+{
+	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+	int index;
+
+	index = qcom_find_src_index(hw, rcg->parent_map, rcg->safe_src_index);
+	if (index < 0)
+		index = 0;
+
+	return clk_hw_get_rate(clk_hw_get_parent_by_index(hw, index));
+}
+
+static int clk_rcg2_shared_enable(struct clk_hw *hw)
+{
+	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+	struct freq_tbl safe_src_freq_tbl = { 0 };
+
+	safe_src_freq_tbl.freq = clk_rcg2_get_safe_src_rate(hw);
+
+	if (rcg->current_freq == safe_src_freq_tbl.freq) {
+		safe_src_freq_tbl.src = rcg->safe_src_index;
+		/*
+		 * Reconfigure the RCG - Incase if any other sub system updates
+		 * the div or src without the knowledge of application processor
+		 * subsystem and RCG could run at different rate other than
+		 * software cached rate.
+		 */
+		clk_rcg2_set_force_enable(hw);
+		clk_rcg2_configure(rcg, &safe_src_freq_tbl);
+		clk_rcg2_clear_force_enable(hw);
+
+		return 0;
+	}
+
+	/*
+	 * Switch from safe source to the stashed mux selection. The current
+	 * parent has already been prepared and enabled at this point, and
+	 * the safe source is always on while application processor subsystem
+	 * is online. Therefore, the RCG can safely switch its source.
+	 */
+
+	return clk_rcg2_shared_force_enable_clear(hw, rcg->current_freq);
+}
+
+static void clk_rcg2_shared_disable(struct clk_hw *hw)
+{
+	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+	struct freq_tbl safe_src_freq_tbl = { 0 };
+
+	safe_src_freq_tbl.src = rcg->safe_src_index;
+	safe_src_freq_tbl.freq = clk_rcg2_get_safe_src_rate(hw);
+
+	/*
+	 * Park the RCG at a safe configuration - sourced off from safe source.
+	 * Force enable and disable the RCG while configuring it to safeguard
+	 * against any update signal coming from the downstream clock.
+	 * The current parent is still prepared and enabled at this point, and
+	 * the safe source is always on while application processor subsystem
+	 * is online. Therefore, the RCG can safely switch its parent.
+	 */
+	clk_rcg2_set_force_enable(hw);
+	clk_rcg2_configure(rcg, &safe_src_freq_tbl);
+	clk_rcg2_clear_force_enable(hw);
+}
+
+const struct clk_ops clk_rcg2_shared_ops = {
+	.enable = clk_rcg2_shared_enable,
+	.disable = clk_rcg2_shared_disable,
+	.get_parent = clk_rcg2_get_parent,
+	.set_parent = clk_rcg2_set_parent,
+	.recalc_rate = clk_rcg2_shared_recalc_rate,
+	.determine_rate = clk_rcg2_determine_rate,
+	.set_rate = clk_rcg2_shared_set_rate,
+	.set_rate_and_parent = clk_rcg2_shared_set_rate_and_parent,
+};
+EXPORT_SYMBOL_GPL(clk_rcg2_shared_ops);