[Patch v2 0/5] Introduce LMh driver for Qualcomm SoCs

[Patch v2 0/5] Introduce LMh driver for Qualcomm SoCs

[Thara Gopinath]

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: qcom: 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 | 162 +++--------------
 drivers/cpufreq/qcom-cpufreq-hw.c    | 103 +++++++++++
 drivers/firmware/qcom_scm.c          |  54 ++++++
 drivers/firmware/qcom_scm.h          |   4 +
 drivers/thermal/qcom/Kconfig         |  10 ++
 drivers/thermal/qcom/Makefile        |   1 +
 drivers/thermal/qcom/lmh.c           | 251 +++++++++++++++++++++++++++
 include/linux/qcom_scm.h             |  14 ++
 8 files changed, 463 insertions(+), 136 deletions(-)
 create mode 100644 drivers/thermal/qcom/lmh.c

-- 
2.25.1

[Patch v2 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh

[Thara Gopinath]

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

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---

v1->v2:
	Changed the input parameters in qcom_scm_lmh_dcvsh from payload_buf and
	payload_size to payload_fn, payload_reg, payload_val as per Bjorn's review
	comments.

 drivers/firmware/qcom_scm.c | 54 +++++++++++++++++++++++++++++++++++++
 drivers/firmware/qcom_scm.h |  4 +++
 include/linux/qcom_scm.h    | 14 ++++++++++
 3 files changed, 72 insertions(+)

diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index ee9cb545e73b..19e9fb91d084 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -1147,6 +1147,60 @@ 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_fn, u32 payload_reg, u32 payload_val,
+		       u64 limit_node, u32 node_id, u64 version)
+{
+	dma_addr_t payload_phys;
+	u32 *payload_buf;
+	int payload_size = 5 * sizeof(u32);
+
+	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;
+
+	payload_buf[0] = payload_fn;
+	payload_buf[1] = 0;
+	payload_buf[2] = payload_reg;
+	payload_buf[3] = 1;
+	payload_buf[4] = payload_val;
+
+	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..c0475d1c9885 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_fn, u32 payload_reg, u32 payload_val,
+			      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,13 @@ 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; }
+
+static inline int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val,
+				     u64 limit_node, u32 node_id, u64 version)
+		{ return -ENODEV; }
+
+static inline int qcom_scm_lmh_profile_change(u32 profile_id) { return -ENODEV; }
+
+static inline bool qcom_scm_lmh_dcvsh_available(void) { return -ENODEV; }
 #endif
 #endif
-- 
2.25.1

Re: [Patch v2 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh

[Matthias Kaehlcke]

On Thu, Jun 24, 2021 at 07:58:09AM -0400, Thara Gopinath wrote:
> Introduce SCM calls to access/configure limits management hardware(LMH).
> 
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
> 
> v1->v2:
> 	Changed the input parameters in qcom_scm_lmh_dcvsh from payload_buf and
> 	payload_size to payload_fn, payload_reg, payload_val as per Bjorn's review
> 	comments.
> 
>  drivers/firmware/qcom_scm.c | 54 +++++++++++++++++++++++++++++++++++++
>  drivers/firmware/qcom_scm.h |  4 +++
>  include/linux/qcom_scm.h    | 14 ++++++++++
>  3 files changed, 72 insertions(+)
> 
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index ee9cb545e73b..19e9fb91d084 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -1147,6 +1147,60 @@ 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_fn, u32 payload_reg, u32 payload_val,
> +		       u64 limit_node, u32 node_id, u64 version)
> +{
> +	dma_addr_t payload_phys;
> +	u32 *payload_buf;
> +	int payload_size = 5 * sizeof(u32);
> +
> +	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;
> +
> +	payload_buf[0] = payload_fn;
> +	payload_buf[1] = 0;
> +	payload_buf[2] = payload_reg;
> +	payload_buf[3] = 1;
> +	payload_buf[4] = payload_val;
> +
> +	desc.args[0] = payload_phys;
> +	return qcom_scm_call(__scm->dev, &desc, NULL);

dma_free_coherent()?

Re: [Patch v2 1/5] firmware: qcom_scm: Introduce SCM calls to access LMh

[Thara Gopinath]

On 6/24/21 1:48 PM, Matthias Kaehlcke wrote:
> On Thu, Jun 24, 2021 at 07:58:09AM -0400, Thara Gopinath wrote:
>> Introduce SCM calls to access/configure limits management hardware(LMH).
>>
>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
>> ---
>>
>> v1->v2:
>> 	Changed the input parameters in qcom_scm_lmh_dcvsh from payload_buf and
>> 	payload_size to payload_fn, payload_reg, payload_val as per Bjorn's review
>> 	comments.
>>
>>   drivers/firmware/qcom_scm.c | 54 +++++++++++++++++++++++++++++++++++++
>>   drivers/firmware/qcom_scm.h |  4 +++
>>   include/linux/qcom_scm.h    | 14 ++++++++++
>>   3 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
>> index ee9cb545e73b..19e9fb91d084 100644
>> --- a/drivers/firmware/qcom_scm.c
>> +++ b/drivers/firmware/qcom_scm.c
>> @@ -1147,6 +1147,60 @@ 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_fn, u32 payload_reg, u32 payload_val,
>> +		       u64 limit_node, u32 node_id, u64 version)
>> +{
>> +	dma_addr_t payload_phys;
>> +	u32 *payload_buf;
>> +	int payload_size = 5 * sizeof(u32);
>> +
>> +	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;
>> +
>> +	payload_buf[0] = payload_fn;
>> +	payload_buf[1] = 0;
>> +	payload_buf[2] = payload_reg;
>> +	payload_buf[3] = 1;
>> +	payload_buf[4] = payload_val;
>> +
>> +	desc.args[0] = payload_phys;
>> +	return qcom_scm_call(__scm->dev, &desc, NULL);
> 
> dma_free_coherent()?

yep.. A free should be done here. Will fix it

> 

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

[Patch v2 2/5] thermal: qcom: Add support for LMh driver

[Thara Gopinath]

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 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 soc's
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>
---

v1->v2:
	- Cosmetic and spelling fixes from review comments from Randy Dunlap
	- Added irq_disable to lmh_irq_ops and removed disabling of irq from
	  lmh_handle_irq. Now cpufreq explicitly disables irq prior to
	  handling it as per Bjorn's suggestion.
	- Rebased to new version of qcom_scm_lmh_dcvsh as changed in patch 1.
	- Removed generic dt compatible string and introduced platform specific one
	  as per Bjorn's suggestion.
	- Take arm, low and high temp thresholds for LMh from dt properties instead of
	  #defines in the driver as per Daniel's suggestion.
	- Other minor fixes.

 drivers/thermal/qcom/Kconfig  |  10 ++
 drivers/thermal/qcom/Makefile |   1 +
 drivers/thermal/qcom/lmh.c    | 251 ++++++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+)
 create mode 100644 drivers/thermal/qcom/lmh.c

diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
index 8d5ac2df26dc..7d942f71e532 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 hardware-enforced mitigation for cpus based on
+	  input from temperature and current sensors.  On many newer Qualcomm SoCs
+	  LMh is configured in the firmware and this feature need not be enabled.
+	  However, on certain SoCs like sdm845 LMh has to be configured from kernel.
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..a14cad83b459
--- /dev/null
+++ b/drivers/thermal/qcom/lmh.c
@@ -0,0 +1,251 @@
+// 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_REG_DCVS_INTR_CLR		0x8
+
+struct lmh_hw_data {
+	void __iomem *base;
+	struct irq_domain *domain;
+	int irq;
+	u32 cpu_id;
+};
+
+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);
+
+	/*
+	 * Call the cpufreq driver to handle the interrupt.
+	 */
+	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(0xff, lmh_data->base + LMH_REG_DCVS_INTR_CLR);
+	enable_irq(lmh_data->irq);
+}
+
+static void lmh_disable_interrupt(struct irq_data *d)
+{
+	struct lmh_hw_data *lmh_data = irq_data_get_irq_chip_data(d);
+
+	disable_irq_nosync(lmh_data->irq);
+}
+
+static struct irq_chip lmh_irq_chip = {
+	.name           = "lmh",
+	.irq_enable	= lmh_enable_interrupt,
+	.irq_disable	= lmh_disable_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 temp_low, temp_high, temp_arm, 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) {
+		dev_err(dev, "missing qcom,lmh-cpu-id property\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "qcom,lmh-temperature-high", &temp_high);
+	if (ret) {
+		dev_err(dev, "missing qcom,lmh-temperature-high property\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "qcom,lmh-temperature-low", &temp_low);
+	if (ret) {
+		dev_err(dev, "missing qcom,lmh-temperature-low property\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "qcom,lmh-temperature-arm", &temp_arm);
+	if (ret) {
+		dev_err(dev, "missing qcom,lmh-temperature-arm property\n");
+		return ret;
+	}
+
+	/*
+	 * 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;
+	}
+
+	platform_set_drvdata(pdev, lmh_data);
+
+	if (!qcom_scm_lmh_dcvsh_available())
+		return -EINVAL;
+
+	/* Enable Thermal Algorithm */
+	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
+		return ret;
+	}
+
+	/* Enable Current Sensing Algorithm */
+	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error %d enabling current subfunction\n", ret);
+		return ret;
+	}
+
+	/* Enable Reliability Algorithm */
+	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
+		return ret;
+	}
+
+	/* Enable BCL Algorithm */
+	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1,
+				 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 */
+	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_TH_ARM_THRESHOLD, temp_arm,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error setting thermal ARM threshold%d\n", ret);
+		return ret;
+	}
+
+	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_TH_HI_THRESHOLD, temp_high,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error setting thermal HI threshold%d\n", ret);
+		return ret;
+	}
+
+	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_TH_LOW_THRESHOLD, temp_low,
+				 LMH_NODE_DCVS, node_id, 0);
+	if (ret) {
+		dev_err(dev, "Error setting thermal ARM threshold%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;
+	}
+
+	/* Disable the irq and let cpufreq enable it when ready to handle the interrupt */
+	disable_irq(lmh_data->irq);
+
+	return 0;
+}
+
+static const struct of_device_id lmh_table[] = {
+	{ .compatible = "qcom,sdm845-lmh", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, lmh_table);
+
+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

Re: [Patch v2 2/5] thermal: qcom: Add support for LMh driver

[Matthias Kaehlcke]

On Thu, Jun 24, 2021 at 07:58:10AM -0400, 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 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 soc's
> 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>
> ---
> 
> v1->v2:
> 	- Cosmetic and spelling fixes from review comments from Randy Dunlap
> 	- Added irq_disable to lmh_irq_ops and removed disabling of irq from
> 	  lmh_handle_irq. Now cpufreq explicitly disables irq prior to
> 	  handling it as per Bjorn's suggestion.
> 	- Rebased to new version of qcom_scm_lmh_dcvsh as changed in patch 1.
> 	- Removed generic dt compatible string and introduced platform specific one
> 	  as per Bjorn's suggestion.
> 	- Take arm, low and high temp thresholds for LMh from dt properties instead of
> 	  #defines in the driver as per Daniel's suggestion.
> 	- Other minor fixes.
> 
>  drivers/thermal/qcom/Kconfig  |  10 ++
>  drivers/thermal/qcom/Makefile |   1 +
>  drivers/thermal/qcom/lmh.c    | 251 ++++++++++++++++++++++++++++++++++
>  3 files changed, 262 insertions(+)
>  create mode 100644 drivers/thermal/qcom/lmh.c
> 
> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
> index 8d5ac2df26dc..7d942f71e532 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 hardware-enforced mitigation for cpus based on
> +	  input from temperature and current sensors.  On many newer Qualcomm SoCs
> +	  LMh is configured in the firmware and this feature need not be enabled.
> +	  However, on certain SoCs like sdm845 LMh has to be configured from kernel.
> 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..a14cad83b459
> --- /dev/null
> +++ b/drivers/thermal/qcom/lmh.c
> @@ -0,0 +1,251 @@
> +// 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_REG_DCVS_INTR_CLR		0x8
> +
> +struct lmh_hw_data {
> +	void __iomem *base;
> +	struct irq_domain *domain;
> +	int irq;
> +	u32 cpu_id;
> +};
> +
> +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);
> +
> +	/*
> +	 * Call the cpufreq driver to handle the interrupt.
> +	 */

