[PATCH v1 0/3] clk: qcom: Misc updates for Root Clock Generators

[PATCH v1 0/3] clk: qcom: Misc updates for Root Clock Generators

[Taniya Das]

This patch adds support for the below

1) There could be failure while updating the RCG and not returning the
   failure could cause the consumer to assume the clock update is a
   success and not handling the failure gracefully.
2) There are few clocks in certain clock controllers which might require
   the hardware control mode to be enabled explicitly.
3) Update the DFS macro as per the hardware plans.

Taniya Das (3):
  clk: qcom: rcg: Return failure for RCG update
  clk: qcom: rcg2: Add support for hardware control mode
  clk: qcom: rcg: update the DFS macro for RCG

 drivers/clk/qcom/clk-rcg.h    |  5 ++-
 drivers/clk/qcom/clk-rcg2.c   |  5 ++-
 drivers/clk/qcom/gcc-sdm845.c | 96 +++++++++++++++++++++----------------------
 3 files changed, 56 insertions(+), 50 deletions(-)

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

[PATCH v1 1/3] clk: qcom: rcg: Return failure for RCG update

[Taniya Das]

In case of update config failure, return -EBUSY, so that consumers could
handle the failure gracefully.

Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 drivers/clk/qcom/clk-rcg2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index 8c02bff..57dbac9 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -119,7 +119,7 @@ static int update_config(struct clk_rcg2 *rcg)
 	}

 	WARN(1, "%s: rcg didn't update its configuration.", name);
-	return 0;
+	return -EBUSY;
 }

 static int clk_rcg2_set_parent(struct clk_hw *hw, u8 index)
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the  Linux Foundation.

Re: [PATCH v1 1/3] clk: qcom: rcg: Return failure for RCG update

[Stephen Boyd]

Quoting Taniya Das (2019-05-08 11:24:53)
> In case of update config failure, return -EBUSY, so that consumers could
> handle the failure gracefully.
> 
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> ---

Applied to clk-next

[PATCH v1 2/3] clk: qcom: rcg2: Add support for hardware control mode

[Taniya Das]

Add a flag to indicate to support and enable hardware control mode
of an RCG.

Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 drivers/clk/qcom/clk-rcg.h  | 3 +++
 drivers/clk/qcom/clk-rcg2.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index c25b57c..5562f38 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -139,6 +139,7 @@ struct clk_dyn_rcg {
  * @freq_tbl: frequency table
  * @clkr: regmap clock handle
  * @cfg_off: defines the cfg register offset from the CMD_RCGR + CFG_REG
+ * @flags: additional flag parameters for the RCG
  */
 struct clk_rcg2 {
 	u32			cmd_rcgr;
@@ -149,6 +150,8 @@ struct clk_rcg2 {
 	const struct freq_tbl	*freq_tbl;
 	struct clk_regmap	clkr;
 	u8			cfg_off;
+	u8			flags;
+#define HW_CLK_CTRL_MODE	BIT(0)
 };

 #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index 57dbac9..5bb6d45 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -289,6 +289,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
 	cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
 	if (rcg->mnd_width && f->n && (f->m != f->n))
 		cfg |= CFG_MODE_DUAL_EDGE;
+	if (rcg->flags & HW_CLK_CTRL_MODE)
+		cfg |= CFG_HW_CLK_CTRL_MASK;
+
 	return regmap_update_bits(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg),
 					mask, cfg);
 }
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the  Linux Foundation.

Re: [PATCH v1 2/3] clk: qcom: rcg2: Add support for hardware control mode

[Stephen Boyd]

Quoting Taniya Das (2019-05-08 11:24:54)
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 57dbac9..5bb6d45 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -289,6 +289,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
>         cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
>         if (rcg->mnd_width && f->n && (f->m != f->n))
>                 cfg |= CFG_MODE_DUAL_EDGE;
> +       if (rcg->flags & HW_CLK_CTRL_MODE)
> +               cfg |= CFG_HW_CLK_CTRL_MASK;
> +

