| Message ID | 20230719073520.2644966-1-alexeymin@postmarketos.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: sdm630: remove refs to nonexistent clocks | expand |
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
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
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
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
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
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>;
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(-)