[PATCH RFC 00/10] qcom: 8084: Cluster idle support

[PATCH RFC 00/10] qcom: 8084: Cluster idle support

[Lina Iyer]

This patchset add support to power down the L2 and the PM domain for the CPU
subsystem on Qualcomm's APQ8084. Only 8084 is supported at this time, because
the firmware for chipsets prior to 8084 (8074, 8064) will allow the domain to
be powered off only when running as a single core system. That is, all
secondary cores must be hotplugged off. With 8084, a new feature in the
firmware allows any core to be the last core to enter power down state. The
state of the L2 power controller will be carried as an argument by the last
CPU, when entering SCM.

This patchset extensively uses the changes to GenPD and the CPU PM domain
introduced by [1]. Also under discussion on the mailing list, is the hwspinlock
changes that are part of this series.

I have tested this on 8084. But invalidating cache causes a system hang and is
under investigation. This patchset at this time is only provided as a proof of
concept for using CPU PM domains.

Thanks,
Lina

[1]. http://www.spinics.net/lists/arm-kernel/msg437091.html

Lina Iyer (10):
  drivers: qcom: spm: Support cache SPMs
  drivers: qcom: spm: Add 8084 L2 SPM register data
  drivers: qcom: spm: Enable runtime suspend/resume of CPU PM domain
  arm: dts: Add L2 power-controller device bindings for APQ8084
  arm: dts: Add power domain device bindings for APQ8084
  drivers: qcom: Enable genpd on selecting QCOM_PM
  hwspinlock: Introduce raw capability for hwspinlocks
  hwspinlock: qcom: Lock #7 is special lock, uses dynamic proc_id
  drivers: qcom: spm: Use hwspinlock to serialize entry into SCM
  arm: dts: qcom: Add TCSR mutex device bindings for APQ8084

 .../devicetree/bindings/arm/msm/qcom,saw2.txt      |   1 +
 Documentation/hwspinlock.txt                       |  16 +++
 arch/arm/boot/dts/qcom-apq8084.dtsi                |  21 +++-
 drivers/hwspinlock/hwspinlock_core.c               |  75 +++++++-----
 drivers/hwspinlock/hwspinlock_internal.h           |   6 +
 drivers/hwspinlock/qcom_hwspinlock.c               |  22 +++-
 drivers/soc/qcom/Kconfig                           |   5 +
 drivers/soc/qcom/spm.c                             | 130 ++++++++++++++++++---
 include/linux/hwspinlock.h                         |  41 +++++++
 9 files changed, 264 insertions(+), 53 deletions(-)

-- 
2.1.4

[PATCH RFC 01/10] drivers: qcom: spm: Support cache SPMs

[Lina Iyer]

Recognize cache SPM devices defined in the DT and configure the
corresponding SPM hardware. SPM controllers for L2 controls the cache's
idle low power state and may also be used to turn off the CPU power
rail.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Andy Gross <agross@codeaurora.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/soc/qcom/spm.c | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
index b04b05a..9f5edaa27 100644
--- a/drivers/soc/qcom/spm.c
+++ b/drivers/soc/qcom/spm.c
@@ -115,6 +115,7 @@ static const struct spm_reg_data spm_reg_8064_cpu = {
 };
 
 static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv);
+static struct spm_driver_data *domain_spm;
 
 typedef int (*idle_fn)(int);
 static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops);
@@ -283,13 +284,18 @@ CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v1, "qcom,kpss-acc-v1", &qcom_cpuidle_ops);
 CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v2, "qcom,kpss-acc-v2", &qcom_cpuidle_ops);
 
 static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
-		int *spm_cpu)
+		int *index, bool *is_domain_spm)
 {
 	struct spm_driver_data *drv = NULL;
 	struct device_node *cpu_node, *saw_node;
 	int cpu;
 	bool found;
 
+	drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
+	if (!drv)
+		return ERR_PTR(-ENOMEM);
+
+	/* Check for a CPU SPM, if found we are done */
 	for_each_possible_cpu(cpu) {
 		cpu_node = of_cpu_device_node_get(cpu);
 		if (!cpu_node)
@@ -302,11 +308,14 @@ static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
 			break;
 	}
 
-	if (found) {
-		drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
-		if (drv)
-			*spm_cpu = cpu;
-	}
+	/* 
+	 * If found, we have a CPU SPM, if not,
+	 * we have a cache SPM
+	 */
+	if (found)
+		*index = cpu;
+
+	*is_domain_spm = !found;
 
 	return drv;
 }
@@ -327,21 +336,22 @@ static int spm_dev_probe(struct platform_device *pdev)
 	struct resource *res;
 	const struct of_device_id *match_id;
 	void __iomem *addr;
-	int cpu;
+	int index;
+	bool is_domain_spm;
 
-	drv = spm_get_drv(pdev, &cpu);
-	if (!drv)
-		return -EINVAL;
+	match_id = of_match_node(spm_match_table, pdev->dev.of_node);
+	if (!match_id)
+		return -ENODEV;
+
+	drv = spm_get_drv(pdev, &index, &is_domain_spm);
+	if (IS_ERR(drv))
+		return PTR_ERR(drv);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	drv->reg_base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(drv->reg_base))
 		return PTR_ERR(drv->reg_base);
 
-	match_id = of_match_node(spm_match_table, pdev->dev.of_node);
-	if (!match_id)
-		return -ENODEV;
-
 	drv->reg_data = match_id->data;
 
 	/* Write the SPM sequences first.. */
@@ -366,8 +376,13 @@ static int spm_dev_probe(struct platform_device *pdev)
 	/* Set up Standby as the default low power mode */
 	spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY);
 
-	per_cpu(cpu_spm_drv, cpu) = drv;
+	/* We are ready to use the CPU/Cache SPM. */
+	if (is_domain_spm)
+		domain_spm = drv;
+	else
+		per_cpu(cpu_spm_drv, index) = drv;
 
