Re: [PATCH v2 2/4] clk: qcom: Configure the RCGs to a safe source as needed

[Amit Nischal]

On 2018-03-20 06:00, Stephen Boyd wrote:

Quoting Amit Nischal (2018-03-07 23:18:13)
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 
actual

downstream clocks can be left enabled before.


s/actual/reality?

Thanks for the review. I will address this in next patch series.





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 actual 
RCG


s/actual/reality?


I will address this in next patch series.


is on and it expects its new source to alredy in enable state  but in


s/alredy/already be/

I will address this in next patch series.



reality new source is in off state. In order to avoid letting the RCG 
to


s/is in off state/is off/
s/letting/having/

I will address this in next patch series.



go into an invalid state, add support to just cache the rate of RCG 
during


s/just//

I will address this in next patch series.




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 
Signed-off-by: Amit Nischal 
---
diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index 2a7489a..205fa34 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -146,6 +146,9 @@ struct clk_dyn_rcg {
  * @hid_width: number of bits in half integer divider
  * @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
+ * @safe_src_freq_tbl : frequency table of safe source when using 
branches

+ * with shared RCGs
  * @clkr: regmap clock handle
  *
  */
@@ -155,6 +158,8 @@ struct clk_rcg2 {
u8  hid_width;
const struct parent_map *parent_map;
const struct freq_tbl   *freq_tbl;
+   unsigned long   current_freq;
+   const struct freq_tbl   *safe_src_freq_tbl;


Can we store safe_src index instead? And then construct the frequency
table entry on the fly at the call site? I think it would greatly
clarify that we don't really care about the rate of the clk at all.
Instead, all we care about is making sure the mux is set to whatever
source selection can provide an always on signal.


We will address the above in the V3 series of the patch set. We will be
generating the safe frequency table on the fly taking the safe source
frequency index input from the RCG clock.


struct clk_regmap   clkr;
 };

@@ -167,5 +172,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 e63db10..a52de54 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -790,3 +790,158 @@ static int clk_gfx3d_set_rate(struct clk_hw *hw, 
unsigned long rate,

.determine_rate = clk_gfx3d_determine_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)) {
+   if (!rcg->current_freq) {
+   if (!clk_rcg2_get_parent(hw))


This is checking for 0 source selection of parent? So basically if
source is XO selected then return what we know is the XO speed? We
should be able to do that by returning parent_rate though?


+   rcg->current_freq =
+   rcg->safe_src_freq_tbl->freq;
+   else
+   rcg->current_freq =
+   clk_rcg2_recalc_rate(hw, 
parent_rate);

+   }
+
+   return rcg->current_freq;
+   }
+
+   return rcg->current_freq = clk_rcg2_recalc_rate(hw, 
parent_rate);


I don't get this function.

To simplify just return cached rate if it's set and clk is off,
otherwise read the hardware?

if (!__clk_is_enabled(hw->clk) && rcg->current_freq)
return rcg->current_freq;

