2022-07-04 11:30:34

by Pierre Gondois

[permalink] [raw]
Subject: [PATCH v2 0/4] qcom-cpufreq-hw LMH irq/hotplug interractions

v2:
- Rebased on v5.19-rc5
- Changed irq checks from '< 0' to '<= 0' to be aligned

The patch-set was tested on a rb5 with an old firmware version:
UEFI Ver : 5.0.210817.BOOT.XF.3.2-00354-SM8250-1
Build Info : 64b Aug 17 2021 23:35:39

commit ffd6cc92ab9c ("arm64: dts: qcom: sm8250: add description of dcvsh
interrupts")
enables DCVS (Dynamic Clock and Voltage Scaling) for sm8250 chips
(so rb5 included). As no LMH (Limits Management Hardware) interrupts
were seen, the firmware used for testing should not be able support
them.

This patch-set should still contain relevant modifications regarding
LMH interrupts and CPU hotplug. Still, it would be good to test
it on a platform which actually uses LMH interrupts.

Pierre Gondois (4):
cpufreq: qcom-hw: Reset cancel_throttle when policy is re-enabled
cpufreq: qcom-hw: Disable LMH irq when disabling policy
cpufreq: qcom-hw: Remove deprecated irq_set_affinity_hint() call
cpufreq: Change order of online() CB and policy->cpus modification

drivers/cpufreq/cpufreq.c | 6 +++---
drivers/cpufreq/qcom-cpufreq-hw.c | 11 ++++++++---
2 files changed, 11 insertions(+), 6 deletions(-)

--
2.25.1


2022-07-04 11:33:27

by Pierre Gondois

[permalink] [raw]
Subject: [PATCH v2 2/4] cpufreq: qcom-hw: Disable LMH irq when disabling policy

If LMH (Limits Management Hardware) is available, when a policy is
disabled by unplugging the last online CPU of policy->cpus, the LMH
irq is left enabled.
When the policy is re-enabled with any of the CPU in policy->cpus
being plugged in, qcom_cpufreq_ready() re-enables the irq. This
triggers the following warning:
[ 379.160106] Unbalanced enable for IRQ 154
[ 379.160120] WARNING: CPU: 7 PID: 48 at kernel/irq/manage.c:774 __enable_irq+0x84/0xc0
Thus disable the irq.

Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/cpufreq/qcom-cpufreq-hw.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 8e5eeb982ebd..729346a62a17 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -470,6 +470,7 @@ static int qcom_cpufreq_hw_cpu_offline(struct cpufreq_policy *policy)

cancel_delayed_work_sync(&data->throttle_work);
irq_set_affinity_hint(data->throttle_irq, NULL);
+ disable_irq_nosync(data->throttle_irq);

return 0;
}
--
2.25.1

2022-07-04 11:37:58

by Pierre Gondois

[permalink] [raw]
Subject: [PATCH v2 1/4] cpufreq: qcom-hw: Reset cancel_throttle when policy is re-enabled

If LMH (Limits Management Hardware) is available, when a policy is
disabled by unplugging the last online CPU of policy->cpus,
qcom_cpufreq_hw_cpu_offline() sets cancel_throttle=true.
cancel_throttle is not reset when the policy is re-enabled with any
of the CPU in policy->cpus being plugged in. So reset it.

This patch also adds an early exit check.

Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/cpufreq/qcom-cpufreq-hw.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 36c79580fba2..8e5eeb982ebd 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -445,6 +445,10 @@ static int qcom_cpufreq_hw_cpu_online(struct cpufreq_policy *policy)
if (data->throttle_irq <= 0)
return 0;

+ mutex_lock(&data->throttle_lock);
+ data->cancel_throttle = false;
+ mutex_unlock(&data->throttle_lock);
+
ret = irq_set_affinity_hint(data->throttle_irq, policy->cpus);
if (ret)
dev_err(&pdev->dev, "Failed to set CPU affinity of %s[%d]\n",
--
2.25.1

2022-07-04 11:46:51

by Pierre Gondois

[permalink] [raw]
Subject: [PATCH v2 4/4] cpufreq: Change order of online() CB and policy->cpus modification

From a state where all policy->related_cpus are offline, putting one
of the policy's CPU back online re-activates the policy by:
1. Calling cpufreq_driver->online()
2. Setting the CPU in policy->cpus

qcom_cpufreq_hw_cpu_online() makes use of policy->cpus. Thus 1. and 2.
should be inverted to avoid having a policy->cpus empty. The
qcom-cpufreq-hw is the only driver affected by this.

Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/cpufreq/cpufreq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 2cad42774164..36043be16d8e 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1350,15 +1350,15 @@ static int cpufreq_online(unsigned int cpu)
}