+	dev_dbg(&pdev->dev, "SPM device probed.\n");
 	return 0;
 }
 
-- 
2.1.4

[PATCH RFC 02/10] drivers: qcom: spm: Add 8084 L2 SPM register data

[Lina Iyer]

Add register data and configure L2 SAW to support voltage control and L2
idle states for QCOM APQ8084 SoC.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 .../devicetree/bindings/arm/msm/qcom,saw2.txt         |  1 +
 drivers/soc/qcom/spm.c                                | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
index ae4afc6..91430ff 100644
--- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
@@ -27,6 +27,7 @@ PROPERTIES
 			"qcom,apq8064-saw2-v1.1-cpu"
 			"qcom,msm8974-saw2-v2.1-cpu"
 			"qcom,apq8084-saw2-v2.1-cpu"
+			"qcom,apq8084-saw2-v2.1-l2"
 
 - reg:
 	Usage: required
diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
index 9f5edaa27..efbf5e5 100644
--- a/drivers/soc/qcom/spm.c
+++ b/drivers/soc/qcom/spm.c
@@ -78,6 +78,8 @@ static const u8 spm_reg_offset_v2_1[SPM_REG_NR] = {
 	[SPM_REG_SPM_CTL]	= 0x30,
 	[SPM_REG_DLY]		= 0x34,
 	[SPM_REG_SEQ_ENTRY]	= 0x80,
+	[SPM_REG_PMIC_DATA_0]	= 0x40,
+	[SPM_REG_PMIC_DATA_1]	= 0x44,
 };
 
 /* SPM register data for 8974, 8084 */
@@ -92,6 +94,20 @@ static const struct spm_reg_data spm_reg_8974_8084_cpu  = {
 	.start_index[PM_SLEEP_MODE_SPC] = 3,
 };
 