return clk_rcg2_recalc_rate(hw, 

Re: [PATCH v2 2/4] clk: qcom: Configure the RCGs to a safe source as needed

[Amit Nischal]

On 2018-03-20 06:00, Stephen Boyd wrote:

Quoting Amit Nischal (2018-03-07 23:18:13)
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 
actual

downstream clocks can be left enabled before.


s/actual/reality?

Thanks for the review. I will address this in next patch series.





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 actual 
RCG


s/actual/reality?


I will address this in next patch series.


is on and it expects its new source to alredy in enable state  but in


s/alredy/already be/

I will address this in next patch series.



reality new source is in off state. In order to avoid letting the RCG 
to


s/is in off state/is off/
s/letting/having/

I will address this in next patch series.



go into an invalid state, add support to just cache the rate of RCG 
during


s/just//

I will address this in next patch series.




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 
Signed-off-by: Amit Nischal 
---
diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index 2a7489a..205fa34 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -146,6 +146,9 @@ struct clk_dyn_rcg {
  * @hid_width: number of bits in half integer divider
  * @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
+ * @safe_src_freq_tbl : frequency table of safe source when using 
branches

+ * with shared RCGs
  * @clkr: regmap clock handle
  *
  */
@@ -155,6 +158,8 @@ struct clk_rcg2 {
u8  hid_width;
const struct parent_map *parent_map;
const struct freq_tbl   *freq_tbl;
+   unsigned long   current_freq;
+   const struct freq_tbl   *safe_src_freq_tbl;


Can we store safe_src index instead? And then construct the frequency
table entry on the fly at the call site? I think it would greatly
clarify that we don't really care about the rate of the clk at all.
Instead, all we care about is making sure the mux is set to whatever
source selection can provide an always on signal.


We will address the above in the V3 series of the patch set. We will be
generating the safe frequency table on the fly taking the safe source
frequency index input from the RCG clock.


struct clk_regmap   clkr;
 };

@@ -167,5 +172,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 e63db10..a52de54 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -790,3 +790,158 @@ static int clk_gfx3d_set_rate(struct clk_hw *hw, 
unsigned long rate,

.determine_rate = clk_gfx3d_determine_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)) {
+   if (!rcg->current_freq) {
+   if (!clk_rcg2_get_parent(hw))


This is checking for 0 source selection of parent? So basically if
source is XO selected then return what we know is the XO speed? We
should be able to do that by returning parent_rate though?


+   rcg->current_freq =
+   rcg->safe_src_freq_tbl->freq;
+   else
+   rcg->current_freq =
+   clk_rcg2_recalc_rate(hw, 
parent_rate);

+   }
+
+   return rcg->current_freq;
+   }
+
+   return rcg->current_freq = clk_rcg2_recalc_rate(hw, 
parent_rate);


I don't get this function.

To simplify just return cached rate if it's set and clk is off,
otherwise read the hardware?

if (!__clk_is_enabled(hw->clk) && rcg->current_freq)
return rcg->current_freq;

return clk_rcg2_recalc_rate(hw, parent_rate);

I would also rename current_freq to 

Re: [PATCH v2 2/4] clk: qcom: Configure the RCGs to a safe source as needed

[Stephen Boyd]

Quoting Amit Nischal (2018-03-07 23:18:13)
> 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 actual
> downstream clocks can be left enabled before.

s/actual/reality?

> 
> 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 actual RCG

s/actual/reality?

> is on and it expects its new source to alredy in enable state  but in

s/alredy/already be/

> reality new source is in off state. In order to avoid letting the RCG to

s/is in off state/is off/
s/letting/having/

> go into an invalid state, add support to just cache the rate of RCG during

s/just//

> 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 
> Signed-off-by: Amit Nischal 
> ---
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index 2a7489a..205fa34 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -146,6 +146,9 @@ struct clk_dyn_rcg {
>   * @hid_width: number of bits in half integer divider
>   * @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
> + * @safe_src_freq_tbl : frequency table of safe source when using branches
> + * with shared RCGs
>   * @clkr: regmap clock handle
>   *
>   */
> @@ -155,6 +158,8 @@ struct clk_rcg2 {
> u8  hid_width;
> const struct parent_map *parent_map;
> const struct freq_tbl   *freq_tbl;
> +   unsigned long   current_freq;
> +   const struct freq_tbl   *safe_src_freq_tbl;

Can we store safe_src index instead? And then construct the frequency
table entry on the fly at the call site? I think it would greatly
clarify that we don't really care about the rate of the clk at all.
Instead, all we care about is making sure the mux is set to whatever
source selection can provide an always on signal.

> struct clk_regmap   clkr;
>  };
> 
> @@ -167,5 +172,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 e63db10..a52de54 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -790,3 +790,158 @@ static int clk_gfx3d_set_rate(struct clk_hw *hw, 
> unsigned long rate,
> .determine_rate = clk_gfx3d_determine_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)) {
> +   if (!rcg->current_freq) {
> +   if (!clk_rcg2_get_parent(hw))

This is checking for 0 source selection of parent? So basically if
source is XO selected then return what we know is the XO speed? We
should be able to do that by returning parent_rate though?

> +   rcg->current_freq =
> +   rcg->safe_src_freq_tbl->freq;
> +   else
> +   rcg->current_freq =
> +   clk_rcg2_recalc_rate(hw, parent_rate);
> +   }
> +
> +   return rcg->current_freq;
> +   }
> +
> +   return rcg->current_freq = clk_rcg2_recalc_rate(hw, parent_rate);

I don't get this function.

To simplify just return cached rate if it's set and clk is off,
otherwise read the hardware?

if (!__clk_is_enabled(hw->clk) && rcg->current_freq)
return rcg->current_freq;

return clk_rcg2_recalc_rate(hw, parent_rate);

I would also rename current_freq to something like cached_freq and then
*only* assign it when someone calls clk_set_rate() (and save around the
frequency table pointer instead of raw frequency). Out of boot, we will
return the right frequency and clk_set_rate() called before enable will
cache the rate to what we want.

> +}
> +
> +static 

Re: [PATCH v2 2/4] clk: qcom: Configure the RCGs to a safe source as needed

[Stephen Boyd]

Quoting Amit Nischal (2018-03-07 23:18:13)
> 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 actual
> downstream clocks can be left enabled before.

s/actual/reality?

> 
> 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 actual RCG

s/actual/reality?

> is on and it expects its new source to alredy in enable state  but in

s/alredy/already be/

> reality new source is in off state. In order to avoid letting the RCG to

s/is in off state/is off/
s/letting/having/

> go into an invalid state, add support to just cache the rate of RCG during

s/just//