Above this we have commit bdc3bbdd40ba ("clk: qcom: Clear hardware clock
control bit of RCG") that clears this bit. Is it possible to always set
this bit and then have an override flag used in sdm845 that says to
_not_ set this bit? Presumably on earlier platforms writing the bit is a
no-op so it's safe to write the bit on those platforms.

This way, if it's going to be the default we can avoid setting the flag
and only set the flag on older platforms where it shouldn't be done for
some reason.

>         return regmap_update_bits(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg),
>                                         mask, cfg);
>  }
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
> of the Code Aurora Forum, hosted by the  Linux Foundation.
> 

Re: [PATCH v1 2/3] clk: qcom: rcg2: Add support for hardware control mode

[Taniya Das]

Hello Stephen,

Thanks for your review.

On 7/16/2019 4:22 AM, Stephen Boyd wrote:
> Quoting Taniya Das (2019-05-08 11:24:54)
>> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
>> index 57dbac9..5bb6d45 100644
>> --- a/drivers/clk/qcom/clk-rcg2.c
>> +++ b/drivers/clk/qcom/clk-rcg2.c
>> @@ -289,6 +289,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
>>          cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
>>          if (rcg->mnd_width && f->n && (f->m != f->n))
>>                  cfg |= CFG_MODE_DUAL_EDGE;
>> +       if (rcg->flags & HW_CLK_CTRL_MODE)
>> +               cfg |= CFG_HW_CLK_CTRL_MASK;
>> +
> 
> Above this we have commit bdc3bbdd40ba ("clk: qcom: Clear hardware clock
> control bit of RCG") that clears this bit. Is it possible to always set
> this bit and then have an override flag used in sdm845 that says to
> _not_ set this bit? Presumably on earlier platforms writing the bit is a
> no-op so it's safe to write the bit on those platforms.
> 
> This way, if it's going to be the default we can avoid setting the flag
> and only set the flag on older platforms where it shouldn't be done for
> some reason.
> 

Not all the subsystem clock controllers might have this hardware control
bit set from design. Thus we want to set them based on the flag.

>>          return regmap_update_bits(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg),
>>                                          mask, cfg);
>>   }
>> --
>> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
>> of the Code Aurora Forum, hosted by the  Linux Foundation.
>>

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

--

Re: [PATCH v1 2/3] clk: qcom: rcg2: Add support for hardware control mode

[Stephen Boyd]

