[PATCH 0/2] cpufreq: few fix on mvebu driver

[PATCH 0/2] cpufreq: few fix on mvebu driver

[Gregory CLEMENT]

Hi,

The second patch of this series was originally part of the series "Add CPU
Frequency scaling support on Armada 37xx" [1].

As requested by Viresh Kumar, it is extracted in a independent series.
In the meantime, Thomas Petazzoni pointed an issue on the first
version that I fixed in this series.

While I was on this driver I found an aother issue that I fixed with
the first patch.

Thanks,

Gregory

[1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2017-December/547650.html

Gregory CLEMENT (2):
  cpufreq: mvebu: Free the clock reference in the normal path
  cpufreq: mvebu: Free opp if registering failed

 drivers/cpufreq/mvebu-cpufreq.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

-- 
2.15.1

[PATCH 1/2] cpufreq: mvebu: Free the clock reference in the normal path

[Gregory CLEMENT]

In case of error the clock reference was freed but not in normal path
once it was nor more used. This patch fixes it.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/cpufreq/mvebu-cpufreq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/mvebu-cpufreq.c b/drivers/cpufreq/mvebu-cpufreq.c
index ed915ee85dd9..c043aad8e3a0 100644
--- a/drivers/cpufreq/mvebu-cpufreq.c
+++ b/drivers/cpufreq/mvebu-cpufreq.c
@@ -99,6 +99,7 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
 		if (ret)
 			dev_err(cpu_dev, "%s: failed to mark OPPs as shared: %d\n",
 				__func__, ret);
+		clk_put(clk);
 	}
 
 	platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
-- 
2.15.1

[PATCH 1/2] cpufreq: mvebu: Free the clock reference in the normal path

[Viresh Kumar]

On 13-12-17, 18:29, Gregory CLEMENT wrote:
> In case of error the clock reference was freed but not in normal path
> once it was nor more used. This patch fixes it.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  drivers/cpufreq/mvebu-cpufreq.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/mvebu-cpufreq.c b/drivers/cpufreq/mvebu-cpufreq.c
> index ed915ee85dd9..c043aad8e3a0 100644
> --- a/drivers/cpufreq/mvebu-cpufreq.c
> +++ b/drivers/cpufreq/mvebu-cpufreq.c
> @@ -99,6 +99,7 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
>  		if (ret)
>  			dev_err(cpu_dev, "%s: failed to mark OPPs as shared: %d\n",
>  				__func__, ret);
> +		clk_put(clk);
>  	}
>  
>  	platform_device_register_simple("cpufreq-dt", -1, NULL, 0);

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

-- 
viresh

[PATCH 2/2] cpufreq: mvebu: Free opp if registering failed

[Gregory CLEMENT]

Since the introduction of this driver, the functions to remove the opp
were added. So stop claiming we can't remove opp and use one of them in
case of failure.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/cpufreq/mvebu-cpufreq.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/cpufreq/mvebu-cpufreq.c b/drivers/cpufreq/mvebu-cpufreq.c
index c043aad8e3a0..31513bd42705 100644
--- a/drivers/cpufreq/mvebu-cpufreq.c
+++ b/drivers/cpufreq/mvebu-cpufreq.c
@@ -76,12 +76,6 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
 			return PTR_ERR(clk);
 		}
 
-		/*
-		 * In case of a failure of dev_pm_opp_add(), we don't
-		 * bother with cleaning up the registered OPP (there's
-		 * no function to do so), and simply cancel the
-		 * registration of the cpufreq device.
-		 */
 		ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk), 0);
 		if (ret) {
 			clk_put(clk);
@@ -91,7 +85,8 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
 		ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk) / 2, 0);
 		if (ret) {
 			clk_put(clk);
-			return ret;
+			dev_err(cpu_dev, "Failed to register OPPs\n");
+			goto opp_register_failed;
 		}
 
 		ret = dev_pm_opp_set_sharing_cpus(cpu_dev,
@@ -104,5 +99,11 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
 
 	platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
 	return 0;
+
+opp_register_failed:
+	/* As registering has failed remove all the opp for all cpus */
+	dev_pm_opp_cpumask_remove_table(cpu_possible_mask);
+
+	return ret;
 }
 device_initcall(armada_xp_pmsu_cpufreq_init);
-- 
2.15.1

[PATCH 2/2] cpufreq: mvebu: Free opp if registering failed

[Viresh Kumar]

On 13-12-17, 18:29, Gregory CLEMENT wrote:
> Since the introduction of this driver, the functions to remove the opp
> were added. So stop claiming we can't remove opp and use one of them in
> case of failure.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  drivers/cpufreq/mvebu-cpufreq.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cpufreq/mvebu-cpufreq.c b/drivers/cpufreq/mvebu-cpufreq.c
> index c043aad8e3a0..31513bd42705 100644
> --- a/drivers/cpufreq/mvebu-cpufreq.c
> +++ b/drivers/cpufreq/mvebu-cpufreq.c
> @@ -76,12 +76,6 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
>  			return PTR_ERR(clk);
>  		}
>  
> -		/*
> -		 * In case of a failure of dev_pm_opp_add(), we don't
> -		 * bother with cleaning up the registered OPP (there's
> -		 * no function to do so), and simply cancel the
> -		 * registration of the cpufreq device.
> -		 */
>  		ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk), 0);
>  		if (ret) {
>  			clk_put(clk);
> @@ -91,7 +85,8 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
>  		ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk) / 2, 0);
>  		if (ret) {
>  			clk_put(clk);
> -			return ret;
> +			dev_err(cpu_dev, "Failed to register OPPs\n");
> +			goto opp_register_failed;
>  		}
>  
>  		ret = dev_pm_opp_set_sharing_cpus(cpu_dev,
> @@ -104,5 +99,11 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
>  
>  	platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
>  	return 0;
> +
> +opp_register_failed:
> +	/* As registering has failed remove all the opp for all cpus */
> +	dev_pm_opp_cpumask_remove_table(cpu_possible_mask);
> +
> +	return ret;
>  }
>  device_initcall(armada_xp_pmsu_cpufreq_init);

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

-- 
viresh

[PATCH 0/2] cpufreq: few fix on mvebu driver

[Rafael J. Wysocki]

On Wednesday, December 13, 2017 6:29:12 PM CET Gregory CLEMENT wrote:
> Hi,
> 
> The second patch of this series was originally part of the series "Add CPU
> Frequency scaling support on Armada 37xx" [1].
> 
> As requested by Viresh Kumar, it is extracted in a independent series.
> In the meantime, Thomas Petazzoni pointed an issue on the first
> version that I fixed in this series.
> 
> While I was on this driver I found an aother issue that I fixed with
> the first patch.
> 
> Thanks,
> 
> Gregory
> 
> [1]: http://lists.infradead.org/pipermail/linux-arm-kernel/2017-December/547650.html
> 
> Gregory CLEMENT (2):
>   cpufreq: mvebu: Free the clock reference in the normal path
>   cpufreq: mvebu: Free opp if registering failed
> 
>  drivers/cpufreq/mvebu-cpufreq.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> 

Both applied, thanks!