linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Introduce LMh driver for Qualcomm SoCs
@ 2021-06-08 22:29 Thara Gopinath
  2021-06-08 22:29 ` [PATCH 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh Thara Gopinath
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Thara Gopinath @ 2021-06-08 22:29 UTC (permalink / raw)
  To: agross, bjorn.andersson, rui.zhang, daniel.lezcano, viresh.kumar,
	rjw, robh+dt
  Cc: linux-arm-msm, linux-pm, linux-kernel, devicetree

Limits Management Hardware(LMh) is a hardware infrastructure on some
Qualcomm SoCs that can enforce temperature and current limits as programmed
by software for certain IPs like CPU. On many newer SoCs LMh is configured
by firmware/TZ and no programming is needed from the kernel side. But on
certain SoCs like sdm845 the firmware does not do a complete programming of
the h/w block. On such SoCs kernel software has to explicitly set up the
temperature limits and turn on various monitoring and enforcing algorithms
on the hardware.

Introduce support for enabling and programming various limit settings and
monitoring capabilities of Limits Management Hardware(LMh) associated with
cpu clusters. Also introduce support in cpufreq hardware driver to monitor
the interrupt associated with cpu frequency throttling so that this
information can be conveyed to the schdeuler via thermal pressure
interface.

With this patch series following cpu performance improvement(30-70%) is
observed on sdm845. The reasoning here is that without LMh being programmed
properly from the kernel, the default settings were enabling thermal
mitigation for CPUs at too low a temperature (around 70-75 degree C).  This
in turn meant that many a time CPUs were never actually allowed to hit the
maximum possible/required frequencies.

UnixBench whets and dhry (./Run whets dhry)
System Benchmarks Index Score

		Without LMh Support		With LMh Support
1 copy test	1353.7				1773.2

8 copy tests	4473.6				7402.3

Sysbench cpu 
sysbench cpu --threads=8 --time=60 --cpu-max-prime=100000 run

		Without LMh Support		With LMh Support
Events per
second			355				614

Avg Latency(ms)		21.84				13.02

Thara Gopinath (5):
  firmware: qcom_scm: Introduce SCM calls to access LMh
  thermal: qcom: Add support for LMh driver
  cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support
  arm64: boot: dts: sdm45: Add support for LMh node
  arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal
    zones 0-7

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 246 +++------------------------
 drivers/cpufreq/qcom-cpufreq-hw.c    | 100 +++++++++++
 drivers/firmware/qcom_scm.c          |  47 +++++
 drivers/firmware/qcom_scm.h          |   4 +
 drivers/thermal/qcom/Kconfig         |  10 ++
 drivers/thermal/qcom/Makefile        |   1 +
 drivers/thermal/qcom/lmh.c           | 244 ++++++++++++++++++++++++++
 include/linux/qcom_scm.h             |  13 ++
 8 files changed, 440 insertions(+), 225 deletions(-)
 create mode 100644 drivers/thermal/qcom/lmh.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh
  2021-06-08 22:29 [PATCH 0/5] Introduce LMh driver for Qualcomm SoCs Thara Gopinath
@ 2021-06-08 22:29 ` Thara Gopinath
  2021-06-09  3:10   ` kernel test robot
  2021-06-08 22:29 ` [PATCH 2/5] thermal: qcom: Add support for LMh driver Thara Gopinath
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Thara Gopinath @ 2021-06-08 22:29 UTC (permalink / raw)
  To: agross, bjorn.andersson, rui.zhang, daniel.lezcano, viresh.kumar,
	rjw, robh+dt
  Cc: linux-arm-msm, linux-pm, linux-kernel, devicetree

Introduce SCM calls to access/configure limits management hardware(LMh).

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/firmware/qcom_scm.c | 47 +++++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h |  4 ++++
 include/linux/qcom_scm.h    | 13 ++++++++++
 3 files changed, 64 insertions(+)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index ee9cb545e73b..0259e9ffb8a1 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1147,6 +1147,53 @@ int qcom_scm_qsmmu500_wait_safe_toggle(bool en)
 }
 EXPORT_SYMBOL(qcom_scm_qsmmu500_wait_safe_toggle);
 
+bool qcom_scm_lmh_dcvsh_available(void)
+{
+	return __qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_LMH, QCOM_SCM_LMH_LIMIT_DCVSH);
+}
+EXPORT_SYMBOL(qcom_scm_lmh_dcvsh_available);
+
+int qcom_scm_lmh_profile_change(u32 profile_id)
+{
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_LMH,
+		.cmd = QCOM_SCM_LMH_LIMIT_PROFILE_CHANGE,
+		.arginfo = QCOM_SCM_ARGS(1, QCOM_SCM_VAL),
+		.args[0] = profile_id,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
+
+	return qcom_scm_call(__scm->dev, &desc, NULL);
+}
+EXPORT_SYMBOL(qcom_scm_lmh_profile_change);
+
+int qcom_scm_lmh_dcvsh(u32 *payload, u32 payload_size, u64 limit_node, u32 node_id, u64 version)
+{
+	dma_addr_t payload_phys;
+	void *payload_buf;
+
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_LMH,
+		.cmd = QCOM_SCM_LMH_LIMIT_DCVSH,
+		.arginfo = QCOM_SCM_ARGS(5, QCOM_SCM_RO, QCOM_SCM_VAL, QCOM_SCM_VAL,
+					QCOM_SCM_VAL, QCOM_SCM_VAL),
+		.args[1] = payload_size,
+		.args[2] = limit_node,
+		.args[3] = node_id,
+		.args[4] = version,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
+
+	payload_buf = dma_alloc_coherent(__scm->dev, payload_size, &payload_phys, GFP_KERNEL);
+	if (!payload_buf)
+		return -ENOMEM;
+	memcpy(payload_buf, payload, payload_size);
+
+	desc.args[0] = payload_phys;
+	return qcom_scm_call(__scm->dev, &desc, NULL);
+}
+EXPORT_SYMBOL(qcom_scm_lmh_dcvsh);
+
 static int qcom_scm_find_dload_address(struct device *dev, u64 *addr)
 {
 	struct device_node *tcsr;
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 632fe3142462..d92156ceb3ac 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -114,6 +114,10 @@ extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
 #define QCOM_SCM_SVC_HDCP		0x11
 #define QCOM_SCM_HDCP_INVOKE		0x01
 
+#define QCOM_SCM_SVC_LMH			0x13
+#define QCOM_SCM_LMH_LIMIT_PROFILE_CHANGE	0x01
+#define QCOM_SCM_LMH_LIMIT_DCVSH		0x10
+
 #define QCOM_SCM_SVC_SMMU_PROGRAM		0x15
 #define QCOM_SCM_SMMU_CONFIG_ERRATA1		0x03
 #define QCOM_SCM_SMMU_CONFIG_ERRATA1_CLIENT_ALL	0x02
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 0165824c5128..0c92197769e7 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -109,6 +109,12 @@ extern int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 			     u32 *resp);
 
 extern int qcom_scm_qsmmu500_wait_safe_toggle(bool en);
+
+extern int qcom_scm_lmh_dcvsh(u32 *payload, u32 payload_size, u64 limit_node,
+			      u32 node_id, u64 version);
+extern int qcom_scm_lmh_profile_change(u32 profile_id);
+extern bool qcom_scm_lmh_dcvsh_available(void);
+
 #else
 
 #include <linux/errno.h>
@@ -170,5 +176,12 @@ static inline int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
 
 static inline int qcom_scm_qsmmu500_wait_safe_toggle(bool en)
 		{ return -ENODEV; }