Quoting Taniya Das (2019-07-15 21:19:02)
> Hello Stephen,
> 
> Thanks for your review.
> 
> On 7/16/2019 4:22 AM, Stephen Boyd wrote:
> > Quoting Taniya Das (2019-05-08 11:24:54)
> >> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> >> index 57dbac9..5bb6d45 100644
> >> --- a/drivers/clk/qcom/clk-rcg2.c
> >> +++ b/drivers/clk/qcom/clk-rcg2.c
> >> @@ -289,6 +289,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> >>          cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
> >>          if (rcg->mnd_width && f->n && (f->m != f->n))
> >>                  cfg |= CFG_MODE_DUAL_EDGE;
> >> +       if (rcg->flags & HW_CLK_CTRL_MODE)
> >> +               cfg |= CFG_HW_CLK_CTRL_MASK;
> >> +
> > 
> > Above this we have commit bdc3bbdd40ba ("clk: qcom: Clear hardware clock
> > control bit of RCG") that clears this bit. Is it possible to always set
> > this bit and then have an override flag used in sdm845 that says to
> > _not_ set this bit? Presumably on earlier platforms writing the bit is a
> > no-op so it's safe to write the bit on those platforms.
> > 
> > This way, if it's going to be the default we can avoid setting the flag
> > and only set the flag on older platforms where it shouldn't be done for
> > some reason.
> > 
> 
> Not all the subsystem clock controllers might have this hardware control
> bit set from design. Thus we want to set them based on the flag.

Yes but what's the percentage of clks that are going to set this flag
vs. not set this flag? If that is low right now then it's fine but if it
eventually becomes the standard mechanism it will be easier to opt-out
of the feature if necessary instead of opt-in.

Re: [PATCH v1 2/3] clk: qcom: rcg2: Add support for hardware control mode

[Taniya Das]

Hello Stephen,

On 7/17/2019 4:48 AM, Stephen Boyd wrote:
> Quoting Taniya Das (2019-07-15 21:19:02)
>> Hello Stephen,
>>
>> Thanks for your review.
>>
>> On 7/16/2019 4:22 AM, Stephen Boyd wrote:
>>> Quoting Taniya Das (2019-05-08 11:24:54)
>>>> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
>>>> index 57dbac9..5bb6d45 100644
>>>> --- a/drivers/clk/qcom/clk-rcg2.c
>>>> +++ b/drivers/clk/qcom/clk-rcg2.c
>>>> @@ -289,6 +289,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
>>>>           cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
>>>>           if (rcg->mnd_width && f->n && (f->m != f->n))
>>>>                   cfg |= CFG_MODE_DUAL_EDGE;
>>>> +       if (rcg->flags & HW_CLK_CTRL_MODE)
>>>> +               cfg |= CFG_HW_CLK_CTRL_MASK;
>>>> +
>>>
>>> Above this we have commit bdc3bbdd40ba ("clk: qcom: Clear hardware clock
>>> control bit of RCG") that clears this bit. Is it possible to always set
>>> this bit and then have an override flag used in sdm845 that says to
>>> _not_ set this bit? Presumably on earlier platforms writing the bit is a
>>> no-op so it's safe to write the bit on those platforms.
>>>
>>> This way, if it's going to be the default we can avoid setting the flag
>>> and only set the flag on older platforms where it shouldn't be done for
>>> some reason.
>>>
>>
>> Not all the subsystem clock controllers might have this hardware control
>> bit set from design. Thus we want to set them based on the flag.
> 
> Yes but what's the percentage of clks that are going to set this flag
> vs. not set this flag? If that is low right now then it's fine but if it
> eventually becomes the standard mechanism it will be easier to opt-out
> of the feature if necessary instead of opt-in.
> 

Currently all the RCGs in GCC need to clear the bit and few RCGs from
the other CCs(DISPCC/VIDEOCC) where the bit is not set from HW requires 
this bit to be set. Thus we want to use this flag mechanism to have the 
flexibility to set.

Once it is a standard we could cleanup to remove this.



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

--

Re: [PATCH v1 2/3] clk: qcom: rcg2: Add support for hardware control mode

[Stephen Boyd]

Quoting Taniya Das (2019-07-30 03:50:08)
> Hello Stephen,
> 
> On 7/17/2019 4:48 AM, Stephen Boyd wrote:
> > Quoting Taniya Das (2019-07-15 21:19:02)
> >> Hello Stephen,
> >>
> >> Thanks for your review.
> >>
> >> On 7/16/2019 4:22 AM, Stephen Boyd wrote:
> >>> Quoting Taniya Das (2019-05-08 11:24:54)
> >>>> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> >>>> index 57dbac9..5bb6d45 100644
> >>>> --- a/drivers/clk/qcom/clk-rcg2.c
> >>>> +++ b/drivers/clk/qcom/clk-rcg2.c
> >>>> @@ -289,6 +289,9 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> >>>>           cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
> >>>>           if (rcg->mnd_width && f->n && (f->m != f->n))
> >>>>                   cfg |= CFG_MODE_DUAL_EDGE;
> >>>> +       if (rcg->flags & HW_CLK_CTRL_MODE)
> >>>> +               cfg |= CFG_HW_CLK_CTRL_MASK;
> >>>> +
> >>>
> >>> Above this we have commit bdc3bbdd40ba ("clk: qcom: Clear hardware clock
> >>> control bit of RCG") that clears this bit. Is it possible to always set
> >>> this bit and then have an override flag used in sdm845 that says to
> >>> _not_ set this bit? Presumably on earlier platforms writing the bit is a
> >>> no-op so it's safe to write the bit on those platforms.
> >>>
> >>> This way, if it's going to be the default we can avoid setting the flag
> >>> and only set the flag on older platforms where it shouldn't be done for
> >>> some reason.
> >>>
> >>
> >> Not all the subsystem clock controllers might have this hardware control
> >> bit set from design. Thus we want to set them based on the flag.
> > 
> > Yes but what's the percentage of clks that are going to set this flag
> > vs. not set this flag? If that is low right now then it's fine but if it
> > eventually becomes the standard mechanism it will be easier to opt-out
> > of the feature if necessary instead of opt-in.
> > 
> 
> Currently all the RCGs in GCC need to clear the bit and few RCGs from
> the other CCs(DISPCC/VIDEOCC) where the bit is not set from HW requires 
> this bit to be set. Thus we want to use this flag mechanism to have the 
> flexibility to set.
> 
> Once it is a standard we could cleanup to remove this.
> 

OK. Please send this patch along with whatever code/driver needs the new
flag.

[PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Taniya Das]

Update the init data name for each of the dynamic frequency switch
controlled clock associated with the RCG clock name, so that it can be
generated as per the hardware plan. Thus update the macro accordingly.

Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 drivers/clk/qcom/clk-rcg.h    |  2 +-
 drivers/clk/qcom/gcc-sdm845.c | 96 +++++++++++++++++++++----------------------
 2 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index 5562f38..e40e8f8 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -171,7 +171,7 @@ struct clk_rcg_dfs_data {
 };

 #define DEFINE_RCG_DFS(r) \
-	{ .rcg = &r##_src, .init = &r##_init }
+	{ .rcg = &r, .init = &r##_init }

 extern int qcom_cc_register_rcg_dfs(struct regmap *regmap,
 				    const struct clk_rcg_dfs_data *rcgs,
diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
index 7131dcf..a76178b 100644
--- a/drivers/clk/qcom/gcc-sdm845.c
+++ b/drivers/clk/qcom/gcc-sdm845.c
@@ -408,7 +408,7 @@ enum {
 	{ }
 };

-static struct clk_init_data gcc_qupv3_wrap0_s0_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap0_s0_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s0_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -421,10 +421,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap0_s0_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap0_s0_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap0_s1_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap0_s1_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s1_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -437,10 +437,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap0_s1_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap0_s1_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap0_s2_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap0_s2_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s2_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -453,10 +453,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap0_s2_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap0_s2_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap0_s3_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap0_s3_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s3_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -469,10 +469,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap0_s3_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap0_s3_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap0_s4_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap0_s4_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s4_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -485,10 +485,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap0_s4_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap0_s4_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap0_s5_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap0_s5_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s5_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -501,10 +501,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap0_s5_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap0_s5_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap0_s6_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap0_s6_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s6_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -517,10 +517,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap0_s6_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap0_s6_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap0_s7_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap0_s7_clk_src_init = {
 	.name = "gcc_qupv3_wrap0_s7_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -533,10 +533,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap0_s7_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap0_s7_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap1_s0_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap1_s0_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s0_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -549,10 +549,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap1_s0_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap1_s0_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap1_s1_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap1_s1_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s1_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -565,10 +565,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap1_s1_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap1_s1_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap1_s2_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap1_s2_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s2_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -581,10 +581,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap1_s2_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap1_s2_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap1_s3_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap1_s3_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s3_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -597,10 +597,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap1_s3_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap1_s3_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap1_s4_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap1_s4_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s4_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -613,10 +613,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap1_s4_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap1_s4_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap1_s5_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap1_s5_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s5_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -629,10 +629,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap1_s5_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap1_s5_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap1_s6_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap1_s6_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s6_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -645,10 +645,10 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap1_s6_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap1_s6_clk_src_init,
 };

-static struct clk_init_data gcc_qupv3_wrap1_s7_clk_init = {
+static struct clk_init_data gcc_qupv3_wrap1_s7_clk_src_init = {
 	.name = "gcc_qupv3_wrap1_s7_clk_src",
 	.parent_names = gcc_parent_names_0,
 	.num_parents = 4,
@@ -661,7 +661,7 @@ enum {
 	.hid_width = 5,
 	.parent_map = gcc_parent_map_0,
 	.freq_tbl = ftbl_gcc_qupv3_wrap0_s0_clk_src,
-	.clkr.hw.init = &gcc_qupv3_wrap1_s7_clk_init,
+	.clkr.hw.init = &gcc_qupv3_wrap1_s7_clk_src_init,
 };

 static const struct freq_tbl ftbl_gcc_sdcc2_apps_clk_src[] = {
@@ -3577,22 +3577,22 @@ enum {
 MODULE_DEVICE_TABLE(of, gcc_sdm845_match_table);

 static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = {
-	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s0_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s1_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s2_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s3_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s4_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s5_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s6_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s7_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s0_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s1_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s2_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s3_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s4_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s5_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s6_clk),
-	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s7_clk),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s0_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s1_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s2_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s3_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s4_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s5_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s6_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap0_s7_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s0_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s1_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s2_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s3_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s4_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s5_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s6_clk_src),
+	DEFINE_RCG_DFS(gcc_qupv3_wrap1_s7_clk_src),
 };

 static int gcc_sdm845_probe(struct platform_device *pdev)
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the  Linux Foundation.

Re: [PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Stephen Boyd]

Quoting Taniya Das (2019-05-08 11:24:55)
> Update the init data name for each of the dynamic frequency switch
> controlled clock associated with the RCG clock name, so that it can be
> generated as per the hardware plan. Thus update the macro accordingly.
> 
> Signed-off-by: Taniya Das <tdas@codeaurora.org>

This patch doesn't make any sense to me.

> ---
>  drivers/clk/qcom/clk-rcg.h    |  2 +-
>  drivers/clk/qcom/gcc-sdm845.c | 96 +++++++++++++++++++++----------------------
>  2 files changed, 49 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index 5562f38..e40e8f8 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -171,7 +171,7 @@ struct clk_rcg_dfs_data {
>  };
> 
>  #define DEFINE_RCG_DFS(r) \
> -       { .rcg = &r##_src, .init = &r##_init }
> +       { .rcg = &r, .init = &r##_init }

Why do we need to rename the init data?

> 
>  extern int qcom_cc_register_rcg_dfs(struct regmap *regmap,
>                                     const struct clk_rcg_dfs_data *rcgs,
> diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
> index 7131dcf..a76178b 100644
> --- a/drivers/clk/qcom/gcc-sdm845.c
> +++ b/drivers/clk/qcom/gcc-sdm845.c
> @@ -408,7 +408,7 @@ enum {
>         { }
>  };
> 
> -static struct clk_init_data gcc_qupv3_wrap0_s0_clk_init = {
> +static struct clk_init_data gcc_qupv3_wrap0_s0_clk_src_init = {
>         .name = "gcc_qupv3_wrap0_s0_clk_src",
>         .parent_names = gcc_parent_names_0,
>         .num_parents = 4,
> @@ -3577,22 +3577,22 @@ enum {
>  MODULE_DEVICE_TABLE(of, gcc_sdm845_match_table);
> 
>  static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = {
> -       DEFINE_RCG_DFS(gcc_qupv3_wrap0_s0_clk),
> +       DEFINE_RCG_DFS(gcc_qupv3_wrap0_s0_clk_src),

I've trimmed the above to try and see what's changed but it doesn't make
sense still.

Re: [PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Taniya Das]

Hello Stephen,

Thanks for the review.

On 5/9/2019 10:57 PM, Stephen Boyd wrote:
> Quoting Taniya Das (2019-05-08 11:24:55)
>> Update the init data name for each of the dynamic frequency switch
>> controlled clock associated with the RCG clock name, so that it can be
>> generated as per the hardware plan. Thus update the macro accordingly.
>>
>> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> 
> This patch doesn't make any sense to me.
> 
>> ---
>>   drivers/clk/qcom/clk-rcg.h    |  2 +-
>>   drivers/clk/qcom/gcc-sdm845.c | 96 +++++++++++++++++++++----------------------
>>   2 files changed, 49 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
>> index 5562f38..e40e8f8 100644
>> --- a/drivers/clk/qcom/clk-rcg.h
>> +++ b/drivers/clk/qcom/clk-rcg.h
>> @@ -171,7 +171,7 @@ struct clk_rcg_dfs_data {
>>   };
>>
>>   #define DEFINE_RCG_DFS(r) \
>> -       { .rcg = &r##_src, .init = &r##_init }
>> +       { .rcg = &r, .init = &r##_init }
> 
> Why do we need to rename the init data?
> 

We want to manage the init data as the clock source name, so that we 
could manage to auto generate our code. So that we do not have to 
re-name the clock init data manually if the DFS source names gets 
updated at any point of time.

>>
>>   extern int qcom_cc_register_rcg_dfs(struct regmap *regmap,
>>                                      const struct clk_rcg_dfs_data *rcgs,
>> diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
>> index 7131dcf..a76178b 100644
>> --- a/drivers/clk/qcom/gcc-sdm845.c
>> +++ b/drivers/clk/qcom/gcc-sdm845.c
>> @@ -408,7 +408,7 @@ enum {
>>          { }
>>   };
>>
>> -static struct clk_init_data gcc_qupv3_wrap0_s0_clk_init = {
>> +static struct clk_init_data gcc_qupv3_wrap0_s0_clk_src_init = {
>>          .name = "gcc_qupv3_wrap0_s0_clk_src",
>>          .parent_names = gcc_parent_names_0,
>>          .num_parents = 4,
>> @@ -3577,22 +3577,22 @@ enum {
>>   MODULE_DEVICE_TABLE(of, gcc_sdm845_match_table);
>>
>>   static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = {
>> -       DEFINE_RCG_DFS(gcc_qupv3_wrap0_s0_clk),
>> +       DEFINE_RCG_DFS(gcc_qupv3_wrap0_s0_clk_src),
> 
> I've trimmed the above to try and see what's changed but it doesn't make
> sense still.
> 

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

--

Re: [PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Stephen Boyd]

Quoting Taniya Das (2019-05-09 19:58:39)
> Hello Stephen,
> 
> Thanks for the review.
> 
> On 5/9/2019 10:57 PM, Stephen Boyd wrote:
> > Quoting Taniya Das (2019-05-08 11:24:55)
> >> Update the init data name for each of the dynamic frequency switch
> >> controlled clock associated with the RCG clock name, so that it can be
> >> generated as per the hardware plan. Thus update the macro accordingly.
> >>
> >> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> > 
> > This patch doesn't make any sense to me.
> > 
> >> ---
> >>   drivers/clk/qcom/clk-rcg.h    |  2 +-
> >>   drivers/clk/qcom/gcc-sdm845.c | 96 +++++++++++++++++++++----------------------
> >>   2 files changed, 49 insertions(+), 49 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> >> index 5562f38..e40e8f8 100644
> >> --- a/drivers/clk/qcom/clk-rcg.h
> >> +++ b/drivers/clk/qcom/clk-rcg.h
> >> @@ -171,7 +171,7 @@ struct clk_rcg_dfs_data {
> >>   };
> >>
> >>   #define DEFINE_RCG_DFS(r) \
> >> -       { .rcg = &r##_src, .init = &r##_init }
> >> +       { .rcg = &r, .init = &r##_init }
> > 
> > Why do we need to rename the init data?
> > 
> 
> We want to manage the init data as the clock source name, so that we 
> could manage to auto generate our code. So that we do not have to 
> re-name the clock init data manually if the DFS source names gets 
> updated at any point of time.
> 

Why is the clk name changing to not have a _src after the "root" of the
clk name? As long as I can remember, RCGs have a "_src" postfix.

Re: [PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Taniya Das]

Hello Stephen,

On 5/10/2019 11:24 PM, Stephen Boyd wrote:
> Quoting Taniya Das (2019-05-09 19:58:39)
>> Hello Stephen,
>>
>> Thanks for the review.
>>
>> On 5/9/2019 10:57 PM, Stephen Boyd wrote:
>>> Quoting Taniya Das (2019-05-08 11:24:55)
>>>> Update the init data name for each of the dynamic frequency switch
>>>> controlled clock associated with the RCG clock name, so that it can be
>>>> generated as per the hardware plan. Thus update the macro accordingly.
>>>>
>>>> Signed-off-by: Taniya Das <tdas@codeaurora.org>
>>>
>>> This patch doesn't make any sense to me.
>>>
>>>> ---
>>>>    drivers/clk/qcom/clk-rcg.h    |  2 +-
>>>>    drivers/clk/qcom/gcc-sdm845.c | 96 +++++++++++++++++++++----------------------
>>>>    2 files changed, 49 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
>>>> index 5562f38..e40e8f8 100644
>>>> --- a/drivers/clk/qcom/clk-rcg.h
>>>> +++ b/drivers/clk/qcom/clk-rcg.h
>>>> @@ -171,7 +171,7 @@ struct clk_rcg_dfs_data {
>>>>    };
>>>>
>>>>    #define DEFINE_RCG_DFS(r) \
>>>> -       { .rcg = &r##_src, .init = &r##_init }
>>>> +       { .rcg = &r, .init = &r##_init }
>>>
>>> Why do we need to rename the init data?
>>>
>>
>> We want to manage the init data as the clock source name, so that we
>> could manage to auto generate our code. So that we do not have to
>> re-name the clock init data manually if the DFS source names gets
>> updated at any point of time.
>>
> 
> Why is the clk name changing to not have a _src after the "root" of the
> clk name? As long as I can remember, RCGs have a "_src" postfix.
> 

Yes, the RCGs would have _src, so we do want the init data also to be
generated with _src postfix. So that we do not have to manually clean up 
the generated code.

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

--

Re: [PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Stephen Boyd]

Quoting Taniya Das (2019-05-12 20:44:46)
> On 5/10/2019 11:24 PM, Stephen Boyd wrote:
> >>>> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> >>>> index 5562f38..e40e8f8 100644
> >>>> --- a/drivers/clk/qcom/clk-rcg.h
> >>>> +++ b/drivers/clk/qcom/clk-rcg.h
> >>>> @@ -171,7 +171,7 @@ struct clk_rcg_dfs_data {
> >>>>    };
> >>>>
> >>>>    #define DEFINE_RCG_DFS(r) \
> >>>> -       { .rcg = &r##_src, .init = &r##_init }
> >>>> +       { .rcg = &r, .init = &r##_init }
> >>>
> >>> Why do we need to rename the init data?
> >>>
> >>
> >> We want to manage the init data as the clock source name, so that we
> >> could manage to auto generate our code. So that we do not have to
> >> re-name the clock init data manually if the DFS source names gets
> >> updated at any point of time.
> >>
> > 
> > Why is the clk name changing to not have a _src after the "root" of the
> > clk name? As long as I can remember, RCGs have a "_src" postfix.
> > 
> 
> Yes, the RCGs would have _src, so we do want the init data also to be
> generated with _src postfix. So that we do not have to manually clean up 
> the generated code.
> 

Please manually cleanup the generated code, or fix the code
generator to do what you want.

Re: [PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Taniya Das]

Hello Stephen,

Thanks for the review.

On 7/16/2019 4:14 AM, Stephen Boyd wrote:
> Quoting Taniya Das (2019-05-12 20:44:46)
>> On 5/10/2019 11:24 PM, Stephen Boyd wrote:
>>>>>> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
>>>>>> index 5562f38..e40e8f8 100644
>>>>>> --- a/drivers/clk/qcom/clk-rcg.h
>>>>>> +++ b/drivers/clk/qcom/clk-rcg.h
>>>>>> @@ -171,7 +171,7 @@ struct clk_rcg_dfs_data {
>>>>>>     };
>>>>>>
>>>>>>     #define DEFINE_RCG_DFS(r) \
>>>>>> -       { .rcg = &r##_src, .init = &r##_init }
>>>>>> +       { .rcg = &r, .init = &r##_init }
>>>>>
>>>>> Why do we need to rename the init data?
>>>>>
>>>>
>>>> We want to manage the init data as the clock source name, so that we
>>>> could manage to auto generate our code. So that we do not have to
>>>> re-name the clock init data manually if the DFS source names gets
>>>> updated at any point of time.
>>>>
>>>
>>> Why is the clk name changing to not have a _src after the "root" of the
>>> clk name? As long as I can remember, RCGs have a "_src" postfix.
>>>
>>
>> Yes, the RCGs would have _src, so we do want the init data also to be
>> generated with _src postfix. So that we do not have to manually clean up
>> the generated code.
>>
> 
> Please manually cleanup the generated code, or fix the code
> generator to do what you want.
> 

Fixing the code manually is not what we intend to do and it is time 
consuming with too many DFS controlled clocks. This really helps us 
align to internal code.

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

--

Re: [PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Stephen Boyd]

Quoting Taniya Das (2019-07-15 21:22:02)
> Hello Stephen,
> 
> Thanks for the review.
> 
> On 7/16/2019 4:14 AM, Stephen Boyd wrote:
> > Quoting Taniya Das (2019-05-12 20:44:46)
> >> On 5/10/2019 11:24 PM, Stephen Boyd wrote:
> >>> Why is the clk name changing to not have a _src after the "root" of the
> >>> clk name? As long as I can remember, RCGs have a "_src" postfix.
> >>>
> >>
> >> Yes, the RCGs would have _src, so we do want the init data also to be
> >> generated with _src postfix. So that we do not have to manually clean up
> >> the generated code.
> >>
> > 
> > Please manually cleanup the generated code, or fix the code
> > generator to do what you want.
> > 
> 
> Fixing the code manually is not what we intend to do and it is time 
> consuming with too many DFS controlled clocks. This really helps us 
> align to internal code.
> 

And you can't fix the code generator to drop the _src part of whatever
is spit out for the DFS lines?

Re: [PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Taniya Das]

On 7/17/2019 4:52 AM, Stephen Boyd wrote:
> Quoting Taniya Das (2019-07-15 21:22:02)
>> Hello Stephen,
>>
>> Thanks for the review.
>>
>> On 7/16/2019 4:14 AM, Stephen Boyd wrote:
>>> Quoting Taniya Das (2019-05-12 20:44:46)
>>>> On 5/10/2019 11:24 PM, Stephen Boyd wrote:
>>>>> Why is the clk name changing to not have a _src after the "root" of the
>>>>> clk name? As long as I can remember, RCGs have a "_src" postfix.
>>>>>
>>>>
>>>> Yes, the RCGs would have _src, so we do want the init data also to be
>>>> generated with _src postfix. So that we do not have to manually clean up
>>>> the generated code.
>>>>
>>>
>>> Please manually cleanup the generated code, or fix the code
>>> generator to do what you want.
>>>
>>
>> Fixing the code manually is not what we intend to do and it is time
>> consuming with too many DFS controlled clocks. This really helps us
>> align to internal code.
>>
> 
> And you can't fix the code generator to drop the _src part of whatever
> is spit out for the DFS lines?
> 

Sure, will drop this.

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

--

Re: [PATCH v1 3/3] clk: qcom: rcg: update the DFS macro for RCG

[Stephen Boyd]

Quoting Taniya Das (2019-07-30 03:51:07)
> 
> 
> On 7/17/2019 4:52 AM, Stephen Boyd wrote:
> > Quoting Taniya Das (2019-07-15 21:22:02)
> >> Hello Stephen,
> >>
> >> Thanks for the review.
> >>
> >> On 7/16/2019 4:14 AM, Stephen Boyd wrote:
> >>> Quoting Taniya Das (2019-05-12 20:44:46)
> >>>> On 5/10/2019 11:24 PM, Stephen Boyd wrote:
> >>>>> Why is the clk name changing to not have a _src after the "root" of the
> >>>>> clk name? As long as I can remember, RCGs have a "_src" postfix.
> >>>>>
> >>>>
> >>>> Yes, the RCGs would have _src, so we do want the init data also to be
> >>>> generated with _src postfix. So that we do not have to manually clean up
> >>>> the generated code.
> >>>>
> >>>
> >>> Please manually cleanup the generated code, or fix the code
> >>> generator to do what you want.
> >>>
> >>
> >> Fixing the code manually is not what we intend to do and it is time
> >> consuming with too many DFS controlled clocks. This really helps us
> >> align to internal code.
> >>
> > 
> > And you can't fix the code generator to drop the _src part of whatever
> > is spit out for the DFS lines?
> > 
> 
> Sure, will drop this.
> 

Actually, I'm OK with this patch, but I'd like to see it in a larger
series that introduces another clk driver using this macro. The reason I
like it is that I can search for the same string name and find the clk
that has DFS enabled on it, instead of finding the branch which doesn't
have DFS. Sorry for the back and forth, I got confused about what was
going on.