+static const struct spm_reg_data spm_reg_8084_l2  = {
+	.reg_offset = spm_reg_offset_v2_1,
+	.spm_cfg = 0x14,
+	.spm_dly = 0x3C102800,
+	.pmic_data[0] = 0x02030080,
+	.pmic_data[1] = 0x00030000,
+	.seq = { 0x1F, 0x00, 0x20, 0x03, 0x22, 0x00, 0x0F, 0x00, 0x10, 0x22,
+		0x12, 0x32, 0x60, 0x70, 0x80, 0xB0, 0x11, 0x42, 0x03, 0x01,
+		0xB0, 0x78, 0x80, 0x12, 0x22, 0x44, 0x50, 0x3B, 0x60, 0x02,
+		0x32, 0x50, 0x0F },
+	.start_index[PM_SLEEP_MODE_STBY] = 0,
+	.start_index[PM_SLEEP_MODE_SPC] = 7,
+};
+
 static const u8 spm_reg_offset_v1_1[SPM_REG_NR] = {
 	[SPM_REG_CFG]		= 0x08,
 	[SPM_REG_SPM_CTL]	= 0x20,
@@ -186,6 +202,7 @@ static int qcom_cpu_spc(int cpu)
 	struct spm_driver_data *drv = per_cpu(cpu_spm_drv, cpu);
 
 	spm_set_low_power_mode(drv, PM_SLEEP_MODE_SPC);
+
 	ret = cpu_suspend(0, qcom_pm_collapse);
 	/*
 	 * ARM common code executes WFI without calling into our driver and
@@ -327,6 +344,8 @@ static const struct of_device_id spm_match_table[] = {
 	  .data = &spm_reg_8974_8084_cpu },
 	{ .compatible = "qcom,apq8064-saw2-v1.1-cpu",
 	  .data = &spm_reg_8064_cpu },
+	{ .compatible = "qcom,apq8084-saw2-v2.1-l2",
+	  .data = &spm_reg_8084_l2 },
 	{ },
 };
 
-- 
2.1.4

[PATCH RFC 03/10] drivers: qcom: spm: Enable runtime suspend/resume of CPU PM domain

[Lina Iyer]

On APQ8084 QCOM SoC's, the CPUs are powered by a single rail controlled
by the L2 cache power controller (L2 SPM). The L2 power domain supplies
power to all the CPUs and L2. It is safe to power down the domain when
all the CPUs and the L2 are powered down.

Powering down of the domain is done through the finite state machine on
the L2 SAW. The L2 SPM can be configured to enter an idle state, when
all CPUs enter their idle state. The L2 SPM state machine would turn off
the cache and possibly power off the power domain as well. The SPM also
guarantees that the h/w is ready for the CPU to resume, when woken up by
an interrupt.

Define a cluster that holds the SPM and possibly other common cluster
elements. The L2 SAW is also the genpd domain provider and the CPUs are
the devices attached to the domain. When CPUIdle powers down each CPU,
the ARM domain framework would callback to notify that the domain may be
powered off. Configure the L2 SPM at that time to flush the L2 cache and
turn off the CPU power rail.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/soc/qcom/spm.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
index efbf5e5..b6d75db 100644
--- a/drivers/soc/qcom/spm.c
+++ b/drivers/soc/qcom/spm.c
@@ -24,8 +24,11 @@
 #include <linux/platform_device.h>
 #include <linux/cpuidle.h>
 #include <linux/cpu_pm.h>
+#include <linux/pm_domain.h>
 #include <linux/qcom_scm.h>
 
+#include <asm/arm-pd.h>
+#include <asm/cacheflush.h>
 #include <asm/cpuidle.h>
 #include <asm/proc-fns.h>
 #include <asm/suspend.h>
@@ -132,6 +135,7 @@ static const struct spm_reg_data spm_reg_8064_cpu = {
 
 static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv);
 static struct spm_driver_data *domain_spm;
+static int l2_flush_flag;
 
 typedef int (*idle_fn)(int);
 static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops);
@@ -187,7 +191,14 @@ static void spm_set_low_power_mode(struct spm_driver_data *drv,
 
 static int qcom_pm_collapse(unsigned long int unused)
 {
-	qcom_scm_cpu_power_down(QCOM_SCM_CPU_PWR_DOWN_L2_ON);
+	/*
+	 * Flush the non-secure cache for Krait processors.
+	 * SCM will flush the secure cache.
+	 */
+	if (l2_flush_flag == QCOM_SCM_CPU_PWR_DOWN_L2_OFF)
+		flush_cache_all();
+
+	qcom_scm_cpu_power_down(l2_flush_flag);
 
 	/*
 	 * Returns here only if there was a pending interrupt and we did not
@@ -300,6 +311,33 @@ static struct cpuidle_ops qcom_cpuidle_ops __initdata = {
 CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v1, "qcom,kpss-acc-v1", &qcom_cpuidle_ops);
 CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v2, "qcom,kpss-acc-v2", &qcom_cpuidle_ops);
 
+static int pd_init(struct device_node *dn, struct generic_pm_domain *domain)
+{
+	return 0;
+}
+
+static int pd_power_on(struct generic_pm_domain *domain)
+{
+	l2_flush_flag = QCOM_SCM_CPU_PWR_DOWN_L2_ON;
+	spm_set_low_power_mode(domain_spm, PM_SLEEP_MODE_STBY);
+	return 0;
+}
+
+static int pd_power_off(struct generic_pm_domain *domain)
+{
+	l2_flush_flag = QCOM_SCM_CPU_PWR_DOWN_L2_OFF;
+	spm_set_low_power_mode(domain_spm, PM_SLEEP_MODE_SPC);
+	return 0;
+}
+
+static struct of_arm_pd_ops pd_ops __initdata = {
+	.init = pd_init,
+	.power_on = pd_power_on,
+	.power_off = pd_power_off,
+};
+
+ARM_PD_METHOD_OF_DECLARE(qcom_cpu_pd, "qcom,apq8084-saw2-v2.1-l2", &pd_ops);
+
 static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
 		int *index, bool *is_domain_spm)
 {
-- 
2.1.4

[PATCH RFC 04/10] arm: dts: Add L2 power-controller device bindings for APQ8084

[Lina Iyer]

Add power controller (SAW) device nodes for L2 caches. L2 SAW enable L2
to enter idle states and be powered off. Also, on 8084 the L2 SAW may be
used to regulate the active voltage for the cpu and L2.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 arch/arm/boot/dts/qcom-apq8084.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 7084010..900ef1f 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -59,7 +59,7 @@
 		};
 
 		L2: l2-cache {
-			compatible = "qcom,arch-cache";
+			compatible = "cache";
 			cache-level = <2>;
 			qcom,saw = <&saw_l2>;
 		};
@@ -183,7 +183,7 @@
 		};
 
 		saw_l2: power-controller@f9012000 {
-			compatible = "qcom,saw2";
+			compatible = "qcom,apq8084-saw2-v2.1-l2", "qcom,saw2";
 			reg = <0xf9012000 0x1000>;
 			regulator;
 		};
-- 
2.1.4

[PATCH RFC 05/10] arm: dts: Add power domain device bindings for APQ8084

[Lina Iyer]

Define L2 SAW as the power domain provider and individual cpus are the
power domain consumers.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 arch/arm/boot/dts/qcom-apq8084.dtsi | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 900ef1f..4a61f8d 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -23,6 +23,7 @@
 			qcom,acc = <&acc0>;
 			qcom,saw = <&saw0>;
 			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&saw_l2>;
 		};
 
 		cpu@1 {
@@ -34,6 +35,7 @@
 			qcom,acc = <&acc1>;
 			qcom,saw = <&saw1>;
 			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&saw_l2>;
 		};
 
 		cpu@2 {
@@ -45,6 +47,7 @@
 			qcom,acc = <&acc2>;
 			qcom,saw = <&saw2>;
 			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&saw_l2>;
 		};
 
 		cpu@3 {
@@ -56,6 +59,7 @@
 			qcom,acc = <&acc3>;
 			qcom,saw = <&saw3>;
 			cpu-idle-states = <&CPU_SPC>;
+			power-domains = <&saw_l2>;
 		};
 
 		L2: l2-cache {
@@ -183,9 +187,11 @@
 		};
 
 		saw_l2: power-controller@f9012000 {
-			compatible = "qcom,apq8084-saw2-v2.1-l2", "qcom,saw2";
+			compatible = "qcom,apq8084-saw2-v2.1-l2", "qcom,saw2",
+					 "arm,pd";
 			reg = <0xf9012000 0x1000>;
 			regulator;
+			#power-domain-cells = <0>;
 		};
 
 		acc0: clock-controller@f9088000 {
-- 
2.1.4

[PATCH RFC 06/10] drivers: qcom: Enable genpd on selecting QCOM_PM

[Lina Iyer]

Enable PM_CPU_DOMAIN and its PM_GENERIC_DOMAINS dependenciesd to provide
cpu domain support for QCOM SoCs.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/soc/qcom/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index ba47b70..b6c2e5d 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -14,6 +14,10 @@ config QCOM_PM
 	bool "Qualcomm Power Management"
 	depends on ARCH_QCOM && !ARM64
 	select QCOM_SCM
+	select ARM_CPU_SUSPEND
+	select PM_GENERIC_DOMAINS
+	select PM_GENERIC_DOMAINS_SLEEP
+	select PM_GENERIC_DOMAINS_OF
 	help
 	  QCOM Platform specific power driver to manage cores and L2 low power
 	  modes. It interface with various system drivers to put the cores in
-- 
2.1.4

[PATCH RFC 07/10] hwspinlock: Introduce raw capability for hwspinlocks

[Lina Iyer]

The hwspinlock framework, uses a s/w spin lock around the hw spinlock to
ensure that only process acquires the lock at any time. This is the most
general use case. A special case is where a hwspinlock may be acquired
in Linux and a remote entity may release the lock. In such a case, the
s/w spinlock may never be unlocked as Linux would never call
hwspin_unlock on the hwlock.

This special case is needed for serializing the processor across context
switches from Linux to firmware. Multiple cores may enter cpu idle and
would switch context to firmware to power off. A cpu holding the
hwspinlock would cause other cpus to wait to acquire the lock, until the
lock is released by the firmware. The last core to power down per Linux
has the correct state of the shared resources and should be the one
considered by the firmware. However, a cpu may be stuck handling FIQs
and therefore the last man view of Linux and the firmware may differ. A
hwspinlock avoids this problem by serializing the entry from Linux to
firmware.

Introduce hwcaps member for hwspinlock_device. The hwcaps represents the
hw capability of each hwlock. The platform driver is responsible for
specifying this capability for each lock in the bank. A lock that has
HWL_CAP_ALLOW_RAW set, would indicate to the framework, the capability
for ensure locking correctness in the platform. Since no sw spinlock
guards the hwspinlock, it is the responsibility of the platform driver
to ensure that an unique value is written to the hwspinlock to ensure
locking correctness.

Drivers may use hwspin_trylock_raw() and hwspin_unlock_raw() api to lock
and unlock a hwlock with raw capability.

Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Ohad Ben-Cohen <ohad@wizery.com>
Cc: Andy Gross <agross@codeaurora.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 Documentation/hwspinlock.txt             | 16 +++++++
 drivers/hwspinlock/hwspinlock_core.c     | 75 +++++++++++++++++++-------------
 drivers/hwspinlock/hwspinlock_internal.h |  6 +++
 include/linux/hwspinlock.h               | 41 +++++++++++++++++
 4 files changed, 108 insertions(+), 30 deletions(-)

diff --git a/Documentation/hwspinlock.txt b/Documentation/hwspinlock.txt
index 61c1ee9..4fd3f55 100644
--- a/Documentation/hwspinlock.txt
+++ b/Documentation/hwspinlock.txt
@@ -132,6 +132,15 @@ independent, drivers.
      notably -EBUSY if the hwspinlock was already taken).
      The function will never sleep.
 
+  int hwspin_trylock_raw(struct hwspinlock *hwlock);
+   - attempt to lock a previously-assigned hwspinlock, but immediately fail if
+     it is already taken. The lock must have been declared by the platform
+     drv code with raw capability support.
+     Returns 0 on success and an appropriate error code otherwise (most
+     notably -EBUSY if the hwspinlock was already taken).
+     This function does not use a sw spinlock around the hwlock. The
+     responsiblity of the lock is guaranteed by the platform code.
+
   void hwspin_unlock(struct hwspinlock *hwlock);
    - unlock a previously-locked hwspinlock. Always succeed, and can be called
      from any context (the function never sleeps). Note: code should _never_
@@ -154,6 +163,13 @@ independent, drivers.
      and the state of the local interrupts is restored to the state saved at
      the given flags. This function will never sleep.
 
+  void hwspin_unlock_raw(struct hwspinlock *hwlock);
+   - unlock a previously-locked hwspinlock. Always succeed, and can be called
+     from any context (the function never sleeps). Note: code should _never_
+     unlock an hwspinlock which is already unlocked (there is no protection
+     against this). The platform driver must support raw capability for this
+     hwlock.
+
   int hwspin_lock_get_id(struct hwspinlock *hwlock);
    - retrieve id number of a given hwspinlock. This is needed when an
      hwspinlock is dynamically assigned: before it can be used to achieve
diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
index 52f708b..0d2baa3 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -80,7 +80,10 @@ static DEFINE_MUTEX(hwspinlock_tree_lock);
  * whether he wants their previous state to be saved. It is up to the user
  * to choose the appropriate @mode of operation, exactly the same way users
  * should decide between spin_trylock, spin_trylock_irq and
- * spin_trylock_irqsave.
+ * spin_trylock_irqsave and even no spinlock, if the hwspinlock is always
+ * acquired in an interrupt disabled context. The platform driver that
+ * registers such a lock, would explicity specify the capability for the
+ * lock with the HWL_CAP_ALLOW_RAW capability flag.
  *
  * Returns 0 if we successfully locked the hwspinlock or -EBUSY if
  * the hwspinlock was already taken.
@@ -92,6 +95,7 @@ int __hwspin_trylock(struct hwspinlock *hwlock, int mode, unsigned long *flags)
 
 	BUG_ON(!hwlock);
 	BUG_ON(!flags && mode == HWLOCK_IRQSTATE);
+	BUG_ON((hwlock->hwcaps & HWL_CAP_ALLOW_RAW) && (mode != HWLOCK_NOLOCK));
 
 	/*
 	 * This spin_lock{_irq, _irqsave} serves three purposes:
@@ -106,32 +110,36 @@ int __hwspin_trylock(struct hwspinlock *hwlock, int mode, unsigned long *flags)
 	 *    problems with hwspinlock usage (e.g. scheduler checks like
 	 *    'scheduling while atomic' etc.)
 	 */
-	if (mode == HWLOCK_IRQSTATE)
-		ret = spin_trylock_irqsave(&hwlock->lock, *flags);
-	else if (mode == HWLOCK_IRQ)
-		ret = spin_trylock_irq(&hwlock->lock);
-	else
-		ret = spin_trylock(&hwlock->lock);
-
-	/* is lock already taken by another context on the local cpu ? */
-	if (!ret)
-		return -EBUSY;
-
-	/* try to take the hwspinlock device */
-	ret = hwlock->bank->ops->trylock(hwlock);
-
-	/* if hwlock is already taken, undo spin_trylock_* and exit */
-	if (!ret) {
+	if (mode != HWLOCK_NOLOCK) {
 		if (mode == HWLOCK_IRQSTATE)
-			spin_unlock_irqrestore(&hwlock->lock, *flags);
+			ret = spin_trylock_irqsave(&hwlock->lock, *flags);
 		else if (mode == HWLOCK_IRQ)
-			spin_unlock_irq(&hwlock->lock);
+			ret = spin_trylock_irq(&hwlock->lock);
 		else
-			spin_unlock(&hwlock->lock);
+			ret = spin_trylock(&hwlock->lock);
 
-		return -EBUSY;
+		/* is lock already taken by another context on the local cpu? */
+		if (!ret)
+			return -EBUSY;
+	}
+
+	/* try to take the hwspinlock device */
+	ret = hwlock->bank->ops->trylock(hwlock);
+
+	if (mode != HWLOCK_NOLOCK) {
+		/* if hwlock is already taken, undo spin_trylock_* and exit */
+		if (!ret) {
+			if (mode == HWLOCK_IRQSTATE)
+				spin_unlock_irqrestore(&hwlock->lock, *flags);
+			else if (mode == HWLOCK_IRQ)
+				spin_unlock_irq(&hwlock->lock);
+			else
+				spin_unlock(&hwlock->lock);
+		}
 	}
 
+	if (!ret)
+		return -EBUSY;
 	/*
 	 * We can be sure the other core's memory operations
 	 * are observable to us only _after_ we successfully take
@@ -223,7 +231,10 @@ EXPORT_SYMBOL_GPL(__hwspin_lock_timeout);
  * if yes, whether he wants their previous state to be restored. It is up
  * to the user to choose the appropriate @mode of operation, exactly the
  * same way users decide between spin_unlock, spin_unlock_irq and
- * spin_unlock_irqrestore.
+ * spin_unlock_irqrestore and even no spinlock, if the hwspinlock is always
+ * acquired in an interrupt disabled context. The platform driver that
+ * registers such a lock, would explicity specify the capability for the
+ * lock with the HWL_CAP_ALLOW_RAW capability flag.
  *
  * The function will never sleep.
  */
@@ -231,6 +242,7 @@ void __hwspin_unlock(struct hwspinlock *hwlock, int mode, unsigned long *flags)
 {
 	BUG_ON(!hwlock);
 	BUG_ON(!flags && mode == HWLOCK_IRQSTATE);
+	BUG_ON((hwlock->hwcaps & HWL_CAP_ALLOW_RAW) && (mode != HWLOCK_NOLOCK));
 
 	/*
 	 * We must make sure that memory operations (both reads and writes),
@@ -248,13 +260,15 @@ void __hwspin_unlock(struct hwspinlock *hwlock, int mode, unsigned long *flags)
 
 	hwlock->bank->ops->unlock(hwlock);
 
-	/* Undo the spin_trylock{_irq, _irqsave} called while locking */
-	if (mode == HWLOCK_IRQSTATE)
-		spin_unlock_irqrestore(&hwlock->lock, *flags);
-	else if (mode == HWLOCK_IRQ)
-		spin_unlock_irq(&hwlock->lock);
-	else
-		spin_unlock(&hwlock->lock);
+	if (mode != HWLOCK_NOLOCK) {
+		/* Undo the spin_trylock{_irq, _irqsave} called while locking */
+		if (mode == HWLOCK_IRQSTATE)
+			spin_unlock_irqrestore(&hwlock->lock, *flags);
+		else if (mode == HWLOCK_IRQ)
+			spin_unlock_irq(&hwlock->lock);
+		else
+			spin_unlock(&hwlock->lock);
+	}
 }
 EXPORT_SYMBOL_GPL(__hwspin_unlock);
 
@@ -421,7 +435,8 @@ int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev,
 	for (i = 0; i < num_locks; i++) {
 		hwlock = &bank->lock[i];
 
-		spin_lock_init(&hwlock->lock);
+		if (!(hwlock->hwcaps & HWL_CAP_ALLOW_RAW))
+			spin_lock_init(&hwlock->lock);
 		hwlock->bank = bank;
 
 		ret = hwspin_lock_register_single(hwlock, base_id + i);
diff --git a/drivers/hwspinlock/hwspinlock_internal.h b/drivers/hwspinlock/hwspinlock_internal.h
index d26f78b..24a4d79 100644
--- a/drivers/hwspinlock/hwspinlock_internal.h
+++ b/drivers/hwspinlock/hwspinlock_internal.h
@@ -21,6 +21,9 @@
 #include <linux/spinlock.h>
 #include <linux/device.h>
 
+/* hwspinlock capability properties */
+#define HWL_CAP_ALLOW_RAW	BIT(1)
+
 struct hwspinlock_device;
 
 /**
@@ -44,11 +47,14 @@ struct hwspinlock_ops {
  * @bank: the hwspinlock_device structure which owns this lock
  * @lock: initialized and used by hwspinlock core
  * @priv: private data, owned by the underlying platform-specific hwspinlock drv
+ * @hwcaps: hardware capablity, like raw lock, that does not need s/w spinlock
+ * around the hwspinlock.
  */
 struct hwspinlock {
 	struct hwspinlock_device *bank;
 	spinlock_t lock;
 	void *priv;
+	int hwcaps;
 };
 
 /**
diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h
index 859d673..f173352 100644
--- a/include/linux/hwspinlock.h
+++ b/include/linux/hwspinlock.h
@@ -24,6 +24,7 @@
 /* hwspinlock mode argument */
 #define HWLOCK_IRQSTATE	0x01	/* Disable interrupts, save state */
 #define HWLOCK_IRQ	0x02	/* Disable interrupts, don't save state */
+#define HWLOCK_NOLOCK	0xFF	/* Dont take any lock */
 
 struct device;
 struct device_node;
@@ -196,6 +197,27 @@ static inline int hwspin_trylock(struct hwspinlock *hwlock)
 }
 
 /**
+ * hwspin_trylock_raw() - attempt to lock a specific hwspinlock without s/w
+ * spinlocks
+ * @hwlock: the hwspinlock which we want to trylock
+ *
+ * This function attempts to lock the hwspinlock without acquiring a s/w
+ * spinlock. The function will return failure if the lock is already taken.
+ *
+ * The function can only be used on a hwlock that has been initialized with
+ * raw capability by the platform drv.
+ *
+ * The function is expected to be called in an interrupt disabled context.
+ *
+ * Returns 0 if we successfully locked the hwspinlock, -EBUSY if the hwspinlock
+ * is already taken.
+ */
+static inline int hwspin_trylock_raw(struct hwspinlock *hwlock)
+{
+	return __hwspin_trylock(hwlock, HWLOCK_NOLOCK, NULL);
+}
+
+/**
  * hwspin_lock_timeout_irqsave() - lock hwspinlock, with timeout, disable irqs
  * @hwlock: the hwspinlock to be locked
  * @to: timeout value in msecs
@@ -317,4 +339,23 @@ static inline void hwspin_unlock(struct hwspinlock *hwlock)
 	__hwspin_unlock(hwlock, 0, NULL);
 }
 
+/**
+ * hwspin_unlock_raw() - unlock hwspinlock
+ * @hwlock: a previously acquired hwspinlock which we want to unlock
+ *
+ * This function will unlock a specific hwspinlock that was acquired using the
+ * hwspin_trylock_raw() call.
+ *
+ * The function can only be used on a hwlock that has been initialized with
+ * raw capability by the platform drv.
+ *
+ * @hwlock must be already locked (e.g. by hwspin_trylock()) before calling
+ * this function: it is a bug to call unlock on a @hwlock that is already
+ * unlocked.
+ */
+static inline void hwspin_unlock_raw(struct hwspinlock *hwlock)
+{
+	__hwspin_unlock(hwlock, HWLOCK_NOLOCK, NULL);
+}
+
 #endif /* __LINUX_HWSPINLOCK_H */
-- 
2.1.4

[PATCH RFC 08/10] hwspinlock: qcom: Lock #7 is special lock, uses dynamic proc_id

[Lina Iyer]

Hwspinlocks are widely used between processors in an SoC, and also
between elevation levels within in the same processor.  QCOM SoC's use
hwspinlock to serialize entry into a low power mode when the context
switches from Linux to secure monitor.

Lock #7 has been assigned for this purpose. In order to differentiate
between one cpu core holding a lock while another cpu is contending for
the same lock, the proc id written into the lock is (128 + cpu id). This
makes it unique value among the cpu cores and therefore when a core
locks the hwspinlock, other cores would wait for the lock to be released
since they would have a different proc id.  This value is specific for
the lock #7 only.

Declare lock #7 as raw capable, so the hwspinlock framework would not
enfore acquiring a s/w spinlock before acquiring the hwspinlock.

Cc: Jeffrey Hugo <jhugo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: Bjorn Andersson <bjorn.andersson-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
Cc: Andy Gross <agross-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/hwspinlock/qcom_hwspinlock.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/hwspinlock/qcom_hwspinlock.c b/drivers/hwspinlock/qcom_hwspinlock.c
index c752447..573f0dd 100644
--- a/drivers/hwspinlock/qcom_hwspinlock.c
+++ b/drivers/hwspinlock/qcom_hwspinlock.c
@@ -25,16 +25,26 @@
 
 #include "hwspinlock_internal.h"
 
-#define QCOM_MUTEX_APPS_PROC_ID	1
-#define QCOM_MUTEX_NUM_LOCKS	32
+#define QCOM_MUTEX_APPS_PROC_ID		1
+#define QCOM_MUTEX_CPUIDLE_OFFSET	128
+#define QCOM_CPUIDLE_LOCK		7
+#define QCOM_MUTEX_NUM_LOCKS		32
+
+static inline u32 __qcom_get_proc_id(struct hwspinlock *lock)
+{
+	return hwspin_lock_get_id(lock) == QCOM_CPUIDLE_LOCK ?
+			(QCOM_MUTEX_CPUIDLE_OFFSET + smp_processor_id()) :
+			QCOM_MUTEX_APPS_PROC_ID;
+}
 
 static int qcom_hwspinlock_trylock(struct hwspinlock *lock)
 {
 	struct regmap_field *field = lock->priv;
 	u32 lock_owner;
 	int ret;
+	u32 proc_id = __qcom_get_proc_id(lock);
 
-	ret = regmap_field_write(field, QCOM_MUTEX_APPS_PROC_ID);
+	ret = regmap_field_write(field, proc_id);
 	if (ret)
 		return ret;
 
@@ -42,7 +52,7 @@ static int qcom_hwspinlock_trylock(struct hwspinlock *lock)
 	if (ret)
 		return ret;
 
-	return lock_owner == QCOM_MUTEX_APPS_PROC_ID;
+	return lock_owner == proc_id;
 }
 
 static void qcom_hwspinlock_unlock(struct hwspinlock *lock)
@@ -57,7 +67,7 @@ static void qcom_hwspinlock_unlock(struct hwspinlock *lock)
 		return;
 	}
 
-	if (lock_owner != QCOM_MUTEX_APPS_PROC_ID) {
+	if (lock_owner != __qcom_get_proc_id(lock)) {
 		pr_err("%s: spinlock not owned by us (actual owner is %d)\n",
 				__func__, lock_owner);
 	}
@@ -129,6 +139,8 @@ static int qcom_hwspinlock_probe(struct platform_device *pdev)
 							     regmap, field);
 	}
 
