arm64: dts: qcom: sc7180: Add LPASS clock controller nodes
Subsystems (5)
PATCH 1/1 arm64: dts: qcom: sc7180: Add LPASS clock controller nodes
Update the clock controller nodes for Low power audio subsystem functionality.
--- arch/arm64/boot/dts/qcom/sc7180.dtsi | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
arch/arm64/boot/dts/qcom/sc7180.dtsi +22 -0
@@ -8,6 +8,7 @@ #include <dt-bindings/clock/qcom,dispcc-sc7180.h> #include <dt-bindings/clock/qcom,gcc-sc7180.h> #include <dt-bindings/clock/qcom,gpucc-sc7180.h> +#include <dt-bindings/clock/qcom,lpasscorecc-sc7180.h> #include <dt-bindings/clock/qcom,rpmh.h> #include <dt-bindings/clock/qcom,videocc-sc7180.h> #include <dt-bindings/interconnect/qcom,osm-l3.h>
@@ -2136,6 +2137,27 @@ }; }; + lpasscc: clock-controller@62d00000 { + compatible = "qcom,sc7180-lpasscorecc"; + reg = <0 0x62d00000 0 0x50000>, + <0 0x62780000 0 0x30000>; + reg-names = "lpass_core_cc", "lpass_audio_cc"; + clocks = <&gcc GCC_LPASS_CFG_NOC_SWAY_CLK>; + clock-names = "iface"; + power-domains = <&lpass_hm LPASS_CORE_HM_GDSCR>; + #clock-cells = <1>; + #power-domain-cells = <1>; + }; + + lpass_hm: clock-controller@63000000 { + compatible = "qcom,sc7180-lpasshm"; + reg = <0 0x63000000 0 0x28>; + clocks = <&gcc GCC_LPASS_CFG_NOC_SWAY_CLK>; + clock-names = "iface"; + #clock-cells = <1>; + #power-domain-cells = <1>; + }; + etm@7040000 { compatible = "arm,coresight-etm4x", "arm,primecell"; reg = <0 0x07040000 0 0x1000>;
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Hi,
Quoted 6 lines · Taniya Das
Update the clock controller nodes for Low power audio subsystem functionality.
Signed-off-by Taniya Das <[email protected]>---
Somewhere here you should be pointing to the unlanded bindings patch, AKA:
https://lore.kernel.org/r/[email protected]
As per usual the fact that are using a new bindings #include file means Qualcomm maintainers and clock maintainers will need to coordinate landing and this needs to be pointed out.
Quoted 29 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)arch/arm64/boot/dts/qcom/sc7180.dtsi +11
-0@@ -8,6 +8,7 @@ #include <dt-bindings/clock/qcom,dispcc-sc7180.h> #include <dt-bindings/clock/qcom,gcc-sc7180.h> #include <dt-bindings/clock/qcom,gpucc-sc7180.h> +#include <dt-bindings/clock/qcom,lpasscorecc-sc7180.h> #include <dt-bindings/clock/qcom,rpmh.h> #include <dt-bindings/clock/qcom,videocc-sc7180.h> #include <dt-bindings/interconnect/qcom,osm-l3.h>@@ -2136,6 +2137,27 @@ }; }; + lpasscc: clock-controller@62d00000 { + compatible = "qcom,sc7180-lpasscorecc"; + reg = <0 0x62d00000 0 0x50000>, + <0 0x62780000 0 0x30000>; + reg-names = "lpass_core_cc", "lpass_audio_cc"; + clocks = <&gcc GCC_LPASS_CFG_NOC_SWAY_CLK>; + clock-names = "iface"; + power-domains = <&lpass_hm LPASS_CORE_HM_GDSCR>; + #clock-cells = <1>; + #power-domain-cells = <1>;
arch/arm64/boot/dts/qcom/sc7180.dtsi line 2159
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +10
-0+ }; + + lpass_hm: clock-controller@63000000 { + compatible = "qcom,sc7180-lpasshm"; + reg = <0 0x63000000 0 0x28>; + clocks = <&gcc GCC_LPASS_CFG_NOC_SWAY_CLK>; + clock-names = "iface"; + #clock-cells = <1>; + #power-domain-cells = <1>; + };
Question: would it ever make sense for a board not to need this clock controller? I ask because the sdm845 "lpass" clock controller is "disabled" by default but yours here isn't. I know sc7180 and sdm845 work pretty differently and perhaps the sdm845's default of "disabled" was just overkill, but I thought I'd ask.
arch/arm64/boot/dts/qcom/sc7180.dtsi line 2163
Quoted 4 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +1
-0+ etm@7040000 { compatible = "arm,coresight-etm4x", "arm,primecell"; reg = <0 0x07040000 0 0x1000>;
Your sort order is off. You should be sorting by unit address. Note that the "ETM" has an extra 0 before its 7, so you're comparing 63 to 07 and you should be after.
Other than those small things above this patch looks like it matches the example in the bindings, so as long as Rob / the clock guys are fine with the bindings then this seems good to go.
-Doug
Hi,
Quoted 72 lines · Doug Anderson
Hi,
Quoted 6 lines · Taniya Das
Update the clock controller nodes for Low power audio subsystem functionality.
Signed-off-by Taniya Das <[email protected]>---
Somewhere here you should be pointing to the unlanded bindings patch, AKA:
https://lore.kernel.org/r/[email protected]
As per usual the fact that are using a new bindings #include file means Qualcomm maintainers and clock maintainers will need to coordinate landing and this needs to be pointed out.
Quoted 29 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)arch/arm64/boot/dts/qcom/sc7180.dtsi +11
-0@@ -8,6 +8,7 @@ #include <dt-bindings/clock/qcom,dispcc-sc7180.h> #include <dt-bindings/clock/qcom,gcc-sc7180.h> #include <dt-bindings/clock/qcom,gpucc-sc7180.h> +#include <dt-bindings/clock/qcom,lpasscorecc-sc7180.h> #include <dt-bindings/clock/qcom,rpmh.h> #include <dt-bindings/clock/qcom,videocc-sc7180.h> #include <dt-bindings/interconnect/qcom,osm-l3.h>@@ -2136,6 +2137,27 @@ }; }; + lpasscc: clock-controller@62d00000 { + compatible = "qcom,sc7180-lpasscorecc"; + reg = <0 0x62d00000 0 0x50000>, + <0 0x62780000 0 0x30000>; + reg-names = "lpass_core_cc", "lpass_audio_cc"; + clocks = <&gcc GCC_LPASS_CFG_NOC_SWAY_CLK>; + clock-names = "iface"; + power-domains = <&lpass_hm LPASS_CORE_HM_GDSCR>; + #clock-cells = <1>; + #power-domain-cells = <1>;arch/arm64/boot/dts/qcom/sc7180.dtsi line 2159
Quoted 10 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +10
-0+ }; + + lpass_hm: clock-controller@63000000 { + compatible = "qcom,sc7180-lpasshm"; + reg = <0 0x63000000 0 0x28>; + clocks = <&gcc GCC_LPASS_CFG_NOC_SWAY_CLK>; + clock-names = "iface"; + #clock-cells = <1>; + #power-domain-cells = <1>; + };Question: would it ever make sense for a board not to need this clock controller? I ask because the sdm845 "lpass" clock controller is "disabled" by default but yours here isn't. I know sc7180 and sdm845 work pretty differently and perhaps the sdm845's default of "disabled" was just overkill, but I thought I'd ask.
arch/arm64/boot/dts/qcom/sc7180.dtsi line 2163
Quoted 4 lines
arch/arm64/boot/dts/qcom/sc7180.dtsi +1
-0+ etm@7040000 { compatible = "arm,coresight-etm4x", "arm,primecell"; reg = <0 0x07040000 0 0x1000>;
Quoted 7 lines
Your sort order is off. You should be sorting by unit address. Note that the "ETM" has an extra 0 before its 7, so you're comparing 63 to 07 and you should be after.
Other than those small things above this patch looks like it matches the example in the bindings, so as long as Rob / the clock guys are fine with the bindings then this seems good to go.
So it looks like the bindings landed and one would think we'd be good to go. Except that when I mixed your device tree patch with the driver that landed things went boom.
Can you please re-post addressing my concerns above and also adding "bi_tcxo" as I have done in the bindings example here:
https://lore.kernel.org/r/20200731133006.1.Iee81b115f5be50d6d69500fe1bda11bba6e16143@changeid
Thanks!
-Doug