linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Add read-only RCG2 ops
@ 2023-06-12  9:22 Konrad Dybcio
  2023-06-12  9:22 ` [PATCH 1/2] clk: qcom: rcg2: Introduce " Konrad Dybcio
  2023-06-12  9:22 ` [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks Konrad Dybcio
  0 siblings, 2 replies; 12+ messages in thread
From: Konrad Dybcio @ 2023-06-12  9:22 UTC (permalink / raw)
  To: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd
  Cc: Marijn Suijten, linux-arm-msm, linux-clk, linux-kernel,
	Stephan Gerhold, Konrad Dybcio

Some of the clocks are managed by remote cores and we're not supposed
to alter their parameters. Monitoring them may still be useful, e.g.
for getting the correct rates of their children. This series attempts
to solve this problem without compromising on either of them.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (2):
      clk: qcom: rcg2: Introduce read-only RCG2 ops
      clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks

 drivers/clk/qcom/clk-rcg.h     | 1 +
 drivers/clk/qcom/clk-rcg2.c    | 8 ++++++++
 drivers/clk/qcom/gcc-msm8996.c | 6 +++---
 3 files changed, 12 insertions(+), 3 deletions(-)
---
base-commit: 53ab6975c12d1ad86c599a8927e8c698b144d669
change-id: 20230612-topic-rcg2_ro-1450a076eda9

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@linaro.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] clk: qcom: rcg2: Introduce read-only RCG2 ops
  2023-06-12  9:22 [PATCH 0/2] Add read-only RCG2 ops Konrad Dybcio
@ 2023-06-12  9:22 ` Konrad Dybcio
  2023-06-12  9:22 ` [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks Konrad Dybcio
  1 sibling, 0 replies; 12+ messages in thread
From: Konrad Dybcio @ 2023-06-12  9:22 UTC (permalink / raw)
  To: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd
  Cc: Marijn Suijten, linux-arm-msm, linux-clk, linux-kernel,
	Stephan Gerhold, Konrad Dybcio

Some clocks are physically part of a clock controller block (e.g. GCC),
but are under no circumstances supposed to be touched from HLOS/APSS, as
another subsystem manages them, and trying to alter its configuration
may (and likely will) wreck total havoc over whatever the clock is
attached to.

Add read-only ops for RCG clocks. This allows us to peak at the rates
(and other configuration parameters) of such clocks without the risk
of messing up half of the SoC due to an erroneous CCF call.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/clk-rcg.h  | 1 +
 drivers/clk/qcom/clk-rcg2.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index e6d84c8c7989..4b4ff156539f 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -170,6 +170,7 @@ struct clk_rcg2_gfx3d {
 extern const struct clk_ops clk_rcg2_ops;
 extern const struct clk_ops clk_rcg2_floor_ops;
 extern const struct clk_ops clk_rcg2_mux_closest_ops;
+extern const struct clk_ops clk_rcg2_ro_ops;
 extern const struct clk_ops clk_edp_pixel_ops;
 extern const struct clk_ops clk_byte_ops;
 extern const struct clk_ops clk_byte2_ops;
diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index e22baf3a7112..71de1cd8d45b 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -518,6 +518,14 @@ const struct clk_ops clk_rcg2_mux_closest_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_rcg2_mux_closest_ops);
 
+const struct clk_ops clk_rcg2_ro_ops = {
+	.is_enabled = clk_rcg2_is_enabled,
+	.get_parent = clk_rcg2_get_parent,
+	.recalc_rate = clk_rcg2_recalc_rate,
+	.get_duty_cycle = clk_rcg2_get_duty_cycle,
+};
+EXPORT_SYMBOL_GPL(clk_rcg2_ro_ops);
+
 struct frac_entry {
 	int num;
 	int den;

-- 
2.41.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-06-12  9:22 [PATCH 0/2] Add read-only RCG2 ops Konrad Dybcio
  2023-06-12  9:22 ` [PATCH 1/2] clk: qcom: rcg2: Introduce " Konrad Dybcio
@ 2023-06-12  9:22 ` Konrad Dybcio
  2023-06-12 10:17   ` Stephan Gerhold
  2023-06-13 17:56   ` Bjorn Andersson
  1 sibling, 2 replies; 12+ messages in thread
From: Konrad Dybcio @ 2023-06-12  9:22 UTC (permalink / raw)
  To: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd
  Cc: Marijn Suijten, linux-arm-msm, linux-clk, linux-kernel,
	Stephan Gerhold, Konrad Dybcio

The config/periph/system NoC clocks are wholly controlled by the
RPM firmware and Linux should never ever alter their configuration.

Switch them over to read-only ops to avoid that.

Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/clk/qcom/gcc-msm8996.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
index 5e44d1bcca9e..588e3b67657a 100644
--- a/drivers/clk/qcom/gcc-msm8996.c
+++ b/drivers/clk/qcom/gcc-msm8996.c
@@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = {
 		.name = "system_noc_clk_src",
 		.parent_data = gcc_xo_gpll0_gpll0_early_div,
 		.num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_ro_ops,
 	},
 };
 
@@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = {
 		.name = "config_noc_clk_src",
 		.parent_data = gcc_xo_gpll0,
 		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_ro_ops,
 	},
 };
 
