[PATCH 00/10] Add APSS RSC to Cluster power domain

[PATCH 00/10] Add APSS RSC to Cluster power domain

[Maulik Shah]

This series patches 1 to 4 adds/corrects the cpuidle states/
apps_rsc TCS configuration to make it same as downstream kernel.

The patches 5, 6 and 7 adds apps_rsc device to cluster power domain such
that when cluster is going to power down the cluster pre off notification
will program the 'sleep' and 'wake' votes in SLEEP TCS and WAKE TCSes.

The patches 8, 9 and 10 are to program the next wakeup in CONTROL_TCS.

[1], [2] was older way of programming CONTROL_TCS (exporting an API and
calling when last CPU was entering deeper low power mode). Now with patch
number 5,6 and 7 the apps RSC is added to to cluster power domain and hence
these patches are no longer needed with this series.

The series is tested on SM8250 with latest linux-next tag next-20220107.

[1] https://patchwork.kernel.org/project/linux-arm-msm/patch/20190218140210.14631-3-rplsssn@codeaurora.org/
[2] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=59613

Lina Iyer (1):
  soc: qcom: rpmh-rsc: Attach RSC to cluster PM domain

Maulik Shah (9):
  arm64: dts: qcom: sm8150: Correct TCS configuration for apps rsc
  arm64: dts: qcom: sm8250: Add cpuidle states
  arm64: dts: qcom: sm8350: Correct TCS configuration for apps rsc
  arm64: dts: qcom: sm8450: Update cpuidle states parameters
  dt-bindings: soc: qcom: Update devicetree binding document for
    rpmh-rsc
  arm64: dts: qcom: Add power-domains property for apps_rsc
  PM: domains: Store the closest hrtimer event of the domain CPUs
  soc: qcom: rpmh-rsc: Save base address of drv
  soc: qcom: rpmh-rsc: Write CONTROL_TCS with next timer wakeup

 .../devicetree/bindings/soc/qcom/rpmh-rsc.txt      |   6 +
 arch/arm64/boot/dts/qcom/sm8150.dtsi               |   7 +-
 arch/arm64/boot/dts/qcom/sm8250.dtsi               | 106 ++++++++++++++++
 arch/arm64/boot/dts/qcom/sm8350.dtsi               |   3 +-
 arch/arm64/boot/dts/qcom/sm8450.dtsi               |  29 ++---
 drivers/base/power/domain_governor.c               |   1 +
 drivers/soc/qcom/rpmh-internal.h                   |   9 +-
 drivers/soc/qcom/rpmh-rsc.c                        | 138 +++++++++++++++++++--
 drivers/soc/qcom/rpmh.c                            |   4 +-
 include/linux/pm_domain.h                          |   1 +
 10 files changed, 271 insertions(+), 33 deletions(-)

-- 
2.7.4

[PATCH 01/10] arm64: dts: qcom: sm8150: Correct TCS configuration for apps rsc

[Maulik Shah]

Correct the TCS config by updating the number of TCSes for each type.

Cc: devicetree@vger.kernel.org
Fixes: d8cf9372b654 ("arm64: dts: qcom: sm8150: Add apps shared nodes")
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8150.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index 6012322..7826564 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -3556,9 +3556,9 @@
 			qcom,tcs-offset = <0xd00>;
 			qcom,drv-id = <2>;
 			qcom,tcs-config = <ACTIVE_TCS  2>,
-					  <SLEEP_TCS   1>,
-					  <WAKE_TCS    1>,
-					  <CONTROL_TCS 0>;
+					  <SLEEP_TCS   3>,
+					  <WAKE_TCS    3>,
+					  <CONTROL_TCS 1>;
 
 			rpmhcc: clock-controller {
 				compatible = "qcom,sm8150-rpmh-clk";
-- 
2.7.4

[PATCH 02/10] arm64: dts: qcom: sm8250: Add cpuidle states

[Maulik Shah]

This change adds various idle states and add devices to power domains.

Cc: devicetree@vger.kernel.org
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 105 +++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 5617a46..077d0ab 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -98,6 +98,8 @@
 			capacity-dmips-mhz = <448>;
 			dynamic-power-coefficient = <205>;
 			next-level-cache = <&L2_0>;
+			power-domains = <&CPU_PD0>;
+			power-domain-names = "psci";
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			operating-points-v2 = <&cpu0_opp_table>;
 			interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -120,6 +122,8 @@
 			capacity-dmips-mhz = <448>;
 			dynamic-power-coefficient = <205>;
 			next-level-cache = <&L2_100>;
+			power-domains = <&CPU_PD1>;
+			power-domain-names = "psci";
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			operating-points-v2 = <&cpu0_opp_table>;
 			interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -139,6 +143,8 @@
 			capacity-dmips-mhz = <448>;
 			dynamic-power-coefficient = <205>;
 			next-level-cache = <&L2_200>;
+			power-domains = <&CPU_PD2>;
+			power-domain-names = "psci";
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			operating-points-v2 = <&cpu0_opp_table>;
 			interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -158,6 +164,8 @@
 			capacity-dmips-mhz = <448>;
 			dynamic-power-coefficient = <205>;
 			next-level-cache = <&L2_300>;
+			power-domains = <&CPU_PD3>;
+			power-domain-names = "psci";
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			operating-points-v2 = <&cpu0_opp_table>;
 			interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -177,6 +185,8 @@
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <379>;
 			next-level-cache = <&L2_400>;
+			power-domains = <&CPU_PD4>;
+			power-domain-names = "psci";
 			qcom,freq-domain = <&cpufreq_hw 1>;
 			operating-points-v2 = <&cpu4_opp_table>;
 			interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -196,6 +206,8 @@
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <379>;
 			next-level-cache = <&L2_500>;
+			power-domains = <&CPU_PD5>;
+			power-domain-names = "psci";
 			qcom,freq-domain = <&cpufreq_hw 1>;
 			operating-points-v2 = <&cpu4_opp_table>;
 			interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -216,6 +228,8 @@
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <379>;
 			next-level-cache = <&L2_600>;
+			power-domains = <&CPU_PD6>;
+			power-domain-names = "psci";
 			qcom,freq-domain = <&cpufreq_hw 1>;
 			operating-points-v2 = <&cpu4_opp_table>;
 			interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -235,6 +249,8 @@
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <444>;
 			next-level-cache = <&L2_700>;
+			power-domains = <&CPU_PD7>;
+			power-domain-names = "psci";
 			qcom,freq-domain = <&cpufreq_hw 2>;
 			operating-points-v2 = <&cpu7_opp_table>;
 			interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
@@ -281,6 +297,42 @@
 				};
 			};
 		};
+
+		idle-states {
+			entry-method = "psci";
+
+			LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
+				compatible = "arm,idle-state";
+				idle-state-name = "silver-rail-power-collapse";
+				arm,psci-suspend-param = <0x40000004>;
+				entry-latency-us = <360>;
+				exit-latency-us = <531>;
+				min-residency-us = <3934>;
+				local-timer-stop;
+			};
+
+			BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
+				compatible = "arm,idle-state";
+				idle-state-name = "gold-rail-power-collapse";
+				arm,psci-suspend-param = <0x40000004>;
+				entry-latency-us = <702>;
+				exit-latency-us = <1061>;
+				min-residency-us = <4488>;
+				local-timer-stop;
+			};
+		};
+
+		domain-idle-states {
+			CLUSTER_SLEEP_0: cluster-sleep-0 {
+				compatible = "domain-idle-state";
+				idle-state-name = "cluster-llcc-off";
+				arm,psci-suspend-param = <0x4100c244>;
+				entry-latency-us = <3264>;
+				exit-latency-us = <6562>;
+				min-residency-us = <9987>;
+				local-timer-stop;
+			};
+		};
 	};
 
 	cpu0_opp_table: cpu0_opp_table {
@@ -594,6 +646,59 @@
 	psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
+
+		CPU_PD0: cpu0 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+		};
+
+		CPU_PD1: cpu1 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+		};
+
+		CPU_PD2: cpu2 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+		};
+
+		CPU_PD3: cpu3 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
+		};
+
+		CPU_PD4: cpu4 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&BIG_CPU_SLEEP_0>;
+		};
+
+		CPU_PD5: cpu5 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&BIG_CPU_SLEEP_0>;
+		};
+
+		CPU_PD6: cpu6 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&BIG_CPU_SLEEP_0>;
+		};
+
+		CPU_PD7: cpu7 {
+			#power-domain-cells = <0>;
+			power-domains = <&CLUSTER_PD>;
+			domain-idle-states = <&BIG_CPU_SLEEP_0>;
+		};
+
+		CLUSTER_PD: cpu-cluster0 {
+			#power-domain-cells = <0>;
+			domain-idle-states = <&CLUSTER_SLEEP_0>;
+		};
 	};
 
 	reserved-memory {
-- 
2.7.4

[PATCH 03/10] arm64: dts: qcom: sm8350: Correct TCS configuration for apps rsc

[Maulik Shah]

Correct the TCS config by updating the number of TCSes for each type.

Cc: devicetree@vger.kernel.org
Fixes: b7e8f433a673 ("arm64: dts: qcom: Add basic devicetree support for SM8350 SoC")
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8350.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index 53b39e7..665f79f 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -1802,7 +1802,7 @@
 			qcom,tcs-offset = <0xd00>;
 			qcom,drv-id = <2>;
 			qcom,tcs-config = <ACTIVE_TCS  2>, <SLEEP_TCS   3>,
-					  <WAKE_TCS    3>, <CONTROL_TCS 1>;
+					  <WAKE_TCS    3>, <CONTROL_TCS 0>;
 
 			rpmhcc: clock-controller {
 				compatible = "qcom,sm8350-rpmh-clk";
-- 
2.7.4

[PATCH 04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters

[Maulik Shah]

This change updates/corrects below cpuidle parameters

1. entry-latency, exit-latency and residency for various idle states.
2. arm,psci-suspend-param which is same for CLUSTER_SLEEP_0/1 states.
3. Add CLUSTER_SLEEP_1 in CLUSTER_PD.

Cc: devicetree@vger.kernel.org
Fixes: 5188049c9b36 ("arm64: dts: qcom: Add base SM8450 DTSI")
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 10c25ad..5e329f8 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -203,9 +203,9 @@
 				compatible = "arm,idle-state";
 				idle-state-name = "silver-rail-power-collapse";
 				arm,psci-suspend-param = <0x40000004>;
-				entry-latency-us = <274>;
-				exit-latency-us = <480>;
-				min-residency-us = <3934>;
+				entry-latency-us = <800>;
+				exit-latency-us = <750>;
+				min-residency-us = <4090>;
 				local-timer-stop;
 			};
 
@@ -213,9 +213,9 @@
 				compatible = "arm,idle-state";
 				idle-state-name = "gold-rail-power-collapse";
 				arm,psci-suspend-param = <0x40000004>;
-				entry-latency-us = <327>;
-				exit-latency-us = <1502>;
-				min-residency-us = <4488>;
+				entry-latency-us = <600>;
+				exit-latency-us = <1550>;
+				min-residency-us = <4791>;
 				local-timer-stop;
 			};
 		};
