[v2,00/14] Clean up RPM bus clocks remnants

  After the recent cleanups ([1], [2]) some in-tree abusers that directly
accessed the RPM bus clocks, effectively circumventing and working
against the efforts of the interconnect framework, were found.

Patches 1-5 drop deprecated references and the rest attempt to stop
direct bus clock abuses.

Depends on [2].

8996 and 8998 remoteproc changes were not tested, they never worked on
my Sony phones.

[1] https://lore.kernel.org/linux-arm-msm/20230526-topic-smd_icc-v7-0-09c78c175546@linaro.org/
[2] https://lore.kernel.org/linux-arm-msm/20230721-topic-icc_bindings-v1-0-93e2bc728fb7@linaro.org/

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Changes in v2:
- Incorporate [3] into the sdm630 patch, add required bindings fixes
- dt-bindings: remoteproc: qcom,adsp: Remove AGGRE2 clock: Merge entries (krzk)
- Pick up a-b (krzk)
- Add "sdm630: Fix USB2 clock-names order"
- Link to v1: https://lore.kernel.org/r/20230721-topic-rpm_clk_cleanup-v1-0-cf6cd5c621d5@linaro.org

[3] https://lore.kernel.org/linux-arm-msm/20230719073520.2644966-1-alexeymin@postmarketos.org/#t

---
Konrad Dybcio (14):
      arm64: dts: qcom: msm8916: Drop RPM bus clocks
      arm64: dts: qcom: msm8996: Drop RPM bus clocks
      arm64: dts: qcom: qcs404: Drop RPM bus clocks
      dt-bindings: arm-smmu: Fix SDM630 clocks description
      dt-bindings: usb: qcom,dwc3: Fix SDM660 clock description
      arm64: dts: qcom: sdm630: Drop RPM bus clocks
      arm64: dts: qcom: msm8939: Drop RPM bus clocks
      dt-bindings: remoteproc: qcom,adsp: Remove AGGRE2 clock
      dt-bindings: remoteproc: qcom,msm8996-mss-pil: Remove PNoC clock
      remoteproc: qcom: q6v5-mss: Remove PNoC clock from 8996 MSS
      arm64: dts: qcom: msm8998: Remove AGGRE2 clock from SLPI
      arm64: dts: qcom: msm8996: Remove AGGRE2 clock from SLPI
      arm64: dts: qcom: msm8996: Remove PNoC clock from MSS
      arm64: dts: qcom: sdm630: Fix USB2 clock-names order

 .../devicetree/bindings/iommu/arm,smmu.yaml        |  2 +-
 .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 20 +-------
 .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  2 -
 .../devicetree/bindings/usb/qcom,dwc3.yaml         |  6 +--
 arch/arm64/boot/dts/qcom/msm8916.dtsi              |  9 ----
 arch/arm64/boot/dts/qcom/msm8939.dtsi              | 12 -----
 arch/arm64/boot/dts/qcom/msm8996.dtsi              | 43 ++++++-----------
 arch/arm64/boot/dts/qcom/msm8998.dtsi              |  5 +-
 arch/arm64/boot/dts/qcom/qcs404.dtsi               |  9 ----
 arch/arm64/boot/dts/qcom/sdm630.dtsi               | 55 +++++-----------------
 drivers/remoteproc/qcom_q6v5_mss.c                 |  1 -
 11 files changed, 34 insertions(+), 130 deletions(-)
---
base-commit: 66d9573193967138cd12e232d4b5bc2b57e0d1ac
change-id: 20230721-topic-rpm_clk_cleanup-1b2f4a1acd01

Best regards,
  