+	bank->lock[QCOM_CPUIDLE_LOCK].hwcaps = HWL_CAP_ALLOW_RAW;
+
 	pm_runtime_enable(&pdev->dev);
 
 	ret = hwspin_lock_register(bank, &pdev->dev, &qcom_hwspinlock_ops,
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 09/10] drivers: qcom: spm: Use hwspinlock to serialize entry into SCM

[Lina Iyer]

When the last CPU enters idle, the state of L2 is determined and the
power controller for the L2 is programmed to power down. The power
controllers' state machine is only triggered when all the CPUs have
executed their WFI instruction. Multiple CPUs may enter SCM to power
down at the same time. Linux identifies the last man down and determines
the state of the cluster, but an FIQ could have kept another CPU busy.
The last CPU to enter SCM may not be the last CPU as determined by
Linux. So the state of the cluster would not be correctly relayed to the
SCM. To ensure that the last CPU in Linux is the same in SCM, serialize
the entry into SCM. Linux is responsible for flushing non-secure L2,
while SCM flushes only the secure lines of L2. Invalidation for secure
and non-secure L2 is done by the SCM when the first CPU resumes from
idle.

An example of the last man race -

Say there are two cores powering down - CoreA and CoreB

CoreA enters idle is not the last core and about to call into SCM
CoreB enters idle and is the last core, determines L2 should be flushed
CoreB receives an FIQ and gets busy handling the FIQ
CoreA has an pending IRQ, bails out of SCM and cpuidle
CoreB is still busy
CoreA enters cpuidle again and now is the last core
CoreA determines L2 should *not* be flushed and calls SCM
CoreB finishes FIQ, enters SCM with a stale L2 state (L2 to be flushed)
SCM records L2 as flushed and invalidates L2 when a core comes up.