if (!new_policy && cpufreq_driver->online) {
+ /* Recover policy->cpus using related_cpus */
+ cpumask_copy(policy->cpus, policy->related_cpus);
+
ret = cpufreq_driver->online(policy);
if (ret) {
pr_debug("%s: %d: initialization failed\n", __func__,
__LINE__);
goto out_exit_policy;
}
-
- /* Recover policy->cpus using related_cpus */
- cpumask_copy(policy->cpus, policy->related_cpus);
} else {
cpumask_copy(policy->cpus, cpumask_of(cpu));

--
2.25.1

2022-07-04 11:48:38

by Pierre Gondois

[permalink] [raw]
Subject: [PATCH v2 3/4] cpufreq: qcom-hw: Remove deprecated irq_set_affinity_hint() call

commit 65c7cdedeb30 ("genirq: Provide new interfaces for affinity hints")
deprecates irq_set_affinity_hint(). Use the new
irq_set_affinity_and_hint() instead.

Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/cpufreq/qcom-cpufreq-hw.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 729346a62a17..2a21f1a2a2bd 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -428,7 +428,7 @@ static int qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy, int index)
return 0;
}

- ret = irq_set_affinity_hint(data->throttle_irq, policy->cpus);
+ ret = irq_set_affinity_and_hint(data->throttle_irq, policy->cpus);
if (ret)
dev_err(&pdev->dev, "Failed to set CPU affinity of %s[%d]\n",
data->irq_name, data->throttle_irq);
@@ -449,7 +449,7 @@ static int qcom_cpufreq_hw_cpu_online(struct cpufreq_policy *policy)
data->cancel_throttle = false;
mutex_unlock(&data->throttle_lock);

- ret = irq_set_affinity_hint(data->throttle_irq, policy->cpus);
+ ret = irq_set_affinity_and_hint(data->throttle_irq, policy->cpus);
if (ret)
dev_err(&pdev->dev, "Failed to set CPU affinity of %s[%d]\n",
data->irq_name, data->throttle_irq);
@@ -469,7 +469,7 @@ static int qcom_cpufreq_hw_cpu_offline(struct cpufreq_policy *policy)
mutex_unlock(&data->throttle_lock);

cancel_delayed_work_sync(&data->throttle_work);
- irq_set_affinity_hint(data->throttle_irq, NULL);
+ irq_set_affinity_and_hint(data->throttle_irq, NULL);
disable_irq_nosync(data->throttle_irq);

return 0;
--
2.25.1

2022-07-11 00:57:33

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] qcom-cpufreq-hw LMH irq/hotplug interractions

On 04-07-22, 13:27, Pierre Gondois wrote:
> v2:
> - Rebased on v5.19-rc5
> - Changed irq checks from '< 0' to '<= 0' to be aligned
>
> The patch-set was tested on a rb5 with an old firmware version:
> UEFI Ver : 5.0.210817.BOOT.XF.3.2-00354-SM8250-1
> Build Info : 64b Aug 17 2021 23:35:39
>
> commit ffd6cc92ab9c ("arm64: dts: qcom: sm8250: add description of dcvsh
> interrupts")
> enables DCVS (Dynamic Clock and Voltage Scaling) for sm8250 chips
> (so rb5 included). As no LMH (Limits Management Hardware) interrupts
> were seen, the firmware used for testing should not be able support
> them.
>
> This patch-set should still contain relevant modifications regarding
> LMH interrupts and CPU hotplug. Still, it would be good to test
> it on a platform which actually uses LMH interrupts.
>
> Pierre Gondois (4):
> cpufreq: qcom-hw: Reset cancel_throttle when policy is re-enabled
> cpufreq: qcom-hw: Disable LMH irq when disabling policy
> cpufreq: qcom-hw: Remove deprecated irq_set_affinity_hint() call
> cpufreq: Change order of online() CB and policy->cpus modification
>
> drivers/cpufreq/cpufreq.c | 6 +++---
> drivers/cpufreq/qcom-cpufreq-hw.c | 11 ++++++++---
> 2 files changed, 11 insertions(+), 6 deletions(-)

Applied. Thanks.

--
viresh