[PATCH] arm64: dts: qcom: sdm630: remove refs to nonexistent clocks

[PATCH] arm64: dts: qcom: sdm630: remove refs to nonexistent clocks

[Alexey Minnekhanov]

Since commit d6edc31f3a68 ("clk: qcom: smd-rpm: Separate out
interconnect bus clocks") rpmcc-sdm660 no longer provides
RPM_SMD_AGGR2_NOC_CLK and RPM_SMD_AGGR2_NOC_A_CLK clocks.
Remove them to fix various probe failures and get devices
booting again.

Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
---
 arch/arm64/boot/dts/qcom/sdm630.dtsi | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index 038ec7a41412..8bea611b246b 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -638,10 +638,6 @@ 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";
 			#global-interrupts = <2>;
 			#iommu-cells = <1>;
 			qcom,bypass-cbndx = /bits/ 8 <6>;
@@ -689,16 +685,12 @@ a2noc: interconnect@1704000 {
 			compatible = "qcom,sdm660-a2noc";
 			reg = <0x01704000 0xc100>;
 			#interconnect-cells = <1>;
-			clock-names = "bus",
-				      "bus_a",
-				      "ipa",
+			clock-names = "ipa",
 				      "ufs_axi",
 				      "aggre2_ufs_axi",
 				      "aggre2_usb3_axi",
 				      "cfg_noc_usb2_axi";
-			clocks = <&rpmcc RPM_SMD_AGGR2_NOC_CLK>,
-				 <&rpmcc RPM_SMD_AGGR2_NOC_A_CLK>,
-				 <&rpmcc RPM_SMD_IPA_CLK>,
+			clocks = <&rpmcc RPM_SMD_IPA_CLK>,
 				 <&gcc GCC_UFS_AXI_CLK>,
 				 <&gcc GCC_AGGRE2_UFS_AXI_CLK>,
 				 <&gcc GCC_AGGRE2_USB3_AXI_CLK>,
@@ -1309,20 +1301,16 @@ usb3: usb@a8f8800 {
 				 <&gcc GCC_USB30_MASTER_CLK>,
 				 <&gcc GCC_AGGRE2_USB3_AXI_CLK>,
 				 <&gcc GCC_USB30_SLEEP_CLK>,
-				 <&gcc GCC_USB30_MOCK_UTMI_CLK>,
-				 <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
+				 <&gcc GCC_USB30_MOCK_UTMI_CLK>;
 			clock-names = "cfg_noc",
 				      "core",
 				      "iface",
 				      "sleep",
-				      "mock_utmi",
-				      "bus";
+				      "mock_utmi";
 
 			assigned-clocks = <&gcc GCC_USB30_MOCK_UTMI_CLK>,
-					  <&gcc GCC_USB30_MASTER_CLK>,
-					  <&rpmcc RPM_SMD_AGGR2_NOC_CLK>;
-			assigned-clock-rates = <19200000>, <120000000>,
-					       <19200000>;
+					  <&gcc GCC_USB30_MASTER_CLK>;
+			assigned-clock-rates = <19200000>, <120000000>;
 
 			interrupts = <GIC_SPI 347 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 243 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.41.0

Re: [PATCH] arm64: dts: qcom: sdm630: remove refs to nonexistent clocks

[Krzysztof Kozlowski]

On 19/07/2023 09:35, Alexey Minnekhanov wrote:
> Since commit d6edc31f3a68 ("clk: qcom: smd-rpm: Separate out
> interconnect bus clocks") rpmcc-sdm660 no longer provides
> RPM_SMD_AGGR2_NOC_CLK and RPM_SMD_AGGR2_NOC_A_CLK clocks.
> Remove them to fix various probe failures and get devices
> booting again.

So that commit broke DTS?

> 
> Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm630.dtsi | 24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> index 038ec7a41412..8bea611b246b 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -638,10 +638,6 @@ 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";

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: sdm630: remove refs to nonexistent clocks

[Alexey Minnekhanov]

On 19.07.2023 10:39, Krzysztof Kozlowski wrote:
> On 19/07/2023 09:35, Alexey Minnekhanov wrote:
>> Since commit d6edc31f3a68 ("clk: qcom: smd-rpm: Separate out
>> interconnect bus clocks") rpmcc-sdm660 no longer provides
>> RPM_SMD_AGGR2_NOC_CLK and RPM_SMD_AGGR2_NOC_A_CLK clocks.
>> Remove them to fix various probe failures and get devices
>> booting again.
> 
> So that commit broke DTS?
> 

Yes, this is my understanding of the situation.
The commit in subject [1] is only in linux-next for a few days, so it 
broke booting only on 6.5-rc (rc2 currently). Konrad said: "these clocks 
references were API abuses; referencing the bus clocks was circumventing 
the interconnect layer. That loophole is now gone and the abusers are 
now apparent"

> 
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check`
> 

If DT schema for interconnect requires bus clocks to be specified, I 
don't even know what to put there now. Can we change schema?

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=d6edc31f3a68d8d0636e0cfcd9eced7460ad32f4

-- 
Regards,
Alexey Minnekhanov
postmarketOS developer

Re: [PATCH] arm64: dts: qcom: sdm630: remove refs to nonexistent clocks

[Stephan Gerhold]

On Wed, Jul 19, 2023 at 11:36:46AM +0300, Alexey Minnekhanov wrote:
> On 19.07.2023 10:39, Krzysztof Kozlowski wrote:
> > It does not look like you tested the DTS against bindings. Please run
> > `make dtbs_check`
> > 
> 
> If DT schema for interconnect requires bus clocks to be specified, I don't
> even know what to put there now. Can we change schema?
> 

I think I mentioned the DT schema updates during the review of Konrad's
interconnect changes and he mentioned he would like to clean those up
after getting the series in. (Which would be sometime soon now I guess)

For now, having the &rpmcc "bus"/"bus_a"/"ipa" clocks specified on the
interconnect@... nodes is still valid. At runtime they will just be
ignored. Feel free to just keep them there for this initial fix.

For the other two usages (iommu@, usb@) these votes with minimal
frequency look a bit related to the "keep_alive" stuff Konrad added. [1]
Maybe that could be used here instead of bypassing interconnect with the
clock votes?

Thanks,
Stephan

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=b979049c38e170286158e97290c892957c836903

Re: [PATCH] arm64: dts: qcom: sdm630: remove refs to nonexistent clocks

[Konrad Dybcio]

On 19.07.2023 11:46, Stephan Gerhold wrote:
> On Wed, Jul 19, 2023 at 11:36:46AM +0300, Alexey Minnekhanov wrote:
>> On 19.07.2023 10:39, Krzysztof Kozlowski wrote:
>>> It does not look like you tested the DTS against bindings. Please run
>>> `make dtbs_check`
>>>
>>
>> If DT schema for interconnect requires bus clocks to be specified, I don't
>> even know what to put there now. Can we change schema?
>>
> 
> I think I mentioned the DT schema updates during the review of Konrad's
> interconnect changes and he mentioned he would like to clean those up
> after getting the series in. (Which would be sometime soon now I guess)
> 
> For now, having the &rpmcc "bus"/"bus_a"/"ipa" clocks specified on the
> interconnect@... nodes is still valid. At runtime they will just be
> ignored. Feel free to just keep them there for this initial fix.
> 
> For the other two usages (iommu@, usb@) these votes with minimal
> frequency look a bit related to the "keep_alive" stuff Konrad added. [1]
> Maybe that could be used here instead of bypassing interconnect with the
> clock votes?
or just pass interconnects=

Konrad

Re: [PATCH] arm64: dts: qcom: sdm630: remove refs to nonexistent clocks

[Konrad Dybcio]

On 19.07.2023 09:35, Alexey Minnekhanov wrote:
> Since commit d6edc31f3a68 ("clk: qcom: smd-rpm: Separate out
> interconnect bus clocks") rpmcc-sdm660 no longer provides
> RPM_SMD_AGGR2_NOC_CLK and RPM_SMD_AGGR2_NOC_A_CLK clocks.
> Remove them to fix various probe failures and get devices
> booting again.
> 
> Signed-off-by: Alexey Minnekhanov <alexeymin@postmarketos.org>
> ---
You need to update the bindings, see commits I made for 8998

Konrad