no need for a multiline comment

> +	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(0xff, lmh_data->base + LMH_REG_DCVS_INTR_CLR);
> +	enable_irq(lmh_data->irq);
> +}
> +
> +static void lmh_disable_interrupt(struct irq_data *d)
> +{
> +	struct lmh_hw_data *lmh_data = irq_data_get_irq_chip_data(d);
> +
> +	disable_irq_nosync(lmh_data->irq);
> +}
> +
> +static struct irq_chip lmh_irq_chip = {
> +	.name           = "lmh",
> +	.irq_enable	= lmh_enable_interrupt,
> +	.irq_disable	= lmh_disable_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 temp_low, temp_high, temp_arm, 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);

binding missing?

> +	if (ret) {
> +		dev_err(dev, "missing qcom,lmh-cpu-id property\n");
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(np, "qcom,lmh-temperature-high", &temp_high);
> +	if (ret) {

ditto

> +		dev_err(dev, "missing qcom,lmh-temperature-high property\n");
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(np, "qcom,lmh-temperature-low", &temp_low);
> +	if (ret) {

ditto

> +		dev_err(dev, "missing qcom,lmh-temperature-low property\n");
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(np, "qcom,lmh-temperature-arm", &temp_arm);
> +	if (ret) {

ditto

> +		dev_err(dev, "missing qcom,lmh-temperature-arm property\n");
> +		return ret;
> +	}
> +
> +	/*
> +	 * 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;
> +	}
> +
> +	platform_set_drvdata(pdev, lmh_data);
> +
> +	if (!qcom_scm_lmh_dcvsh_available())
> +		return -EINVAL;
> +
> +	/* Enable Thermal Algorithm */

nit: thermal algorithm

Same for other comments below.

Actually I think you can delete the comments, the error log a few lines below
provides the same information.

> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable Current Sensing Algorithm */
> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {
> +		dev_err(dev, "Error %d enabling current subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable Reliability Algorithm */
> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {

disable thermal subfunction??

> +		dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable BCL Algorithm */
> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {

disable previously enabled stuff?

> +		dev_err(dev, "Error %d enabling BCL subfunction\n", ret);
> +		return ret;
> +	}
> +
> +	ret = qcom_scm_lmh_profile_change(0x1);

Does profile 1 represent something specific, i.e. should this be a constant?
If not, why a hex instead of a decimal value?

> +	if (ret) {

disable previously enabled stuff?

> +		dev_err(dev, "Error %d changing profile\n", ret);
> +		return ret;
> +	}
> +
> +	/* Set default thermal trips */
> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_TH_ARM_THRESHOLD, temp_arm,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {

undo stuff?

> +		dev_err(dev, "Error setting thermal ARM threshold%d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_TH_HI_THRESHOLD, temp_high,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {

undo stuff?

> +		dev_err(dev, "Error setting thermal HI threshold%d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_TH_LOW_THRESHOLD, temp_low,
> +				 LMH_NODE_DCVS, node_id, 0);
> +	if (ret) {

undo stuff?

> +		dev_err(dev, "Error setting thermal ARM threshold%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) {

undo stuff?

> +		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) {

undo stuff?

> +		dev_err(dev, "Error %d registering irq %x\n", ret, lmh_data->irq);
> +		irq_domain_remove(lmh_data->domain);
> +		return ret;
> +	}
> +
> +	/* Disable the irq and let cpufreq enable it when ready to handle the interrupt */
> +	disable_irq(lmh_data->irq);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id lmh_table[] = {
> +	{ .compatible = "qcom,sdm845-lmh", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, lmh_table);
> +
> +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
> 

Re: [Patch v2 2/5] thermal: qcom: Add support for LMh driver

[Thara Gopinath]

On 6/24/21 1:24 PM, Matthias Kaehlcke wrote:
> On Thu, Jun 24, 2021 at 07:58:10AM -0400, 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 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 soc's
>> 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>
>> ---
>>
>> v1->v2:
>> 	- Cosmetic and spelling fixes from review comments from Randy Dunlap
>> 	- Added irq_disable to lmh_irq_ops and removed disabling of irq from
>> 	  lmh_handle_irq. Now cpufreq explicitly disables irq prior to
>> 	  handling it as per Bjorn's suggestion.
>> 	- Rebased to new version of qcom_scm_lmh_dcvsh as changed in patch 1.
>> 	- Removed generic dt compatible string and introduced platform specific one
>> 	  as per Bjorn's suggestion.
>> 	- Take arm, low and high temp thresholds for LMh from dt properties instead of
>> 	  #defines in the driver as per Daniel's suggestion.
>> 	- Other minor fixes.
>>
>>   drivers/thermal/qcom/Kconfig  |  10 ++
>>   drivers/thermal/qcom/Makefile |   1 +
>>   drivers/thermal/qcom/lmh.c    | 251 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 262 insertions(+)
>>   create mode 100644 drivers/thermal/qcom/lmh.c
>>
>> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
>> index 8d5ac2df26dc..7d942f71e532 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 hardware-enforced mitigation for cpus based on
>> +	  input from temperature and current sensors.  On many newer Qualcomm SoCs
>> +	  LMh is configured in the firmware and this feature need not be enabled.
>> +	  However, on certain SoCs like sdm845 LMh has to be configured from kernel.
>> 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..a14cad83b459
>> --- /dev/null
>> +++ b/drivers/thermal/qcom/lmh.c
>> @@ -0,0 +1,251 @@
>> +// 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_REG_DCVS_INTR_CLR		0x8
>> +
>> +struct lmh_hw_data {
>> +	void __iomem *base;
>> +	struct irq_domain *domain;
>> +	int irq;
>> +	u32 cpu_id;
>> +};
>> +
>> +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);
>> +
>> +	/*
>> +	 * Call the cpufreq driver to handle the interrupt.
>> +	 */
> 
> no need for a multiline comment

ok..

> 
>> +	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(0xff, lmh_data->base + LMH_REG_DCVS_INTR_CLR);
>> +	enable_irq(lmh_data->irq);
>> +}
>> +
>> +static void lmh_disable_interrupt(struct irq_data *d)
>> +{
>> +	struct lmh_hw_data *lmh_data = irq_data_get_irq_chip_data(d);
>> +
>> +	disable_irq_nosync(lmh_data->irq);
>> +}
>> +
>> +static struct irq_chip lmh_irq_chip = {
>> +	.name           = "lmh",
>> +	.irq_enable	= lmh_enable_interrupt,
>> +	.irq_disable	= lmh_disable_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 temp_low, temp_high, temp_arm, 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);
> 
> binding missing?

ya. I am waiting for feeback on the bindings before documenting them. I 
was not exactly sure how this driver will look like after reviews. But 
once it is more or less reviewed, I will add the patch documenting the 
bindings. Same for all the below bindings.

> 
>> +	if (ret) {
>> +		dev_err(dev, "missing qcom,lmh-cpu-id property\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = of_property_read_u32(np, "qcom,lmh-temperature-high", &temp_high);
>> +	if (ret) {
> 
> ditto
> 
>> +		dev_err(dev, "missing qcom,lmh-temperature-high property\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = of_property_read_u32(np, "qcom,lmh-temperature-low", &temp_low);
>> +	if (ret) {
> 
> ditto
> 
>> +		dev_err(dev, "missing qcom,lmh-temperature-low property\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = of_property_read_u32(np, "qcom,lmh-temperature-arm", &temp_arm);
>> +	if (ret) {
> 
> ditto
> 
>> +		dev_err(dev, "missing qcom,lmh-temperature-arm property\n");
>> +		return ret;
>> +	}
>> +
>> +	/*
>> +	 * 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;
>> +	}
>> +
>> +	platform_set_drvdata(pdev, lmh_data);
>> +
>> +	if (!qcom_scm_lmh_dcvsh_available())
>> +		return -EINVAL;
>> +
>> +	/* Enable Thermal Algorithm */
> 
> nit: thermal algorithm

sure.

> 
> Same for other comments below.
> 
> Actually I think you can delete the comments, the error log a few lines below
> provides the same information.
> 
>> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_ALGO_MODE_ENABLE, 1,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling thermal subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Enable Current Sensing Algorithm */
>> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_CRNT, LMH_ALGO_MODE_ENABLE, 1,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
>> +		dev_err(dev, "Error %d enabling current subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Enable Reliability Algorithm */
>> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_REL, LMH_ALGO_MODE_ENABLE, 1,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
> 
> disable thermal subfunction??

So, my main reference to this h/w is downstream code which does no 
disabling of any sorts.. I do have some documentation but not complete. 
My understanding is that each of these pieces(current, reliability etc) 
are separate entities that provide an input to LMh h/w in deciding 
whether to throttle and how much to throttle. Now that you brought this 
up, I am thinking maybe I should remove returning on error all-together.
Instead allow for separate pieces to be enabled even if one piece 
returns an error. I will have to move thermal all the way to the bottom 
because there is no point is setting the trips if thermal algorithm is 
not enabled. If this does not work, I will keep this as is because I see 
no disable code for this downstream.

> 
>> +		dev_err(dev, "Error %d enabling reliability subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Enable BCL Algorithm */
>> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_BCL, LMH_ALGO_MODE_ENABLE, 1,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
> 
> disable previously enabled stuff?
> 
>> +		dev_err(dev, "Error %d enabling BCL subfunction\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = qcom_scm_lmh_profile_change(0x1);
> 
> Does profile 1 represent something specific, i.e. should this be a constant?
> If not, why a hex instead of a decimal value?

Again zero documentation and just the downstream code for reference. So 
it is hex in the downstream code. I am going to make this a constant.

> 
>> +	if (ret) {
> 
> disable previously enabled stuff?
> 
>> +		dev_err(dev, "Error %d changing profile\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Set default thermal trips */
>> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_TH_ARM_THRESHOLD, temp_arm,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
> 
> undo stuff?
> 
>> +		dev_err(dev, "Error setting thermal ARM threshold%d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_TH_HI_THRESHOLD, temp_high,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
> 
> undo stuff?
> 
>> +		dev_err(dev, "Error setting thermal HI threshold%d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = qcom_scm_lmh_dcvsh(LMH_SUB_FN_THERMAL, LMH_TH_LOW_THRESHOLD, temp_low,
>> +				 LMH_NODE_DCVS, node_id, 0);
>> +	if (ret) {
> 
> undo stuff?
> 
>> +		dev_err(dev, "Error setting thermal ARM threshold%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) {
> 
> undo stuff?

Even without irq support, lest of LMh can function. So there is no undo 
needed here.

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

> 
>> +		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) {
> 
> undo stuff?
> 
>> +		dev_err(dev, "Error %d registering irq %x\n", ret, lmh_data->irq);
>> +		irq_domain_remove(lmh_data->domain);
>> +		return ret;
>> +	}
>> +
>> +	/* Disable the irq and let cpufreq enable it when ready to handle the interrupt */
>> +	disable_irq(lmh_data->irq);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id lmh_table[] = {
>> +	{ .compatible = "qcom,sdm845-lmh", },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, lmh_table);
>> +
>> +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
>>

[Patch v2 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support

[Thara Gopinath]

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>
---

v1->v2:
	- Introduced qcom_cpufreq_hw_lmh_init to consolidate LMh related initializations
	  as per Viresh's review comment.
	- Moved the piece of code restarting polling/re-enabling LMh interrupt to
	  qcom_lmh_dcvs_notify therby simplifying isr and timer callback as per Viresh's
	  suggestion.
	- Droped cpus from qcom_cpufreq_data and instead using cpus from cpufreq_policy in
	  qcom_lmh_dcvs_notify as per Viresh's review comment.
	- Dropped dt property qcom,support-lmh as per Bjorn's suggestion.
	- Other minor/cosmetic fixes

 drivers/cpufreq/qcom-cpufreq-hw.c | 103 ++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index f86859bf76f1..241f6f2b441f 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,10 @@ 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;
+	struct cpufreq_policy *policy;
+	int lmh_dcvs_irq;
 };
 
 static unsigned long cpu_hw_rate, xo_rate;
@@ -251,10 +258,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 = data->policy;
+	struct dev_pm_opp *opp;
+	struct device *dev;
+	unsigned long max_capacity, capacity, freq_hz, throttled_freq;
+	unsigned int val, freq;
+
+	/*
+	 * Get the h/w throttled frequency, normalize it using the
+	 * registered opp table and use it to calculate thermal pressure.
+	 */
+	val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
+	freq = qcom_lmh_vote_to_freq(val);
+	freq_hz = freq * HZ_PER_KHZ;
+
+	dev = get_cpu_device(cpumask_first(policy->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);
+
+	throttled_freq = freq_hz / HZ_PER_KHZ;
+
+	/* Update thermal pressure */
+	max_capacity = arch_scale_cpu_capacity(cpumask_first(policy->cpus));
+	capacity = 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(policy->cpus, max_capacity - capacity);
+	/*
+	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
+	 * polling and switch back to interrupt mechanism
+	 */
+	if (throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(policy->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 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);
+}
+
+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);
+
+	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,
 };
@@ -274,6 +350,23 @@ static const struct of_device_id qcom_cpufreq_hw_match[] = {
 };
 MODULE_DEVICE_TABLE(of, qcom_cpufreq_hw_match);
 
+static void qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy)
+{
+	struct qcom_cpufreq_data *data = policy->driver_data;
+	struct platform_device *pdev = cpufreq_get_driver_data();
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	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);
+		return;
+	}
+	data->policy = policy;
+	INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
+}
+
 static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 {
 	struct platform_device *pdev = cpufreq_get_driver_data();
@@ -370,6 +463,16 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
 	}
 
+	/* Look for LMh interrupt. If no interrupt line is specified /
+	 * if there is an error, allow cpufreq to be enabled as usual.
+	 */
+	data->lmh_dcvs_irq = platform_get_irq(pdev, index);
+	if (data->lmh_dcvs_irq > 0) {
+		qcom_cpufreq_hw_lmh_init(policy);
+	} else if (data->lmh_dcvs_irq != -ENXIO) {
+		ret = data->lmh_dcvs_irq;
+		goto error;
+	}
 	return 0;
 error:
 	kfree(data);
-- 
2.25.1

Re: [Patch v2 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support

[Viresh Kumar]

On 24-06-21, 07:58, 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>
> ---
> 
> v1->v2:
> 	- Introduced qcom_cpufreq_hw_lmh_init to consolidate LMh related initializations
> 	  as per Viresh's review comment.
> 	- Moved the piece of code restarting polling/re-enabling LMh interrupt to
> 	  qcom_lmh_dcvs_notify therby simplifying isr and timer callback as per Viresh's
> 	  suggestion.
> 	- Droped cpus from qcom_cpufreq_data and instead using cpus from cpufreq_policy in
> 	  qcom_lmh_dcvs_notify as per Viresh's review comment.
> 	- Dropped dt property qcom,support-lmh as per Bjorn's suggestion.
> 	- Other minor/cosmetic fixes
> 
>  drivers/cpufreq/qcom-cpufreq-hw.c | 103 ++++++++++++++++++++++++++++++
>  1 file changed, 103 insertions(+)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index f86859bf76f1..241f6f2b441f 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>

Please don't break the alphabetical order here.

>  #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,10 @@ 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;
> +	struct cpufreq_policy *policy;
> +	int lmh_dcvs_irq;
>  };
>  
>  static unsigned long cpu_hw_rate, xo_rate;
> @@ -251,10 +258,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 = data->policy;
> +	struct dev_pm_opp *opp;
> +	struct device *dev;
> +	unsigned long max_capacity, capacity, freq_hz, throttled_freq;
> +	unsigned int val, freq;
> +
> +	/*
> +	 * Get the h/w throttled frequency, normalize it using the
> +	 * registered opp table and use it to calculate thermal pressure.
> +	 */
> +	val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
> +	freq = qcom_lmh_vote_to_freq(val);
> +	freq_hz = freq * HZ_PER_KHZ;
> +
> +	dev = get_cpu_device(cpumask_first(policy->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);
> +
> +	throttled_freq = freq_hz / HZ_PER_KHZ;
> +
> +	/* Update thermal pressure */
> +	max_capacity = arch_scale_cpu_capacity(cpumask_first(policy->cpus));
> +	capacity = throttled_freq * max_capacity;
> +	capacity /= policy->cpuinfo.max_freq;
> +	/* Don't pass boost capacity to scheduler */
> +	if (capacity > max_capacity)
> +		capacity = max_capacity;

I wonder why this check isn't present for cpufreq_cooling.c .

> +	arch_set_thermal_pressure(policy->cpus, max_capacity - capacity);
> +	/*

Whenever you mix code and comments, please separate them with a blank
line, else it becomes a bit messy and harder to read.

> +	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
> +	 * polling and switch back to interrupt mechanism
> +	 */
> +	if (throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(policy->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 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);
> +}
> +
> +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);
> +
> +	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,
>  };
> @@ -274,6 +350,23 @@ static const struct of_device_id qcom_cpufreq_hw_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, qcom_cpufreq_hw_match);
>  
> +static void qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy)
> +{
> +	struct qcom_cpufreq_data *data = policy->driver_data;
> +	struct platform_device *pdev = cpufreq_get_driver_data();
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	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);
> +		return;
> +	}
> +	data->policy = policy;
> +	INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
> +}
> +
>  static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  {
>  	struct platform_device *pdev = cpufreq_get_driver_data();
> @@ -370,6 +463,16 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>  			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
>  	}
>  
> +	/* Look for LMh interrupt. If no interrupt line is specified /
> +	 * if there is an error, allow cpufreq to be enabled as usual.
> +	 */

Proper comment style please..

> +	data->lmh_dcvs_irq = platform_get_irq(pdev, index);
> +	if (data->lmh_dcvs_irq > 0) {
> +		qcom_cpufreq_hw_lmh_init(policy);
> +	} else if (data->lmh_dcvs_irq != -ENXIO) {
> +		ret = data->lmh_dcvs_irq;
> +		goto error;
> +	}

Move all of this to qcom_cpufreq_hw_lmh_init().

And I don't see any cleanup for this stuff. There is no guarantee that
the irq won't fire and queue up a work right after cpufreq driver is
unregistered and before the devm_ stuff gets released.

>  	return 0;
>  error:
>  	kfree(data);
> -- 
> 2.25.1

-- 
viresh

Re: [Patch v2 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support

[Thara Gopinath]

On 6/28/21 10:35 PM, Viresh Kumar wrote:
> On 24-06-21, 07:58, 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>
>> ---
>>
>> v1->v2:
>> 	- Introduced qcom_cpufreq_hw_lmh_init to consolidate LMh related initializations
>> 	  as per Viresh's review comment.
>> 	- Moved the piece of code restarting polling/re-enabling LMh interrupt to
>> 	  qcom_lmh_dcvs_notify therby simplifying isr and timer callback as per Viresh's
>> 	  suggestion.
>> 	- Droped cpus from qcom_cpufreq_data and instead using cpus from cpufreq_policy in
>> 	  qcom_lmh_dcvs_notify as per Viresh's review comment.
>> 	- Dropped dt property qcom,support-lmh as per Bjorn's suggestion.
>> 	- Other minor/cosmetic fixes
>>
>>   drivers/cpufreq/qcom-cpufreq-hw.c | 103 ++++++++++++++++++++++++++++++
>>   1 file changed, 103 insertions(+)
>>
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
>> index f86859bf76f1..241f6f2b441f 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>
> 
> Please don't break the alphabetical order here.
> 
>>   #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,10 @@ 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;
>> +	struct cpufreq_policy *policy;
>> +	int lmh_dcvs_irq;
>>   };
>>   
>>   static unsigned long cpu_hw_rate, xo_rate;
>> @@ -251,10 +258,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 = data->policy;
>> +	struct dev_pm_opp *opp;
>> +	struct device *dev;
>> +	unsigned long max_capacity, capacity, freq_hz, throttled_freq;
>> +	unsigned int val, freq;
>> +
>> +	/*
>> +	 * Get the h/w throttled frequency, normalize it using the
>> +	 * registered opp table and use it to calculate thermal pressure.
>> +	 */
>> +	val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
>> +	freq = qcom_lmh_vote_to_freq(val);
>> +	freq_hz = freq * HZ_PER_KHZ;
>> +
>> +	dev = get_cpu_device(cpumask_first(policy->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);
>> +
>> +	throttled_freq = freq_hz / HZ_PER_KHZ;
>> +
>> +	/* Update thermal pressure */
>> +	max_capacity = arch_scale_cpu_capacity(cpumask_first(policy->cpus));
>> +	capacity = throttled_freq * max_capacity;
>> +	capacity /= policy->cpuinfo.max_freq;
>> +	/* Don't pass boost capacity to scheduler */
>> +	if (capacity > max_capacity)
>> +		capacity = max_capacity;
> 
> I wonder why this check isn't present for cpufreq_cooling.c .

Hi Viresh,

I don't think cpufreq_cooling recognizes boost frequencies. The max 
state there is the max of nominal frequencies , right? If not, it might 
be a good idea to add this check there as well.

I will fix rest of your comments in v3.

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

> 
>> +	arch_set_thermal_pressure(policy->cpus, max_capacity - capacity);
>> +	/*
> 
> Whenever you mix code and comments, please separate them with a blank
> line, else it becomes a bit messy and harder to read.
> 
>> +	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
>> +	 * polling and switch back to interrupt mechanism
>> +	 */
>> +	if (throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(policy->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 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);
>> +}
>> +
>> +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);
>> +
>> +	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,
>>   };
>> @@ -274,6 +350,23 @@ static const struct of_device_id qcom_cpufreq_hw_match[] = {
>>   };
>>   MODULE_DEVICE_TABLE(of, qcom_cpufreq_hw_match);
>>   
>> +static void qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy)
>> +{
>> +	struct qcom_cpufreq_data *data = policy->driver_data;
>> +	struct platform_device *pdev = cpufreq_get_driver_data();
>> +	struct device *dev = &pdev->dev;
>> +	int ret;
>> +
>> +	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);
>> +		return;
>> +	}
>> +	data->policy = policy;
>> +	INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
>> +}
>> +
>>   static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   {
>>   	struct platform_device *pdev = cpufreq_get_driver_data();
>> @@ -370,6 +463,16 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
>>   	}
>>   
>> +	/* Look for LMh interrupt. If no interrupt line is specified /
>> +	 * if there is an error, allow cpufreq to be enabled as usual.
>> +	 */
> 
> Proper comment style please..
> 
>> +	data->lmh_dcvs_irq = platform_get_irq(pdev, index);
>> +	if (data->lmh_dcvs_irq > 0) {
>> +		qcom_cpufreq_hw_lmh_init(policy);
>> +	} else if (data->lmh_dcvs_irq != -ENXIO) {
>> +		ret = data->lmh_dcvs_irq;
>> +		goto error;
>> +	}
> 
> Move all of this to qcom_cpufreq_hw_lmh_init().
> 
> And I don't see any cleanup for this stuff. There is no guarantee that
> the irq won't fire and queue up a work right after cpufreq driver is
> unregistered and before the devm_ stuff gets released.
> 
>>   	return 0;
>>   error:
>>   	kfree(data);
>> -- 
>> 2.25.1
> 

Re: [Patch v2 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support

[Viresh Kumar]

On 29-06-21, 22:25, Thara Gopinath wrote:
> I don't think cpufreq_cooling recognizes boost frequencies. The max state
> there is the max of nominal frequencies , right? If not, it might be a good
> idea to add this check there as well.

Ahh, that explains it.

-- 
viresh

Re: [Patch v2 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support

[Taniya Das]

On 6/24/2021 5:28 PM, 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>
> ---
> 
> v1->v2:
> 	- Introduced qcom_cpufreq_hw_lmh_init to consolidate LMh related initializations
> 	  as per Viresh's review comment.
> 	- Moved the piece of code restarting polling/re-enabling LMh interrupt to
> 	  qcom_lmh_dcvs_notify therby simplifying isr and timer callback as per Viresh's
> 	  suggestion.
> 	- Droped cpus from qcom_cpufreq_data and instead using cpus from cpufreq_policy in
> 	  qcom_lmh_dcvs_notify as per Viresh's review comment.
> 	- Dropped dt property qcom,support-lmh as per Bjorn's suggestion.
> 	- Other minor/cosmetic fixes
> 
>   drivers/cpufreq/qcom-cpufreq-hw.c | 103 ++++++++++++++++++++++++++++++
>   1 file changed, 103 insertions(+)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index f86859bf76f1..241f6f2b441f 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,10 @@ 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;
> +	struct cpufreq_policy *policy;
> +	int lmh_dcvs_irq;
>   };
>   
>   static unsigned long cpu_hw_rate, xo_rate;
> @@ -251,10 +258,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 = data->policy;
> +	struct dev_pm_opp *opp;
> +	struct device *dev;
> +	unsigned long max_capacity, capacity, freq_hz, throttled_freq;
> +	unsigned int val, freq;
> +
> +	/*
> +	 * Get the h/w throttled frequency, normalize it using the
> +	 * registered opp table and use it to calculate thermal pressure.
> +	 */
> +	val = readl_relaxed(data->base + data->soc_data->reg_current_vote);
> +	freq = qcom_lmh_vote_to_freq(val);
> +	freq_hz = freq * HZ_PER_KHZ;
> +
> +	dev = get_cpu_device(cpumask_first(policy->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);
> +
> +	throttled_freq = freq_hz / HZ_PER_KHZ;
> +
> +	/* Update thermal pressure */
> +	max_capacity = arch_scale_cpu_capacity(cpumask_first(policy->cpus));
> +	capacity = 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(policy->cpus, max_capacity - capacity);
> +	/*
> +	 * If h/w throttled frequency is higher than what cpufreq has requested for, stop
> +	 * polling and switch back to interrupt mechanism
> +	 */
> +	if (throttled_freq >= qcom_cpufreq_hw_get(cpumask_first(policy->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 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);
> +}
> +
> +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);
> +
> +	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,
>   };
> @@ -274,6 +350,23 @@ static const struct of_device_id qcom_cpufreq_hw_match[] = {
>   };
>   MODULE_DEVICE_TABLE(of, qcom_cpufreq_hw_match);
>   
> +static void qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy)
> +{
> +	struct qcom_cpufreq_data *data = policy->driver_data;
> +	struct platform_device *pdev = cpufreq_get_driver_data();
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	ret = devm_request_irq(dev, data->lmh_dcvs_irq, qcom_lmh_dcvs_handle_irq,
> +			       0, "dcvsh-irq", data);


It is better if you tag the CPU id while registering the IRQ.
"dcvsh-irq-x" (0/4/7)

> +	if (ret) {
> +		dev_err(dev, "Error %d registering irq %x\n", ret, data->lmh_dcvs_irq);
> +		return;
> +	}
> +	data->policy = policy;
> +	INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
> +}
> +
>   static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>   {
>   	struct platform_device *pdev = cpufreq_get_driver_data();
> @@ -370,6 +463,16 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>   			dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
>   	}
>   
> +	/* Look for LMh interrupt. If no interrupt line is specified /
> +	 * if there is an error, allow cpufreq to be enabled as usual.
> +	 */
> +	data->lmh_dcvs_irq = platform_get_irq(pdev, index);
> +	if (data->lmh_dcvs_irq > 0) {
> +		qcom_cpufreq_hw_lmh_init(policy);
> +	} else if (data->lmh_dcvs_irq != -ENXIO) {
> +		ret = data->lmh_dcvs_irq;
> +		goto error;
> +	}
>   	return 0;
>   error:
>   	kfree(data);
> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

Re: [Patch v2 3/5] cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support

[Thara Gopinath]

On 6/28/21 10:50 PM, Taniya Das wrote:
> 
> 
> On 6/24/2021 5:28 PM, 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>
>> ---
>>
>> v1->v2:
>>     - Introduced qcom_cpufreq_hw_lmh_init to consolidate LMh related 
>> initializations
>>       as per Viresh's review comment.
>>     - Moved the piece of code restarting polling/re-enabling LMh 
>> interrupt to
>>       qcom_lmh_dcvs_notify therby simplifying isr and timer callback 
>> as per Viresh's
>>       suggestion.
>>     - Droped cpus from qcom_cpufreq_data and instead using cpus from 
>> cpufreq_policy in
>>       qcom_lmh_dcvs_notify as per Viresh's review comment.
>>     - Dropped dt property qcom,support-lmh as per Bjorn's suggestion.
>>     - Other minor/cosmetic fixes
>>
>>   drivers/cpufreq/qcom-cpufreq-hw.c | 103 ++++++++++++++++++++++++++++++
>>   1 file changed, 103 insertions(+)
>>
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c 
>> b/drivers/cpufreq/qcom-cpufreq-hw.c
>> index f86859bf76f1..241f6f2b441f 100644
>> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c

[snip]

>>   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,
>>   };
>> @@ -274,6 +350,23 @@ static const struct of_device_id 
>> qcom_cpufreq_hw_match[] = {
>>   };
>>   MODULE_DEVICE_TABLE(of, qcom_cpufreq_hw_match);
>> +static void qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy)
>> +{
>> +    struct qcom_cpufreq_data *data = policy->driver_data;
>> +    struct platform_device *pdev = cpufreq_get_driver_data();
>> +    struct device *dev = &pdev->dev;
>> +    int ret;
>> +
>> +    ret = devm_request_irq(dev, data->lmh_dcvs_irq, 
>> qcom_lmh_dcvs_handle_irq,
>> +                   0, "dcvsh-irq", data);
> 
> 
> It is better if you tag the CPU id while registering the IRQ.
> "dcvsh-irq-x" (0/4/7)

Sure. Will fix it.

> 
>> +    if (ret) {
>> +        dev_err(dev, "Error %d registering irq %x\n", ret, 
>> data->lmh_dcvs_irq);
>> +        return;
>> +    }
>> +    data->policy = policy;
>> +    INIT_DEFERRABLE_WORK(&data->lmh_dcvs_poll_work, qcom_lmh_dcvs_poll);
>> +}
>> +
>>   static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   {
>>       struct platform_device *pdev = cpufreq_get_driver_data();
>> @@ -370,6 +463,16 @@ static int qcom_cpufreq_hw_cpu_init(struct 
>> cpufreq_policy *policy)
>>               dev_warn(cpu_dev, "failed to enable boost: %d\n", ret);
>>       }
>> +    /* Look for LMh interrupt. If no interrupt line is specified /
>> +     * if there is an error, allow cpufreq to be enabled as usual.
>> +     */
>> +    data->lmh_dcvs_irq = platform_get_irq(pdev, index);
>> +    if (data->lmh_dcvs_irq > 0) {
>> +        qcom_cpufreq_hw_lmh_init(policy);
>> +    } else if (data->lmh_dcvs_irq != -ENXIO) {
>> +        ret = data->lmh_dcvs_irq;
>> +        goto error;
>> +    }
>>       return 0;
>>   error:
>>       kfree(data);
>>
> 

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

[Patch v2 4/5] arm64: boot: dts: qcom: sdm45: Add support for LMh node

[Thara Gopinath]

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>
---

v1->v2:
	- Dropped dt property qcom,support-lmh as per Bjorn's review comments.
	- Changed lmh compatible from generic to platform specific.
	- Introduced properties specifying arm, low and high temp thresholds for LMh
	  as per Daniel's suggestion.

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0a86fe71a66d..202fec09becd 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -3646,6 +3646,30 @@ swm: swm@c85 {
 			};
 		};
 
+		lmh_cluster1: lmh@17d70800 {
+			compatible = "qcom,sdm845-lmh";
+			reg = <0 0x17d70800 0 0x401>;
+			interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+			qcom,lmh-cpu-id = <0x4>;
+			qcom,lmh-temperature-arm = <65000>;
+			qcom,lmh-temperature-low = <74500>;
+			qcom,lmh-temperature-high = <75000>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+		};
+
+		lmh_cluster0: lmh@17d78800 {
+			compatible = "qcom,sdm845-lmh";
+			reg = <0 0x17d78800 0 0x401>;
+			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+			qcom,lmh-cpu-id = <0x0>;
+			qcom,lmh-temperature-arm = <65000>;
+			qcom,lmh-temperature-low = <74500>;
+			qcom,lmh-temperature-high = <75000>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+		};
+
 		sound: sound {
 		};
 
@@ -4911,6 +4935,8 @@ 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";
 
-- 
2.25.1

[Patch v2 5/5] arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal zones 0-7

[Thara Gopinath]

Now that Limits h/w is enabled to monitor thermal events around cpus and
throttle the cpu frequencies, remove cpufreq cooling device for the cpus which
does software throttling of cpu frequencies.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---

v1->v2:
	Removing only cooling maps for cpu specific thermal zones keeping the
	trip point definitions intact as per Daniel's suggestion. This is to
	ensure that thermal zone temparature and trip violation information is
	available to any userspace daemon monitoring these zones.

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 136 ---------------------------
 1 file changed, 136 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 202fec09becd..7819f87d97ac 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -4994,23 +4994,6 @@ cpu0_crit: cpu_crit {
 					type = "critical";
 				};
 			};
-
-			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 {
@@ -5038,23 +5021,6 @@ cpu1_crit: cpu_crit {
 					type = "critical";
 				};
 			};
-
-			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 {
@@ -5082,23 +5048,6 @@ cpu2_crit: cpu_crit {
 					type = "critical";
 				};
 			};
-
-			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 {
@@ -5126,23 +5075,6 @@ cpu3_crit: cpu_crit {
 					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 {
@@ -5170,23 +5102,6 @@ cpu4_crit: cpu_crit {
 					type = "critical";
 				};
 			};
-
-			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 {
@@ -5214,23 +5129,6 @@ cpu5_crit: cpu_crit {
 					type = "critical";
 				};
 			};
-
-			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 {
@@ -5258,23 +5156,6 @@ cpu6_crit: cpu_crit {
 					type = "critical";
 				};
 			};
-
-			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 {
@@ -5302,23 +5183,6 @@ cpu7_crit: cpu_crit {
 					type = "critical";
 				};
 			};
-
-			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

Re: [Patch v2 5/5] arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal zones 0-7

[Matthias Kaehlcke]

On Thu, Jun 24, 2021 at 07:58:13AM -0400, Thara Gopinath wrote:

> Subject: arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal zones 0-7

The patch doesn't remove the passive trip points (anymore?), but the cooling
maps/devices. Also talking about 'thermal zones 0-7' doesn't really convey
any useful information (and the enumeration could potentially change in the
future), better talk about the CPU thermal zones.

> Now that Limits h/w is enabled to monitor thermal events around cpus and
> throttle the cpu frequencies, remove cpufreq cooling device for the cpus which
> does software throttling of cpu frequencies.

Re: [Patch v2 5/5] arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal zones 0-7

[Thara Gopinath]

On 6/24/21 12:51 PM, Matthias Kaehlcke wrote:
> On Thu, Jun 24, 2021 at 07:58:13AM -0400, Thara Gopinath wrote:
> 
>> Subject: arm64: boot: dts: qcom: sdm845: Remove passive trip points for thermal zones 0-7
> 
> The patch doesn't remove the passive trip points (anymore?), but the cooling
> maps/devices. Also talking about 'thermal zones 0-7' doesn't really convey
> any useful information (and the enumeration could potentially change in the
> future), better talk about the CPU thermal zones.

Hi Matthias,

Yes you are right. I forgot to change the subject. Will fix the subject 
and provide better description of the zones in the next rev

> 
>> Now that Limits h/w is enabled to monitor thermal events around cpus and
>> throttle the cpu frequencies, remove cpufreq cooling device for the cpus which
>> does software throttling of cpu frequencies.

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