To avoid this race, serialize all entry from Linux to SCM for cpuidle. A
hwspinlock is locked by Linux and released by the SCM when the context
switches to Secure. This way, the last man view of both Linux and SCM,
match with that of the L2 power controller configuration. A raw
hwspinlock would not use s/w spinlocks around the hwspinlock.
Since there is no possiblity of preemption in cpuidle, it is safe to
use the raw variant of the hwspinlock.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/soc/qcom/Kconfig |  1 +
 drivers/soc/qcom/spm.c   | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index b6c2e5d..cb50efb 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -18,6 +18,7 @@ config QCOM_PM
 	select PM_GENERIC_DOMAINS
 	select PM_GENERIC_DOMAINS_SLEEP
 	select PM_GENERIC_DOMAINS_OF
+	select HWSPINLOCK_QCOM
 	help
 	  QCOM Platform specific power driver to manage cores and L2 low power
 	  modes. It interface with various system drivers to put the cores in
diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
index b6d75db..bd09514 100644
--- a/drivers/soc/qcom/spm.c
+++ b/drivers/soc/qcom/spm.c
@@ -26,6 +26,8 @@
 #include <linux/cpu_pm.h>
 #include <linux/pm_domain.h>
 #include <linux/qcom_scm.h>
+#include <linux/pm_domain.h>
+#include <linux/hwspinlock.h>
 
 #include <asm/arm-pd.h>
 #include <asm/cacheflush.h>