On 12/09/2023 15:31, Konrad Dybcio wrote:
> These clocks are now handled from within the icc framework and are
> no longer registered from within the CCF. Remove them.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm630.dtsi | 49 +++++++-----------------------------
>  1 file changed, 9 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> index ec6003212c4d..f11d2a07508c 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -605,9 +605,6 @@ bimc: interconnect@1008000 {
>  			compatible = "qcom,sdm660-bimc";
>  			reg = <0x01008000 0x78000>;
>  			#interconnect-cells = <1>;
> -			clock-names = "bus", "bus_a";
> -			clocks = <&rpmcc RPM_SMD_BIMC_CLK>,
> -				 <&rpmcc RPM_SMD_BIMC_A_CLK>;

Bindings expect here two clocks, so you miss some bindings patches.

>  		};
>  
>  		restart@10ac000 {
> @@ -619,28 +616,17 @@ cnoc: interconnect@1500000 {
>  			compatible = "qcom,sdm660-cnoc";
>  			reg = <0x01500000 0x10000>;
>  			#interconnect-cells = <1>;
> -			clock-names = "bus", "bus_a";
> -			clocks = <&rpmcc RPM_SMD_CNOC_CLK>,
> -				 <&rpmcc RPM_SMD_CNOC_A_CLK>;
>  		};
>  
>  		snoc: interconnect@1626000 {
>  			compatible = "qcom,sdm660-snoc";
>  			reg = <0x01626000 0x7090>;
>  			#interconnect-cells = <1>;
> -			clock-names = "bus", "bus_a";
> -			clocks = <&rpmcc RPM_SMD_SNOC_CLK>,
> -				 <&rpmcc RPM_SMD_SNOC_A_CLK>;
>  		};
>  
>  		anoc2_smmu: iommu@16c0000 {
>  			compatible = "qcom,sdm630-smmu-v2", "qcom,smmu-v2";
>  			reg = <0x016c0000 0x40000>;
> -
> -			assigned-clocks = <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
> -			assigned-clock-rates = <1000>;
> -			clocks = <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
> -			clock-names = "bus";

This is also against bindings. After your patch #4, such bus clock (or
other combinations) is still required.


>  			#global-interrupts = <2>;
>  			#iommu-cells = <1>;
>  
> @@ -685,16 +671,12 @@ a2noc: interconnect@1704000 {
>  			compatible = "qcom,sdm660-a2noc";
>  			reg = <0x01704000 0xc100>;
>  			#interconnect-cells = <1>;
> -			clock-names = "bus",
> -				      "bus_a",
> -				      "ipa",
> +			clock-names = "ipa",

And which bindings does this match?

Best regards,
Krzysztof
  

On 12/09/2023 15:31, Konrad Dybcio wrote:
> The last 2 clock-names entries for the USB2 controller were swapped,
> resulting in schema warnings:
> 
> ['cfg_noc', 'core', 'mock_utmi', 'sleep'] is too short
>         'iface' was expected
>         'sleep' was expected
>         'mock_utmi' was expected
> 
> Fix it and take the liberty to make the clock-names entries more
> readable.

This was already fixed:

https://lore.kernel.org/all/20230723141849.93078-2-krzysztof.kozlowski@linaro.org/

Best regards,
Krzysztof
  

On 12/09/2023 15:31, Konrad Dybcio wrote:
> The last 2 clock-names entries for the USB2 controller were swapped,
> resulting in schema warnings:
> 
> ['cfg_noc', 'core', 'mock_utmi', 'sleep'] is too short
>         'iface' was expected
>         'sleep' was expected
>         'mock_utmi' was expected
> 
> Fix it and take the liberty to make the clock-names entries more
> readable.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm630.dtsi | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> index f11d2a07508c..316c8fd224e0 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -1394,8 +1394,10 @@ usb2: usb@c2f8800 {
>  				 <&gcc GCC_USB20_MASTER_CLK>,
>  				 <&gcc GCC_USB20_MOCK_UTMI_CLK>,
>  				 <&gcc GCC_USB20_SLEEP_CLK>;
> -			clock-names = "cfg_noc", "core",
> -				      "mock_utmi", "sleep";
> +			clock-names = "cfg_noc",
> +				      "core",
> +				      "sleep",
> +				      "mock_utmi";

Plus this is just incorrect... :(

Best regards,
Krzysztof
  

On 13.09.2023 09:13, Krzysztof Kozlowski wrote:
> On 12/09/2023 15:31, Konrad Dybcio wrote:
>> These clocks are now handled from within the icc framework and are
>> no longer registered from within the CCF. Remove them.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
[...]

>>  		anoc2_smmu: iommu@16c0000 {
>>  			compatible = "qcom,sdm630-smmu-v2", "qcom,smmu-v2";
>>  			reg = <0x016c0000 0x40000>;
>> -
>> -			assigned-clocks = <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
>> -			assigned-clock-rates = <1000>;
>> -			clocks = <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
>> -			clock-names = "bus";
> 
> This is also against bindings. After your patch #4, such bus clock (or
> other combinations) is still required.
So, we have 4 SMMU instances on this platform:

MMSS (described, iface, mem, mem_iface)
GPU (described, iface-mm, iface-smmu, bus-smmu)

ANOC2 (this one, no clocks after removing rpmcc bus)
LPASS (no clocks)

Should I then create a new entry in the bindings, replicating
what's there for msm8998[1] and dropping the entry with just "bus"
from anyOf?

Konrad

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/iommu/arm,smmu.yaml?h=next-20230913#n272
  

On 14.09.2023 08:26, Krzysztof Kozlowski wrote:
> On 13/09/2023 14:08, Konrad Dybcio wrote:
>> On 13.09.2023 09:13, Krzysztof Kozlowski wrote:
>>> On 12/09/2023 15:31, Konrad Dybcio wrote:
>>>> These clocks are now handled from within the icc framework and are
>>>> no longer registered from within the CCF. Remove them.
>>>>
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>> ---
>> [...]
>>
>>>>  		anoc2_smmu: iommu@16c0000 {
>>>>  			compatible = "qcom,sdm630-smmu-v2", "qcom,smmu-v2";
>>>>  			reg = <0x016c0000 0x40000>;
>>>> -
>>>> -			assigned-clocks = <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
>>>> -			assigned-clock-rates = <1000>;
>>>> -			clocks = <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
>>>> -			clock-names = "bus";
>>>
>>> This is also against bindings. After your patch #4, such bus clock (or
>>> other combinations) is still required.
>> So, we have 4 SMMU instances on this platform:
>>
>> MMSS (described, iface, mem, mem_iface)
>> GPU (described, iface-mm, iface-smmu, bus-smmu)
>>
>> ANOC2 (this one, no clocks after removing rpmcc bus)
>> LPASS (no clocks)
> 
> Ah, I did not notice it.
> 
>>
>> Should I then create a new entry in the bindings, replicating
>> what's there for msm8998[1] and dropping the entry with just "bus"
>> from anyOf?
> 
> So this passes the bindings, right?
Yes

anyOf: in the binding should allow
> also no match, so this should be fine. However indeed we need to drop
> the "bus" entry, because it is not valid anymore.
Actually, looks like the LPASS smmu may require a single
clock. We can reuse that single-"bus"-clock entry for
HLOS1_VOTE_LPASS_ADSP_SMMU_CLK.

The device didn't crash when trying to access LPASS SMMU
with that clock absent, but I guess it may have just
been luck, things may change once more hardware is parked..

Konrad
  

On Tue, 12 Sep 2023 15:31:38 +0200, Konrad Dybcio wrote:
> After the recent cleanups ([1], [2]) some in-tree abusers that directly
> accessed the RPM bus clocks, effectively circumventing and working
> against the efforts of the interconnect framework, were found.
> 
> Patches 1-5 drop deprecated references and the rest attempt to stop
> direct bus clock abuses.
> 
> [...]

Applied SMMU bindings fix to will (for-joerg/arm-smmu/fixes), thanks!

[04/14] dt-bindings: arm-smmu: Fix SDM630 clocks description
        https://git.kernel.org/will/c/938ba2f252a5

Cheers,
  

On Tue, 12 Sep 2023 15:31:38 +0200, Konrad Dybcio wrote:
> After the recent cleanups ([1], [2]) some in-tree abusers that directly
> accessed the RPM bus clocks, effectively circumventing and working
> against the efforts of the interconnect framework, were found.
> 
> Patches 1-5 drop deprecated references and the rest attempt to stop
> direct bus clock abuses.
> 
> [...]

Applied, thanks!

[08/14] dt-bindings: remoteproc: qcom,adsp: Remove AGGRE2 clock
        commit: c4c5b47958529bc1de10260df0c583710853b516
[09/14] dt-bindings: remoteproc: qcom,msm8996-mss-pil: Remove PNoC clock
        commit: e7781901449cbcff129d80a5d9021e9e96084ec4
[10/14] remoteproc: qcom: q6v5-mss: Remove PNoC clock from 8996 MSS
        commit: e1592981c51bac38ea2041b642777b3ba30606a8

Best regards,