@@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = {
 		.name = "periph_noc_clk_src",
 		.parent_data = gcc_xo_gpll0,
 		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
-		.ops = &clk_rcg2_ops,
+		.ops = &clk_rcg2_ro_ops,
 	},
 };
 

-- 
2.41.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-06-12  9:22 ` [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks Konrad Dybcio
@ 2023-06-12 10:17   ` Stephan Gerhold
  2023-06-12 11:46     ` Konrad Dybcio
  2023-06-13 17:56   ` Bjorn Andersson
  1 sibling, 1 reply; 12+ messages in thread
From: Stephan Gerhold @ 2023-06-12 10:17 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd,
	Marijn Suijten, linux-arm-msm, linux-clk, linux-kernel

On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote:
> The config/periph/system NoC clocks are wholly controlled by the
> RPM firmware and Linux should never ever alter their configuration.
> 
> Switch them over to read-only ops to avoid that.
> 
> Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/clk/qcom/gcc-msm8996.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
> index 5e44d1bcca9e..588e3b67657a 100644
> --- a/drivers/clk/qcom/gcc-msm8996.c
> +++ b/drivers/clk/qcom/gcc-msm8996.c
> @@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = {
>  		.name = "system_noc_clk_src",
>  		.parent_data = gcc_xo_gpll0_gpll0_early_div,
>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_ro_ops,
>  	},
>  };
>  
> @@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = {
>  		.name = "config_noc_clk_src",
>  		.parent_data = gcc_xo_gpll0,
>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_ro_ops,
>  	},
>  };
>  
> @@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = {
>  		.name = "periph_noc_clk_src",
>  		.parent_data = gcc_xo_gpll0,
>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_ro_ops,
>  	},
>  };
>  

If the rcg is read-only then it shouldn't need a .freq_tbl (the
.recalc_rate() works without it). These clocks have one for some reason.

Maybe we should also set .flags = CLK_GET_RATE_NOCACHE to ensure that
clk_get_rate() doesn't return some stale value? Although I'm not sure if
it's enough to set it here or if it would need to be set on all child
clocks as well.

Thanks,
Stephan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-06-12 10:17   ` Stephan Gerhold
@ 2023-06-12 11:46     ` Konrad Dybcio
  2023-06-12 12:23       ` Stephan Gerhold
  0 siblings, 1 reply; 12+ messages in thread
From: Konrad Dybcio @ 2023-06-12 11:46 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd,
	Marijn Suijten, linux-arm-msm, linux-clk, linux-kernel



On 12.06.2023 12:17, Stephan Gerhold wrote:
> On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote:
>> The config/periph/system NoC clocks are wholly controlled by the
>> RPM firmware and Linux should never ever alter their configuration.
>>
>> Switch them over to read-only ops to avoid that.
>>
>> Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>  drivers/clk/qcom/gcc-msm8996.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
>> index 5e44d1bcca9e..588e3b67657a 100644
>> --- a/drivers/clk/qcom/gcc-msm8996.c
>> +++ b/drivers/clk/qcom/gcc-msm8996.c
>> @@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = {
>>  		.name = "system_noc_clk_src",
>>  		.parent_data = gcc_xo_gpll0_gpll0_early_div,
>>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_ro_ops,
>>  	},
>>  };
>>  
>> @@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = {
>>  		.name = "config_noc_clk_src",
>>  		.parent_data = gcc_xo_gpll0,
>>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_ro_ops,
>>  	},
>>  };
>>  
>> @@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = {
>>  		.name = "periph_noc_clk_src",
>>  		.parent_data = gcc_xo_gpll0,
>>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_ro_ops,
>>  	},
>>  };
>>  
> 
> If the rcg is read-only then it shouldn't need a .freq_tbl (the
> .recalc_rate() works without it). These clocks have one for some reason.
.get_parent needs it

> 
> Maybe we should also set .flags = CLK_GET_RATE_NOCACHE to ensure that
> clk_get_rate() doesn't return some stale value? Although I'm not sure if
> it's enough to set it here or if it would need to be set on all child
> clocks as well.
Hm, I'm not sure if it makes sense for the children..

Konrad

> 
> Thanks,
> Stephan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-06-12 11:46     ` Konrad Dybcio
@ 2023-06-12 12:23       ` Stephan Gerhold
  2023-06-12 12:25         ` Konrad Dybcio
  0 siblings, 1 reply; 12+ messages in thread