@@ -38,6 +40,7 @@
 #define SPM_CTL_INDEX		0x7f
 #define SPM_CTL_INDEX_SHIFT	4
 #define SPM_CTL_EN		BIT(0)
+#define QCOM_PC_HWLOCK		7
 
 enum pm_sleep_mode {
 	PM_SLEEP_MODE_STBY,
@@ -139,6 +142,7 @@ static int l2_flush_flag;
 
 typedef int (*idle_fn)(int);
 static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops);
+static struct hwspinlock *remote_lock;
 
 static inline void spm_register_write(struct spm_driver_data *drv,
 					enum spm_reg reg, u32 val)
@@ -198,6 +202,24 @@ static int qcom_pm_collapse(unsigned long int unused)
 	if (l2_flush_flag == QCOM_SCM_CPU_PWR_DOWN_L2_OFF)
 		flush_cache_all();
 
+	/*
+	 * Wait and acquire the hwspin lock to synchronize
+	 * the entry into SCM. The view of the last core in Linux
+	 * should be same for SCM so the l2_flush_flag is correct.
+	 *
+	 * * IMPORTANT *
+	 * 1. SCM unlocks this lock.
+	 * 2. We do not want to call api that would spinlock before
+	 *    acquiring the hwspin lock. It will not be unlocked.
+	 *    So call raw variant
+	 * 3. Every core needs to acquire this lock and the entry into
+	 *    SCM would be serialized after this point.
+	 */
+	if (remote_lock) {
+		while (hwspin_trylock_raw(remote_lock))
+			;
+	}
+
 	qcom_scm_cpu_power_down(l2_flush_flag);
 
 	/*
@@ -439,6 +461,10 @@ static int spm_dev_probe(struct platform_device *pdev)
 	else
 		per_cpu(cpu_spm_drv, index) = drv;
 
+	/* Initialize hwspinlock, to serialize entry into secure */
+	if (!remote_lock && of_match_node(cache_spm_table, pdev->dev.of_node))
+		remote_lock = hwspin_lock_request_specific(QCOM_PC_HWLOCK);
+
 	dev_dbg(&pdev->dev, "SPM device probed.\n");
 	return 0;
 }
