diff mbox series

[09/11] arm64: dts: qcom: msm8998: Remove AGGRE2 clock from SLPI

Message ID 20230721-topic-rpm_clk_cleanup-v1-9-cf6cd5c621d5@linaro.org
State New
Headers
Series Clean up RPM bus clocks remnants |

Commit Message

Konrad Dybcio July 21, 2023, 3:36 p.m. UTC 
  The AGGRE2 clock is a clock for the entire AGGRE2 bus, managed from
within the interconnect driver. Attaching it to SLPI was a total hack.
Get rid of it.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8998.dtsi | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Jeffrey Hugo July 21, 2023, 3:50 p.m. UTC | #1 
On 7/21/2023 9:36 AM, Konrad Dybcio wrote:
> The AGGRE2 clock is a clock for the entire AGGRE2 bus, managed from
> within the interconnect driver. Attaching it to SLPI was a total hack.
> Get rid of it.

Nit - why do we care what driver manages the clock?  DT describes 
hardware...

The entire SLPI block hangs off the AGGRE2 bus, so that bus needs to be 
on for the SLPI.  I agree that AGGRE2 is really an interconnect device 
and SLPI should be a consumer of that, but we don't have 8998 
interconnects defined yet.  Seems like this hack is still needed.

> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/msm8998.dtsi | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index 360fe3edcc08..547c3f9654a6 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -1599,9 +1599,8 @@ remoteproc_slpi: remoteproc@5800000 {
>   
>   			px-supply = <&vreg_lvs2a_1p8>;
>   
> -			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> -				 <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
> -			clock-names = "xo", "aggre2";
> +			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>;
> +			clock-names = "xo";
>   
>   			memory-region = <&slpi_mem>;
>   
>
Konrad Dybcio July 26, 2023, 3:48 p.m. UTC | #2 
On 21.07.2023 17:50, Jeffrey Hugo wrote:
> On 7/21/2023 9:36 AM, Konrad Dybcio wrote:
>> The AGGRE2 clock is a clock for the entire AGGRE2 bus, managed from
>> within the interconnect driver. Attaching it to SLPI was a total hack.
>> Get rid of it.
> 
> Nit - why do we care what driver manages the clock?  DT describes hardware...
> 
> The entire SLPI block hangs off the AGGRE2 bus, so that bus needs to be on for the SLPI.  I agree that AGGRE2 is really an interconnect device and SLPI should be a consumer of that, but we don't have 8998 interconnects defined yet.  Seems like this hack is still needed.
I'll try to get somebody to check on a device with working SLPI, but
the DT-side explanation here is: this is not a clock. It's previously
been misrepresented as such, but it lies within the "interconnect"
class.

Konrad
> 
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/msm8998.dtsi | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
>> index 360fe3edcc08..547c3f9654a6 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
>> @@ -1599,9 +1599,8 @@ remoteproc_slpi: remoteproc@5800000 {
>>                 px-supply = <&vreg_lvs2a_1p8>;
>>   -            clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>> -                 <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
>> -            clock-names = "xo", "aggre2";
>> +            clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>;
>> +            clock-names = "xo";
>>                 memory-region = <&slpi_mem>;
>>  
>
Konrad Dybcio Aug. 12, 2023, 11:46 a.m. UTC | #3 
On 21.07.2023 17:50, Jeffrey Hugo wrote:
> On 7/21/2023 9:36 AM, Konrad Dybcio wrote:
>> The AGGRE2 clock is a clock for the entire AGGRE2 bus, managed from
>> within the interconnect driver. Attaching it to SLPI was a total hack.
>> Get rid of it.
> 
> Nit - why do we care what driver manages the clock?  DT describes hardware...
> 
> The entire SLPI block hangs off the AGGRE2 bus, so that bus needs to be on for the SLPI.  I agree that AGGRE2 is really an interconnect device and SLPI should be a consumer of that, but we don't have 8998 interconnects defined yet.  Seems like this hack is still needed.
As we concluded in private, this has no effect as the clk-smd-rpm driver
leaves that clock dangling at FMAX anyway

Konrad
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index 360fe3edcc08..547c3f9654a6 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -1599,9 +1599,8 @@  remoteproc_slpi: remoteproc@5800000 {
 
 			px-supply = <&vreg_lvs2a_1p8>;
 
-			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
-				 <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
-			clock-names = "xo", "aggre2";
+			clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>;
+			clock-names = "xo";
 
 			memory-region = <&slpi_mem>;