+
+int qcom_scm_lmh_dcvsh(u32 *payload, u32 payload_size, u64 limit_node,
+		       u32 node_id, u64 version)
+		{ return -ENODEV; }
+int qcom_scm_lmh_profile_change(u32 profile_id) { return -ENODEV; }
+
+bool qcom_scm_lmh_dcvsh_available(void) { return -ENODEV; }
 #endif
 #endif
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/5] thermal: qcom: Add support for LMh driver
  2021-06-08 22:29 [PATCH 0/5] Introduce LMh driver for Qualcomm SoCs Thara Gopinath
  2021-06-08 22:29 ` [PATCH 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh Thara Gopinath
@ 2021-06-08 22:29 ` Thara Gopinath
  2021-06-09  2:25   ` Randy Dunlap
  2021-06-14 20:53   ` Bjorn Andersson
  2021-06-08 22:29 ` [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support Thara Gopinath
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 18+ messages in thread
From: Thara Gopinath @ 2021-06-08 22:29 UTC (permalink / raw)
  To: agross, bjorn.andersson, rui.zhang, daniel.lezcano, viresh.kumar,
	rjw, robh+dt
  Cc: linux-arm-msm, linux-pm, linux-kernel, devicetree

Driver enabling various pieces of Limits Management Hardware(LMh) for cpu
cluster0 and cpu cluster1 namely kick starting monitoring of temperature,
current, battery current violations, enabling reliability algorithm and
setting up various temperature limits.

The following has been explained in the cover letter. I am including this
here so that this remains in the commit message as well.

LMh is a hardware infrastructure on some Qualcomm SoCs that can enforce
temperature and current limits as programmed by software for certain IPs
like CPU. On many newer SoCs LMh is configured by firmware/TZ and no
programming is needed from the kernel side. But on certain SoCs like sdm845
the firmware does not do a complete programming of the h/w. On such SoCs
kernel software has to explicitly set up the temperature limits and turn on
various monitoring and enforcing algorithms on the hardware.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/thermal/qcom/Kconfig  |  10 ++
 drivers/thermal/qcom/Makefile |   1 +
 drivers/thermal/qcom/lmh.c    | 244 ++++++++++++++++++++++++++++++++++
 3 files changed, 255 insertions(+)
 create mode 100644 drivers/thermal/qcom/lmh.c

diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
index 8d5ac2df26dc..c95b95e254d7 100644
--- a/drivers/thermal/qcom/Kconfig
+++ b/drivers/thermal/qcom/Kconfig
@@ -31,3 +31,13 @@ config QCOM_SPMI_TEMP_ALARM
 	  trip points. The temperature reported by the thermal sensor reflects the
 	  real time die temperature if an ADC is present or an estimate of the
 	  temperature based upon the over temperature stage value.
+
+config QCOM_LMH
+	tristate "Qualcomm Limits Management Hardware"
+	depends on ARCH_QCOM
+	help
+	  This enables initialization of Qualcomm limits management
+	  hardware(LMh). LMh allows for h/w enforced mitigation for cpus based on
+	  input from temperature and current sensors.  On many newer Qualcomm SoCs
+	  LMH is configure in the firmware and this feature need not be enabled.
+	  However, on certain SoCs like sdm845 LMH has to be configured from HLOS.
diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
index 252ea7d9da0b..0fa2512042e7 100644
--- a/drivers/thermal/qcom/Makefile
+++ b/drivers/thermal/qcom/Makefile
@@ -5,3 +5,4 @@ qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
 				   tsens-8960.o
 obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
+obj-$(CONFIG_QCOM_LMH)		+= lmh.o
diff --git a/drivers/thermal/qcom/lmh.c b/drivers/thermal/qcom/lmh.c
new file mode 100644
index 000000000000..8741a36cb674
--- /dev/null
+++ b/drivers/thermal/qcom/lmh.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright (C) 2021, Linaro Limited. All rights reserved.
+ */
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+#include <linux/qcom_scm.h>
+
+#define LMH_NODE_DCVS			0x44435653
+#define LMH_CLUSTER0_NODE_ID		0x6370302D
+#define LMH_CLUSTER1_NODE_ID		0x6370312D
+
+#define LMH_SUB_FN_THERMAL		0x54484D4C
+#define LMH_SUB_FN_CRNT			0x43524E54
+#define LMH_SUB_FN_REL			0x52454C00
+#define LMH_SUB_FN_BCL			0x42434C00
+
+#define LMH_ALGO_MODE_ENABLE		0x454E424C
+#define LMH_TH_HI_THRESHOLD		0x48494748
+#define LMH_TH_LOW_THRESHOLD		0x4C4F5700
+#define LMH_TH_ARM_THRESHOLD		0x41524D00
+
+#define LMH_TH_HI_TEMP			95000
+#define LMH_TH_LOW_TEMP			94500
+#define LMH_TH_ARM_TEMP			65000
+
+#define LMH_REG_DCVS_INTR_CLR		0x8
+
+struct lmh_hw_data {
+	void __iomem *base;
+	struct irq_domain *domain;
+	int irq;
+	u32 payload[5];
+	u32 payload_size;
+	u32 cpu_id;
+};
+
+static void update_payload(struct lmh_hw_data *lmh_data, u32 fn, u32 reg, u32 val)
+{
+	lmh_data->payload[0] = fn;
+	lmh_data->payload[1] = 0;
+	lmh_data->payload[2] = reg;
+	lmh_data->payload[3] = 1;
+	lmh_data->payload[4] = val;
+}
+
+static irqreturn_t lmh_handle_irq(int hw_irq, void *data)
+{
+	struct lmh_hw_data *lmh_data = data;
+	int irq = irq_find_mapping(lmh_data->domain, 0);
+
+	/*
+	 * Disable interrupt and call the cpufreq driver to handle the interrupt
+	 * cpufreq will enable the interrupt once finished processing.
+	 */
+	disable_irq_nosync(lmh_data->irq);
+	if (irq)
+		generic_handle_irq(irq);
+
+	return 0;
+}
+
+static void lmh_enable_interrupt(struct irq_data *d)
+{
+	struct lmh_hw_data *lmh_data = irq_data_get_irq_chip_data(d);
+
+	/* Clear the existing interrupt */
+	writel_relaxed(0xFF, lmh_data->base + LMH_REG_DCVS_INTR_CLR);
+	enable_irq(lmh_data->irq);
+}
+
+static struct irq_chip lmh_irq_chip = {
+	.name           = "lmh",
+	.irq_enable	= lmh_enable_interrupt,
+};
+
+static int lmh_irq_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
+{
+	struct lmh_hw_data *lmh_data = d->host_data;
+
+	irq_set_chip_and_handler(irq, &lmh_irq_chip, handle_simple_irq);
+	irq_set_chip_data(irq, lmh_data);
+
+	return 0;
+}
+
+static const struct irq_domain_ops lmh_irq_ops = {
+	.map = lmh_irq_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static int lmh_probe(struct platform_device *pdev)
+{
+	struct device *dev;
+	struct device_node *np;
+	struct lmh_hw_data *lmh_data;
+	u32 node_id;
+	int ret;
+
+	dev = &pdev->dev;
+	np = dev->of_node;
+	if (!np)
+		return -EINVAL;
+
+	lmh_data = devm_kzalloc(dev, sizeof(*lmh_data), GFP_KERNEL);
+	if (!lmh_data)
+		return -ENOMEM;
+
+	lmh_data->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(lmh_data->base))
+		return PTR_ERR(lmh_data->base);
+
+	ret = of_property_read_u32(np, "qcom,lmh-cpu-id", &lmh_data->cpu_id);
+	if (ret)
+		return -ENODEV;
+
+	/*
+	 * Only sdm845 has lmh hardware currently enabled from hlos. If this is needed
+	 * for other platforms, revisit this to check if the <cpu-id, node-id> should be part
+	 * of a dt match table.
+	 */
+	if (lmh_data->cpu_id == 0) {
+		node_id = LMH_CLUSTER0_NODE_ID;
+	} else if (lmh_data->cpu_id == 4) {
+		node_id = LMH_CLUSTER1_NODE_ID;
+	} else {
+		dev_err(dev, "Wrong cpu id associated with lmh node\n");
+		return -EINVAL;
+	}
+
+	/* Payload size is five bytes for now */
+	lmh_data->payload_size = 5 * sizeof(u32);
+
+	platform_set_drvdata(pdev, lmh_data);
+
+	if (!qcom_scm_lmh_dcvsh_available())
+		return -EINVAL;
+
+	/* Enable Thermal Algorithm */
+	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1);
+	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
+		return ret;
+	}
+
+	/* Enable Current Sensing Algorithm */
+	update_payload(lmh_data, LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1);
+	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error %d enabling current subfunction\n", ret);
+		return ret;
+	}
+
+	/* Enable Reliability Algorithm */
+	update_payload(lmh_data, LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1);
+	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
+		return ret;
+	}
+
+	/* Enable BCL Algorithm */
+	update_payload(lmh_data, LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1);
+	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error %d enabling BCL subfunction\n", ret);
+		return ret;
+	}
+
+	ret = qcom_scm_lmh_profile_change(0x1);
+	if (ret) {
+		dev_err(dev, "Error %d changing profile\n", ret);
+		return ret;
+	}
+
+	/* Set default thermal trips */
+	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_ARM_THRESHOLD, LMH_TH_ARM_TEMP);
+	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);
+		return ret;
+	}
+
+	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_HI_THRESHOLD, LMH_TH_HI_TEMP);
+	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error setting thermal HI thershold%d\n", ret);
+		return ret;
+	}
+	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_LOW_THRESHOLD, LMH_TH_LOW_TEMP);
+	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);
+		return ret;
+	}
+
+	lmh_data->irq = platform_get_irq(pdev, 0);
+	lmh_data->domain = irq_domain_add_linear(np, 1, &lmh_irq_ops, lmh_data);
+	if (!lmh_data->domain) {
+		dev_err(dev, "Error adding irq_domain\n");
+		return -EINVAL;
+	}
+
+	ret = devm_request_irq(dev, lmh_data->irq, lmh_handle_irq,
+			       IRQF_TRIGGER_HIGH | IRQF_ONESHOT | IRQF_NO_SUSPEND,
+			       "lmh-irq", lmh_data);
+	if (ret) {
+		dev_err(dev, "Error %d registering irq %x\n", ret, lmh_data->irq);
+		irq_domain_remove(lmh_data->domain);
+		return ret;
+	}
+	return 0;
+}
+
+static const struct of_device_id lmh_table[] = {
+	{ .compatible = "qcom,msm-hw-limits", },
+	{},
+};
+
+static struct platform_driver lmh_driver = {
+	.probe = lmh_probe,
+	.driver = {
+		.name = "qcom-lmh",
+		.of_match_table = lmh_table,
+	},
+};
+module_platform_driver(lmh_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("QCOM LMH driver");
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support
  2021-06-08 22:29 [PATCH 0/5] Introduce LMh driver for Qualcomm SoCs Thara Gopinath
  2021-06-08 22:29 ` [PATCH 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh Thara Gopinath
  2021-06-08 22:29 ` [PATCH 2/5] thermal: qcom: Add support for LMh driver Thara Gopinath
@ 2021-06-08 22:29 ` Thara Gopinath
  2021-06-14 10:31   ` Viresh Kumar
  2021-06-18 18:16   ` Bjorn Andersson
  2021-06-08 22:29 ` [PATCH 4/5] arm64: boot: dts: sdm45: Add support for LMh node Thara Gopinath
  2021-06-08 22:29 ` [PATCH 5/5] arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal zones 0-7 Thara Gopinath
  4 siblings, 2 replies; 18+ messages in thread
From: Thara Gopinath @ 2021-06-08 22:29 UTC (permalink / raw)
  To: agross, bjorn.andersson, rui.zhang, daniel.lezcano, viresh.kumar,
	rjw, robh+dt
  Cc: linux-arm-msm, linux-pm, linux-kernel, devicetree

Add interrupt support to notify the kernel of h/w initiated frequency
throttling by LMh. Convey this to scheduler via thermal presssure
interface.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/cpufreq/qcom-cpufreq-hw.c | 100 ++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index f86859bf76f1..95e17330aa9d 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -13,6 +13,7 @@
 #include <linux/of_platform.h>
 #include <linux/pm_opp.h>
 #include <linux/slab.h>
+#include <linux/interrupt.h>
 
 #define LUT_MAX_ENTRIES			40U
 #define LUT_SRC				GENMASK(31, 30)
@@ -22,10 +23,13 @@
 #define CLK_HW_DIV			2
 #define LUT_TURBO_IND			1
 
+#define HZ_PER_KHZ			1000
+
 struct qcom_cpufreq_soc_data {
 	u32 reg_enable;
 	u32 reg_freq_lut;
 	u32 reg_volt_lut;
+	u32 reg_current_vote;
 	u32 reg_perf_state;
 	u8 lut_row_size;
 };
@@ -33,7 +37,11 @@ struct qcom_cpufreq_soc_data {
 struct qcom_cpufreq_data {
 	void __iomem *base;
 	struct resource *res;
+	struct delayed_work lmh_dcvs_poll_work;
 	const struct qcom_cpufreq_soc_data *soc_data;
+	cpumask_var_t cpus;
+	unsigned long throttled_freq;
+	int lmh_dcvs_irq;
 };
 
 static unsigned long cpu_hw_rate, xo_rate;
@@ -251,10 +259,79 @@ static void qcom_get_related_cpus(int index, struct cpumask *m)
 	}
 }
 
+static inline unsigned long qcom_lmh_vote_to_freq(u32 val)
+{
+	return (val & 0x3FF) * 19200;
+}
+
+static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
+{
+	struct cpufreq_policy policy;
+	struct dev_pm_opp *opp;
+	struct device *dev;
+	unsigned long max_capacity, capacity, freq_hz;
+	unsigned int val, freq;
+
+	val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
+	freq = qcom_lmh_vote_to_freq(val);
+	freq_hz = freq * HZ_PER_KHZ;
+
+	/* Do I need to calculate ceil and floor ? */
+	dev = get_cpu_device(cpumask_first(data->cpus));
+	opp = dev_pm_opp_find_freq_floor(dev, &freq_hz);
+	if (IS_ERR(opp) && PTR_ERR(opp) == -ERANGE)
+		opp = dev_pm_opp_find_freq_ceil(dev, &freq_hz);
+
+	data->throttled_freq = freq_hz / HZ_PER_KHZ;
+
+	cpufreq_get_policy(&policy, cpumask_first(data->cpus));
+
+	/* Update thermal pressure */
+	max_capacity = arch_scale_cpu_capacity(cpumask_first(data->cpus));
+	capacity = data->throttled_freq * max_capacity;
+	capacity /= policy.cpuinfo.max_freq;
+	/* Don't pass boost capacity to scheduler */
+	if (capacity > max_capacity)
+		capacity = max_capacity;
+	arch_set_thermal_pressure(data->cpus, max_capacity - capacity);
+}
+
+static void qcom_lmh_dcvs_poll(struct work_struct *work)
+{
+	struct qcom_cpufreq_data *data;
+
+	data = container_of(work, struct qcom_cpufreq_data, lmh_dcvs_poll_work.work);
+
+	qcom_lmh_dcvs_notify(data);
+	/**
+	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
+	 * polling and switch back to interrupt mechanism
+	 */
+	if (data->throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(data->cpus)))
+		/* Clear the existing interrupts and enable it back */
+		enable_irq(data->lmh_dcvs_irq);
+	else
+		mod_delayed_work(system_highpri_wq, &data->lmh_dcvs_poll_work,
+				 msecs_to_jiffies(10));
+}
+
+static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
+{
+	struct qcom_cpufreq_data *c_data = data;
+
+	/* Disable interrupt and enable polling */
+	disable_irq_nosync(c_data->lmh_dcvs_irq);
+	qcom_lmh_dcvs_notify(c_data);
+	mod_delayed_work(system_highpri_wq, &c_data->lmh_dcvs_poll_work, msecs_to_jiffies(10));
+
+	return 0;
+}
+
 static const struct qcom_cpufreq_soc_data qcom_soc_data = {
 	.reg_enable = 0x0,
 	.reg_freq_lut = 0x110,
 	.reg_volt_lut = 0x114,
+	.reg_current_vote = 0x704,
 	.reg_perf_state = 0x920,
 	.lut_row_size = 32,
 };
@@ -285,6 +362,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 	void __iomem *base;
 	struct qcom_cpufreq_data *data;
 	int ret, index;
+	bool lmh_mitigation_enabled = false;
 
 	cpu_dev = get_cpu_device(policy->cpu);
 	if (!cpu_dev) {
@@ -305,6 +383,8 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 
 	index = args.args[0];
 
+	lmh_mitigation_enabled = of_property_read_bool(pdev->dev.of_node, "qcom,support-lmh");
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
 	if (!res) {
 		dev_err(dev, "failed to get mem resource %d\n", index);
@@ -329,6 +409,11 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 		goto unmap_base;
 	}
 
+	if (!alloc_cpumask_var(&data->cpus, GFP_KERNEL)) {
+		ret = -ENOMEM;
+		goto unmap_base;
+	}
+
 	data->soc_data = of_device_get_match_data(&pdev->dev);
 	data->base = base;
 	data->res = res;
@@ -347,6 +432,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 		goto error;
 	}
 
+	cpumask_copy(data->cpus, policy->cpus);
 	policy->driver_data = data;
 
 	ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy);
@@ -370,6 +456,20 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
 	}
 
+	if (lmh_mitigation_enabled) {
+		data->lmh_dcvs_irq = platform_get_irq(pdev, index);
+		if (data->lmh_dcvs_irq < 0) {
+			ret = data->lmh_dcvs_irq;
+			goto error;
+		}
+		ret = devm_request_irq(dev, data->lmh_dcvs_irq, qcom_lmh_dcvs_handle_irq,
+				       0, "dcvsh-irq", data);
+		if (ret) {
+			dev_err(dev, "Error %d registering irq %x\n", ret, data->lmh_dcvs_irq);
+			goto error;
+		}
+		INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
+	}
 	return 0;
 error:
 	kfree(data);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/5] arm64: boot: dts: sdm45: Add support for LMh node
  2021-06-08 22:29 [PATCH 0/5] Introduce LMh driver for Qualcomm SoCs Thara Gopinath
                   ` (2 preceding siblings ...)
  2021-06-08 22:29 ` [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support Thara Gopinath
@ 2021-06-08 22:29 ` Thara Gopinath
  2021-06-08 22:29 ` [PATCH 5/5] arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal zones 0-7 Thara Gopinath
  4 siblings, 0 replies; 18+ messages in thread
From: Thara Gopinath @ 2021-06-08 22:29 UTC (permalink / raw)
  To: agross, bjorn.andersson, rui.zhang, daniel.lezcano, viresh.kumar,
	rjw, robh+dt
  Cc: linux-arm-msm, linux-pm, linux-kernel, devicetree

Add LMh nodes for cpu cluster0 and cpu cluster1. Also add interrupt
support in cpufreq node to capture the LMh interrupt and let the scheduler
know of the max frequency throttling.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0a86fe71a66d..fdd8d816f728 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -3646,6 +3646,24 @@ swm: swm@c85 {
 			};
 		};
 
+		lmh_cluster1: lmh@17d70800 {
+			compatible = "qcom,msm-hw-limits";
+			reg = <0 0x17d70800 0 0x401>;
+			interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+			qcom,lmh-cpu-id = <0x4>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+		};
+
+		lmh_cluster0: lmh@17d78800 {
+			compatible = "qcom,msm-hw-limits";
+			reg = <0 0x17d78800 0 0x401>;
+			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+			qcom,lmh-cpu-id = <0x0>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+		};
+
 		sound: sound {
 		};
 
@@ -4911,10 +4929,13 @@ cpufreq_hw: cpufreq@17d43000 {
 			reg = <0 0x17d43000 0 0x1400>, <0 0x17d45800 0 0x1400>;
 			reg-names = "freq-domain0", "freq-domain1";
 
+			interrupts-extended = <&lmh_cluster0 0>, <&lmh_cluster1 0>;
+
 			clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
 			clock-names = "xo", "alternate";
 
 			#freq-domain-cells = <1>;
+			qcom,support-lmh = <1>;
 		};
 
 		wifi: wifi@18800000 {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 5/5] arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal zones 0-7
  2021-06-08 22:29 [PATCH 0/5] Introduce LMh driver for Qualcomm SoCs Thara Gopinath
                   ` (3 preceding siblings ...)
  2021-06-08 22:29 ` [PATCH 4/5] arm64: boot: dts: sdm45: Add support for LMh node Thara Gopinath