-- 
2.1.4

[PATCH RFC 10/10] arm: dts: qcom: Add TCSR mutex device bindings for APQ8084

[Lina Iyer]

Add device binding for hwspinlock support on QCOM 8084 SoCs.

Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/boot/dts/qcom-apq8084.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 4a61f8d..2a08741 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -286,5 +286,16 @@
 			interrupt-controller;
 			#interrupt-cells = <4>;
 		};
+
+		tcsr_mutex_regs: syscon@fd484000{
+			compatible = "syscon";
+			reg = <0xfd484000 0x20000>;
+		};
+
+		tcsr_mutex: hwlock {
+			compatible = "qcom,tcsr-mutex";
+			syscon = <&tcsr_mutex_regs 0 0x80>;
+			#hwlock-cells = <1>;
+		};
 	};
 };
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 06/10] drivers: qcom: Enable genpd on selecting QCOM_PM

[Andy Gross]

On Wed, Aug 05, 2015 at 10:32:42AM -0600, Lina Iyer wrote:
> Enable PM_CPU_DOMAIN and its PM_GENERIC_DOMAINS dependenciesd to provide
> cpu domain support for QCOM SoCs.

Fix dependencies <sic>

> 
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
>  drivers/soc/qcom/Kconfig | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index ba47b70..b6c2e5d 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -14,6 +14,10 @@ config QCOM_PM
>  	bool "Qualcomm Power Management"
>  	depends on ARCH_QCOM && !ARM64
>  	select QCOM_SCM
> +	select ARM_CPU_SUSPEND
> +	select PM_GENERIC_DOMAINS
> +	select PM_GENERIC_DOMAINS_SLEEP
> +	select PM_GENERIC_DOMAINS_OF
>  	help
>  	  QCOM Platform specific power driver to manage cores and L2 low power
>  	  modes. It interface with various system drivers to put the cores in
> -- 
> 2.1.4
> 

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH RFC 08/10] hwspinlock: qcom: Lock #7 is special lock, uses dynamic proc_id