@@ -224,10 +224,10 @@
 			CLUSTER_SLEEP_0: cluster-sleep-0 {
 				compatible = "domain-idle-state";
 				idle-state-name = "cluster-l3-off";
-				arm,psci-suspend-param = <0x4100c344>;
-				entry-latency-us = <584>;
-				exit-latency-us = <2332>;
-				min-residency-us = <6118>;
+				arm,psci-suspend-param = <0x41000044>;
+				entry-latency-us = <1050>;
+				exit-latency-us = <2500>;
+				min-residency-us = <5309>;
 				local-timer-stop;
 			};
 
@@ -235,9 +235,9 @@
 				compatible = "domain-idle-state";
 				idle-state-name = "cluster-power-collapse";
 				arm,psci-suspend-param = <0x4100c344>;
-				entry-latency-us = <2893>;
-				exit-latency-us = <4023>;
-				min-residency-us = <9987>;
+				entry-latency-us = <2700>;
+				exit-latency-us = <3500>;
+				min-residency-us = <13959>;
 				local-timer-stop;
 			};
 		};
@@ -315,7 +315,7 @@
 
 		CLUSTER_PD: cpu-cluster0 {
 			#power-domain-cells = <0>;
-			domain-idle-states = <&CLUSTER_SLEEP_0>;
+			domain-idle-states = <&CLUSTER_SLEEP_0 &CLUSTER_SLEEP_1>;
 		};
 	};
 
-- 
2.7.4

[PATCH 05/10] dt-bindings: soc: qcom: Update devicetree binding document for rpmh-rsc

[Maulik Shah]

The change documents power-domains property for RSC device.
This optional property points to corresponding PM domain node.

Cc: devicetree@vger.kernel.org
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
index 9b86d1e..85b9859 100644
--- a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
+++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
@@ -78,6 +78,11 @@ Properties:
 	            CONTROL_TCS
 	- Cell #2 (Number of TCS): <u32>
 
+- power-domains:
+	Usage: optional
+	Value type: <prop-encoded-power-domains>
+	Definition: Phandle pointing to the corresponding PM domain node.
+
 - label:
 	Usage: optional
 	Value type: <string>
@@ -112,6 +117,7 @@ TCS-OFFSET: 0xD00
 				  <SLEEP_TCS   3>,
 				  <WAKE_TCS    3>,
 				  <CONTROL_TCS 1>;
+		power-domains = <&CLUSTER_PD>;
 	};
 
 Example 2:
-- 
2.7.4

[PATCH 06/10] soc: qcom: rpmh-rsc: Attach RSC to cluster PM domain

[Maulik Shah]

From: Lina Iyer <ilina@codeaurora.org>

RSC is part the CPU subsystem and powers off the CPU domains when all
the CPUs and no RPMH transactions are pending from any of the drivers.
The RSC needs to flush the 'sleep' and 'wake' votes that are critical
for saving power when all the CPUs are in idle.

