[PATCH 0/4] cpufreq: qcom-hw: LMH irq/hotplug interractions

[PATCH 0/4] cpufreq: qcom-hw: LMH irq/hotplug interractions

[Pierre Gondois]

This patch-set provides fixes for the qcom-cpufreq-hw driver regarding
LMH irqs configurations and (un)plugging CPUs.
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.

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

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.

LMH irqs can be identified with:
  cat /proc/interrupts | grep "dcvsh\-irq"
and their configuration can be seen at:
  /proc/irqs/XXX/*

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 | 16 ++++++++++++----
 2 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.25.1

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

[Pierre Gondois]

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.

Fixes: a1eb080a0447 ("cpufreq: qcom-hw: provide online/offline operations")
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 drivers/cpufreq/qcom-cpufreq-hw.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 0253731d6d25..8aba73698eda 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -442,6 +442,13 @@ static int qcom_cpufreq_hw_cpu_online(struct cpufreq_policy *policy)
 	struct platform_device *pdev = cpufreq_get_driver_data();
 	int ret;
 
+	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

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

[Pierre Gondois]

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.

This patch also makes the check against throttle_irq consistent,
0 being theoretically valid.

Fixes: a1eb080a0447 ("cpufreq: qcom-hw: provide online/offline operations")
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 drivers/cpufreq/qcom-cpufreq-hw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 8aba73698eda..8cad3744cff4 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -461,7 +461,7 @@ static int qcom_cpufreq_hw_cpu_offline(struct cpufreq_policy *policy)
 {
 	struct qcom_cpufreq_data *data = policy->driver_data;
 
-	if (data->throttle_irq <= 0)
+	if (data->throttle_irq < 0)
 		return 0;
 
 	mutex_lock(&data->throttle_lock);
@@ -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

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

[Pierre Gondois]

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 <pierre.gondois@arm.com>
---
 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 8cad3744cff4..9cba3b9d3035 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

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

[Pierre Gondois]

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.

Fixes: a1eb080a0447 ("cpufreq: qcom-hw: provide online/offline operations")
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 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

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

[Viresh Kumar]

On 15-06-22, 16:43, Pierre Gondois wrote:
> >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.
> 
> Fixes: a1eb080a0447 ("cpufreq: qcom-hw: provide online/offline operations")
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>  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));

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

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

[Pierre Gondois]

Hello,
I saw that:
https://lore.kernel.org/all/20220617064421.l4vshytmqtittzee@vireshk-i7/

was applied, so this patch-set would need to be rebased. Please let me
know if you think it requires modifications before it gets rebased,

Regards,
Pierre

On 6/15/22 16:43, Pierre Gondois wrote:
> This patch-set provides fixes for the qcom-cpufreq-hw driver regarding
> LMH irqs configurations and (un)plugging CPUs.
> 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.
> 
> 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
> 
> 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.
> 
> LMH irqs can be identified with:
>    cat /proc/interrupts | grep "dcvsh\-irq"
> and their configuration can be seen at:
>    /proc/irqs/XXX/*
> 
> 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 | 16 ++++++++++++----
>   2 files changed, 15 insertions(+), 7 deletions(-)
> 

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

[Viresh Kumar]

On 04-07-22, 11:55, Pierre Gondois wrote:
> Hello,
> I saw that:
> https://lore.kernel.org/all/20220617064421.l4vshytmqtittzee@vireshk-i7/
> 
> was applied, so this patch-set would need to be rebased. Please let me
> know if you think it requires modifications before it gets rebased,

Everything else looks fine, just rebase and resend. Thanks.

-- 
viresh