Re: [PATCH V2 RESEND 6/6] arm64: dts: qcom: sm8650: Add video and camera clock controllers

From: Jagadeesh Kona
Date: Thu Apr 04 2024 - 06:07:11 EST




On 4/4/2024 11:00 AM, Dmitry Baryshkov wrote:
On Thu, 4 Apr 2024 at 08:13, Jagadeesh Kona <quic_jkona@xxxxxxxxxxx> wrote:



On 4/3/2024 9:24 PM, Dmitry Baryshkov wrote:
On Wed, 3 Apr 2024 at 10:16, Jagadeesh Kona <quic_jkona@xxxxxxxxxxx> wrote:



On 3/25/2024 11:38 AM, Jagadeesh Kona wrote:


On 3/21/2024 6:43 PM, Dmitry Baryshkov wrote:
On Thu, 21 Mar 2024 at 11:27, Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>
wrote:

Add device nodes for video and camera clock controllers on Qualcomm
SM8650 platform.

Signed-off-by: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>
---
arch/arm64/boot/dts/qcom/sm8650.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi
b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 32c0a7b9aded..d862aa6be824 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -4,6 +4,8 @@
*/

#include <dt-bindings/clock/qcom,rpmh.h>
+#include <dt-bindings/clock/qcom,sm8450-videocc.h>
+#include <dt-bindings/clock/qcom,sm8650-camcc.h>
#include <dt-bindings/clock/qcom,sm8650-dispcc.h>
#include <dt-bindings/clock/qcom,sm8650-gcc.h>
#include <dt-bindings/clock/qcom,sm8650-gpucc.h>
@@ -3110,6 +3112,32 @@ opp-202000000 {
};
};

+ videocc: clock-controller@aaf0000 {
+ compatible = "qcom,sm8650-videocc";
+ reg = <0 0x0aaf0000 0 0x10000>;
+ clocks = <&bi_tcxo_div2>,
+ <&gcc GCC_VIDEO_AHB_CLK>;
+ power-domains = <&rpmhpd RPMHPD_MMCX>;
+ required-opps = <&rpmhpd_opp_low_svs>;

The required-opps should no longer be necessary.


Sure, will check and remove this if not required.


I checked further on this and without required-opps, if there is no vote
on the power-domain & its peer from any other consumers, when runtime
get is called on device, it enables the power domain just at the minimum
non-zero level. But in some cases, the minimum non-zero level of
power-domain could be just retention and is not sufficient for clock
controller to operate, hence required-opps property is needed to specify
the minimum level required on power-domain for this clock controller.

In which cases? If it ends up with the retention vote, it is a bug
which must be fixed.


The minimum non-zero level(configured from bootloaders) of MMCX is
retention on few chipsets but it can vary across the chipsets. Hence to
be on safer side from our end, it is good to have required-opps in DT to
specify the minimum level required for this clock controller.

We are discussing sm8650, not some abstract chipset. Does it list
retention or low_svs as a minimal level for MMCX?


Actually, the minimum level for MMCX is external to the clock controllers. But the clock controller requires MMCX to be atleast at lowsvs for it to be functional. Hence we need to keep required-opps to ensure the same without relying on the actual minimum level for MMCX.

Thanks,
Jagadeesh


+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ #power-domain-cells = <1>;
+ };
+
+ camcc: clock-controller@ade0000 {
+ compatible = "qcom,sm8650-camcc";
+ reg = <0 0x0ade0000 0 0x20000>;
+ clocks = <&gcc GCC_CAMERA_AHB_CLK>,
+ <&bi_tcxo_div2>,
+ <&bi_tcxo_ao_div2>,
+ <&sleep_clk>;
+ power-domains = <&rpmhpd RPMHPD_MMCX>;
+ required-opps = <&rpmhpd_opp_low_svs>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ #power-domain-cells = <1>;
+ };
+
mdss: display-subsystem@ae00000 {
compatible = "qcom,sm8650-mdss";
reg = <0 0x0ae00000 0 0x1000>;
--
2.43.0