Let's make RSC part of the CPU PM domains, by attaching it to the
cluster power domain. Registering for PM domain notifications, RSC
driver can be notified that the last CPU is powering down. When the last
CPU is powering down the domain, let's flush the 'sleep' and 'wake'
votes that are stored in the data buffers into the hardware and also
write next wakeup in CONTROL_TCS.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 drivers/soc/qcom/rpmh-internal.h |  6 +++-
 drivers/soc/qcom/rpmh-rsc.c      | 60 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 344ba68..32ac117 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -97,7 +97,9 @@ struct rpmh_ctrlr {
  * @rsc_pm:             CPU PM notifier for controller.
  *                      Used when solver mode is not present.
  * @cpus_in_pm:         Number of CPUs not in idle power collapse.
- *                      Used when solver mode is not present.
+ *                      Used when solver mode and "power-domains" is not present.
+ * @genpd_nb:           PM Domain notifier for cluster genpd notifications.
+ * @genpdb:             PM Domain for cluster genpd.
  * @tcs:                TCS groups.
  * @tcs_in_use:         S/W state of the TCS; only set for ACTIVE_ONLY
  *                      transfers, but might show a sleep/wake TCS in use if
@@ -117,6 +119,8 @@ struct rsc_drv {
 	int id;
 	int num_tcs;
 	struct notifier_block rsc_pm;
+	struct notifier_block genpd_nb;
+	struct generic_pm_domain *genpd;
 	atomic_t cpus_in_pm;
 	struct tcs_group tcs[TCS_TYPE_NR];
 	DECLARE_BITMAP(tcs_in_use, MAX_TCS_NR);
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 01c2f50c..5875ad5 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -14,10 +14,13 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/wait.h>
@@ -834,6 +837,51 @@ static int rpmh_rsc_cpu_pm_callback(struct notifier_block *nfb,
 	return ret;
 }
 
+/**
+ * rpmh_rsc_pd_callback() - Check if any of the AMCs are busy.
+ * @nfb:    Pointer to the genpd notifier block in struct rsc_drv.
+ * @action: GENPD_NOTIFY_PRE_OFF, GENPD_NOTIFY_OFF, GENPD_NOTIFY_PRE_ON or GENPD_NOTIFY_ON.
+ * @v:      Unused
+ *
+ * This function is given to dev_pm_genpd_add_notifier() so we can be informed
+ * about when cluster-pd is going down. When cluster go down we know no more active
+ * transfers will be started so we write sleep/wake sets. This function gets
+ * called from cpuidle code paths and also at system suspend time.
+ *
+ * If AMCs are not busy then writes cached sleep and wake messages to TCSes.
+ * The firmware then takes care of triggering them when entering deepest low power modes.
+ *
+ * Return:
+ * * NOTIFY_OK          - success
+ * * NOTIFY_BAD         - failure
+ */
+static int rpmh_rsc_pd_callback(struct notifier_block *nfb,
+				unsigned long action, void *v)
+{
+	struct rsc_drv *drv = container_of(nfb, struct rsc_drv, genpd_nb);
+
+	/* We don't need to lock as domin on/off are serialized */
+	if ((action == GENPD_NOTIFY_PRE_OFF) &&
+	    (rpmh_rsc_ctrlr_is_busy(drv) || rpmh_flush(&drv->client)))
+		return NOTIFY_BAD;
+
+	return NOTIFY_OK;
+}
+
+static int rpmh_rsc_pd_attach(struct rsc_drv *drv, struct device *dev)
+{
+	int ret;
+
+	pm_runtime_enable(dev);
+	ret = dev_pm_domain_attach(dev, false);
+	if (ret)
+		return ret;
+
+	drv->genpd = pd_to_genpd(dev->pm_domain);
+	drv->genpd_nb.notifier_call = rpmh_rsc_pd_callback;
+	return dev_pm_genpd_add_notifier(dev, &drv->genpd_nb);
+}
+
 static int rpmh_probe_tcs_config(struct platform_device *pdev,
 				 struct rsc_drv *drv, void __iomem *base)
 {
@@ -963,7 +1011,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
 		return ret;
 
 	/*
-	 * CPU PM notification are not required for controllers that support
+	 * CPU PM/genpd notification are not required for controllers that support
 	 * 'HW solver' mode where they can be in autonomous mode executing low
 	 * power mode to power down.
 	 */
@@ -971,8 +1019,14 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
 	solver_config &= DRV_HW_SOLVER_MASK << DRV_HW_SOLVER_SHIFT;
 	solver_config = solver_config >> DRV_HW_SOLVER_SHIFT;
 	if (!solver_config) {
-		drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback;
-		cpu_pm_register_notifier(&drv->rsc_pm);
+		if (of_find_property(dn, "power-domains", NULL)) {
+			ret = rpmh_rsc_pd_attach(drv, &pdev->dev);
+			if (ret)
+				return ret;
+		} else {
+			drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback;
+			cpu_pm_register_notifier(&drv->rsc_pm);
+		}
 	}
 
 	/* Enable the active TCS to send requests immediately */
-- 
2.7.4

[PATCH 07/10] arm64: dts: qcom: Add power-domains property for apps_rsc

[Maulik Shah]

Add power-domains property which allows apps_rsc device to attach
to cluster power domain on sm8150, sm8250, sm8350 and sm8450.

Cc: devicetree@vger.kernel.org
Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 +
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 1 +
 arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 +
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 1 +
 4 files changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
index 7826564..83a44f5 100644
--- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
@@ -3559,6 +3559,7 @@
 					  <SLEEP_TCS   3>,
 					  <WAKE_TCS    3>,
 					  <CONTROL_TCS 1>;
+			power-domains = <&CLUSTER_PD>;
 
 			rpmhcc: clock-controller {
 				compatible = "qcom,sm8150-rpmh-clk";
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 077d0ab..ebb4a4e 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -4593,6 +4593,7 @@
 			qcom,drv-id = <2>;
 			qcom,tcs-config = <ACTIVE_TCS  2>, <SLEEP_TCS   3>,
 					  <WAKE_TCS    3>, <CONTROL_TCS 1>;
+			power-domains = <&CLUSTER_PD>;
 
 			rpmhcc: clock-controller {
 				compatible = "qcom,sm8250-rpmh-clk";
diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index 665f79f..2c5dc305 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -1803,6 +1803,7 @@
 			qcom,drv-id = <2>;
 			qcom,tcs-config = <ACTIVE_TCS  2>, <SLEEP_TCS   3>,
 					  <WAKE_TCS    3>, <CONTROL_TCS 0>;
+			power-domains = <&CLUSTER_PD>;
 
 			rpmhcc: clock-controller {
 				compatible = "qcom,sm8350-rpmh-clk";
diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 5e329f8..acd122a 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -910,6 +910,7 @@
 			qcom,drv-id = <2>;
 			qcom,tcs-config = <ACTIVE_TCS  3>, <SLEEP_TCS   2>,
 					  <WAKE_TCS    2>, <CONTROL_TCS 0>;
+			power-domains = <&CLUSTER_PD>;
 
 			apps_bcm_voter: bcm-voter {
 				compatible = "qcom,bcm-voter";
-- 
2.7.4

[PATCH 08/10] PM: domains: Store the closest hrtimer event of the domain CPUs

[Maulik Shah]

The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI)
SoCs when the deepest CPUidle modes results in the SoC also to enter
the low power mode.

RSC is part of CPU subsystem and APSS rsc device is attached to cluster
power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which
can wakeup the SoC from deepest low power states. The CONTROL_TCS does
this by writing next wakeup in always on domain timer when the SoC is
entering the low power state.

Store the domain wakeup time from all the CPUs which can be used from
domain power off callback by RSC device.

Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 drivers/base/power/domain_governor.c | 1 +
 include/linux/pm_domain.h            | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
index cd08c58..a4c7dd8 100644
--- a/drivers/base/power/domain_governor.c
+++ b/drivers/base/power/domain_governor.c
@@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
 				domain_wakeup = next_hrtimer;
 		}
 	}
+	genpd->next_hrtimer = domain_wakeup;
 
 	/* The minimum idle duration is from now - until the next wakeup. */
 	idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now));
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 67017c9..682b372 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -136,6 +136,7 @@ struct generic_pm_domain {
 	struct gpd_dev_ops dev_ops;
 	s64 max_off_time_ns;	/* Maximum allowed "suspended" time. */
 	ktime_t next_wakeup;	/* Maintained by the domain governor */
+	ktime_t next_hrtimer;	/* Closest hrtimer event of the domain CPUs */
 	bool max_off_time_changed;
 	bool cached_power_down_ok;
 	bool cached_power_down_state_idx;
-- 
2.7.4

[PATCH 09/10] soc: qcom: rpmh-rsc: Save base address of drv

[Maulik Shah]

Add changes to save drv's base address for rsc. This is
used to read drv's configuration such as solver mode is
supported or to write into CONTROL_TCS registers.

Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 drivers/soc/qcom/rpmh-internal.h |  2 ++
 drivers/soc/qcom/rpmh-rsc.c      | 18 ++++++++----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 32ac117..6770bbb 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -91,6 +91,7 @@ struct rpmh_ctrlr {
  * Resource State Coordinator controller (RSC)
  *
  * @name:               Controller identifier.
+ * @base:               Start address of the DRV registers in this controller.
  * @tcs_base:           Start address of the TCS registers in this controller.
  * @id:                 Instance id in the controller (Direct Resource Voter).
  * @num_tcs:            Number of TCSes in this DRV.
@@ -115,6 +116,7 @@ struct rpmh_ctrlr {
  */
 struct rsc_drv {
 	const char *name;
+	void __iomem *base;
 	void __iomem *tcs_base;
 	int id;
 	int num_tcs;
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 5875ad5..c2a7c6c 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -882,8 +882,7 @@ static int rpmh_rsc_pd_attach(struct rsc_drv *drv, struct device *dev)
 	return dev_pm_genpd_add_notifier(dev, &drv->genpd_nb);
 }
 
-static int rpmh_probe_tcs_config(struct platform_device *pdev,
-				 struct rsc_drv *drv, void __iomem *base)
+static int rpmh_probe_tcs_config(struct platform_device *pdev, struct rsc_drv *drv)
 {
 	struct tcs_type_config {
 		u32 type;
@@ -897,9 +896,9 @@ static int rpmh_probe_tcs_config(struct platform_device *pdev,
 	ret = of_property_read_u32(dn, "qcom,tcs-offset", &offset);
 	if (ret)
 		return ret;
-	drv->tcs_base = base + offset;
+	drv->tcs_base = drv->base + offset;
 
-	config = readl_relaxed(base + DRV_PRNT_CHLD_CONFIG);
+	config = readl_relaxed(drv->base + DRV_PRNT_CHLD_CONFIG);
 
 	max_tcs = config;
 	max_tcs &= DRV_NUM_TCS_MASK << (DRV_NUM_TCS_SHIFT * drv->id);
@@ -961,7 +960,6 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
 	char drv_id[10] = {0};
 	int ret, irq;
 	u32 solver_config;
-	void __iomem *base;
 
 	/*
 	 * Even though RPMh doesn't directly use cmd-db, all of its children
@@ -988,11 +986,11 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
 		drv->name = dev_name(&pdev->dev);
 
 	snprintf(drv_id, ARRAY_SIZE(drv_id), "drv-%d", drv->id);
-	base = devm_platform_ioremap_resource_byname(pdev, drv_id);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	drv->base = devm_platform_ioremap_resource_byname(pdev, drv_id);
+	if (IS_ERR(drv->base))
+		return PTR_ERR(drv->base);
 
-	ret = rpmh_probe_tcs_config(pdev, drv, base);
+	ret = rpmh_probe_tcs_config(pdev, drv);
 	if (ret)
 		return ret;
 
@@ -1015,7 +1013,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
 	 * 'HW solver' mode where they can be in autonomous mode executing low
 	 * power mode to power down.
 	 */
-	solver_config = readl_relaxed(base + DRV_SOLVER_CONFIG);
+	solver_config = readl_relaxed(drv->base + DRV_SOLVER_CONFIG);
 	solver_config &= DRV_HW_SOLVER_MASK << DRV_HW_SOLVER_SHIFT;
 	solver_config = solver_config >> DRV_HW_SOLVER_SHIFT;
 	if (!solver_config) {
-- 
2.7.4

[PATCH 10/10] soc: qcom: rpmh-rsc: Write CONTROL_TCS with next timer wakeup

[Maulik Shah]

The next wakeup timer value needs to be set in always on domain timer
as the arch timer interrupt can not wakeup the SoC if after the deepest
CPUidle states the SoC also enters deepest low power state.

To wakeup the SoC in such scenarios the earliest wakeup time is set in
CONTROL_TCS and the firmware takes care of setting up its own timer in
always on domain with next wakeup time. The timer wakes up the RSC and
sets resources back to wake state.

Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
---
 drivers/soc/qcom/rpmh-internal.h |  1 +
 drivers/soc/qcom/rpmh-rsc.c      | 60 ++++++++++++++++++++++++++++++++++++++++
 drivers/soc/qcom/rpmh.c          |  4 ++-
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 6770bbb..04789a37 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -135,6 +135,7 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, const struct tcs_request *msg);
 int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
 			     const struct tcs_request *msg);
 void rpmh_rsc_invalidate(struct rsc_drv *drv);
+void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv);
 
 void rpmh_tx_done(const struct tcs_request *msg, int r);
 int rpmh_flush(struct rpmh_ctrlr *ctrlr);
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index c2a7c6c..b3b85f1 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -12,6 +12,7 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
+#include <linux/ktime.h>
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/notifier.h>
@@ -25,6 +26,7 @@
 #include <linux/spinlock.h>
 #include <linux/wait.h>
 
+#include <clocksource/arm_arch_timer.h>
 #include <soc/qcom/cmd-db.h>
 #include <soc/qcom/tcs.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
@@ -49,6 +51,14 @@
 #define DRV_NCPT_MASK			0x1F
 #define DRV_NCPT_SHIFT			27
 
+/* Offsets for CONTROL TCS Registers */
+#define RSC_DRV_CTL_TCS_DATA_HI		0x38
+#define RSC_DRV_CTL_TCS_DATA_HI_MASK	0xFFFFFF
+#define RSC_DRV_CTL_TCS_DATA_HI_VALID	BIT(31)
+#define RSC_DRV_CTL_TCS_DATA_LO		0x40
+#define RSC_DRV_CTL_TCS_DATA_LO_MASK	0xFFFFFFFF
+#define RSC_DRV_CTL_TCS_DATA_SIZE	32
+
 /* Offsets for common TCS Registers, one bit per TCS */
 #define RSC_DRV_IRQ_ENABLE		0x00
 #define RSC_DRV_IRQ_STATUS		0x04
@@ -142,6 +152,14 @@
  *  +---------------------------------------------------+
  */
 
+#define USECS_TO_CYCLES(time_usecs)			\
+	xloops_to_cycles((time_usecs) * 0x10C7UL)
+
+static inline unsigned long xloops_to_cycles(unsigned long xloops)
+{
+	return (xloops * loops_per_jiffy * HZ) >> 32;
+}
+
 static inline void __iomem *
 tcs_reg_addr(const struct rsc_drv *drv, int reg, int tcs_id)
 {
@@ -757,6 +775,48 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv)
 }
 
 /**
+ * rpmh_rsc_write_next_wakeup() - Write next wakeup in CONTROL_TCS.
+ * @drv: The controller
+ *
+ * Writes maximum wakeup cycles when called from suspend.
+ * Writes earliest hrtimer wakeup when called from idle.
+ */
+void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv)
+{
+	ktime_t now, wakeup;
+	u64 wakeup_us, wakeup_cycles = ~0;
+	u32 lo, hi;
+
+	if (!drv->tcs[CONTROL_TCS].num_tcs || !drv->genpd)
+		return;
+
+	/* Set highest time when system (timekeeping) is suspended */
+	if (system_state == SYSTEM_SUSPEND)
+		goto exit;
+
+	/* Find the earliest hrtimer wakeup from online cpus */
+	wakeup = drv->genpd->next_hrtimer;
+
+	/* Find the relative wakeup in kernel time scale */
+	now = ktime_get();
+	wakeup = ktime_sub(wakeup, now);
+	wakeup_us = ktime_to_us(wakeup);
+
+	/* Convert the wakeup to arch timer scale */
+	wakeup_cycles = USECS_TO_CYCLES(wakeup_us);
+	wakeup_cycles += arch_timer_read_counter();
+
+exit:
+	lo = wakeup_cycles & RSC_DRV_CTL_TCS_DATA_LO_MASK;
+	hi = wakeup_cycles >> RSC_DRV_CTL_TCS_DATA_SIZE;
+	hi &= RSC_DRV_CTL_TCS_DATA_HI_MASK;
+	hi |= RSC_DRV_CTL_TCS_DATA_HI_VALID;
+
+	writel_relaxed(lo, drv->base + RSC_DRV_CTL_TCS_DATA_LO);
+	writel_relaxed(hi, drv->base + RSC_DRV_CTL_TCS_DATA_HI);
+}
+
+/**
  * rpmh_rsc_cpu_pm_callback() - Check if any of the AMCs are busy.
  * @nfb:    Pointer to the notifier block in struct rsc_drv.
  * @action: CPU_PM_ENTER, CPU_PM_ENTER_FAILED, or CPU_PM_EXIT.
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 01765ee..3a53ed9 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -450,7 +450,7 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
 
 	if (!ctrlr->dirty) {
 		pr_debug("Skipping flush, TCS has latest data.\n");
-		goto exit;
+		goto write_next_wakeup;
 	}
 
 	/* Invalidate the TCSes first to avoid stale data */
@@ -479,6 +479,8 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
 
 	ctrlr->dirty = false;
 
+write_next_wakeup:
+	rpmh_rsc_write_next_wakeup(ctrlr_to_drv(ctrlr));
 exit:
 	spin_unlock(&ctrlr->cache_lock);
 	return ret;
-- 
2.7.4

Re: [PATCH 06/10] soc: qcom: rpmh-rsc: Attach RSC to cluster PM domain

[Ulf Hansson]

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> From: Lina Iyer <ilina@codeaurora.org>
>
> RSC is part the CPU subsystem and powers off the CPU domains when all
> the CPUs and no RPMH transactions are pending from any of the drivers.
> The RSC needs to flush the 'sleep' and 'wake' votes that are critical
> for saving power when all the CPUs are in idle.
>
> Let's make RSC part of the CPU PM domains, by attaching it to the
> cluster power domain. Registering for PM domain notifications, RSC
> driver can be notified that the last CPU is powering down. When the last
> CPU is powering down the domain, let's flush the 'sleep' and 'wake'
> votes that are stored in the data buffers into the hardware and also
> write next wakeup in CONTROL_TCS.
>
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> ---
>  drivers/soc/qcom/rpmh-internal.h |  6 +++-
>  drivers/soc/qcom/rpmh-rsc.c      | 60 ++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 62 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 344ba68..32ac117 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -97,7 +97,9 @@ struct rpmh_ctrlr {
>   * @rsc_pm:             CPU PM notifier for controller.
>   *                      Used when solver mode is not present.
>   * @cpus_in_pm:         Number of CPUs not in idle power collapse.
> - *                      Used when solver mode is not present.
> + *                      Used when solver mode and "power-domains" is not present.
> + * @genpd_nb:           PM Domain notifier for cluster genpd notifications.
> + * @genpdb:             PM Domain for cluster genpd.

/s/genpdb/genpd

>   * @tcs:                TCS groups.
>   * @tcs_in_use:         S/W state of the TCS; only set for ACTIVE_ONLY
>   *                      transfers, but might show a sleep/wake TCS in use if
> @@ -117,6 +119,8 @@ struct rsc_drv {
>         int id;
>         int num_tcs;
>         struct notifier_block rsc_pm;
> +       struct notifier_block genpd_nb;
> +       struct generic_pm_domain *genpd;
>         atomic_t cpus_in_pm;
>         struct tcs_group tcs[TCS_TYPE_NR];
>         DECLARE_BITMAP(tcs_in_use, MAX_TCS_NR);
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index 01c2f50c..5875ad5 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -14,10 +14,13 @@
>  #include <linux/kernel.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
> +#include <linux/notifier.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/wait.h>
> @@ -834,6 +837,51 @@ static int rpmh_rsc_cpu_pm_callback(struct notifier_block *nfb,
>         return ret;
>  }
>
> +/**
> + * rpmh_rsc_pd_callback() - Check if any of the AMCs are busy.
> + * @nfb:    Pointer to the genpd notifier block in struct rsc_drv.
> + * @action: GENPD_NOTIFY_PRE_OFF, GENPD_NOTIFY_OFF, GENPD_NOTIFY_PRE_ON or GENPD_NOTIFY_ON.
> + * @v:      Unused
> + *
> + * This function is given to dev_pm_genpd_add_notifier() so we can be informed
> + * about when cluster-pd is going down. When cluster go down we know no more active
> + * transfers will be started so we write sleep/wake sets. This function gets
> + * called from cpuidle code paths and also at system suspend time.
> + *
> + * If AMCs are not busy then writes cached sleep and wake messages to TCSes.
> + * The firmware then takes care of triggering them when entering deepest low power modes.
> + *
> + * Return:
> + * * NOTIFY_OK          - success
> + * * NOTIFY_BAD         - failure
> + */
> +static int rpmh_rsc_pd_callback(struct notifier_block *nfb,
> +                               unsigned long action, void *v)
> +{
> +       struct rsc_drv *drv = container_of(nfb, struct rsc_drv, genpd_nb);
> +
> +       /* We don't need to lock as domin on/off are serialized */

/s/domin/genpd

> +       if ((action == GENPD_NOTIFY_PRE_OFF) &&
> +           (rpmh_rsc_ctrlr_is_busy(drv) || rpmh_flush(&drv->client)))
> +               return NOTIFY_BAD;
> +
> +       return NOTIFY_OK;
> +}
> +
> +static int rpmh_rsc_pd_attach(struct rsc_drv *drv, struct device *dev)
> +{
> +       int ret;
> +
> +       pm_runtime_enable(dev);
> +       ret = dev_pm_domain_attach(dev, false);

Unless I have missed something, this should not be needed.

This is because it's a regular platform driver and we only have a
single PM domain to attach for the rsc device. In this case, the
platform bus is capable of managing the attach to the genpd. See
platform_probe() in drivers/base/platform.c.

> +       if (ret)
> +               return ret;
 > +
> +       drv->genpd = pd_to_genpd(dev->pm_domain);

I couldn't find where this pointer is being used later in the driver.
In any case, you can probably use dev->pm_domain directly wherever
needed instead.

> +       drv->genpd_nb.notifier_call = rpmh_rsc_pd_callback;
> +       return dev_pm_genpd_add_notifier(dev, &drv->genpd_nb);

You should call pm_runtime_disable() in the error path.

> +}
> +
>  static int rpmh_probe_tcs_config(struct platform_device *pdev,
>                                  struct rsc_drv *drv, void __iomem *base)
>  {
> @@ -963,7 +1011,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>                 return ret;
>
>         /*
> -        * CPU PM notification are not required for controllers that support
> +        * CPU PM/genpd notification are not required for controllers that support
>          * 'HW solver' mode where they can be in autonomous mode executing low
>          * power mode to power down.
>          */
> @@ -971,8 +1019,14 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>         solver_config &= DRV_HW_SOLVER_MASK << DRV_HW_SOLVER_SHIFT;
>         solver_config = solver_config >> DRV_HW_SOLVER_SHIFT;
>         if (!solver_config) {
> -               drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback;
> -               cpu_pm_register_notifier(&drv->rsc_pm);
> +               if (of_find_property(dn, "power-domains", NULL)) {

Rather than parsing the DT, I think it's better to check if
"dev->pm_domain" has been assigned. As I indicated above, the platform
bus manages the attach before the driver's ->probe() callback is
invoked.

> +                       ret = rpmh_rsc_pd_attach(drv, &pdev->dev);
> +                       if (ret)
> +                               return ret;
> +               } else {
> +                       drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback;
> +                       cpu_pm_register_notifier(&drv->rsc_pm);
> +               }
>         }
>
>         /* Enable the active TCS to send requests immediately */

Beyond this point, you need to call the below to manage the error path
correctly:
dev_pm_genpd_remove_notifier()
pm_runtime_disable()

> --
> 2.7.4
>

Kind regards
Uffe

Re: [PATCH 02/10] arm64: dts: qcom: sm8250: Add cpuidle states

[Ulf Hansson]

On Sun, 9 Jan 2022 at 18:25, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> This change adds various idle states and add devices to power domains.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe


> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 105 +++++++++++++++++++++++++++++++++++
>  1 file changed, 105 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 5617a46..077d0ab 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -98,6 +98,8 @@
>                         capacity-dmips-mhz = <448>;
>                         dynamic-power-coefficient = <205>;
>                         next-level-cache = <&L2_0>;
> +                       power-domains = <&CPU_PD0>;
> +                       power-domain-names = "psci";
>                         qcom,freq-domain = <&cpufreq_hw 0>;
>                         operating-points-v2 = <&cpu0_opp_table>;
>                         interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -120,6 +122,8 @@
>                         capacity-dmips-mhz = <448>;
>                         dynamic-power-coefficient = <205>;
>                         next-level-cache = <&L2_100>;
> +                       power-domains = <&CPU_PD1>;
> +                       power-domain-names = "psci";
>                         qcom,freq-domain = <&cpufreq_hw 0>;
>                         operating-points-v2 = <&cpu0_opp_table>;
>                         interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -139,6 +143,8 @@
>                         capacity-dmips-mhz = <448>;
>                         dynamic-power-coefficient = <205>;
>                         next-level-cache = <&L2_200>;
> +                       power-domains = <&CPU_PD2>;
> +                       power-domain-names = "psci";
>                         qcom,freq-domain = <&cpufreq_hw 0>;
>                         operating-points-v2 = <&cpu0_opp_table>;
>                         interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -158,6 +164,8 @@
>                         capacity-dmips-mhz = <448>;
>                         dynamic-power-coefficient = <205>;
>                         next-level-cache = <&L2_300>;
> +                       power-domains = <&CPU_PD3>;
> +                       power-domain-names = "psci";
>                         qcom,freq-domain = <&cpufreq_hw 0>;
>                         operating-points-v2 = <&cpu0_opp_table>;
>                         interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -177,6 +185,8 @@
>                         capacity-dmips-mhz = <1024>;
>                         dynamic-power-coefficient = <379>;
>                         next-level-cache = <&L2_400>;
> +                       power-domains = <&CPU_PD4>;
> +                       power-domain-names = "psci";
>                         qcom,freq-domain = <&cpufreq_hw 1>;
>                         operating-points-v2 = <&cpu4_opp_table>;
>                         interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -196,6 +206,8 @@
>                         capacity-dmips-mhz = <1024>;
>                         dynamic-power-coefficient = <379>;
>                         next-level-cache = <&L2_500>;
> +                       power-domains = <&CPU_PD5>;
> +                       power-domain-names = "psci";
>                         qcom,freq-domain = <&cpufreq_hw 1>;
>                         operating-points-v2 = <&cpu4_opp_table>;
>                         interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -216,6 +228,8 @@
>                         capacity-dmips-mhz = <1024>;
>                         dynamic-power-coefficient = <379>;
>                         next-level-cache = <&L2_600>;
> +                       power-domains = <&CPU_PD6>;
> +                       power-domain-names = "psci";
>                         qcom,freq-domain = <&cpufreq_hw 1>;
>                         operating-points-v2 = <&cpu4_opp_table>;
>                         interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -235,6 +249,8 @@
>                         capacity-dmips-mhz = <1024>;
>                         dynamic-power-coefficient = <444>;
>                         next-level-cache = <&L2_700>;
> +                       power-domains = <&CPU_PD7>;
> +                       power-domain-names = "psci";
>                         qcom,freq-domain = <&cpufreq_hw 2>;
>                         operating-points-v2 = <&cpu7_opp_table>;
>                         interconnects = <&gem_noc MASTER_AMPSS_M0 &mc_virt SLAVE_EBI_CH0>,
> @@ -281,6 +297,42 @@
>                                 };
>                         };
>                 };
> +
> +               idle-states {
> +                       entry-method = "psci";
> +
> +                       LITTLE_CPU_SLEEP_0: cpu-sleep-0-0 {
> +                               compatible = "arm,idle-state";
> +                               idle-state-name = "silver-rail-power-collapse";
> +                               arm,psci-suspend-param = <0x40000004>;
> +                               entry-latency-us = <360>;
> +                               exit-latency-us = <531>;
> +                               min-residency-us = <3934>;
> +                               local-timer-stop;
> +                       };
> +
> +                       BIG_CPU_SLEEP_0: cpu-sleep-1-0 {
> +                               compatible = "arm,idle-state";
> +                               idle-state-name = "gold-rail-power-collapse";
> +                               arm,psci-suspend-param = <0x40000004>;
> +                               entry-latency-us = <702>;
> +                               exit-latency-us = <1061>;
> +                               min-residency-us = <4488>;
> +                               local-timer-stop;
> +                       };
> +               };
> +
> +               domain-idle-states {
> +                       CLUSTER_SLEEP_0: cluster-sleep-0 {
> +                               compatible = "domain-idle-state";
> +                               idle-state-name = "cluster-llcc-off";
> +                               arm,psci-suspend-param = <0x4100c244>;
> +                               entry-latency-us = <3264>;
> +                               exit-latency-us = <6562>;
> +                               min-residency-us = <9987>;
> +                               local-timer-stop;
> +                       };
> +               };
>         };
>
>         cpu0_opp_table: cpu0_opp_table {
> @@ -594,6 +646,59 @@
>         psci {
>                 compatible = "arm,psci-1.0";
>                 method = "smc";
> +
> +               CPU_PD0: cpu0 {
> +                       #power-domain-cells = <0>;
> +                       power-domains = <&CLUSTER_PD>;
> +                       domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
> +               };
> +
> +               CPU_PD1: cpu1 {
> +                       #power-domain-cells = <0>;
> +                       power-domains = <&CLUSTER_PD>;
> +                       domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
> +               };
> +
> +               CPU_PD2: cpu2 {
> +                       #power-domain-cells = <0>;
> +                       power-domains = <&CLUSTER_PD>;
> +                       domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
> +               };
> +
> +               CPU_PD3: cpu3 {
> +                       #power-domain-cells = <0>;
> +                       power-domains = <&CLUSTER_PD>;
> +                       domain-idle-states = <&LITTLE_CPU_SLEEP_0>;
> +               };
> +
> +               CPU_PD4: cpu4 {
> +                       #power-domain-cells = <0>;
> +                       power-domains = <&CLUSTER_PD>;
> +                       domain-idle-states = <&BIG_CPU_SLEEP_0>;
> +               };
> +
> +               CPU_PD5: cpu5 {
> +                       #power-domain-cells = <0>;
> +                       power-domains = <&CLUSTER_PD>;
> +                       domain-idle-states = <&BIG_CPU_SLEEP_0>;
> +               };
> +
> +               CPU_PD6: cpu6 {
> +                       #power-domain-cells = <0>;
> +                       power-domains = <&CLUSTER_PD>;
> +                       domain-idle-states = <&BIG_CPU_SLEEP_0>;
> +               };
> +
> +               CPU_PD7: cpu7 {
> +                       #power-domain-cells = <0>;
> +                       power-domains = <&CLUSTER_PD>;
> +                       domain-idle-states = <&BIG_CPU_SLEEP_0>;
> +               };
> +
> +               CLUSTER_PD: cpu-cluster0 {
> +                       #power-domain-cells = <0>;
> +                       domain-idle-states = <&CLUSTER_SLEEP_0>;
> +               };
>         };
>
>         reserved-memory {
> --
> 2.7.4
>

Re: [PATCH 04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters

[Ulf Hansson]

On Sun, 9 Jan 2022 at 18:25, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> This change updates/corrects below cpuidle parameters
>
> 1. entry-latency, exit-latency and residency for various idle states.
> 2. arm,psci-suspend-param which is same for CLUSTER_SLEEP_0/1 states.
> 3. Add CLUSTER_SLEEP_1 in CLUSTER_PD.
>
> Cc: devicetree@vger.kernel.org
> Fixes: 5188049c9b36 ("arm64: dts: qcom: Add base SM8450 DTSI")
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 10c25ad..5e329f8 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -203,9 +203,9 @@
>                                 compatible = "arm,idle-state";
>                                 idle-state-name = "silver-rail-power-collapse";
>                                 arm,psci-suspend-param = <0x40000004>;
> -                               entry-latency-us = <274>;
> -                               exit-latency-us = <480>;
> -                               min-residency-us = <3934>;
> +                               entry-latency-us = <800>;
> +                               exit-latency-us = <750>;
> +                               min-residency-us = <4090>;
>                                 local-timer-stop;
>                         };
>
> @@ -213,9 +213,9 @@
>                                 compatible = "arm,idle-state";
>                                 idle-state-name = "gold-rail-power-collapse";
>                                 arm,psci-suspend-param = <0x40000004>;
> -                               entry-latency-us = <327>;
> -                               exit-latency-us = <1502>;
> -                               min-residency-us = <4488>;
> +                               entry-latency-us = <600>;
> +                               exit-latency-us = <1550>;
> +                               min-residency-us = <4791>;
>                                 local-timer-stop;
>                         };
>                 };
> @@ -224,10 +224,10 @@
>                         CLUSTER_SLEEP_0: cluster-sleep-0 {
>                                 compatible = "domain-idle-state";
>                                 idle-state-name = "cluster-l3-off";
> -                               arm,psci-suspend-param = <0x4100c344>;
> -                               entry-latency-us = <584>;
> -                               exit-latency-us = <2332>;
> -                               min-residency-us = <6118>;
> +                               arm,psci-suspend-param = <0x41000044>;
> +                               entry-latency-us = <1050>;
> +                               exit-latency-us = <2500>;
> +                               min-residency-us = <5309>;
>                                 local-timer-stop;
>                         };
>
> @@ -235,9 +235,9 @@
>                                 compatible = "domain-idle-state";
>                                 idle-state-name = "cluster-power-collapse";
>                                 arm,psci-suspend-param = <0x4100c344>;
> -                               entry-latency-us = <2893>;
> -                               exit-latency-us = <4023>;
> -                               min-residency-us = <9987>;
> +                               entry-latency-us = <2700>;
> +                               exit-latency-us = <3500>;
> +                               min-residency-us = <13959>;
>                                 local-timer-stop;
>                         };
>                 };
> @@ -315,7 +315,7 @@
>
>                 CLUSTER_PD: cpu-cluster0 {
>                         #power-domain-cells = <0>;
> -                       domain-idle-states = <&CLUSTER_SLEEP_0>;
> +                       domain-idle-states = <&CLUSTER_SLEEP_0 &CLUSTER_SLEEP_1>;

Should this be like the below instead?

<&CLUSTER_SLEEP_0>,  <&CLUSTER_SLEEP_1>;

>                 };
>         };
>
> --
> 2.7.4
>