From: Stephan Gerhold @ 2023-06-12 12:23 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd,
	Marijn Suijten, linux-arm-msm, linux-clk, linux-kernel

On Mon, Jun 12, 2023 at 01:46:26PM +0200, Konrad Dybcio wrote:
> On 12.06.2023 12:17, Stephan Gerhold wrote:
> > On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote:
> >> The config/periph/system NoC clocks are wholly controlled by the
> >> RPM firmware and Linux should never ever alter their configuration.
> >>
> >> Switch them over to read-only ops to avoid that.
> >>
> >> Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver")
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> >> ---
> >>  drivers/clk/qcom/gcc-msm8996.c | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
> >> index 5e44d1bcca9e..588e3b67657a 100644
> >> --- a/drivers/clk/qcom/gcc-msm8996.c
> >> +++ b/drivers/clk/qcom/gcc-msm8996.c
> >> @@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = {
> >>  		.name = "system_noc_clk_src",
> >>  		.parent_data = gcc_xo_gpll0_gpll0_early_div,
> >>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div),
> >> -		.ops = &clk_rcg2_ops,
> >> +		.ops = &clk_rcg2_ro_ops,
> >>  	},
> >>  };
> >>  
> >> @@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = {
> >>  		.name = "config_noc_clk_src",
> >>  		.parent_data = gcc_xo_gpll0,
> >>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
> >> -		.ops = &clk_rcg2_ops,
> >> +		.ops = &clk_rcg2_ro_ops,
> >>  	},
> >>  };
> >>  
> >> @@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = {
> >>  		.name = "periph_noc_clk_src",
> >>  		.parent_data = gcc_xo_gpll0,
> >>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
> >> -		.ops = &clk_rcg2_ops,
> >> +		.ops = &clk_rcg2_ro_ops,
> >>  	},
> >>  };
> >>  
> > 
> > If the rcg is read-only then it shouldn't need a .freq_tbl (the
> > .recalc_rate() works without it). These clocks have one for some reason.
> .get_parent needs it
> 

Hm? .get_parent uses the ->parent_map but not the ->freq_tbl as far as I
can see. :)

Stephan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-06-12 12:23       ` Stephan Gerhold
@ 2023-06-12 12:25         ` Konrad Dybcio
  0 siblings, 0 replies; 12+ messages in thread
From: Konrad Dybcio @ 2023-06-12 12:25 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd,
	Marijn Suijten, linux-arm-msm, linux-clk, linux-kernel



On 12.06.2023 14:23, Stephan Gerhold wrote:
> On Mon, Jun 12, 2023 at 01:46:26PM +0200, Konrad Dybcio wrote:
>> On 12.06.2023 12:17, Stephan Gerhold wrote:
>>> On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote:
>>>> The config/periph/system NoC clocks are wholly controlled by the
>>>> RPM firmware and Linux should never ever alter their configuration.
>>>>
>>>> Switch them over to read-only ops to avoid that.
>>>>
>>>> Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver")
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>> ---
>>>>  drivers/clk/qcom/gcc-msm8996.c | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
>>>> index 5e44d1bcca9e..588e3b67657a 100644
>>>> --- a/drivers/clk/qcom/gcc-msm8996.c
>>>> +++ b/drivers/clk/qcom/gcc-msm8996.c
>>>> @@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = {
>>>>  		.name = "system_noc_clk_src",
>>>>  		.parent_data = gcc_xo_gpll0_gpll0_early_div,
>>>>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div),
>>>> -		.ops = &clk_rcg2_ops,
>>>> +		.ops = &clk_rcg2_ro_ops,
>>>>  	},
>>>>  };
>>>>  
>>>> @@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = {
>>>>  		.name = "config_noc_clk_src",
>>>>  		.parent_data = gcc_xo_gpll0,
>>>>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
>>>> -		.ops = &clk_rcg2_ops,
>>>> +		.ops = &clk_rcg2_ro_ops,
>>>>  	},
>>>>  };
>>>>  
>>>> @@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = {
>>>>  		.name = "periph_noc_clk_src",
>>>>  		.parent_data = gcc_xo_gpll0,
>>>>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
>>>> -		.ops = &clk_rcg2_ops,
>>>> +		.ops = &clk_rcg2_ro_ops,
>>>>  	},
>>>>  };
>>>>  
>>>
>>> If the rcg is read-only then it shouldn't need a .freq_tbl (the
>>> .recalc_rate() works without it). These clocks have one for some reason.
>> .get_parent needs it
>>
> 
> Hm? .get_parent uses the ->parent_map but not the ->freq_tbl as far as I
> can see. :)
Brainfart. I think the ftbls can be removed.