@ 2021-06-08 22:29 ` Thara Gopinath
  4 siblings, 0 replies; 18+ messages in thread
From: Thara Gopinath @ 2021-06-08 22:29 UTC (permalink / raw)
  To: agross, bjorn.andersson, rui.zhang, daniel.lezcano, viresh.kumar,
	rjw, robh+dt
  Cc: linux-arm-msm, linux-pm, linux-kernel, devicetree

Now that Limits h/w is enabled to monitor thermal events around cpus and
throttle the cpu frequencies, remove the s/w montoring of the same which
was happening via tsens. We keep critical trip points for these zones so
that system shutdown can be initiated if the temperature exceeds critical
trip.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 225 ---------------------------
 1 file changed, 225 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index fdd8d816f728..9a494a1b7a09 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -4971,18 +4971,6 @@ cpu0-thermal {
 			thermal-sensors = <&tsens0 1>;
 
 			trips {
-				cpu0_alert0: trip-point0 {
-					temperature = <90000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
-				cpu0_alert1: trip-point1 {
-					temperature = <95000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
 				cpu0_crit: cpu_crit {
 					temperature = <110000>;
 					hysteresis = <1000>;
@@ -4990,22 +4978,6 @@ cpu0_crit: cpu_crit {
 				};
 			};
 
-			cooling-maps {
-				map0 {
-					trip = <&cpu0_alert0>;
-					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-				map1 {
-					trip = <&cpu0_alert1>;
-					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-			};
 		};
 
 		cpu1-thermal {
@@ -5015,18 +4987,6 @@ cpu1-thermal {
 			thermal-sensors = <&tsens0 2>;
 
 			trips {
-				cpu1_alert0: trip-point0 {
-					temperature = <90000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
-				cpu1_alert1: trip-point1 {
-					temperature = <95000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
 				cpu1_crit: cpu_crit {
 					temperature = <110000>;
 					hysteresis = <1000>;
@@ -5034,22 +4994,6 @@ cpu1_crit: cpu_crit {
 				};
 			};
 
-			cooling-maps {
-				map0 {
-					trip = <&cpu1_alert0>;
-					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-				map1 {
-					trip = <&cpu1_alert1>;
-					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-			};
 		};
 
 		cpu2-thermal {
@@ -5059,18 +5003,6 @@ cpu2-thermal {
 			thermal-sensors = <&tsens0 3>;
 
 			trips {
-				cpu2_alert0: trip-point0 {
-					temperature = <90000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
-				cpu2_alert1: trip-point1 {
-					temperature = <95000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
 				cpu2_crit: cpu_crit {
 					temperature = <110000>;
 					hysteresis = <1000>;
@@ -5078,22 +5010,6 @@ cpu2_crit: cpu_crit {
 				};
 			};
 
-			cooling-maps {
-				map0 {
-					trip = <&cpu2_alert0>;
-					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-				map1 {
-					trip = <&cpu2_alert1>;
-					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-			};
 		};
 
 		cpu3-thermal {
@@ -5103,41 +5019,12 @@ cpu3-thermal {
 			thermal-sensors = <&tsens0 4>;
 
 			trips {
-				cpu3_alert0: trip-point0 {
-					temperature = <90000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
-				cpu3_alert1: trip-point1 {
-					temperature = <95000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
 				cpu3_crit: cpu_crit {
 					temperature = <110000>;
 					hysteresis = <1000>;
 					type = "critical";
 				};
 			};
-
-			cooling-maps {
-				map0 {
-					trip = <&cpu3_alert0>;
-					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-				map1 {
-					trip = <&cpu3_alert1>;
-					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-			};
 		};
 
 		cpu4-thermal {
@@ -5147,18 +5034,6 @@ cpu4-thermal {
 			thermal-sensors = <&tsens0 7>;
 
 			trips {
-				cpu4_alert0: trip-point0 {
-					temperature = <90000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
-				cpu4_alert1: trip-point1 {
-					temperature = <95000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
 				cpu4_crit: cpu_crit {
 					temperature = <110000>;
 					hysteresis = <1000>;
@@ -5166,22 +5041,6 @@ cpu4_crit: cpu_crit {
 				};
 			};
 
-			cooling-maps {
-				map0 {
-					trip = <&cpu4_alert0>;
-					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-				map1 {
-					trip = <&cpu4_alert1>;
-					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-			};
 		};
 
 		cpu5-thermal {
@@ -5191,18 +5050,6 @@ cpu5-thermal {
 			thermal-sensors = <&tsens0 8>;
 
 			trips {
-				cpu5_alert0: trip-point0 {
-					temperature = <90000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
-				cpu5_alert1: trip-point1 {
-					temperature = <95000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
 				cpu5_crit: cpu_crit {
 					temperature = <110000>;
 					hysteresis = <1000>;
@@ -5210,22 +5057,6 @@ cpu5_crit: cpu_crit {
 				};
 			};
 
-			cooling-maps {
-				map0 {
-					trip = <&cpu5_alert0>;
-					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-				map1 {
-					trip = <&cpu5_alert1>;
-					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-			};
 		};
 
 		cpu6-thermal {
@@ -5235,18 +5066,6 @@ cpu6-thermal {
 			thermal-sensors = <&tsens0 9>;
 
 			trips {
-				cpu6_alert0: trip-point0 {
-					temperature = <90000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
-				cpu6_alert1: trip-point1 {
-					temperature = <95000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
 				cpu6_crit: cpu_crit {
 					temperature = <110000>;
 					hysteresis = <1000>;
@@ -5254,22 +5073,6 @@ cpu6_crit: cpu_crit {
 				};
 			};
 
-			cooling-maps {
-				map0 {
-					trip = <&cpu6_alert0>;
-					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-				map1 {
-					trip = <&cpu6_alert1>;
-					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-			};
 		};
 
 		cpu7-thermal {
@@ -5279,18 +5082,6 @@ cpu7-thermal {
 			thermal-sensors = <&tsens0 10>;
 
 			trips {
-				cpu7_alert0: trip-point0 {
-					temperature = <90000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
-				cpu7_alert1: trip-point1 {
-					temperature = <95000>;
-					hysteresis = <2000>;
-					type = "passive";
-				};
-
 				cpu7_crit: cpu_crit {
 					temperature = <110000>;
 					hysteresis = <1000>;
@@ -5298,22 +5089,6 @@ cpu7_crit: cpu_crit {
 				};
 			};
 
-			cooling-maps {
-				map0 {
-					trip = <&cpu7_alert0>;
-					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-				map1 {
-					trip = <&cpu7_alert1>;
-					cooling-device = <&CPU4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&CPU7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-			};
 		};
 
 		aoss0-thermal {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] thermal: qcom: Add support for LMh driver
  2021-06-08 22:29 ` [PATCH 2/5] thermal: qcom: Add support for LMh driver Thara Gopinath