Other than the above, feel free to add:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

Re: [PATCH 05/10] dt-bindings: soc: qcom: Update devicetree binding document for rpmh-rsc

[Ulf Hansson]

On Sun, 9 Jan 2022 at 18:25, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> The change documents power-domains property for RSC device.
> This optional property points to corresponding PM domain node.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

BTW, it would be nice to get the binding converted to the new DT yaml
formal, perhaps something we can look at doing on top?

Kind regards
Uffe


> ---
>  Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> index 9b86d1e..85b9859 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> @@ -78,6 +78,11 @@ Properties:
>                     CONTROL_TCS
>         - Cell #2 (Number of TCS): <u32>
>
> +- power-domains:
> +       Usage: optional
> +       Value type: <prop-encoded-power-domains>
> +       Definition: Phandle pointing to the corresponding PM domain node.
> +
>  - label:
>         Usage: optional
>         Value type: <string>
> @@ -112,6 +117,7 @@ TCS-OFFSET: 0xD00
>                                   <SLEEP_TCS   3>,
>                                   <WAKE_TCS    3>,
>                                   <CONTROL_TCS 1>;
> +               power-domains = <&CLUSTER_PD>;
>         };
>
>  Example 2:
> --
> 2.7.4
>

Re: [PATCH 07/10] arm64: dts: qcom: Add power-domains property for apps_rsc