Konrad
> 
> Stephan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-06-13 17:56   ` Bjorn Andersson
@ 2023-06-13 17:54     ` Konrad Dybcio
  2023-07-31 11:01       ` Konrad Dybcio
  0 siblings, 1 reply; 12+ messages in thread
From: Konrad Dybcio @ 2023-06-13 17:54 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Michael Turquette, Stephen Boyd, Marijn Suijten,
	linux-arm-msm, linux-clk, linux-kernel, Stephan Gerhold



On 13.06.2023 19:56, Bjorn Andersson wrote:
> On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote:
>> The config/periph/system NoC clocks are wholly controlled by the
>> RPM firmware and Linux should never ever alter their configuration.
>>
> 
> Does Linux need to know about them?
Not really, but it allows us to get rates of their children.

We can get rid of them if one can argue debugcc is enough. Unless
we need clk_get_rate for some reason.

Konrad
> 
> Regards,
> Bjorn
> 
>> Switch them over to read-only ops to avoid that.
>>
>> Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>  drivers/clk/qcom/gcc-msm8996.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
>> index 5e44d1bcca9e..588e3b67657a 100644
>> --- a/drivers/clk/qcom/gcc-msm8996.c
>> +++ b/drivers/clk/qcom/gcc-msm8996.c
>> @@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = {
>>  		.name = "system_noc_clk_src",
>>  		.parent_data = gcc_xo_gpll0_gpll0_early_div,
>>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_ro_ops,
>>  	},
>>  };
>>  
>> @@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = {
>>  		.name = "config_noc_clk_src",
>>  		.parent_data = gcc_xo_gpll0,
>>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_ro_ops,
>>  	},
>>  };
>>  
>> @@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = {
>>  		.name = "periph_noc_clk_src",
>>  		.parent_data = gcc_xo_gpll0,
>>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
>> -		.ops = &clk_rcg2_ops,
>> +		.ops = &clk_rcg2_ro_ops,
>>  	},
>>  };
>>  
>>
>> -- 
>> 2.41.0
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-06-12  9:22 ` [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks Konrad Dybcio
  2023-06-12 10:17   ` Stephan Gerhold
@ 2023-06-13 17:56   ` Bjorn Andersson
  2023-06-13 17:54     ` Konrad Dybcio
  1 sibling, 1 reply; 12+ messages in thread
From: Bjorn Andersson @ 2023-06-13 17:56 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Andy Gross, Michael Turquette, Stephen Boyd, Marijn Suijten,
	linux-arm-msm, linux-clk, linux-kernel, Stephan Gerhold

On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote:
> The config/periph/system NoC clocks are wholly controlled by the
> RPM firmware and Linux should never ever alter their configuration.
> 

Does Linux need to know about them?

Regards,
Bjorn

> Switch them over to read-only ops to avoid that.
> 
> Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/clk/qcom/gcc-msm8996.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
> index 5e44d1bcca9e..588e3b67657a 100644
> --- a/drivers/clk/qcom/gcc-msm8996.c
> +++ b/drivers/clk/qcom/gcc-msm8996.c
> @@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = {
>  		.name = "system_noc_clk_src",
>  		.parent_data = gcc_xo_gpll0_gpll0_early_div,
>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_ro_ops,
>  	},
>  };
>  
> @@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = {
>  		.name = "config_noc_clk_src",
>  		.parent_data = gcc_xo_gpll0,
>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_ro_ops,
>  	},
>  };
>  
> @@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = {
>  		.name = "periph_noc_clk_src",
>  		.parent_data = gcc_xo_gpll0,
>  		.num_parents = ARRAY_SIZE(gcc_xo_gpll0),
> -		.ops = &clk_rcg2_ops,
> +		.ops = &clk_rcg2_ro_ops,
>  	},
>  };
>  
> 
> -- 
> 2.41.0
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-06-13 17:54     ` Konrad Dybcio
@ 2023-07-31 11:01       ` Konrad Dybcio
  2023-08-24 10:27         ` Konrad Dybcio
  0 siblings, 1 reply; 12+ messages in thread
From: Konrad Dybcio @ 2023-07-31 11:01 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Michael Turquette, Stephen Boyd, Marijn Suijten,
	linux-arm-msm, linux-clk, linux-kernel, Stephan Gerhold

On 13.06.2023 19:54, Konrad Dybcio wrote:
> 
> 
> On 13.06.2023 19:56, Bjorn Andersson wrote:
>> On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote:
>>> The config/periph/system NoC clocks are wholly controlled by the
>>> RPM firmware and Linux should never ever alter their configuration.
>>>
>>
>> Does Linux need to know about them?
> Not really, but it allows us to get rates of their children.
> 
> We can get rid of them if one can argue debugcc is enough. Unless
> we need clk_get_rate for some reason.
> 
Any opinions?

Konrad

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-07-31 11:01       ` Konrad Dybcio
@ 2023-08-24 10:27         ` Konrad Dybcio
  2023-08-24 11:32           ` Dmitry Baryshkov
  0 siblings, 1 reply; 12+ messages in thread
From: Konrad Dybcio @ 2023-08-24 10:27 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Michael Turquette, Stephen Boyd, Marijn Suijten,
	linux-arm-msm, linux-clk, linux-kernel, Stephan Gerhold

On 31.07.2023 13:01, Konrad Dybcio wrote:
> On 13.06.2023 19:54, Konrad Dybcio wrote:
>>
>>
>> On 13.06.2023 19:56, Bjorn Andersson wrote:
>>> On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote:
>>>> The config/periph/system NoC clocks are wholly controlled by the
>>>> RPM firmware and Linux should never ever alter their configuration.
>>>>
>>>
>>> Does Linux need to know about them?
>> Not really, but it allows us to get rates of their children.
>>
>> We can get rid of them if one can argue debugcc is enough. Unless
>> we need clk_get_rate for some reason.
>>
> Any opinions?
So, do we drop them?

Konrad

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks
  2023-08-24 10:27         ` Konrad Dybcio
@ 2023-08-24 11:32           ` Dmitry Baryshkov
  0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2023-08-24 11:32 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, Andy Gross, Michael Turquette, Stephen Boyd,
	Marijn Suijten, linux-arm-msm, linux-clk, linux-kernel,
	Stephan Gerhold

On Thu, 24 Aug 2023 at 13:28, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 31.07.2023 13:01, Konrad Dybcio wrote:
> > On 13.06.2023 19:54, Konrad Dybcio wrote:
> >>
> >>
> >> On 13.06.2023 19:56, Bjorn Andersson wrote:
> >>> On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote:
> >>>> The config/periph/system NoC clocks are wholly controlled by the
> >>>> RPM firmware and Linux should never ever alter their configuration.
> >>>>
> >>>
> >>> Does Linux need to know about them?
> >> Not really, but it allows us to get rates of their children.
> >>
> >> We can get rid of them if one can argue debugcc is enough. Unless
> >> we need clk_get_rate for some reason.
> >>
> > Any opinions?
> So, do we drop them?

My opinion would be to drop the NoC clocks unless there is any extra
value (like child clocks).

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-08-24 11:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-12  9:22 [PATCH 0/2] Add read-only RCG2 ops Konrad Dybcio
2023-06-12  9:22 ` [PATCH 1/2] clk: qcom: rcg2: Introduce " Konrad Dybcio
2023-06-12  9:22 ` [PATCH 2/2] clk: qcom: gcc-msm8996: Use read-only RCG ops for RPM bus clocks Konrad Dybcio
2023-06-12 10:17   ` Stephan Gerhold
2023-06-12 11:46     ` Konrad Dybcio
2023-06-12 12:23       ` Stephan Gerhold
2023-06-12 12:25         ` Konrad Dybcio
2023-06-13 17:56   ` Bjorn Andersson
2023-06-13 17:54     ` Konrad Dybcio
2023-07-31 11:01       ` Konrad Dybcio
2023-08-24 10:27         ` Konrad Dybcio
2023-08-24 11:32           ` Dmitry Baryshkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).