[Bjorn Andersson]

On Wed 05 Aug 09:32 PDT 2015, Lina Iyer wrote:

> Hwspinlocks are widely used between processors in an SoC, and also
> between elevation levels within in the same processor.  QCOM SoC's use
> hwspinlock to serialize entry into a low power mode when the context
> switches from Linux to secure monitor.
> 
> Lock #7 has been assigned for this purpose. In order to differentiate
> between one cpu core holding a lock while another cpu is contending for
> the same lock, the proc id written into the lock is (128 + cpu id). This
> makes it unique value among the cpu cores and therefore when a core
> locks the hwspinlock, other cores would wait for the lock to be released
> since they would have a different proc id.  This value is specific for
> the lock #7 only.
> 

As I was thinking about this, I came to the conclusion that my argument
that it's system configuration and not a property of the block that lock
#7 is special is actually biting myself.

Just as #7 is system configuration, so is the fact that 1 is the lock
value for all other locks.

I've been meaning to answer you, but I haven't come to a good conclusion
in this matter. I think that both of these properties should be possible
to express in DT, but I don't know how.


Perhaps we should just list each lock that we expose as a subnode in DT
with a property to indicate the lock value - with the possibility of
indicating cpu_id.

Something like:

tcsr-mutex {
	compatible = "qcom,tcsr-mutex";
	syscon = <&tcsr_mutex_block 0 0x80>;

	#hwlock-cells = <1>;
	#address-cells = <1>;
	#size-cells = <0>;

	smem-lock@3 {
		reg = <3>;
		qcom,lock-value = <1>;
	};

	scm-lock@7 {
		reg = <7>;
		qcom,lock-value-from-cpu-id;
		qcom,lock-raw;
	};
};

As we don't reference most of these locks anyways I don't think this
would still be reasonable. And if reg is an array it's quite compact.

> Declare lock #7 as raw capable, so the hwspinlock framework would not
> enfore acquiring a s/w spinlock before acquiring the hwspinlock.
> 

I'm fine with this part!


Sorry for the extremely slow response, but this took some time to
process...

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 08/10] hwspinlock: qcom: Lock #7 is special lock, uses dynamic proc_id

[Lina Iyer]

On Wed, Aug 12 2015 at 22:30 -0600, Bjorn Andersson wrote:
>On Wed 05 Aug 09:32 PDT 2015, Lina Iyer wrote:
>
>> Hwspinlocks are widely used between processors in an SoC, and also
>> between elevation levels within in the same processor.  QCOM SoC's use
>> hwspinlock to serialize entry into a low power mode when the context
>> switches from Linux to secure monitor.
>>
>> Lock #7 has been assigned for this purpose. In order to differentiate
>> between one cpu core holding a lock while another cpu is contending for
>> the same lock, the proc id written into the lock is (128 + cpu id). This
>> makes it unique value among the cpu cores and therefore when a core
>> locks the hwspinlock, other cores would wait for the lock to be released
>> since they would have a different proc id.  This value is specific for
>> the lock #7 only.
>>
>
>As I was thinking about this, I came to the conclusion that my argument
>that it's system configuration and not a property of the block that lock
>#7 is special is actually biting myself.
>
>Just as #7 is system configuration, so is the fact that 1 is the lock
>value for all other locks.
>
>I've been meaning to answer you, but I haven't come to a good conclusion
>in this matter. I think that both of these properties should be possible
>to express in DT, but I don't know how.
>
I thought about that. These are s/w imposed behavior. As a far as the
h/w is concerned, you could just write a non-zero value and the lock is
considered acquired. So placing that in DT may not be appropriate.

>
>Perhaps we should just list each lock that we expose as a subnode in DT
>with a property to indicate the lock value - with the possibility of
>indicating cpu_id.
>
>Something like:
>
>tcsr-mutex {
>	compatible = "qcom,tcsr-mutex";
>	syscon = <&tcsr_mutex_block 0 0x80>;
>
>	#hwlock-cells = <1>;
>	#address-cells = <1>;
>	#size-cells = <0>;
>
>	smem-lock@3 {
>		reg = <3>;
>		qcom,lock-value = <1>;
>	};
>
>	scm-lock@7 {
>		reg = <7>;
>		qcom,lock-value-from-cpu-id;
>		qcom,lock-raw;
>	};
>};
>
>As we don't reference most of these locks anyways I don't think this
>would still be reasonable. And if reg is an array it's quite compact.
>
But, looking at the DT, its not evident what lock-value-from-cpu-id,
would mean.  It is an implementation detail of Linux (as a result of
firmware). May be better to just hide it in the hwspinlock driver.

I am not sure about lock-raw, but I would think its not QCOM specific.
Imagine the case, when hwspinlock framework does not s/w spinlock around
the hwspinlock, we wouldnt have this property. The reason why we
spinlock around the hwspinlock is because we dont have a good way to
generate a unique value for the hwspinlock, so multiple threads
acquiring the same locks could safely be assured that they have acquired
the lock. While convenient and probably more effecient to just have s/w
spinlocks around the hwspinlock, the problem here is it is mandated
across all locks.

To me it seems OK to explicity call out lock #7 as unique entity in the
bank that is compatible with a raw hwspinlock in the DT. But the value
that lock #7 uses is an implementation detail and should rest wth the
drivers.

Thanks,
Lina
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html