[Ulf Hansson]

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> Add power-domains property which allows apps_rsc device to attach
> to cluster power domain on sm8150, sm8250, sm8350 and sm8450.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe


> ---
>  arch/arm64/boot/dts/qcom/sm8150.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sm8350.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 1 +
>  4 files changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> index 7826564..83a44f5 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> @@ -3559,6 +3559,7 @@
>                                           <SLEEP_TCS   3>,
>                                           <WAKE_TCS    3>,
>                                           <CONTROL_TCS 1>;
> +                       power-domains = <&CLUSTER_PD>;
>
>                         rpmhcc: clock-controller {
>                                 compatible = "qcom,sm8150-rpmh-clk";
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 077d0ab..ebb4a4e 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4593,6 +4593,7 @@
>                         qcom,drv-id = <2>;
>                         qcom,tcs-config = <ACTIVE_TCS  2>, <SLEEP_TCS   3>,
>                                           <WAKE_TCS    3>, <CONTROL_TCS 1>;
> +                       power-domains = <&CLUSTER_PD>;
>
>                         rpmhcc: clock-controller {
>                                 compatible = "qcom,sm8250-rpmh-clk";
> diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> index 665f79f..2c5dc305 100644
> --- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
> @@ -1803,6 +1803,7 @@
>                         qcom,drv-id = <2>;
>                         qcom,tcs-config = <ACTIVE_TCS  2>, <SLEEP_TCS   3>,
>                                           <WAKE_TCS    3>, <CONTROL_TCS 0>;
> +                       power-domains = <&CLUSTER_PD>;
>
>                         rpmhcc: clock-controller {
>                                 compatible = "qcom,sm8350-rpmh-clk";
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 5e329f8..acd122a 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -910,6 +910,7 @@
>                         qcom,drv-id = <2>;
>                         qcom,tcs-config = <ACTIVE_TCS  3>, <SLEEP_TCS   2>,
>                                           <WAKE_TCS    2>, <CONTROL_TCS 0>;
> +                       power-domains = <&CLUSTER_PD>;
>
>                         apps_bcm_voter: bcm-voter {
>                                 compatible = "qcom,bcm-voter";
> --
> 2.7.4
>

Re: [PATCH 09/10] soc: qcom: rpmh-rsc: Save base address of drv

[Ulf Hansson]

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> Add changes to save drv's base address for rsc. This is
> used to read drv's configuration such as solver mode is
> supported or to write into CONTROL_TCS registers.
>
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/soc/qcom/rpmh-internal.h |  2 ++
>  drivers/soc/qcom/rpmh-rsc.c      | 18 ++++++++----------
>  2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 32ac117..6770bbb 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -91,6 +91,7 @@ struct rpmh_ctrlr {
>   * Resource State Coordinator controller (RSC)
>   *
>   * @name:               Controller identifier.
> + * @base:               Start address of the DRV registers in this controller.
>   * @tcs_base:           Start address of the TCS registers in this controller.
>   * @id:                 Instance id in the controller (Direct Resource Voter).
>   * @num_tcs:            Number of TCSes in this DRV.
> @@ -115,6 +116,7 @@ struct rpmh_ctrlr {
>   */
>  struct rsc_drv {
>         const char *name;
> +       void __iomem *base;
>         void __iomem *tcs_base;
>         int id;
>         int num_tcs;
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index 5875ad5..c2a7c6c 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -882,8 +882,7 @@ static int rpmh_rsc_pd_attach(struct rsc_drv *drv, struct device *dev)
>         return dev_pm_genpd_add_notifier(dev, &drv->genpd_nb);
>  }
>
> -static int rpmh_probe_tcs_config(struct platform_device *pdev,
> -                                struct rsc_drv *drv, void __iomem *base)
> +static int rpmh_probe_tcs_config(struct platform_device *pdev, struct rsc_drv *drv)
>  {
>         struct tcs_type_config {
>                 u32 type;
> @@ -897,9 +896,9 @@ static int rpmh_probe_tcs_config(struct platform_device *pdev,
>         ret = of_property_read_u32(dn, "qcom,tcs-offset", &offset);
>         if (ret)
>                 return ret;
> -       drv->tcs_base = base + offset;
> +       drv->tcs_base = drv->base + offset;
>
> -       config = readl_relaxed(base + DRV_PRNT_CHLD_CONFIG);
> +       config = readl_relaxed(drv->base + DRV_PRNT_CHLD_CONFIG);
>
>         max_tcs = config;
>         max_tcs &= DRV_NUM_TCS_MASK << (DRV_NUM_TCS_SHIFT * drv->id);
> @@ -961,7 +960,6 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>         char drv_id[10] = {0};
>         int ret, irq;
>         u32 solver_config;
> -       void __iomem *base;
>
>         /*
>          * Even though RPMh doesn't directly use cmd-db, all of its children
> @@ -988,11 +986,11 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>                 drv->name = dev_name(&pdev->dev);
>
>         snprintf(drv_id, ARRAY_SIZE(drv_id), "drv-%d", drv->id);
> -       base = devm_platform_ioremap_resource_byname(pdev, drv_id);
> -       if (IS_ERR(base))
> -               return PTR_ERR(base);
> +       drv->base = devm_platform_ioremap_resource_byname(pdev, drv_id);
> +       if (IS_ERR(drv->base))
> +               return PTR_ERR(drv->base);
>
> -       ret = rpmh_probe_tcs_config(pdev, drv, base);
> +       ret = rpmh_probe_tcs_config(pdev, drv);
>         if (ret)
>                 return ret;
>
> @@ -1015,7 +1013,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>          * 'HW solver' mode where they can be in autonomous mode executing low
>          * power mode to power down.
>          */
> -       solver_config = readl_relaxed(base + DRV_SOLVER_CONFIG);
> +       solver_config = readl_relaxed(drv->base + DRV_SOLVER_CONFIG);
>         solver_config &= DRV_HW_SOLVER_MASK << DRV_HW_SOLVER_SHIFT;
>         solver_config = solver_config >> DRV_HW_SOLVER_SHIFT;
>         if (!solver_config) {
> --
> 2.7.4
>

Re: [PATCH 10/10] soc: qcom: rpmh-rsc: Write CONTROL_TCS with next timer wakeup

[Ulf Hansson]

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> The next wakeup timer value needs to be set in always on domain timer
> as the arch timer interrupt can not wakeup the SoC if after the deepest
> CPUidle states the SoC also enters deepest low power state.
>
> To wakeup the SoC in such scenarios the earliest wakeup time is set in
> CONTROL_TCS and the firmware takes care of setting up its own timer in
> always on domain with next wakeup time. The timer wakes up the RSC and
> sets resources back to wake state.
>
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> ---
>  drivers/soc/qcom/rpmh-internal.h |  1 +
>  drivers/soc/qcom/rpmh-rsc.c      | 60 ++++++++++++++++++++++++++++++++++++++++
>  drivers/soc/qcom/rpmh.c          |  4 ++-
>  3 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
> index 6770bbb..04789a37 100644
> --- a/drivers/soc/qcom/rpmh-internal.h
> +++ b/drivers/soc/qcom/rpmh-internal.h
> @@ -135,6 +135,7 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, const struct tcs_request *msg);
>  int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
>                              const struct tcs_request *msg);
>  void rpmh_rsc_invalidate(struct rsc_drv *drv);
> +void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv);
>
>  void rpmh_tx_done(const struct tcs_request *msg, int r);
>  int rpmh_flush(struct rpmh_ctrlr *ctrlr);
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index c2a7c6c..b3b85f1 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -12,6 +12,7 @@
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
>  #include <linux/kernel.h>
> +#include <linux/ktime.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/notifier.h>
> @@ -25,6 +26,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/wait.h>
>
> +#include <clocksource/arm_arch_timer.h>
>  #include <soc/qcom/cmd-db.h>
>  #include <soc/qcom/tcs.h>
>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> @@ -49,6 +51,14 @@
>  #define DRV_NCPT_MASK                  0x1F
>  #define DRV_NCPT_SHIFT                 27
>
> +/* Offsets for CONTROL TCS Registers */
> +#define RSC_DRV_CTL_TCS_DATA_HI                0x38
> +#define RSC_DRV_CTL_TCS_DATA_HI_MASK   0xFFFFFF
> +#define RSC_DRV_CTL_TCS_DATA_HI_VALID  BIT(31)
> +#define RSC_DRV_CTL_TCS_DATA_LO                0x40
> +#define RSC_DRV_CTL_TCS_DATA_LO_MASK   0xFFFFFFFF
> +#define RSC_DRV_CTL_TCS_DATA_SIZE      32
> +
>  /* Offsets for common TCS Registers, one bit per TCS */
>  #define RSC_DRV_IRQ_ENABLE             0x00
>  #define RSC_DRV_IRQ_STATUS             0x04
> @@ -142,6 +152,14 @@
>   *  +---------------------------------------------------+
>   */
>
> +#define USECS_TO_CYCLES(time_usecs)                    \
> +       xloops_to_cycles((time_usecs) * 0x10C7UL)
> +
> +static inline unsigned long xloops_to_cycles(unsigned long xloops)
> +{
> +       return (xloops * loops_per_jiffy * HZ) >> 32;
> +}
> +
>  static inline void __iomem *
>  tcs_reg_addr(const struct rsc_drv *drv, int reg, int tcs_id)
>  {
> @@ -757,6 +775,48 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv)
>  }
>
>  /**
> + * rpmh_rsc_write_next_wakeup() - Write next wakeup in CONTROL_TCS.
> + * @drv: The controller
> + *
> + * Writes maximum wakeup cycles when called from suspend.
> + * Writes earliest hrtimer wakeup when called from idle.
> + */
> +void rpmh_rsc_write_next_wakeup(struct rsc_drv *drv)
> +{
> +       ktime_t now, wakeup;
> +       u64 wakeup_us, wakeup_cycles = ~0;
> +       u32 lo, hi;
> +
> +       if (!drv->tcs[CONTROL_TCS].num_tcs || !drv->genpd)

Just curious, but in case you don't have a genpd attached, but are
rather using the CPU PM notifiers to determine the last CPU - in that
case, don't you need to write a new value for the timer/wakeup?

> +               return;
> +
> +       /* Set highest time when system (timekeeping) is suspended */
> +       if (system_state == SYSTEM_SUSPEND)
> +               goto exit;
> +
> +       /* Find the earliest hrtimer wakeup from online cpus */
> +       wakeup = drv->genpd->next_hrtimer;
> +
> +       /* Find the relative wakeup in kernel time scale */
> +       now = ktime_get();
> +       wakeup = ktime_sub(wakeup, now);
> +       wakeup_us = ktime_to_us(wakeup);
> +
> +       /* Convert the wakeup to arch timer scale */
> +       wakeup_cycles = USECS_TO_CYCLES(wakeup_us);
> +       wakeup_cycles += arch_timer_read_counter();
> +
> +exit:
> +       lo = wakeup_cycles & RSC_DRV_CTL_TCS_DATA_LO_MASK;
> +       hi = wakeup_cycles >> RSC_DRV_CTL_TCS_DATA_SIZE;
> +       hi &= RSC_DRV_CTL_TCS_DATA_HI_MASK;
> +       hi |= RSC_DRV_CTL_TCS_DATA_HI_VALID;
> +
> +       writel_relaxed(lo, drv->base + RSC_DRV_CTL_TCS_DATA_LO);
> +       writel_relaxed(hi, drv->base + RSC_DRV_CTL_TCS_DATA_HI);
> +}
> +
> +/**
>   * rpmh_rsc_cpu_pm_callback() - Check if any of the AMCs are busy.
>   * @nfb:    Pointer to the notifier block in struct rsc_drv.
>   * @action: CPU_PM_ENTER, CPU_PM_ENTER_FAILED, or CPU_PM_EXIT.
> diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
> index 01765ee..3a53ed9 100644
> --- a/drivers/soc/qcom/rpmh.c
> +++ b/drivers/soc/qcom/rpmh.c
> @@ -450,7 +450,7 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
>
>         if (!ctrlr->dirty) {
>                 pr_debug("Skipping flush, TCS has latest data.\n");
> -               goto exit;
> +               goto write_next_wakeup;
>         }
>
>         /* Invalidate the TCSes first to avoid stale data */
> @@ -479,6 +479,8 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr)
>
>         ctrlr->dirty = false;
>
> +write_next_wakeup:
> +       rpmh_rsc_write_next_wakeup(ctrlr_to_drv(ctrlr));
>  exit:
>         spin_unlock(&ctrlr->cache_lock);
>         return ret;

Kind regards
Uffe

Re: [PATCH 08/10] PM: domains: Store the closest hrtimer event of the domain CPUs

[Ulf Hansson]

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI)
> SoCs when the deepest CPUidle modes results in the SoC also to enter
> the low power mode.
>
> RSC is part of CPU subsystem and APSS rsc device is attached to cluster
> power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which
> can wakeup the SoC from deepest low power states. The CONTROL_TCS does
> this by writing next wakeup in always on domain timer when the SoC is
> entering the low power state.
>
> Store the domain wakeup time from all the CPUs which can be used from
> domain power off callback by RSC device.
>
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>

I need to think a little bit more about this one, so I have to get
back with some more detailed comments next week.

Kind regards
Uffe

> ---
>  drivers/base/power/domain_governor.c | 1 +
>  include/linux/pm_domain.h            | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
> index cd08c58..a4c7dd8 100644
> --- a/drivers/base/power/domain_governor.c
> +++ b/drivers/base/power/domain_governor.c
> @@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
>                                 domain_wakeup = next_hrtimer;
>                 }
>         }
> +       genpd->next_hrtimer = domain_wakeup;
>
>         /* The minimum idle duration is from now - until the next wakeup. */
>         idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now));
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 67017c9..682b372 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -136,6 +136,7 @@ struct generic_pm_domain {
>         struct gpd_dev_ops dev_ops;
>         s64 max_off_time_ns;    /* Maximum allowed "suspended" time. */
>         ktime_t next_wakeup;    /* Maintained by the domain governor */
> +       ktime_t next_hrtimer;   /* Closest hrtimer event of the domain CPUs */
>         bool max_off_time_changed;
>         bool cached_power_down_ok;
>         bool cached_power_down_state_idx;
> --
> 2.7.4
>

Re: [PATCH 04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters

[Maulik Shah]

Hi,

On 1/14/2022 6:00 PM, Ulf Hansson wrote:
>
>> @@ -315,7 +315,7 @@
>>
>>                  CLUSTER_PD: cpu-cluster0 {
>>                          #power-domain-cells = <0>;
>> -                       domain-idle-states = <&CLUSTER_SLEEP_0>;
>> +                       domain-idle-states = <&CLUSTER_SLEEP_0 &CLUSTER_SLEEP_1>;
> Should this be like the below instead?
>
> <&CLUSTER_SLEEP_0>,  <&CLUSTER_SLEEP_1>;

Thanks for catching this. Will correct in v2.

Thanks,
Maulik

Re: [PATCH 05/10] dt-bindings: soc: qcom: Update devicetree binding document for rpmh-rsc

[Rob Herring]

On Sun, 09 Jan 2022 22:55:02 +0530, Maulik Shah wrote:
> The change documents power-domains property for RSC device.
> This optional property points to corresponding PM domain node.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> ---
>  Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 08/10] PM: domains: Store the closest hrtimer event of the domain CPUs

[Ulf Hansson]

On Sun, 9 Jan 2022 at 18:26, Maulik Shah <quic_mkshah@quicinc.com> wrote:
>
> The arch timer can not wake up the Qualcomm Technologies, Inc. (QTI)
> SoCs when the deepest CPUidle modes results in the SoC also to enter
> the low power mode.
>
> RSC is part of CPU subsystem and APSS rsc device is attached to cluster
> power domain. RSC has to setup next hrtimer wakeup in CONTROL_TCS which
> can wakeup the SoC from deepest low power states. The CONTROL_TCS does
> this by writing next wakeup in always on domain timer when the SoC is
> entering the low power state.
>
> Store the domain wakeup time from all the CPUs which can be used from
> domain power off callback by RSC device.
>
> Signed-off-by: Maulik Shah <quic_mkshah@quicinc.com>
> ---
>  drivers/base/power/domain_governor.c | 1 +
>  include/linux/pm_domain.h            | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c
> index cd08c58..a4c7dd8 100644
> --- a/drivers/base/power/domain_governor.c
> +++ b/drivers/base/power/domain_governor.c
> @@ -363,6 +363,7 @@ static bool cpu_power_down_ok(struct dev_pm_domain *pd)
>                                 domain_wakeup = next_hrtimer;
>                 }
>         }
> +       genpd->next_hrtimer = domain_wakeup;
>
>         /* The minimum idle duration is from now - until the next wakeup. */
>         idle_duration_ns = ktime_to_ns(ktime_sub(domain_wakeup, now));
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 67017c9..682b372 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -136,6 +136,7 @@ struct generic_pm_domain {
>         struct gpd_dev_ops dev_ops;
>         s64 max_off_time_ns;    /* Maximum allowed "suspended" time. */
>         ktime_t next_wakeup;    /* Maintained by the domain governor */
> +       ktime_t next_hrtimer;   /* Closest hrtimer event of the domain CPUs */

Would you mind clarifying the comment into something along the lines
of: "/* Next hrtimer for the CPU PM domain */

>         bool max_off_time_changed;
>         bool cached_power_down_ok;
>         bool cached_power_down_state_idx;
> --
> 2.7.4
>

Beside the nitpick above, I have a few additional minor comments.

*) Users of genpd->next_hrtimer should not access this member in the
struct generic_pm_domain themselves. Instead, I suggest we add a genpd
helper function to deal with this. In this regard, we should also add
a description to the helper function to explain under what *specific*
conditions it's allowed to be called for.