> 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 
> Signed-off-by: Amit Nischal 
> ---
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index 2a7489a..205fa34 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -146,6 +146,9 @@ struct clk_dyn_rcg {
>   * @hid_width: number of bits in half integer divider
>   * @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
> + * @safe_src_freq_tbl : frequency table of safe source when using branches
> + * with shared RCGs
>   * @clkr: regmap clock handle
>   *
>   */
> @@ -155,6 +158,8 @@ struct clk_rcg2 {
> u8  hid_width;
> const struct parent_map *parent_map;
> const struct freq_tbl   *freq_tbl;
> +   unsigned long   current_freq;
> +   const struct freq_tbl   *safe_src_freq_tbl;

Can we store safe_src index instead? And then construct the frequency
table entry on the fly at the call site? I think it would greatly
clarify that we don't really care about the rate of the clk at all.
Instead, all we care about is making sure the mux is set to whatever
source selection can provide an always on signal.

> struct clk_regmap   clkr;
>  };
> 
> @@ -167,5 +172,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 e63db10..a52de54 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -790,3 +790,158 @@ static int clk_gfx3d_set_rate(struct clk_hw *hw, 
> unsigned long rate,
> .determine_rate = clk_gfx3d_determine_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)) {
> +   if (!rcg->current_freq) {
> +   if (!clk_rcg2_get_parent(hw))

This is checking for 0 source selection of parent? So basically if
source is XO selected then return what we know is the XO speed? We
should be able to do that by returning parent_rate though?

> +   rcg->current_freq =
> +   rcg->safe_src_freq_tbl->freq;
> +   else
> +   rcg->current_freq =
> +   clk_rcg2_recalc_rate(hw, parent_rate);
> +   }
> +
> +   return rcg->current_freq;
> +   }
> +
> +   return rcg->current_freq = clk_rcg2_recalc_rate(hw, parent_rate);

I don't get this function.

To simplify just return cached rate if it's set and clk is off,
otherwise read the hardware?

if (!__clk_is_enabled(hw->clk) && rcg->current_freq)
return rcg->current_freq;

return clk_rcg2_recalc_rate(hw, parent_rate);

I would also rename current_freq to something like cached_freq and then
*only* assign it when someone calls clk_set_rate() (and save around the
frequency table pointer instead of raw frequency). Out of boot, we will
return the right frequency and clk_set_rate() called before enable will
cache the rate to what we want.

> +}
> +
> +static int clk_rcg2_shared_enable(struct clk_hw *hw)
> 

[PATCH v2 2/4] clk: qcom: Configure the RCGs to a safe source as needed

[Amit Nischal]

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 actual
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 actual RCG
is on and it expects its new source to alredy in enable state  but in
reality new source is in off state. In order to avoid letting the RCG to
go into an invalid state, add support to just 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 
Signed-off-by: Amit Nischal 
---
 drivers/clk/qcom/clk-rcg.h  |   8 ++-
 drivers/clk/qcom/clk-rcg2.c | 155 
 2 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index 2a7489a..205fa34 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
@@ -146,6 +146,9 @@ struct clk_dyn_rcg {
  * @hid_width: number of bits in half integer divider
  * @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
+ * @safe_src_freq_tbl : frequency table of safe source when using branches
+ * with shared RCGs
  * @clkr: regmap clock handle
  *
  */
@@ -155,6 +158,8 @@ struct clk_rcg2 {
u8  hid_width;
const struct parent_map *parent_map;
const struct freq_tbl   *freq_tbl;
+   unsigned long   current_freq;
+   const struct freq_tbl   *safe_src_freq_tbl;
struct clk_regmap   clkr;
 };

@@ -167,5 +172,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 e63db10..a52de54 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -790,3 +790,158 @@ 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;
+
+   /*

[PATCH v2 2/4] clk: qcom: Configure the RCGs to a safe source as needed

[Amit Nischal]

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 actual
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 actual RCG
is on and it expects its new source to alredy in enable state  but in
reality new source is in off state. In order to avoid letting the RCG to
go into an invalid state, add support to just 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 
Signed-off-by: Amit Nischal 
---
 drivers/clk/qcom/clk-rcg.h  |   8 ++-
 drivers/clk/qcom/clk-rcg2.c | 155 
 2 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index 2a7489a..205fa34 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
@@ -146,6 +146,9 @@ struct clk_dyn_rcg {
  * @hid_width: number of bits in half integer divider
  * @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
+ * @safe_src_freq_tbl : frequency table of safe source when using branches
+ * with shared RCGs
  * @clkr: regmap clock handle
  *
  */
@@ -155,6 +158,8 @@ struct clk_rcg2 {
u8  hid_width;
const struct parent_map *parent_map;
const struct freq_tbl   *freq_tbl;
+   unsigned long   current_freq;
+   const struct freq_tbl   *safe_src_freq_tbl;
struct clk_regmap   clkr;
 };

@@ -167,5 +172,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 e63db10..a52de54 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -790,3 +790,158 @@ 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