@ 2021-06-09  2:25   ` Randy Dunlap
  2021-06-15  1:37     ` Thara Gopinath
  2021-06-14 20:53   ` Bjorn Andersson
  1 sibling, 1 reply; 18+ messages in thread
From: Randy Dunlap @ 2021-06-09  2:25 UTC (permalink / raw)
  To: Thara Gopinath, agross, bjorn.andersson, rui.zhang,
	daniel.lezcano, viresh.kumar, rjw, robh+dt
  Cc: linux-arm-msm, linux-pm, linux-kernel, devicetree

On 6/8/21 3:29 PM, Thara Gopinath wrote:
> Driver enabling various pieces of Limits Management Hardware(LMh) for cpu
> cluster0 and cpu cluster1 namely kick starting monitoring of temperature,
> current, battery current violations, enabling reliability algorithm and
> setting up various temperature limits.
> 
> The following has been explained in the cover letter. I am including this
> here so that this remains in the commit message as well.
> 
> LMh is a hardware infrastructure on some Qualcomm SoCs that can enforce
> temperature and current limits as programmed by software for certain IPs
> like CPU. On many newer SoCs LMh is configured by firmware/TZ and no
> programming is needed from the kernel side. But on certain SoCs like sdm845
> the firmware does not do a complete programming of the h/w. On such SoCs
> kernel software has to explicitly set up the temperature limits and turn on
> various monitoring and enforcing algorithms on the hardware.
> 
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
>  drivers/thermal/qcom/Kconfig  |  10 ++
>  drivers/thermal/qcom/Makefile |   1 +
>  drivers/thermal/qcom/lmh.c    | 244 ++++++++++++++++++++++++++++++++++
>  3 files changed, 255 insertions(+)
>  create mode 100644 drivers/thermal/qcom/lmh.c
> 
> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
> index 8d5ac2df26dc..c95b95e254d7 100644
> --- a/drivers/thermal/qcom/Kconfig
> +++ b/drivers/thermal/qcom/Kconfig
> @@ -31,3 +31,13 @@ config QCOM_SPMI_TEMP_ALARM
>  	  trip points. The temperature reported by the thermal sensor reflects the
>  	  real time die temperature if an ADC is present or an estimate of the
>  	  temperature based upon the over temperature stage value.
> +
> +config QCOM_LMH
> +	tristate "Qualcomm Limits Management Hardware"
> +	depends on ARCH_QCOM
> +	help
> +	  This enables initialization of Qualcomm limits management
> +	  hardware(LMh). LMh allows for h/w enforced mitigation for cpus based on

	                                hardware-enforced           CPUs

> +	  input from temperature and current sensors.  On many newer Qualcomm SoCs
> +	  LMH is configure in the firmware and this feature need not be enabled.

	  LMh

> +	  However, on certain SoCs like sdm845 LMH has to be configured from HLOS.

	                                       LMh

What is HLOS?


> diff --git a/drivers/thermal/qcom/lmh.c b/drivers/thermal/qcom/lmh.c
> new file mode 100644
> index 000000000000..8741a36cb674
> --- /dev/null
> +++ b/drivers/thermal/qcom/lmh.c
> @@ -0,0 +1,244 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (C) 2021, Linaro Limited. All rights reserved.
> + */

[snip]

> +static int lmh_probe(struct platform_device *pdev)
> +{
> +	struct device *dev;
> +	struct device_node *np;
> +	struct lmh_hw_data *lmh_data;
> +	u32 node_id;
> +	int ret;
> +
> +	dev = &pdev->dev;
> +	np = dev->of_node;
> +	if (!np)
> +		return -EINVAL;
> +
> +	lmh_data = devm_kzalloc(dev, sizeof(*lmh_data), GFP_KERNEL);
> +	if (!lmh_data)
> +		return -ENOMEM;
> +
> +	lmh_data->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(lmh_data->base))
> +		return PTR_ERR(lmh_data->base);
> +
> +	ret = of_property_read_u32(np, "qcom,lmh-cpu-id", &lmh_data->cpu_id);
> +	if (ret)
> +		return -ENODEV;
> +
> +	/*
> +	 * Only sdm845 has lmh hardware currently enabled from hlos. If this is needed
> +	 * for other platforms, revisit this to check if the <cpu-id, node-id> should be part
> +	 * of a dt match table.
> +	 */
> +	if (lmh_data->cpu_id == 0) {
> +		node_id = LMH_CLUSTER0_NODE_ID;
> +	} else if (lmh_data->cpu_id == 4) {
> +		node_id = LMH_CLUSTER1_NODE_ID;
> +	} else {
> +		dev_err(dev, "Wrong cpu id associated with lmh node\n");

		                    CPU                    LMh

> +		return -EINVAL;
> +	}
> +
> +	/* Payload size is five bytes for now */
> +	lmh_data->payload_size = 5 * sizeof(u32);
> +
> +	platform_set_drvdata(pdev, lmh_data);
> +
> +	if (!qcom_scm_lmh_dcvsh_available())
> +		return -EINVAL;
> +
> +	/* Enable Thermal Algorithm */
> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable Current Sensing Algorithm */
> +	update_payload(lmh_data, LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling current subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable Reliability Algorithm */
> +	update_payload(lmh_data, LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable BCL Algorithm */
> +	update_payload(lmh_data, LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling BCL subfunction\n", ret);

What is BCL?

> +		return ret;
> +	}
> +
> +	ret = qcom_scm_lmh_profile_change(0x1);
> +	if (ret) {
> +		dev_err(dev, "Error %d changing profile\n", ret);
> +		return ret;
> +	}
> +
> +	/* Set default thermal trips */
> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_ARM_THRESHOLD, LMH_TH_ARM_TEMP);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);

		                                        threshold

> +		return ret;
> +	}
> +
> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_HI_THRESHOLD, LMH_TH_HI_TEMP);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error setting thermal HI thershold%d\n", ret);

		                                       threshold

> +		return ret;
> +	}
> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_LOW_THRESHOLD, LMH_TH_LOW_TEMP);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);

		                                        threshold

> +		return ret;
> +	}
> +
> +	lmh_data->irq = platform_get_irq(pdev, 0);
> +	lmh_data->domain = irq_domain_add_linear(np, 1, &lmh_irq_ops, lmh_data);
> +	if (!lmh_data->domain) {
> +		dev_err(dev, "Error adding irq_domain\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = devm_request_irq(dev, lmh_data->irq, lmh_handle_irq,
> +			       IRQF_TRIGGER_HIGH | IRQF_ONESHOT | IRQF_NO_SUSPEND,
> +			       "lmh-irq", lmh_data);
> +	if (ret) {
> +		dev_err(dev, "Error %d registering irq %x\n", ret, lmh_data->irq);
> +		irq_domain_remove(lmh_data->domain);
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static const struct of_device_id lmh_table[] = {
> +	{ .compatible = "qcom,msm-hw-limits", },
> +	{},
> +};
> +
> +static struct platform_driver lmh_driver = {
> +	.probe = lmh_probe,
> +	.driver = {
> +		.name = "qcom-lmh",
> +		.of_match_table = lmh_table,
> +	},
> +};
> +module_platform_driver(lmh_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("QCOM LMH driver");

                            LMh


thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh
  2021-06-08 22:29 ` [PATCH 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh Thara Gopinath
@ 2021-06-09  3:10   ` kernel test robot
  0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-06-09  3:10 UTC (permalink / raw)
  To: Thara Gopinath, agross, bjorn.andersson, rui.zhang,
	daniel.lezcano, viresh.kumar, rjw, robh+dt
  Cc: kbuild-all, clang-built-linux, linux-arm-msm, linux-pm, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 6517 bytes --]

Hi Thara,

I love your patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on pm/linux-next linus/master v5.13-rc5 next-20210608]
[cannot apply to thermal/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Thara-Gopinath/Introduce-LMh-driver-for-Qualcomm-SoCs/20210609-063135
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: powerpc-randconfig-r024-20210608 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project d2012d965d60c3258b3a69d024491698f8aec386)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc cross compiling tool for clang build
        # apt-get install binutils-powerpc-linux-gnu
        # https://github.com/0day-ci/linux/commit/bd39209abfb69da45acdc24c969d69620106f24e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Thara-Gopinath/Introduce-LMh-driver-for-Qualcomm-SoCs/20210609-063135
        git checkout bd39209abfb69da45acdc24c969d69620106f24e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/mmc/host/sdhci-msm.c:8:
   In file included from include/linux/module.h:12:
   In file included from include/linux/list.h:9:
   In file included from include/linux/kernel.h:12:
   In file included from include/linux/bitops.h:32:
   In file included from arch/powerpc/include/asm/bitops.h:62:
   arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
   #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
           ^
   <built-in>:308:9: note: previous definition is here
   #define __lwsync __builtin_ppc_lwsync
           ^
   In file included from drivers/mmc/host/sdhci-msm.c:16:
>> include/linux/qcom_scm.h:180:5: warning: no previous prototype for function 'qcom_scm_lmh_dcvsh' [-Wmissing-prototypes]
   int qcom_scm_lmh_dcvsh(u32 *payload, u32 payload_size, u64 limit_node,
       ^
   include/linux/qcom_scm.h:180:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int qcom_scm_lmh_dcvsh(u32 *payload, u32 payload_size, u64 limit_node,
   ^
   static 
>> include/linux/qcom_scm.h:183:5: warning: no previous prototype for function 'qcom_scm_lmh_profile_change' [-Wmissing-prototypes]
   int qcom_scm_lmh_profile_change(u32 profile_id) { return -ENODEV; }
       ^
   include/linux/qcom_scm.h:183:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int qcom_scm_lmh_profile_change(u32 profile_id) { return -ENODEV; }
   ^
   static 
>> include/linux/qcom_scm.h:185:6: warning: no previous prototype for function 'qcom_scm_lmh_dcvsh_available' [-Wmissing-prototypes]
   bool qcom_scm_lmh_dcvsh_available(void) { return -ENODEV; }
        ^
   include/linux/qcom_scm.h:185:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool qcom_scm_lmh_dcvsh_available(void) { return -ENODEV; }
   ^
   static 
   4 warnings generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for HOTPLUG_CPU
   Depends on SMP && (PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE
   Selected by
   - PM_SLEEP_SMP && SMP && (ARCH_SUSPEND_POSSIBLE || ARCH_HIBERNATION_POSSIBLE && PM_SLEEP


vim +/qcom_scm_lmh_dcvsh +180 include/linux/qcom_scm.h

   140	
   141	static inline int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val)
   142			{ return -ENODEV; }
   143	static inline int qcom_scm_io_writel(phys_addr_t addr, unsigned int val)
   144			{ return -ENODEV; }
   145	
   146	static inline bool qcom_scm_restore_sec_cfg_available(void) { return false; }
   147	static inline int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare)
   148			{ return -ENODEV; }
   149	static inline int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size)
   150			{ return -ENODEV; }
   151	static inline int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare)
   152			{ return -ENODEV; }
   153	extern inline int qcom_scm_mem_protect_video_var(u32 cp_start, u32 cp_size,
   154							 u32 cp_nonpixel_start,
   155							 u32 cp_nonpixel_size)
   156			{ return -ENODEV; }
   157	static inline int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
   158			unsigned int *src, const struct qcom_scm_vmperm *newvm,
   159			unsigned int dest_cnt) { return -ENODEV; }
   160	
   161	static inline bool qcom_scm_ocmem_lock_available(void) { return false; }
   162	static inline int qcom_scm_ocmem_lock(enum qcom_scm_ocmem_client id, u32 offset,
   163			u32 size, u32 mode) { return -ENODEV; }
   164	static inline int qcom_scm_ocmem_unlock(enum qcom_scm_ocmem_client id,
   165			u32 offset, u32 size) { return -ENODEV; }
   166	
   167	static inline bool qcom_scm_ice_available(void) { return false; }
   168	static inline int qcom_scm_ice_invalidate_key(u32 index) { return -ENODEV; }
   169	static inline int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
   170					       enum qcom_scm_ice_cipher cipher,
   171					       u32 data_unit_size) { return -ENODEV; }
   172	
   173	static inline bool qcom_scm_hdcp_available(void) { return false; }
   174	static inline int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
   175			u32 *resp) { return -ENODEV; }
   176	
   177	static inline int qcom_scm_qsmmu500_wait_safe_toggle(bool en)
   178			{ return -ENODEV; }
   179	
 > 180	int qcom_scm_lmh_dcvsh(u32 *payload, u32 payload_size, u64 limit_node,
   181			       u32 node_id, u64 version)
   182			{ return -ENODEV; }
 > 183	int qcom_scm_lmh_profile_change(u32 profile_id) { return -ENODEV; }
   184	
 > 185	bool qcom_scm_lmh_dcvsh_available(void) { return -ENODEV; }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36078 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support
  2021-06-08 22:29 ` [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support Thara Gopinath
@ 2021-06-14 10:31   ` Viresh Kumar
  2021-06-15  1:58     ` Thara Gopinath
  2021-06-18 18:16   ` Bjorn Andersson
  1 sibling, 1 reply; 18+ messages in thread
From: Viresh Kumar @ 2021-06-14 10:31 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: agross, bjorn.andersson, rui.zhang, daniel.lezcano, rjw, robh+dt,
	linux-arm-msm, linux-pm, linux-kernel, devicetree

On 08-06-21, 18:29, Thara Gopinath wrote:
> Add interrupt support to notify the kernel of h/w initiated frequency
> throttling by LMh. Convey this to scheduler via thermal presssure
> interface.
> 
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
>  drivers/cpufreq/qcom-cpufreq-hw.c | 100 ++++++++++++++++++++++++++++++
>  1 file changed, 100 insertions(+)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index f86859bf76f1..95e17330aa9d 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -13,6 +13,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/pm_opp.h>
>  #include <linux/slab.h>
> +#include <linux/interrupt.h>
>  
>  #define LUT_MAX_ENTRIES			40U
>  #define LUT_SRC				GENMASK(31, 30)
> @@ -22,10 +23,13 @@
>  #define CLK_HW_DIV			2
>  #define LUT_TURBO_IND			1
>  
> +#define HZ_PER_KHZ			1000
> +
>  struct qcom_cpufreq_soc_data {
>  	u32 reg_enable;
>  	u32 reg_freq_lut;
>  	u32 reg_volt_lut;
> +	u32 reg_current_vote;
>  	u32 reg_perf_state;
>  	u8 lut_row_size;
>  };
> @@ -33,7 +37,11 @@ struct qcom_cpufreq_soc_data {
>  struct qcom_cpufreq_data {
>  	void __iomem *base;
>  	struct resource *res;
> +	struct delayed_work lmh_dcvs_poll_work;
>  	const struct qcom_cpufreq_soc_data *soc_data;
> +	cpumask_var_t cpus;
> +	unsigned long throttled_freq;
> +	int lmh_dcvs_irq;
>  };
>  
>  static unsigned long cpu_hw_rate, xo_rate;
> @@ -251,10 +259,79 @@ static void qcom_get_related_cpus(int index, struct cpumask *m)
>  	}
>  }
>  
> +static inline unsigned long qcom_lmh_vote_to_freq(u32 val)
> +{
> +	return (val & 0x3FF) * 19200;
> +}
> +
> +static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
> +{
> +	struct cpufreq_policy policy;
> +	struct dev_pm_opp *opp;
> +	struct device *dev;
> +	unsigned long max_capacity, capacity, freq_hz;
> +	unsigned int val, freq;
> +
> +	val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
> +	freq = qcom_lmh_vote_to_freq(val);
> +	freq_hz = freq * HZ_PER_KHZ;
> +
> +	/* Do I need to calculate ceil and floor ? */

You don't know ?

> +	dev = get_cpu_device(cpumask_first(data->cpus));
> +	opp = dev_pm_opp_find_freq_floor(dev, &freq_hz);
> +	if (IS_ERR(opp) && PTR_ERR(opp) == -ERANGE)
> +		opp = dev_pm_opp_find_freq_ceil(dev, &freq_hz);
> +
> +	data->throttled_freq = freq_hz / HZ_PER_KHZ;
> +

What exactly are we trying to do here ? A comment would be good as
well.

> +	cpufreq_get_policy(&policy, cpumask_first(data->cpus));
> +
> +	/* Update thermal pressure */
> +	max_capacity = arch_scale_cpu_capacity(cpumask_first(data->cpus));

Set capacity of a single CPU from a policy ?

> +	capacity = data->throttled_freq * max_capacity;
> +	capacity /= policy.cpuinfo.max_freq;
> +	/* Don't pass boost capacity to scheduler */
> +	if (capacity > max_capacity)
> +		capacity = max_capacity;
> +	arch_set_thermal_pressure(data->cpus, max_capacity - capacity);

You should really be using policy->cpus instead of allocating
data->cpus..

> +}
> +
> +static void qcom_lmh_dcvs_poll(struct work_struct *work)
> +{
> +	struct qcom_cpufreq_data *data;
> +
> +	data = container_of(work, struct qcom_cpufreq_data, lmh_dcvs_poll_work.work);
> +
> +	qcom_lmh_dcvs_notify(data);

You should really move the below stuff the disable_irq_nosync(), it
will make your life easier.

> +	/**
> +	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
> +	 * polling and switch back to interrupt mechanism
> +	 */
> +	if (data->throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(data->cpus)))
> +		/* Clear the existing interrupts and enable it back */
> +		enable_irq(data->lmh_dcvs_irq);
> +	else
> +		mod_delayed_work(system_highpri_wq, &data->lmh_dcvs_poll_work,
> +				 msecs_to_jiffies(10));
> +}
> +
> +static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
> +{
> +	struct qcom_cpufreq_data *c_data = data;
> +
> +	/* Disable interrupt and enable polling */
> +	disable_irq_nosync(c_data->lmh_dcvs_irq);
> +	qcom_lmh_dcvs_notify(c_data);
> +	mod_delayed_work(system_highpri_wq, &c_data->lmh_dcvs_poll_work, msecs_to_jiffies(10));
> +
> +	return 0;
> +}
> +
>  static const struct qcom_cpufreq_soc_data qcom_soc_data = {
>  	.reg_enable = 0x0,
>  	.reg_freq_lut = 0x110,
>  	.reg_volt_lut = 0x114,
> +	.reg_current_vote = 0x704,

Should this be a different patch ?

>  	.reg_perf_state = 0x920,
>  	.lut_row_size = 32,
>  };
> @@ -285,6 +362,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  	void __iomem *base;
>  	struct qcom_cpufreq_data *data;
>  	int ret, index;
> +	bool lmh_mitigation_enabled = false;

You just overwrite it below, no need to initialize it.

>  
>  	cpu_dev = get_cpu_device(policy->cpu);
>  	if (!cpu_dev) {
> @@ -305,6 +383,8 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  
>  	index = args.args[0];
>  
> +	lmh_mitigation_enabled = of_property_read_bool(pdev->dev.of_node, "qcom,support-lmh");
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
>  	if (!res) {
>  		dev_err(dev, "failed to get mem resource %d\n", index);
> @@ -329,6 +409,11 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  		goto unmap_base;
>  	}
>  
> +	if (!alloc_cpumask_var(&data->cpus, GFP_KERNEL)) {
> +		ret = -ENOMEM;
> +		goto unmap_base;
> +	}
> +
>  	data->soc_data = of_device_get_match_data(&pdev->dev);
>  	data->base = base;
>  	data->res = res;
> @@ -347,6 +432,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  		goto error;
>  	}
>  
> +	cpumask_copy(data->cpus, policy->cpus);
>  	policy->driver_data = data;
>  
>  	ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy);
> @@ -370,6 +456,20 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
>  	}
>  
> +	if (lmh_mitigation_enabled) {

Shouldn't you move the allocation and setting of data->cpus here ? I
suggest creating a separate routine for all initialization around this
stuff.

> +		data->lmh_dcvs_irq = platform_get_irq(pdev, index);
> +		if (data->lmh_dcvs_irq < 0) {
> +			ret = data->lmh_dcvs_irq;
> +			goto error;
> +		}
> +		ret = devm_request_irq(dev, data->lmh_dcvs_irq, qcom_lmh_dcvs_handle_irq,
> +				       0, "dcvsh-irq", data);

I would rather pass policy as data here.

> +		if (ret) {
> +			dev_err(dev, "Error %d registering irq %x\n", ret, data->lmh_dcvs_irq);
> +			goto error;
> +		}
> +		INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
> +	}
>  	return 0;
>  error:
>  	kfree(data);

-- 
viresh

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] thermal: qcom: Add support for LMh driver
  2021-06-08 22:29 ` [PATCH 2/5] thermal: qcom: Add support for LMh driver Thara Gopinath
  2021-06-09  2:25   ` Randy Dunlap
@ 2021-06-14 20:53   ` Bjorn Andersson
  2021-06-15  1:38     ` Thara Gopinath
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Andersson @ 2021-06-14 20:53 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: agross, rui.zhang, daniel.lezcano, viresh.kumar, rjw, robh+dt,
	linux-arm-msm, linux-pm, linux-kernel, devicetree

On Tue 08 Jun 17:29 CDT 2021, Thara Gopinath wrote:

> Driver enabling various pieces of Limits Management Hardware(LMh) for cpu
> cluster0 and cpu cluster1 namely kick starting monitoring of temperature,
> current, battery current violations, enabling reliability algorithm and
> setting up various temperature limits.
> 
> The following has been explained in the cover letter. I am including this
> here so that this remains in the commit message as well.
> 
> LMh is a hardware infrastructure on some Qualcomm SoCs that can enforce
> temperature and current limits as programmed by software for certain IPs
> like CPU. On many newer SoCs LMh is configured by firmware/TZ and no
> programming is needed from the kernel side. But on certain SoCs like sdm845
> the firmware does not do a complete programming of the h/w. On such SoCs
> kernel software has to explicitly set up the temperature limits and turn on
> various monitoring and enforcing algorithms on the hardware.
> 
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
>  drivers/thermal/qcom/Kconfig  |  10 ++
>  drivers/thermal/qcom/Makefile |   1 +
>  drivers/thermal/qcom/lmh.c    | 244 ++++++++++++++++++++++++++++++++++
>  3 files changed, 255 insertions(+)
>  create mode 100644 drivers/thermal/qcom/lmh.c
> 
> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
> index 8d5ac2df26dc..c95b95e254d7 100644
> --- a/drivers/thermal/qcom/Kconfig
> +++ b/drivers/thermal/qcom/Kconfig
> @@ -31,3 +31,13 @@ config QCOM_SPMI_TEMP_ALARM
>  	  trip points. The temperature reported by the thermal sensor reflects the
>  	  real time die temperature if an ADC is present or an estimate of the
>  	  temperature based upon the over temperature stage value.
> +
> +config QCOM_LMH
> +	tristate "Qualcomm Limits Management Hardware"
> +	depends on ARCH_QCOM
> +	help
> +	  This enables initialization of Qualcomm limits management
> +	  hardware(LMh). LMh allows for h/w enforced mitigation for cpus based on
> +	  input from temperature and current sensors.  On many newer Qualcomm SoCs
> +	  LMH is configure in the firmware and this feature need not be enabled.
> +	  However, on certain SoCs like sdm845 LMH has to be configured from HLOS.
> diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
> index 252ea7d9da0b..0fa2512042e7 100644
> --- a/drivers/thermal/qcom/Makefile
> +++ b/drivers/thermal/qcom/Makefile
> @@ -5,3 +5,4 @@ qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
>  				   tsens-8960.o
>  obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
>  obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
> +obj-$(CONFIG_QCOM_LMH)		+= lmh.o
> diff --git a/drivers/thermal/qcom/lmh.c b/drivers/thermal/qcom/lmh.c
> new file mode 100644
> index 000000000000..8741a36cb674
> --- /dev/null
> +++ b/drivers/thermal/qcom/lmh.c
> @@ -0,0 +1,244 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (C) 2021, Linaro Limited. All rights reserved.
> + */
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqdomain.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/slab.h>
> +#include <linux/qcom_scm.h>
> +
> +#define LMH_NODE_DCVS			0x44435653
> +#define LMH_CLUSTER0_NODE_ID		0x6370302D
> +#define LMH_CLUSTER1_NODE_ID		0x6370312D
> +
> +#define LMH_SUB_FN_THERMAL		0x54484D4C
> +#define LMH_SUB_FN_CRNT			0x43524E54
> +#define LMH_SUB_FN_REL			0x52454C00
> +#define LMH_SUB_FN_BCL			0x42434C00
> +
> +#define LMH_ALGO_MODE_ENABLE		0x454E424C
> +#define LMH_TH_HI_THRESHOLD		0x48494748
> +#define LMH_TH_LOW_THRESHOLD		0x4C4F5700
> +#define LMH_TH_ARM_THRESHOLD		0x41524D00
> +
> +#define LMH_TH_HI_TEMP			95000
> +#define LMH_TH_LOW_TEMP			94500
> +#define LMH_TH_ARM_TEMP			65000
> +
> +#define LMH_REG_DCVS_INTR_CLR		0x8
> +
> +struct lmh_hw_data {
> +	void __iomem *base;
> +	struct irq_domain *domain;
> +	int irq;
> +	u32 payload[5];
> +	u32 payload_size;
> +	u32 cpu_id;
> +};
> +
> +static void update_payload(struct lmh_hw_data *lmh_data, u32 fn, u32 reg, u32 val)

Please pass fn, reg and val in the scm function call instead and stuff
the payload array in the scm driver instead.

> +{
> +	lmh_data->payload[0] = fn;
> +	lmh_data->payload[1] = 0;
> +	lmh_data->payload[2] = reg;
> +	lmh_data->payload[3] = 1;
> +	lmh_data->payload[4] = val;
> +}
> +
> +static irqreturn_t lmh_handle_irq(int hw_irq, void *data)
> +{
> +	struct lmh_hw_data *lmh_data = data;
> +	int irq = irq_find_mapping(lmh_data->domain, 0);
> +
> +	/*
> +	 * Disable interrupt and call the cpufreq driver to handle the interrupt
> +	 * cpufreq will enable the interrupt once finished processing.
> +	 */
> +	disable_irq_nosync(lmh_data->irq);

The contract between this driver's disabling of the IRQ and the
cpufreq-hw driver's enabling it when we're done polling does worry me.

In the case of EPSS, don't we disable the interrupt during the polling
there as well? If that's the case wouldn't it be better to implement
irq_chip->irq_disable and have the cpufreq-hw driver do the disable in
both cases?

> +	if (irq)
> +		generic_handle_irq(irq);
> +
> +	return 0;
> +}
> +
> +static void lmh_enable_interrupt(struct irq_data *d)
> +{
> +	struct lmh_hw_data *lmh_data = irq_data_get_irq_chip_data(d);
> +
> +	/* Clear the existing interrupt */
> +	writel_relaxed(0xFF, lmh_data->base + LMH_REG_DCVS_INTR_CLR);

Please avoid using _relaxed versions of writel, unless there's a strong
reason and please lowercase the hex digits.

> +	enable_irq(lmh_data->irq);
> +}
> +
> +static struct irq_chip lmh_irq_chip = {
> +	.name           = "lmh",
> +	.irq_enable	= lmh_enable_interrupt,
> +};
> +
> +static int lmh_irq_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
> +{
> +	struct lmh_hw_data *lmh_data = d->host_data;
> +
> +	irq_set_chip_and_handler(irq, &lmh_irq_chip, handle_simple_irq);
> +	irq_set_chip_data(irq, lmh_data);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops lmh_irq_ops = {
> +	.map = lmh_irq_map,
> +	.xlate = irq_domain_xlate_onecell,
> +};
> +
> +static int lmh_probe(struct platform_device *pdev)
> +{
> +	struct device *dev;
> +	struct device_node *np;
> +	struct lmh_hw_data *lmh_data;
> +	u32 node_id;
> +	int ret;
> +
> +	dev = &pdev->dev;
> +	np = dev->of_node;
> +	if (!np)
> +		return -EINVAL;
> +
> +	lmh_data = devm_kzalloc(dev, sizeof(*lmh_data), GFP_KERNEL);
> +	if (!lmh_data)
> +		return -ENOMEM;
> +
> +	lmh_data->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(lmh_data->base))
> +		return PTR_ERR(lmh_data->base);
> +
> +	ret = of_property_read_u32(np, "qcom,lmh-cpu-id", &lmh_data->cpu_id);
> +	if (ret)
> +		return -ENODEV;
> +
> +	/*
> +	 * Only sdm845 has lmh hardware currently enabled from hlos. If this is needed
> +	 * for other platforms, revisit this to check if the <cpu-id, node-id> should be part
> +	 * of a dt match table.
> +	 */
> +	if (lmh_data->cpu_id == 0) {
> +		node_id = LMH_CLUSTER0_NODE_ID;
> +	} else if (lmh_data->cpu_id == 4) {
> +		node_id = LMH_CLUSTER1_NODE_ID;
> +	} else {
> +		dev_err(dev, "Wrong cpu id associated with lmh node\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Payload size is five bytes for now */
> +	lmh_data->payload_size = 5 * sizeof(u32);
> +
> +	platform_set_drvdata(pdev, lmh_data);
> +
> +	if (!qcom_scm_lmh_dcvsh_available())
> +		return -EINVAL;
> +
> +	/* Enable Thermal Algorithm */
> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable Current Sensing Algorithm */
> +	update_payload(lmh_data, LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling current subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable Reliability Algorithm */
> +	update_payload(lmh_data, LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable BCL Algorithm */
> +	update_payload(lmh_data, LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling BCL subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	ret = qcom_scm_lmh_profile_change(0x1);
> +	if (ret) {
> +		dev_err(dev, "Error %d changing profile\n", ret);
> +		return ret;
> +	}
> +
> +	/* Set default thermal trips */
> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_ARM_THRESHOLD, LMH_TH_ARM_TEMP);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);
> +		return ret;
> +	}
> +
> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_HI_THRESHOLD, LMH_TH_HI_TEMP);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error setting thermal HI thershold%d\n", ret);
> +		return ret;
> +	}
> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_LOW_THRESHOLD, LMH_TH_LOW_TEMP);
> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);
> +		return ret;
> +	}
> +
> +	lmh_data->irq = platform_get_irq(pdev, 0);
> +	lmh_data->domain = irq_domain_add_linear(np, 1, &lmh_irq_ops, lmh_data);
> +	if (!lmh_data->domain) {
> +		dev_err(dev, "Error adding irq_domain\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = devm_request_irq(dev, lmh_data->irq, lmh_handle_irq,
> +			       IRQF_TRIGGER_HIGH | IRQF_ONESHOT | IRQF_NO_SUSPEND,
> +			       "lmh-irq", lmh_data);
> +	if (ret) {
> +		dev_err(dev, "Error %d registering irq %x\n", ret, lmh_data->irq);
> +		irq_domain_remove(lmh_data->domain);
> +		return ret;
> +	}
> +	return 0;
> +}
> +
> +static const struct of_device_id lmh_table[] = {
> +	{ .compatible = "qcom,msm-hw-limits", },

Don't we need platform specific compatibles? Perhaps
"qcom,<platform>-lmh"?

If we're going with a generic compatible I think that should be done in
addition to a platform-specific one (and qcom,lmh should be sufficient?)

> +	{},

Please omit the comma here.

> +};

Driver is tristate, so you need a MODULE_DEVICE_TABLE(of, lmh_table);
here, to make sure the module is loaded automatically based on the
compatible strings in DT.

Regards,
Bjorn

> +
> +static struct platform_driver lmh_driver = {
> +	.probe = lmh_probe,
> +	.driver = {
> +		.name = "qcom-lmh",
> +		.of_match_table = lmh_table,
> +	},
> +};
> +module_platform_driver(lmh_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("QCOM LMH driver");
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] thermal: qcom: Add support for LMh driver
  2021-06-09  2:25   ` Randy Dunlap
@ 2021-06-15  1:37     ` Thara Gopinath
  0 siblings, 0 replies; 18+ messages in thread
From: Thara Gopinath @ 2021-06-15  1:37 UTC (permalink / raw)
  To: Randy Dunlap, agross, bjorn.andersson, rui.zhang, daniel.lezcano,
	viresh.kumar, rjw, robh+dt
  Cc: linux-arm-msm, linux-pm, linux-kernel, devicetree

Hi Randy,

Thanks for the review. I somehow did not see your review earlier. I 
noticed it today morning when Bjorn replied to this patch. Apologies for 
the delay

On 6/8/21 10:25 PM, Randy Dunlap wrote:
> On 6/8/21 3:29 PM, Thara Gopinath wrote:
>> Driver enabling various pieces of Limits Management Hardware(LMh) for cpu
>> cluster0 and cpu cluster1 namely kick starting monitoring of temperature,
>> current, battery current violations, enabling reliability algorithm and
>> setting up various temperature limits.
>>
>> The following has been explained in the cover letter. I am including this
>> here so that this remains in the commit message as well.
>>
>> LMh is a hardware infrastructure on some Qualcomm SoCs that can enforce
>> temperature and current limits as programmed by software for certain IPs
>> like CPU. On many newer SoCs LMh is configured by firmware/TZ and no
>> programming is needed from the kernel side. But on certain SoCs like sdm845
>> the firmware does not do a complete programming of the h/w. On such SoCs
>> kernel software has to explicitly set up the temperature limits and turn on
>> various monitoring and enforcing algorithms on the hardware.
>>
>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
>> ---
>>   drivers/thermal/qcom/Kconfig  |  10 ++
>>   drivers/thermal/qcom/Makefile |   1 +
>>   drivers/thermal/qcom/lmh.c    | 244 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 255 insertions(+)
>>   create mode 100644 drivers/thermal/qcom/lmh.c
>>
>> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
>> index 8d5ac2df26dc..c95b95e254d7 100644
>> --- a/drivers/thermal/qcom/Kconfig
>> +++ b/drivers/thermal/qcom/Kconfig
>> @@ -31,3 +31,13 @@ config QCOM_SPMI_TEMP_ALARM
>>   	  trip points. The temperature reported by the thermal sensor reflects the
>>   	  real time die temperature if an ADC is present or an estimate of the
>>   	  temperature based upon the over temperature stage value.
>> +
>> +config QCOM_LMH
>> +	tristate "Qualcomm Limits Management Hardware"
>> +	depends on ARCH_QCOM
>> +	help
>> +	  This enables initialization of Qualcomm limits management
>> +	  hardware(LMh). LMh allows for h/w enforced mitigation for cpus based on
> 
> 	                                hardware-enforced           CPUs
> 
>> +	  input from temperature and current sensors.  On many newer Qualcomm SoCs
>> +	  LMH is configure in the firmware and this feature need not be enabled.
> 
> 	  LMh
> 
>> +	  However, on certain SoCs like sdm845 LMH has to be configured from HLOS.
> 
> 	                                       LMh
> 
> What is HLOS?

High Level Operating System. But I will change it to Linux kernel.
> 
> 
>> diff --git a/drivers/thermal/qcom/lmh.c b/drivers/thermal/qcom/lmh.c
>> new file mode 100644
>> index 000000000000..8741a36cb674
>> --- /dev/null
>> +++ b/drivers/thermal/qcom/lmh.c
>> @@ -0,0 +1,244 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +/*
>> + * Copyright (C) 2021, Linaro Limited. All rights reserved.
>> + */
> 
> [snip]
> 
>> +static int lmh_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev;
>> +	struct device_node *np;
>> +	struct lmh_hw_data *lmh_data;
>> +	u32 node_id;
>> +	int ret;
>> +
>> +	dev = &pdev->dev;
>> +	np = dev->of_node;
>> +	if (!np)
>> +		return -EINVAL;
>> +
>> +	lmh_data = devm_kzalloc(dev, sizeof(*lmh_data), GFP_KERNEL);
>> +	if (!lmh_data)
>> +		return -ENOMEM;
>> +
>> +	lmh_data->base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(lmh_data->base))
>> +		return PTR_ERR(lmh_data->base);
>> +
>> +	ret = of_property_read_u32(np, "qcom,lmh-cpu-id", &lmh_data->cpu_id);
>> +	if (ret)
>> +		return -ENODEV;
>> +
>> +	/*
>> +	 * Only sdm845 has lmh hardware currently enabled from hlos. If this is needed
>> +	 * for other platforms, revisit this to check if the <cpu-id, node-id> should be part
>> +	 * of a dt match table.
>> +	 */
>> +	if (lmh_data->cpu_id == 0) {
>> +		node_id = LMH_CLUSTER0_NODE_ID;
>> +	} else if (lmh_data->cpu_id == 4) {
>> +		node_id = LMH_CLUSTER1_NODE_ID;
>> +	} else {
>> +		dev_err(dev, "Wrong cpu id associated with lmh node\n");
> 
> 		                    CPU                    LMh

will fix it. Also will fix all the typos you have caught below.

> 
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Payload size is five bytes for now */
>> +	lmh_data->payload_size = 5 * sizeof(u32);
>> +
>> +	platform_set_drvdata(pdev, lmh_data);
>> +
>> +	if (!qcom_scm_lmh_dcvsh_available())
>> +		return -EINVAL;
>> +
>> +	/* Enable Thermal Algorithm */
>> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Enable Current Sensing Algorithm */
>> +	update_payload(lmh_data, LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling current subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Enable Reliability Algorithm */
>> +	update_payload(lmh_data, LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Enable BCL Algorithm */
>> +	update_payload(lmh_data, LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling BCL subfunction\n", ret);
> 
> What is BCL?

Battery Current Limits

-- 
Warm Regards
Thara
> 
>> +		return ret;
>> +	}
>> +
>> +	ret = qcom_scm_lmh_profile_change(0x1);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d changing profile\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Set default thermal trips */
>> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_ARM_THRESHOLD, LMH_TH_ARM_TEMP);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);
> 
> 		                                        threshold
> 
>> +		return ret;
>> +	}
>> +
>> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_HI_THRESHOLD, LMH_TH_HI_TEMP);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error setting thermal HI thershold%d\n", ret);
> 
> 		                                       threshold
> 
>> +		return ret;
>> +	}
>> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_LOW_THRESHOLD, LMH_TH_LOW_TEMP);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);
> 
> 		                                        threshold
> 
>> +		return ret;
>> +	}
>> +
>> +	lmh_data->irq = platform_get_irq(pdev, 0);
>> +	lmh_data->domain = irq_domain_add_linear(np, 1, &lmh_irq_ops, lmh_data);
>> +	if (!lmh_data->domain) {
>> +		dev_err(dev, "Error adding irq_domain\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = devm_request_irq(dev, lmh_data->irq, lmh_handle_irq,
>> +			       IRQF_TRIGGER_HIGH | IRQF_ONESHOT | IRQF_NO_SUSPEND,
>> +			       "lmh-irq", lmh_data);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d registering irq %x\n", ret, lmh_data->irq);
>> +		irq_domain_remove(lmh_data->domain);
>> +		return ret;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id lmh_table[] = {
>> +	{ .compatible = "qcom,msm-hw-limits", },
>> +	{},
>> +};
>> +
>> +static struct platform_driver lmh_driver = {
>> +	.probe = lmh_probe,
>> +	.driver = {
>> +		.name = "qcom-lmh",
>> +		.of_match_table = lmh_table,
>> +	},
>> +};
>> +module_platform_driver(lmh_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("QCOM LMH driver");
> 
>                              LMh
> 
> 
> thanks.
> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] thermal: qcom: Add support for LMh driver
  2021-06-14 20:53   ` Bjorn Andersson
@ 2021-06-15  1:38     ` Thara Gopinath
  2021-06-18 17:54       ` Bjorn Andersson
  0 siblings, 1 reply; 18+ messages in thread
From: Thara Gopinath @ 2021-06-15  1:38 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: agross, rui.zhang, daniel.lezcano, viresh.kumar, rjw, robh+dt,
	linux-arm-msm, linux-pm, linux-kernel, devicetree

Hi Bjorn,

Thanks for the review


On 6/14/21 4:53 PM, Bjorn Andersson wrote:
> On Tue 08 Jun 17:29 CDT 2021, Thara Gopinath wrote:
> 
>> Driver enabling various pieces of Limits Management Hardware(LMh) for cpu
>> cluster0 and cpu cluster1 namely kick starting monitoring of temperature,
>> current, battery current violations, enabling reliability algorithm and
>> setting up various temperature limits.
>>
>> The following has been explained in the cover letter. I am including this
>> here so that this remains in the commit message as well.
>>
>> LMh is a hardware infrastructure on some Qualcomm SoCs that can enforce
>> temperature and current limits as programmed by software for certain IPs
>> like CPU. On many newer SoCs LMh is configured by firmware/TZ and no
>> programming is needed from the kernel side. But on certain SoCs like sdm845
>> the firmware does not do a complete programming of the h/w. On such SoCs
>> kernel software has to explicitly set up the temperature limits and turn on
>> various monitoring and enforcing algorithms on the hardware.
>>
>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
>> ---
>>   drivers/thermal/qcom/Kconfig  |  10 ++
>>   drivers/thermal/qcom/Makefile |   1 +
>>   drivers/thermal/qcom/lmh.c    | 244 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 255 insertions(+)
>>   create mode 100644 drivers/thermal/qcom/lmh.c
>>
>> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
>> index 8d5ac2df26dc..c95b95e254d7 100644
>> --- a/drivers/thermal/qcom/Kconfig
>> +++ b/drivers/thermal/qcom/Kconfig
>> @@ -31,3 +31,13 @@ config QCOM_SPMI_TEMP_ALARM
>>   	  trip points. The temperature reported by the thermal sensor reflects the
>>   	  real time die temperature if an ADC is present or an estimate of the
>>   	  temperature based upon the over temperature stage value.
>> +
>> +config QCOM_LMH
>> +	tristate "Qualcomm Limits Management Hardware"
>> +	depends on ARCH_QCOM
>> +	help
>> +	  This enables initialization of Qualcomm limits management
>> +	  hardware(LMh). LMh allows for h/w enforced mitigation for cpus based on
>> +	  input from temperature and current sensors.  On many newer Qualcomm SoCs
>> +	  LMH is configure in the firmware and this feature need not be enabled.
>> +	  However, on certain SoCs like sdm845 LMH has to be configured from HLOS.
>> diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
>> index 252ea7d9da0b..0fa2512042e7 100644
>> --- a/drivers/thermal/qcom/Makefile
>> +++ b/drivers/thermal/qcom/Makefile
>> @@ -5,3 +5,4 @@ qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
>>   				   tsens-8960.o
>>   obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
>>   obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
>> +obj-$(CONFIG_QCOM_LMH)		+= lmh.o
>> diff --git a/drivers/thermal/qcom/lmh.c b/drivers/thermal/qcom/lmh.c
>> new file mode 100644
>> index 000000000000..8741a36cb674
>> --- /dev/null
>> +++ b/drivers/thermal/qcom/lmh.c
>> @@ -0,0 +1,244 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +/*
>> + * Copyright (C) 2021, Linaro Limited. All rights reserved.
>> + */
>> +#include <linux/module.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/err.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/slab.h>
>> +#include <linux/qcom_scm.h>
>> +
>> +#define LMH_NODE_DCVS			0x44435653
>> +#define LMH_CLUSTER0_NODE_ID		0x6370302D
>> +#define LMH_CLUSTER1_NODE_ID		0x6370312D
>> +
>> +#define LMH_SUB_FN_THERMAL		0x54484D4C
>> +#define LMH_SUB_FN_CRNT			0x43524E54
>> +#define LMH_SUB_FN_REL			0x52454C00
>> +#define LMH_SUB_FN_BCL			0x42434C00
>> +
>> +#define LMH_ALGO_MODE_ENABLE		0x454E424C
>> +#define LMH_TH_HI_THRESHOLD		0x48494748
>> +#define LMH_TH_LOW_THRESHOLD		0x4C4F5700
>> +#define LMH_TH_ARM_THRESHOLD		0x41524D00
>> +
>> +#define LMH_TH_HI_TEMP			95000
>> +#define LMH_TH_LOW_TEMP			94500
>> +#define LMH_TH_ARM_TEMP			65000
>> +
>> +#define LMH_REG_DCVS_INTR_CLR		0x8
>> +
>> +struct lmh_hw_data {
>> +	void __iomem *base;
>> +	struct irq_domain *domain;
>> +	int irq;
>> +	u32 payload[5];
>> +	u32 payload_size;
>> +	u32 cpu_id;
>> +};
>> +
>> +static void update_payload(struct lmh_hw_data *lmh_data, u32 fn, u32 reg, u32 val)
> 
> Please pass fn, reg and val in the scm function call instead and stuff
> the payload array in the scm driver instead.

Sure . I will redo this part.

> 
>> +{
>> +	lmh_data->payload[0] = fn;
>> +	lmh_data->payload[1] = 0;
>> +	lmh_data->payload[2] = reg;
>> +	lmh_data->payload[3] = 1;
>> +	lmh_data->payload[4] = val;
>> +}
>> +
>> +static irqreturn_t lmh_handle_irq(int hw_irq, void *data)
>> +{
>> +	struct lmh_hw_data *lmh_data = data;
>> +	int irq = irq_find_mapping(lmh_data->domain, 0);
>> +
>> +	/*
>> +	 * Disable interrupt and call the cpufreq driver to handle the interrupt
>> +	 * cpufreq will enable the interrupt once finished processing.
>> +	 */
>> +	disable_irq_nosync(lmh_data->irq);
> 
> The contract between this driver's disabling of the IRQ and the
> cpufreq-hw driver's enabling it when we're done polling does worry me.
> 
> In the case of EPSS, don't we disable the interrupt during the polling
> there as well? If that's the case wouldn't it be better to implement
> irq_chip->irq_disable and have the cpufreq-hw driver do the disable in
> both cases?

Yes. You are right. In case of EPSS, the cpufreq-hw will have to disable 
the interrupt. I did think of the approach you suggested here. My only 
issue is that we will dispatch the interrupt to cpufreq-hw without it 
disabling it and hence the interrupt could fire again, right ?


> 
>> +	if (irq)
>> +		generic_handle_irq(irq);
>> +
>> +	return 0;
>> +}
>> +
>> +static void lmh_enable_interrupt(struct irq_data *d)
>> +{
>> +	struct lmh_hw_data *lmh_data = irq_data_get_irq_chip_data(d);
>> +
>> +	/* Clear the existing interrupt */
>> +	writel_relaxed(0xFF, lmh_data->base + LMH_REG_DCVS_INTR_CLR);
> 
> Please avoid using _relaxed versions of writel, unless there's a strong
> reason and please lowercase the hex digits.

Sure.

> 
>> +	enable_irq(lmh_data->irq);
>> +}
>> +
>> +static struct irq_chip lmh_irq_chip = {
>> +	.name           = "lmh",
>> +	.irq_enable	= lmh_enable_interrupt,
>> +};
>> +
>> +static int lmh_irq_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
>> +{
>> +	struct lmh_hw_data *lmh_data = d->host_data;
>> +
>> +	irq_set_chip_and_handler(irq, &lmh_irq_chip, handle_simple_irq);
>> +	irq_set_chip_data(irq, lmh_data);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct irq_domain_ops lmh_irq_ops = {
>> +	.map = lmh_irq_map,
>> +	.xlate = irq_domain_xlate_onecell,
>> +};
>> +
>> +static int lmh_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev;
>> +	struct device_node *np;
>> +	struct lmh_hw_data *lmh_data;
>> +	u32 node_id;
>> +	int ret;
>> +
>> +	dev = &pdev->dev;
>> +	np = dev->of_node;
>> +	if (!np)
>> +		return -EINVAL;
>> +
>> +	lmh_data = devm_kzalloc(dev, sizeof(*lmh_data), GFP_KERNEL);
>> +	if (!lmh_data)
>> +		return -ENOMEM;
>> +
>> +	lmh_data->base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(lmh_data->base))
>> +		return PTR_ERR(lmh_data->base);
>> +
>> +	ret = of_property_read_u32(np, "qcom,lmh-cpu-id", &lmh_data->cpu_id);
>> +	if (ret)
>> +		return -ENODEV;
>> +
>> +	/*
>> +	 * Only sdm845 has lmh hardware currently enabled from hlos. If this is needed
>> +	 * for other platforms, revisit this to check if the <cpu-id, node-id> should be part
>> +	 * of a dt match table.
>> +	 */
>> +	if (lmh_data->cpu_id == 0) {
>> +		node_id = LMH_CLUSTER0_NODE_ID;
>> +	} else if (lmh_data->cpu_id == 4) {
>> +		node_id = LMH_CLUSTER1_NODE_ID;
>> +	} else {
>> +		dev_err(dev, "Wrong cpu id associated with lmh node\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Payload size is five bytes for now */
>> +	lmh_data->payload_size = 5 * sizeof(u32);
>> +
>> +	platform_set_drvdata(pdev, lmh_data);
>> +
>> +	if (!qcom_scm_lmh_dcvsh_available())
>> +		return -EINVAL;
>> +
>> +	/* Enable Thermal Algorithm */
>> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Enable Current Sensing Algorithm */
>> +	update_payload(lmh_data, LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling current subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Enable Reliability Algorithm */
>> +	update_payload(lmh_data, LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Enable BCL Algorithm */
>> +	update_payload(lmh_data, LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling BCL subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = qcom_scm_lmh_profile_change(0x1);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d changing profile\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Set default thermal trips */
>> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_ARM_THRESHOLD, LMH_TH_ARM_TEMP);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_HI_THRESHOLD, LMH_TH_HI_TEMP);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error setting thermal HI thershold%d\n", ret);
>> +		return ret;
>> +	}
>> +	update_payload(lmh_data, LMH_SUB_FN_THERMAL, LMH_TH_LOW_THRESHOLD, LMH_TH_LOW_TEMP);
>> +	ret = qcom_scm_lmh_dcvsh(lmh_data->payload, lmh_data->payload_size,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error setting thermal ARM thershold%d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	lmh_data->irq = platform_get_irq(pdev, 0);
>> +	lmh_data->domain = irq_domain_add_linear(np, 1, &lmh_irq_ops, lmh_data);
>> +	if (!lmh_data->domain) {
>> +		dev_err(dev, "Error adding irq_domain\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = devm_request_irq(dev, lmh_data->irq, lmh_handle_irq,
>> +			       IRQF_TRIGGER_HIGH | IRQF_ONESHOT | IRQF_NO_SUSPEND,
>> +			       "lmh-irq", lmh_data);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d registering irq %x\n", ret, lmh_data->irq);
>> +		irq_domain_remove(lmh_data->domain);
>> +		return ret;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id lmh_table[] = {
>> +	{ .compatible = "qcom,msm-hw-limits", },
> 
> Don't we need platform specific compatibles? Perhaps
> "qcom,<platform>-lmh"?

Yes considering that is the norm, I will follow it.

> 
> If we're going with a generic compatible I think that should be done in
> addition to a platform-specific one (and qcom,lmh should be sufficien >
>> +	{},
> 
> Please omit the comma here.

Ok.

> 
>> +};
> 
> Driver is tristate, so you need a MODULE_DEVICE_TABLE(of, lmh_table);
> here, to make sure the module is loaded automatically based on the
> compatible strings in DT.

Right. I will fix it .
> 
> Regards,
> Bjorn
> 
>> +
>> +static struct platform_driver lmh_driver = {
>> +	.probe = lmh_probe,
>> +	.driver = {
>> +		.name = "qcom-lmh",
>> +		.of_match_table = lmh_table,
>> +	},
>> +};
>> +module_platform_driver(lmh_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("QCOM LMH driver");
>> -- 
>> 2.25.1
>>

-- 
Warm Regards
Thara (She/Her/Hers)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support
  2021-06-14 10:31   ` Viresh Kumar
@ 2021-06-15  1:58     ` Thara Gopinath
  2021-06-15  5:16       ` Viresh Kumar
  0 siblings, 1 reply; 18+ messages in thread
From: Thara Gopinath @ 2021-06-15  1:58 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: agross, bjorn.andersson, rui.zhang, daniel.lezcano, rjw, robh+dt,
	linux-arm-msm, linux-pm, linux-kernel, devicetree



On 6/14/21 6:31 AM, Viresh Kumar wrote:
> On 08-06-21, 18:29, Thara Gopinath wrote:
>> Add interrupt support to notify the kernel of h/w initiated frequency
>> throttling by LMh. Convey this to scheduler via thermal presssure
>> interface.
>>
>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
>> ---
>>   drivers/cpufreq/qcom-cpufreq-hw.c | 100 ++++++++++++++++++++++++++++++
>>   1 file changed, 100 insertions(+)
>>
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
>> index f86859bf76f1..95e17330aa9d 100644
>> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
>> @@ -13,6 +13,7 @@
>>   #include <linux/of_platform.h>
>>   #include <linux/pm_opp.h>
>>   #include <linux/slab.h>
>> +#include <linux/interrupt.h>
>>   
>>   #define LUT_MAX_ENTRIES			40U
>>   #define LUT_SRC				GENMASK(31, 30)
>> @@ -22,10 +23,13 @@
>>   #define CLK_HW_DIV			2
>>   #define LUT_TURBO_IND			1
>>   
>> +#define HZ_PER_KHZ			1000
>> +
>>   struct qcom_cpufreq_soc_data {
>>   	u32 reg_enable;
>>   	u32 reg_freq_lut;
>>   	u32 reg_volt_lut;
>> +	u32 reg_current_vote;
>>   	u32 reg_perf_state;
>>   	u8 lut_row_size;
>>   };
>> @@ -33,7 +37,11 @@ struct qcom_cpufreq_soc_data {
>>   struct qcom_cpufreq_data {
>>   	void __iomem *base;
>>   	struct resource *res;
>> +	struct delayed_work lmh_dcvs_poll_work;
>>   	const struct qcom_cpufreq_soc_data *soc_data;
>> +	cpumask_var_t cpus;
>> +	unsigned long throttled_freq;
>> +	int lmh_dcvs_irq;
>>   };
>>   
>>   static unsigned long cpu_hw_rate, xo_rate;
>> @@ -251,10 +259,79 @@ static void qcom_get_related_cpus(int index, struct cpumask *m)
>>   	}
>>   }
>>   
>> +static inline unsigned long qcom_lmh_vote_to_freq(u32 val)
>> +{
>> +	return (val & 0x3FF) * 19200;
>> +}
>> +
>> +static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
>> +{
>> +	struct cpufreq_policy policy;
>> +	struct dev_pm_opp *opp;
>> +	struct device *dev;
>> +	unsigned long max_capacity, capacity, freq_hz;
>> +	unsigned int val, freq;
>> +
>> +	val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
>> +	freq = qcom_lmh_vote_to_freq(val);
>> +	freq_hz = freq * HZ_PER_KHZ;
>> +
>> +	/* Do I need to calculate ceil and floor ? */
> 
> You don't know ?

stray comment! Will remove it.

> 
>> +	dev = get_cpu_device(cpumask_first(data->cpus));
>> +	opp = dev_pm_opp_find_freq_floor(dev, &freq_hz);
>> +	if (IS_ERR(opp) && PTR_ERR(opp) == -ERANGE)
>> +		opp = dev_pm_opp_find_freq_ceil(dev, &freq_hz);
>> +
>> +	data->throttled_freq = freq_hz / HZ_PER_KHZ;
>> +
> 
> What exactly are we trying to do here ? A comment would be good as
> well.

You want me to put a comment saying converting frequency in hz to khz ?

> 
>> +	cpufreq_get_policy(&policy, cpumask_first(data->cpus));
>> +
>> +	/* Update thermal pressure */
>> +	max_capacity = arch_scale_cpu_capacity(cpumask_first(data->cpus));
> 
> Set capacity of a single CPU from a policy ?

Get maximum capacity of a cpu.

> 
>> +	capacity = data->throttled_freq * max_capacity;
>> +	capacity /= policy.cpuinfo.max_freq;
>> +	/* Don't pass boost capacity to scheduler */
>> +	if (capacity > max_capacity)
>> +		capacity = max_capacity;
>> +	arch_set_thermal_pressure(data->cpus, max_capacity - capacity);
> 
> You should really be using policy->cpus instead of allocating
> data->cpus..

Yes I should be. But I still need data->cpus to get the policy.

> 
>> +}
>> +
>> +static void qcom_lmh_dcvs_poll(struct work_struct *work)
>> +{
>> +	struct qcom_cpufreq_data *data;
>> +
>> +	data = container_of(work, struct qcom_cpufreq_data, lmh_dcvs_poll_work.work);
>> +
>> +	qcom_lmh_dcvs_notify(data);
> 
> You should really move the below stuff the disable_irq_nosync(), it
> will make your life easier.

I don't understand your comment here. I want to disable irq. call 
notify. Start polling. And in polling I want to call notify and if the 
thermal event has passed stop polling else continue polling.
> 
>> +	/**
>> +	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
>> +	 * polling and switch back to interrupt mechanism
>> +	 */
>> +	if (data->throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(data->cpus)))
>> +		/* Clear the existing interrupts and enable it back */
>> +		enable_irq(data->lmh_dcvs_irq);
>> +	else
>> +		mod_delayed_work(system_highpri_wq, &data->lmh_dcvs_poll_work,
>> +				 msecs_to_jiffies(10));
>> +}
>> +
>> +static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
>> +{
>> +	struct qcom_cpufreq_data *c_data = data;
>> +
>> +	/* Disable interrupt and enable polling */
>> +	disable_irq_nosync(c_data->lmh_dcvs_irq);
>> +	qcom_lmh_dcvs_notify(c_data);
>> +	mod_delayed_work(system_highpri_wq, &c_data->lmh_dcvs_poll_work, msecs_to_jiffies(10));
>> +
>> +	return 0;
>> +}
>> +
>>   static const struct qcom_cpufreq_soc_data qcom_soc_data = {
>>   	.reg_enable = 0x0,
>>   	.reg_freq_lut = 0x110,
>>   	.reg_volt_lut = 0x114,
>> +	.reg_current_vote = 0x704,
> 
> Should this be a different patch ?

Why ? This is the register to read the throttled frequency.

> 
>>   	.reg_perf_state = 0x920,
>>   	.lut_row_size = 32,
>>   };
>> @@ -285,6 +362,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   	void __iomem *base;
>>   	struct qcom_cpufreq_data *data;
>>   	int ret, index;
>> +	bool lmh_mitigation_enabled = false;
> 
> You just overwrite it below, no need to initialize it.

Sure.

> 
>>   
>>   	cpu_dev = get_cpu_device(policy->cpu);
>>   	if (!cpu_dev) {
>> @@ -305,6 +383,8 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   
>>   	index = args.args[0];
>>   
>> +	lmh_mitigation_enabled = of_property_read_bool(pdev->dev.of_node, "qcom,support-lmh");
>> +
>>   	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
>>   	if (!res) {
>>   		dev_err(dev, "failed to get mem resource %d\n", index);
>> @@ -329,6 +409,11 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   		goto unmap_base;
>>   	}
>>   
>> +	if (!alloc_cpumask_var(&data->cpus, GFP_KERNEL)) {
>> +		ret = -ENOMEM;
>> +		goto unmap_base;
>> +	}
>> +
>>   	data->soc_data = of_device_get_match_data(&pdev->dev);
>>   	data->base = base;
>>   	data->res = res;
>> @@ -347,6 +432,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   		goto error;
>>   	}
>>   
>> +	cpumask_copy(data->cpus, policy->cpus);
>>   	policy->driver_data = data;
>>   
>>   	ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy);
>> @@ -370,6 +456,20 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
>>   	}
>>   
>> +	if (lmh_mitigation_enabled) {
> 
> Shouldn't you move the allocation and setting of data->cpus here ? I
> suggest creating a separate routine for all initialization around this
> stuff.

I should considering nothing else is using data->cpus. Yes I will create 
a separate init function.

> 
>> +		data->lmh_dcvs_irq = platform_get_irq(pdev, index);
>> +		if (data->lmh_dcvs_irq < 0) {
>> +			ret = data->lmh_dcvs_irq;
>> +			goto error;
>> +		}
>> +		ret = devm_request_irq(dev, data->lmh_dcvs_irq, qcom_lmh_dcvs_handle_irq,
>> +				       0, "dcvsh-irq", data);
> 
> I would rather pass policy as data here.

So policy for a cpu can change runtime, right ?

> 
>> +		if (ret) {
>> +			dev_err(dev, "Error %d registering irq %x\n", ret, data->lmh_dcvs_irq);
>> +			goto error;
>> +		}
>> +		INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
>> +	}
>>   	return 0;
>>   error:
>>   	kfree(data);
> 

-- 
Warm Regards
Thara (She/Her/Hers)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support
  2021-06-15  1:58     ` Thara Gopinath
@ 2021-06-15  5:16       ` Viresh Kumar
  0 siblings, 0 replies; 18+ messages in thread
From: Viresh Kumar @ 2021-06-15  5:16 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: agross, bjorn.andersson, rui.zhang, daniel.lezcano, rjw, robh+dt,
	linux-arm-msm, linux-pm, linux-kernel, devicetree

On 14-06-21, 21:58, Thara Gopinath wrote:
> On 6/14/21 6:31 AM, Viresh Kumar wrote:
> > On 08-06-21, 18:29, Thara Gopinath wrote:
> > > +static void qcom_lmh_dcvs_notify(struct qcom_cpufreq_data *data)
> > > +{
> > > +	struct cpufreq_policy policy;
> > > +	struct dev_pm_opp *opp;
> > > +	struct device *dev;
> > > +	unsigned long max_capacity, capacity, freq_hz;
> > > +	unsigned int val, freq;
> > > +
> > > +	val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
> > > +	freq = qcom_lmh_vote_to_freq(val);
> > > +	freq_hz = freq * HZ_PER_KHZ;
> > > +
> > > +	/* Do I need to calculate ceil and floor ? */
> > 
> > You don't know ?
> 
> stray comment! Will remove it.
> 
> > 
> > > +	dev = get_cpu_device(cpumask_first(data->cpus));
> > > +	opp = dev_pm_opp_find_freq_floor(dev, &freq_hz);
> > > +	if (IS_ERR(opp) && PTR_ERR(opp) == -ERANGE)
> > > +		opp = dev_pm_opp_find_freq_ceil(dev, &freq_hz);
> > > +
> > > +	data->throttled_freq = freq_hz / HZ_PER_KHZ;
> > > +
> > 
> > What exactly are we trying to do here ? A comment would be good as
> > well.
> 
> You want me to put a comment saying converting frequency in hz to khz ?

Not that, but for the entire routine. What exactly are we looking to
do here and why?

> > 
> > > +	cpufreq_get_policy(&policy, cpumask_first(data->cpus));
> > > +
> > > +	/* Update thermal pressure */
> > > +	max_capacity = arch_scale_cpu_capacity(cpumask_first(data->cpus));
> > 
> > Set capacity of a single CPU from a policy ?
> 
> Get maximum capacity of a cpu.

Ahh, I thought you are setting it :(

> > 
> > > +	capacity = data->throttled_freq * max_capacity;
> > > +	capacity /= policy.cpuinfo.max_freq;
> > > +	/* Don't pass boost capacity to scheduler */
> > > +	if (capacity > max_capacity)
> > > +		capacity = max_capacity;
> > > +	arch_set_thermal_pressure(data->cpus, max_capacity - capacity);
> > 
> > You should really be using policy->cpus instead of allocating
> > data->cpus..
> 
> Yes I should be. But I still need data->cpus to get the policy.

From the comment which comes later on, you shall get the policy here
anyway.
 
> > > +}
> > > +
> > > +static void qcom_lmh_dcvs_poll(struct work_struct *work)
> > > +{
> > > +	struct qcom_cpufreq_data *data;
> > > +
> > > +	data = container_of(work, struct qcom_cpufreq_data, lmh_dcvs_poll_work.work);
> > > +
> > > +	qcom_lmh_dcvs_notify(data);
> > 
> > You should really move the below stuff the disable_irq_nosync(), it
> > will make your life easier.

Damn, s/disable_irq_nosync/qcom_lmh_dcvs_notify/

> I don't understand your comment here. I want to disable irq. call notify.
> Start polling. And in polling I want to call notify and if the thermal event
> has passed stop polling else continue polling.

Yeah, I messed up in the comment. I was asking to move the enable-irq
and mod_delayed_work to qcom_lmh_dcvs_notify() itself.

> > > +	/**
> > > +	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
> > > +	 * polling and switch back to interrupt mechanism
> > > +	 */
> > > +	if (data->throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(data->cpus)))
> > > +		/* Clear the existing interrupts and enable it back */
> > > +		enable_irq(data->lmh_dcvs_irq);
> > > +	else
> > > +		mod_delayed_work(system_highpri_wq, &data->lmh_dcvs_poll_work,
> > > +				 msecs_to_jiffies(10));
> > > +}
> > > +
> > > +static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
> > > +{
> > > +	struct qcom_cpufreq_data *c_data = data;
> > > +
> > > +	/* Disable interrupt and enable polling */
> > > +	disable_irq_nosync(c_data->lmh_dcvs_irq);
> > > +	qcom_lmh_dcvs_notify(c_data);
> > > +	mod_delayed_work(system_highpri_wq, &c_data->lmh_dcvs_poll_work, msecs_to_jiffies(10));
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   static const struct qcom_cpufreq_soc_data qcom_soc_data = {
> > >   	.reg_enable = 0x0,
> > >   	.reg_freq_lut = 0x110,
> > >   	.reg_volt_lut = 0x114,
> > > +	.reg_current_vote = 0x704,
> > 
> > Should this be a different patch ?
> 
> Why ? This is the register to read the throttled frequency.

Okay, it looked this is separate. Leave it.

> > > +		ret = devm_request_irq(dev, data->lmh_dcvs_irq, qcom_lmh_dcvs_handle_irq,
> > > +				       0, "dcvsh-irq", data);
> > 
> > I would rather pass policy as data here.
> 
> So policy for a cpu can change runtime, right ?

No, it is allocated just once.

-- 
viresh

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] thermal: qcom: Add support for LMh driver
  2021-06-15  1:38     ` Thara Gopinath
@ 2021-06-18 17:54       ` Bjorn Andersson
  2021-06-18 21:53         ` Thara Gopinath
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Andersson @ 2021-06-18 17:54 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: agross, rui.zhang, daniel.lezcano, viresh.kumar, rjw, robh+dt,
	linux-arm-msm, linux-pm, linux-kernel, devicetree

On Mon 14 Jun 20:38 CDT 2021, Thara Gopinath wrote:
> On 6/14/21 4:53 PM, Bjorn Andersson wrote:
> > On Tue 08 Jun 17:29 CDT 2021, Thara Gopinath wrote:
> > > diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
[..]
> > > +static irqreturn_t lmh_handle_irq(int hw_irq, void *data)
> > > +{
> > > +	struct lmh_hw_data *lmh_data = data;
> > > +	int irq = irq_find_mapping(lmh_data->domain, 0);
> > > +
> > > +	/*
> > > +	 * Disable interrupt and call the cpufreq driver to handle the interrupt
> > > +	 * cpufreq will enable the interrupt once finished processing.
> > > +	 */
> > > +	disable_irq_nosync(lmh_data->irq);
> > 
> > The contract between this driver's disabling of the IRQ and the
> > cpufreq-hw driver's enabling it when we're done polling does worry me.
> > 
> > In the case of EPSS, don't we disable the interrupt during the polling
> > there as well? If that's the case wouldn't it be better to implement
> > irq_chip->irq_disable and have the cpufreq-hw driver do the disable in
> > both cases?
> 
> Yes. You are right. In case of EPSS, the cpufreq-hw will have to disable the
> interrupt. I did think of the approach you suggested here. My only issue is
> that we will dispatch the interrupt to cpufreq-hw without it disabling it
> and hence the interrupt could fire again, right ?
> 

Does it fire again before you INTR_CLK it?

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support
  2021-06-08 22:29 ` [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support Thara Gopinath
  2021-06-14 10:31   ` Viresh Kumar
@ 2021-06-18 18:16   ` Bjorn Andersson
  2021-06-18 21:55     ` Thara Gopinath
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Andersson @ 2021-06-18 18:16 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: agross, rui.zhang, daniel.lezcano, viresh.kumar, rjw, robh+dt,
	linux-arm-msm, linux-pm, linux-kernel, devicetree

On Tue 08 Jun 17:29 CDT 2021, Thara Gopinath wrote:
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
[..]
> @@ -305,6 +383,8 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  
>  	index = args.args[0];
>  
> +	lmh_mitigation_enabled = of_property_read_bool(pdev->dev.of_node, "qcom,support-lmh");

Rather than adding a new interrupt _and_ a flag to tell the driver that
this new interrupt should be used, wouldn't it be sufficient to just see
if the interrupt is specified?

> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
>  	if (!res) {
>  		dev_err(dev, "failed to get mem resource %d\n", index);
> @@ -329,6 +409,11 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  		goto unmap_base;
>  	}
>  
> +	if (!alloc_cpumask_var(&data->cpus, GFP_KERNEL)) {
> +		ret = -ENOMEM;
> +		goto unmap_base;
> +	}
> +
>  	data->soc_data = of_device_get_match_data(&pdev->dev);
>  	data->base = base;
>  	data->res = res;
> @@ -347,6 +432,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  		goto error;
>  	}
>  
> +	cpumask_copy(data->cpus, policy->cpus);
>  	policy->driver_data = data;
>  
>  	ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy);
> @@ -370,6 +456,20 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
>  	}
>  
> +	if (lmh_mitigation_enabled) {
> +		data->lmh_dcvs_irq = platform_get_irq(pdev, index);
> +		if (data->lmh_dcvs_irq < 0) {

This will be -ENXIO if the interrupt isn't specified and <0 for other
errors, so you should be able to distinguish the two failure cases.

Regards,
Bjorn

> +			ret = data->lmh_dcvs_irq;
> +			goto error;
> +		}
> +		ret = devm_request_irq(dev, data->lmh_dcvs_irq, qcom_lmh_dcvs_handle_irq,
> +				       0, "dcvsh-irq", data);
> +		if (ret) {
> +			dev_err(dev, "Error %d registering irq %x\n", ret, data->lmh_dcvs_irq);
> +			goto error;
> +		}
> +		INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
> +	}
>  	return 0;
>  error:
>  	kfree(data);
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] thermal: qcom: Add support for LMh driver
  2021-06-18 17:54       ` Bjorn Andersson
@ 2021-06-18 21:53         ` Thara Gopinath
  0 siblings, 0 replies; 18+ messages in thread
From: Thara Gopinath @ 2021-06-18 21:53 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: agross, rui.zhang, daniel.lezcano, viresh.kumar, rjw, robh+dt,
	linux-arm-msm, linux-pm, linux-kernel, devicetree



On 6/18/21 1:54 PM, Bjorn Andersson wrote:
> On Mon 14 Jun 20:38 CDT 2021, Thara Gopinath wrote:
>> On 6/14/21 4:53 PM, Bjorn Andersson wrote:
>>> On Tue 08 Jun 17:29 CDT 2021, Thara Gopinath wrote:
>>>> diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
> [..]
>>>> +static irqreturn_t lmh_handle_irq(int hw_irq, void *data)
>>>> +{
>>>> +	struct lmh_hw_data *lmh_data = data;
>>>> +	int irq = irq_find_mapping(lmh_data->domain, 0);
>>>> +
>>>> +	/*
>>>> +	 * Disable interrupt and call the cpufreq driver to handle the interrupt
>>>> +	 * cpufreq will enable the interrupt once finished processing.
>>>> +	 */
>>>> +	disable_irq_nosync(lmh_data->irq);
>>>
>>> The contract between this driver's disabling of the IRQ and the
>>> cpufreq-hw driver's enabling it when we're done polling does worry me.
>>>
>>> In the case of EPSS, don't we disable the interrupt during the polling
>>> there as well? If that's the case wouldn't it be better to implement
>>> irq_chip->irq_disable and have the cpufreq-hw driver do the disable in
>>> both cases?
>>
>> Yes. You are right. In case of EPSS, the cpufreq-hw will have to disable the
>> interrupt. I did think of the approach you suggested here. My only issue is
>> that we will dispatch the interrupt to cpufreq-hw without it disabling it
>> and hence the interrupt could fire again, right ?
>>
> 
> Does it fire again before you INTR_CLK it?

You mean clear it ? I couldn't reproduce it either way. I did not try 
the irq_chip->irq_disable either. So I will give it a try and if my 
tests pass , I will post it.

> 
> Regards,
> Bjorn
> 

-- 
Warm Regards
Thara (She/Her/Hers)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support
  2021-06-18 18:16   ` Bjorn Andersson
@ 2021-06-18 21:55     ` Thara Gopinath
  0 siblings, 0 replies; 18+ messages in thread
From: Thara Gopinath @ 2021-06-18 21:55 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: agross, rui.zhang, daniel.lezcano, viresh.kumar, rjw, robh+dt,
	linux-arm-msm, linux-pm, linux-kernel, devicetree



On 6/18/21 2:16 PM, Bjorn Andersson wrote:
> On Tue 08 Jun 17:29 CDT 2021, Thara Gopinath wrote:
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> [..]
>> @@ -305,6 +383,8 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   
>>   	index = args.args[0];
>>   
>> +	lmh_mitigation_enabled = of_property_read_bool(pdev->dev.of_node, "qcom,support-lmh");
> 
> Rather than adding a new interrupt _and_ a flag to tell the driver that
> this new interrupt should be used, wouldn't it be sufficient to just see
> if the interrupt is specified?

Yes. you are right. It should be. Though when I wrote it there was some 
reason which I forget now. I will remove it.

-- 
Warm Regards
Thara (She/Her/Hers)

> 
>> +
>>   	res = platform_get_resource(pdev, IORESOURCE_MEM, index);
>>   	if (!res) {
>>   		dev_err(dev, "failed to get mem resource %d\n", index);
>> @@ -329,6 +409,11 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   		goto unmap_base;
>>   	}
>>   
>> +	if (!alloc_cpumask_var(&data->cpus, GFP_KERNEL)) {
>> +		ret = -ENOMEM;
>> +		goto unmap_base;
>> +	}
>> +
>>   	data->soc_data = of_device_get_match_data(&pdev->dev);
>>   	data->base = base;
>>   	data->res = res;
>> @@ -347,6 +432,7 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   		goto error;
>>   	}
>>   
>> +	cpumask_copy(data->cpus, policy->cpus);
>>   	policy->driver_data = data;
>>   
>>   	ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy);
>> @@ -370,6 +456,20 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
>>   	}
>>   
>> +	if (lmh_mitigation_enabled) {
>> +		data->lmh_dcvs_irq = platform_get_irq(pdev, index);
>> +		if (data->lmh_dcvs_irq < 0) {
> 
> This will be -ENXIO if the interrupt isn't specified and <0 for other
> errors, so you should be able to distinguish the two failure cases.
> 
> Regards,
> Bjorn
> 
>> +			ret = data->lmh_dcvs_irq;
>> +			goto error;
>> +		}
>> +		ret = devm_request_irq(dev, data->lmh_dcvs_irq, qcom_lmh_dcvs_handle_irq,
>> +				       0, "dcvsh-irq", data);
>> +		if (ret) {
>> +			dev_err(dev, "Error %d registering irq %x\n", ret, data->lmh_dcvs_irq);
>> +			goto error;
>> +		}
>> +		INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
>> +	}
>>   	return 0;
>>   error:
>>   	kfree(data);
>> -- 
>> 2.25.1
>>



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-06-18 21:55 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 22:29 [PATCH 0/5] Introduce LMh driver for Qualcomm SoCs Thara Gopinath
2021-06-08 22:29 ` [PATCH 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh Thara Gopinath
2021-06-09  3:10   ` kernel test robot
2021-06-08 22:29 ` [PATCH 2/5] thermal: qcom: Add support for LMh driver Thara Gopinath
2021-06-09  2:25   ` Randy Dunlap
2021-06-15  1:37     ` Thara Gopinath
2021-06-14 20:53   ` Bjorn Andersson
2021-06-15  1:38     ` Thara Gopinath
2021-06-18 17:54       ` Bjorn Andersson
2021-06-18 21:53         ` Thara Gopinath
2021-06-08 22:29 ` [PATCH 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support Thara Gopinath
2021-06-14 10:31   ` Viresh Kumar
2021-06-15  1:58     ` Thara Gopinath
2021-06-15  5:16       ` Viresh Kumar
2021-06-18 18:16   ` Bjorn Andersson
2021-06-18 21:55     ` Thara Gopinath
2021-06-08 22:29 ` [PATCH 4/5] arm64: boot: dts: sdm45: Add support for LMh node Thara Gopinath
2021-06-08 22:29 ` [PATCH 5/5] arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal zones 0-7 Thara Gopinath

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).