**) We should assign genpd->next_hrtimer a default value in
pm_genpd_init(). Perhaps that can also be used in a way to make sure
the helper function always returns a valid value!?

Other than this, I think the approach looks sane to me!

Kind regards
Uffe

Re: (subset) [PATCH 00/10] Add APSS RSC to Cluster power domain

[Bjorn Andersson]

On Sun, 9 Jan 2022 22:54:57 +0530, Maulik Shah wrote:
> This series patches 1 to 4 adds/corrects the cpuidle states/
> apps_rsc TCS configuration to make it same as downstream kernel.
> 
> The patches 5, 6 and 7 adds apps_rsc device to cluster power domain such
> that when cluster is going to power down the cluster pre off notification
> will program the 'sleep' and 'wake' votes in SLEEP TCS and WAKE TCSes.
> 
> [...]

Applied, thanks!

[01/10] arm64: dts: qcom: sm8150: Correct TCS configuration for apps rsc
        commit: 17ac8af678b6da6a8f1df7da8ebf2c5198741827
[02/10] arm64: dts: qcom: sm8250: Add cpuidle states
        commit: 32bc936d732171d48c9c8f96c4fa25ac3ed7e1c7
[03/10] arm64: dts: qcom: sm8350: Correct TCS configuration for apps rsc
        commit: a131255e4ad1ef8d4873ecba21561ba272b2547a
[04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters
        commit: 6574702b0d394d2acc9ff808c4a79df8b9999173

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

Re: (subset) [PATCH 00/10] Add APSS RSC to Cluster power domain

[Amit Pundir]

On Tue, 1 Feb 2022 at 10:52, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
>
> On Sun, 9 Jan 2022 22:54:57 +0530, Maulik Shah wrote:
> > This series patches 1 to 4 adds/corrects the cpuidle states/
> > apps_rsc TCS configuration to make it same as downstream kernel.
> >
> > The patches 5, 6 and 7 adds apps_rsc device to cluster power domain such
> > that when cluster is going to power down the cluster pre off notification
> > will program the 'sleep' and 'wake' votes in SLEEP TCS and WAKE TCSes.
> >
> > [...]
>
> Applied, thanks!
>
> [01/10] arm64: dts: qcom: sm8150: Correct TCS configuration for apps rsc
>         commit: 17ac8af678b6da6a8f1df7da8ebf2c5198741827
> [02/10] arm64: dts: qcom: sm8250: Add cpuidle states
>         commit: 32bc936d732171d48c9c8f96c4fa25ac3ed7e1c7

Hi, These patches landed upstream and I see a boot regression on RB5
with this sm8250 patch:

[    T1] vreg_l11c_3p3: failed to enable: -ETIMEDOUT
[    T1] qcom-rpmh-regulator 18200000.rsc:pm8150l-rpmh-regulators:
ldo11: devm_regulator_register() failed, ret=-110
[    T1] qcom-rpmh-regulator: probe of
18200000.rsc:pm8150l-rpmh-regulators failed with error -110

Doesn't regress everytime but It is fairly reproducible. RB5 fails to
boot on every alternate reboot or so. But the device boots everytime
if I revert this patch.

> [03/10] arm64: dts: qcom: sm8350: Correct TCS configuration for apps rsc
>         commit: a131255e4ad1ef8d4873ecba21561ba272b2547a
> [04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters
>         commit: 6574702b0d394d2acc9ff808c4a79df8b9999173
>
> Best regards,
> --
> Bjorn Andersson <bjorn.andersson@linaro.org>

Re: (subset) [PATCH 00/10] Add APSS RSC to Cluster power domain

[Amit Pundir]

On Tue, 29 Mar 2022 at 15:25, Amit Pundir <amit.pundir@linaro.org> wrote:
>
> On Tue, 1 Feb 2022 at 10:52, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
> >
> > On Sun, 9 Jan 2022 22:54:57 +0530, Maulik Shah wrote:
> > > This series patches 1 to 4 adds/corrects the cpuidle states/
> > > apps_rsc TCS configuration to make it same as downstream kernel.
> > >
> > > The patches 5, 6 and 7 adds apps_rsc device to cluster power domain such
> > > that when cluster is going to power down the cluster pre off notification
> > > will program the 'sleep' and 'wake' votes in SLEEP TCS and WAKE TCSes.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [01/10] arm64: dts: qcom: sm8150: Correct TCS configuration for apps rsc
> >         commit: 17ac8af678b6da6a8f1df7da8ebf2c5198741827
> > [02/10] arm64: dts: qcom: sm8250: Add cpuidle states
> >         commit: 32bc936d732171d48c9c8f96c4fa25ac3ed7e1c7
>
> Hi, These patches landed upstream and I see a boot regression on RB5
> with this sm8250 patch:
>
> [    T1] vreg_l11c_3p3: failed to enable: -ETIMEDOUT
> [    T1] qcom-rpmh-regulator 18200000.rsc:pm8150l-rpmh-regulators:
> ldo11: devm_regulator_register() failed, ret=-110
> [    T1] qcom-rpmh-regulator: probe of
> 18200000.rsc:pm8150l-rpmh-regulators failed with error -110
>
> Doesn't regress everytime but It is fairly reproducible. RB5 fails to
> boot on every alternate reboot or so. But the device boots everytime
> if I revert this patch.

Hi, I'm still waiting on a resolution/fix for this. I'm happy to test
or run any debug patches, if it would help narrow the issue down.
Fwiw, this is the defconfig I'm using
https://git.linaro.org/people/amit.pundir/linux.git/plain/arch/arm64/configs/rbX_aosp_defconfig?h=rb5-rpmh-regression

Regards,
Amit Pundir

>
> > [03/10] arm64: dts: qcom: sm8350: Correct TCS configuration for apps rsc
> >         commit: a131255e4ad1ef8d4873ecba21561ba272b2547a
> > [04/10] arm64: dts: qcom: sm8450: Update cpuidle states parameters
> >         commit: 6574702b0d394d2acc9ff808c4a79df8b9999173
> >
> > Best regards,
> > --
> > Bjorn Andersson <bjorn.andersson@linaro.org>